-
Notifications
You must be signed in to change notification settings - Fork 40.3k
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
Critical pods should not be limited to kube-system namespace #60596
Comments
While I am fine with the approach, it seems the only way to ensure that those critical pods are not coming from some random namespace is whitelisting them using priority admission controller or using quota. Atleast previously we have one more level of security that critical pod has to be created in kube-system namespace. |
The requirement to limit the critical pod annotation meaning to the kube-system namespace was a workaround for the lack of access control and quota around the annotation. We actively work to prevent special handling of the kube-system namespace wherever possible. |
Yes, which is why those aspects are essential to a functional priority system that works for a variety of cluster configurations |
Agree with Ravi and Jordan on what is needed for final solution. |
There is no doubt that we need to have quota for priority. It is needed not only for critical priorities, but also for any other high priority classes that users may define. That effort is going forward and we will have it soon. |
Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>. Critical pods shouldn't be restricted to kube-system **What this PR does / why we need it**: To make sure that critical pods are not restricted to kube-system namespace. **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 #60596 **Special notes for your reviewer**: @bsalamat @liggitt @aveshagarwal - Can we hold this till we merge quota restriction PR #57963. **Release note**: ```release-note NONE ```
@ravisantoshgudimetla , I think we also need to update Priority admission controller for that.
|
see later PR re-restricting this until the quota controls graduate to at least beta state: #65593 |
Got that, thanks :) That makes sense to me. |
If this is closed and there even was a merged pull request for this type of functionality, what is the current status? And is the documentation in sync with current status? |
It seems that |
Waiting for this to be available myself. No reason my Node Exporter dataset has to run in the |
Confirmed code + behavior. |
/reopen |
@vllry: Reopened this issue. 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 kubernetes/test-infra repository. |
I too am confused based on that documentation. Can we get an update here with an answer and fix the documentation if it's wrong? |
This is resolved in 1.17 See #76310 |
Due to kubernetes/kubernetes#60596, we cannot use 'priorityClass: system-node-critical' with Kubernetes versions prior to 1.17.
Due to kubernetes/kubernetes#60596, we cannot use 'priorityClass: system-node-critical' with Kubernetes versions prior to 1.17.
Due to kubernetes/kubernetes#60596, we cannot use 'priorityClass: system-node-critical' with Kubernetes versions prior to 1.17.
When we used annotations to mark critical pods, we considered pods with such an annotation critical only when they were created in "kube-system" namespace. Going to the new era of using priority to indicate critical pods, we would like to remove the restriction of "kube-system" namespace and consider pods with critical priority as critical no matter what namespace they are in.
ref/ #60519 (comment)
/kind bug
/sig scheduling
/assign @ravisantoshgudimetla
cc/ @liggitt
The text was updated successfully, but these errors were encountered: