-
Notifications
You must be signed in to change notification settings - Fork 39.1k
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
fix apiserver crashed when priority classs already exists #67372
Conversation
/test pull-kubernetes-e2e-kops-aws |
@@ -101,7 +101,7 @@ func AddSystemPriorityClasses() genericapiserver.PostStartHookFunc { | |||
return true, nil | |||
}) | |||
// if we're never able to make it through initialization, kill the API server. | |||
if err != nil { | |||
if err != nil && !apierrors.IsAlreadyExists(err) { |
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 don't really follow but where does the AlreadyExistsError
come from?
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.
the AlreadyExists error should be tolerated on line 87, not here
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.
the AlreadyExists error should be tolerated on line 87, not here
agree. returning a false, nil
here will give a retry, amiright?
/hold /cc @liggitt i'd prefer a create-on-update to simplify all such initializers. it would be cleaner and hassle-free. wdyt? (xref similar complex logic) kubernetes/pkg/master/controller.go Line 287 in cfb4a5e
|
tolerating AlreadyExists here is a fine change to make, and is a much smaller change, and we'd want that picked back to 1.11 |
@tanshanshan were you planning on reopening this PR? I think this fix was on the right track |
/hold cancel |
6219e51
to
a99b619
Compare
/assign @liggitt |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: liggitt, tanshanshan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Automatic merge from submit-queue (batch tested with PRs 67137, 67372, 67505, 67373, 67357). If you want to cherry-pick this change to another branch, please follow the instructions here. |
/sig api-machinery |
…67372-upstream-release-1.11 Automatic merge from submit-queue. Automated cherry pick of #67372: fix apiserver crashed when priority classs already exists Cherry pick of #67372 on release-1.11. #67372: fix apiserver crashed when priority classs already exists ```release-note kube-apiserver: fixes error creating system priority classes when starting multiple apiservers simultaneously ```
What this PR does / why we need it:
when priority classs already exists , ignore error
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #67367
Special notes for your reviewer:
Release note: