-
Notifications
You must be signed in to change notification settings - Fork 337
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
webhook: fix priority admission conflict #1968
webhook: fix priority admission conflict #1968
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1968 +/- ##
==========================================
+ Coverage 67.54% 67.55% +0.01%
==========================================
Files 420 420
Lines 47058 47059 +1
==========================================
+ Hits 31785 31792 +7
Misses 12960 12960
+ Partials 2313 2307 -6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
when we set priorityclass koord-xxx with preemptionPolicy=Never, priority admission refuse to create pod with error like this: ```text pods "test" is forbidden: the string value of PreemptionPolicy (PreemptLowerPriority) must not be provided in pod spec; priority admission controller computed Never from the given PriorityClass name ``` we overwrite PriorityClassName/Priority in cluster_colocation_profile, so as well PreemptionPolicy. see https://github.com/kubernetes/kubernetes/blob/master/plugin/pkg/admission/priority/admission.go#L188 Signed-off-by: j4ckstraw <j4ckstraw@foxmail.com>
8457454
to
21befaf
Compare
We change priorityclass preemptionPolicy from PreemptLowerPriority to Never for some reason, then new pod refused by priority admission. priority admission check if priority and preemptionPolicy is corresponding to priorityClass link while koordinator only overwrite PriorityClassName and Priority in cluster colocation profile link, preemptionPolicy overwrite is also needed. |
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.
/lgtm
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eahydra 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 |
Signed-off-by: j4ckstraw <j4ckstraw@foxmail.com>
when we set priorityclass koord-xxx with preemptionPolicy=Never, priority admission refuse to create pod with error like this:
we overwrite PriorityClassName/Priority in cluster_colocation_profile, so as well PreemptionPolicy.
see
https://github.com/kubernetes/kubernetes/blob/master/plugin/pkg/admission/priority/admission.go#L188
Ⅰ. Describe what this PR does
Ⅱ. Does this pull request fix one issue?
Ⅲ. Describe how to verify it
Ⅳ. Special notes for reviews
V. Checklist
make test