-
Notifications
You must be signed in to change notification settings - Fork 100
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
[release-4.14] OCPBUGS-35912: Fix race to mark node Joined #860
base: release-4.14
Are you sure you want to change the base?
[release-4.14] OCPBUGS-35912: Fix race to mark node Joined #860
Conversation
If a host is in the Installed state already (which can occur when the assisted-installer-controller sets the progress to Done), don't try to set the progress to Joined as it will not only never succeed, but also take 30+ minutes of unlogged retries inside the client before an error is returned. This narrows the window in which this can occur, but if the bootstrap assisted-installer reads the Host before the assisted-installer-controller updates the status, this could still occur. Ensure any failed requests are retried by not adding the Node to the readyMasters list until the Progress has been set to either Joined or Done (the latter triggers a change of Status to Installed). Improve debugging by not logging different request_ids for messages corresponding to a single request.
Since 4xx error codes indicate a problem on the client side, most of them cannot be usefully retried at the HTTP transport level. e.g. if a 409 Conflict is returned in response to a PUT request, then we need to fetch the resource again with a GET before creating a new PUT request. Blocking for 30+ minutes in the original PUT call (without logging) is not helpful; we want the transport to return immediately so we can try again. Retry on only those 4xx error codes where it is conceivable that trying the same request again might work.
@openshift-cherrypick-robot: Jira Issue OCPBUGS-35894 has been cloned as Jira Issue OCPBUGS-35912. Will retitle bug to link to clone. In 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. |
@openshift-cherrypick-robot: This pull request references Jira Issue OCPBUGS-35912, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. In 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. |
/retest |
/lgtm |
/lgtm |
/jira refresh |
@zaneb: This pull request references Jira Issue OCPBUGS-35912, which is valid. The bug has been moved to the POST state. 7 validation(s) were run on this bug
Requesting review from QA contact: In 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. |
/hold |
In a race between the assisted-installer and assisted-installer-controller to mark a control plane node as ready, the former will get a 409 Conflict response from assisted-service if the latter wins. Since f548d32 we retry that check promptly. However, prior to this change we never read fresh inventory data (which would show the host has already reached the "installed" state and thus prevent us from attempting another (unnecessary) write that is doomed to fail due to a conflict. Reload the host inventory in the event of a failure to update the host status, so that we can take into account that the change may already have been performed by the assisted-installer-controller.
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: openshift-cherrypick-robot The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Backport of #891 is now included. |
@openshift-cherrypick-robot: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
/lgtm |
Issues go stale after 90d of inactivity. Mark the issue as fresh by commenting If this issue is safe to close now please do so with /lifecycle stale |
This is an automated cherry-pick of #859
/assign zaneb