-
Notifications
You must be signed in to change notification settings - Fork 1.5k
OCPBUGS-50534: Fix error messages in available IP code #9503
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
OCPBUGS-50534: Fix error messages in available IP code #9503
Conversation
|
@rna-afk: This pull request references Jira Issue OCPBUGS-50534, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
pkg/asset/manifests/azure/cluster.go
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: should we include a reason, for example, no available IP (i.e. same for line 286 above)?
I feel like failed to ... alone could mean some unknown internal error, which might not be descriptive enough :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for line 299.
Currently, installer seems always running into line 286 when lack of permissions, maybe add reason what Patrick suggested for line 271?
babd7d5 to
759b731
Compare
|
The shared vpc job is permafailing until we have openshift/release#61374, so the silver lining is we can see these changes in the ci test results! /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: patrickdillon The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/jira refresh |
|
@rna-afk: This pull request references Jira Issue OCPBUGS-50534, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
pkg/asset/manifests/azure/cluster.go
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: I think here *ipAvail.AvailableIPAddresses is non-empty and we could not find any suitable IP right? How about no available IP or similar?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is IP available but at this stage there's no Ip that we can use that's in the machine network so I think it's fine but I am really bad at error messages or commenting so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, what do you think of using what you just wrote as error message instead (i.e. that is There is IP available but at this stage there's no Ip that we can use that's in the machine network)? Otherwise, LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realized the wordings in my original question was kinda odd. Your latest commit looks great! 👏🏼
Fixing the error messages in the check for available IPs. The error message is always empty since there was no error but there are no IPs available.
759b731 to
be7f5b5
Compare
|
/lgtm ^-^ |
|
/lgtm |
|
/jira refresh The requirements for Jira bugs have changed (Jira issues linked to PRs on main branch need to target different OCP), recalculating validity. |
|
@openshift-bot: This pull request references Jira Issue OCPBUGS-50534, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
2 similar comments
|
/override ci/prow/okd-scos-images |
|
@tthvo: tthvo unauthorized: /override is restricted to Repo administrators, approvers in top level OWNERS file, and the following github teams:openshift: openshift-release-oversight openshift-staff-engineers. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Ah no, I don't have the power 😅 /retest-required |
|
/retest-required |
|
/hold Revision be7f5b5 was retested 3 times: holding |
|
/unhold |
|
@rna-afk: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
b20e29d
into
openshift:main
|
@rna-afk: Jira Issue OCPBUGS-50534: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-50534 has been moved to the MODIFIED state. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
[ART PR BUILD NOTIFIER] Distgit: ose-installer-altinfra |
|
[ART PR BUILD NOTIFIER] Distgit: ose-installer-terraform-providers |
|
[ART PR BUILD NOTIFIER] Distgit: ose-baremetal-installer |
|
[ART PR BUILD NOTIFIER] Distgit: ose-installer-artifacts |
Fixing the error messages in the check for available IPs. The error message is always empty since there was no error but there are no IPs available.