Set the csi-secrets-store to have a priority class #3909
Conversation
Daemonsets are needed to run on all nodes but in a busy cluster with dynamic scaling, it is possible to have enough pending pods from replica sets that a new node fills up before the daemonset pod is ready to be scheduled. Without priority classes, this can cause daemonsets to be pending to the pod they are bound to and can not run elsewhere. In this change I added a priority class and use it in the daemonsets. We could look at using the priority class system-node-critical which is reserved for kubernetes system daemonsets. I don't think it is valid to use that here. Just having a priority higher than 0 will usually address this as 99.99% of all workload pods are 0 (or below zero for the easily evicted buffer pods and such)
|
💖 Thanks for opening your first pull request! 💖 We use semantic commit messages to streamline the release process. Before your pull request can be merged, you should make sure your first commit and PR title start with a semantic prefix. Examples of commit messages with semantic prefixes: - |
| @@ -282,6 +282,18 @@ status: | |||
| conditions: [] | |||
| storedVersions: [] | |||
| --- | |||
| # A priority class for the daemonset such that they are not | |||
There was a problem hiding this comment.
Could you use go template comments for these lines instead of bash comments to save cloud-init data overhead? :/
E.g.:
{{/* A priority class for the daemonset such that they are not */}}
There was a problem hiding this comment.
Ok - annoying, but ok.
(We could look loading and dumping yaml in optimal form after templating such that you are sure to not have waste in general)
|
@Michael-Sinz and one more thing after the comment change, run Thanks so much for this! |
Change the comments such that they go away in template expansion (saves space in cloud-init)
| metadata: | ||
| name: csi-secrets-store | ||
| labels: | ||
| addonmanager.kubernetes.io/mode: EnsureExists |
There was a problem hiding this comment.
Added this label so that kube-addon-manager would interpret this resource for loading
There was a problem hiding this comment.
Thanks - sorry I missed that.
| {{- /* frozen out of a node due to the node filling up with "normal" */}} | ||
| {{- /* pods before the daemonset controller can get the daemonset */}} | ||
| {{- /* pods to be scheduled. */}} | ||
| apiVersion: scheduling.k8s.io/v1 |
There was a problem hiding this comment.
|
Congrats on merging your first pull request! 🎉🎉🎉 |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jackfrancis, Michael-Sinz 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 |
Daemonsets are needed to run on all nodes but in a busy cluster with dynamic scaling, it is possible to have enough pending pods from replica sets that a new node fills up before the daemonset pod is ready to be scheduled. Without priority classes, this can cause daemonsets to be pending to the pod they are bound to and can not run elsewhere.
In this change I added a priority class and use it in the daemonsets. We could look at using the priority class system-node-critical which is reserved for kubernetes system daemonsets. I don't think it is valid to use that here.
Just having a priority higher than 0 will usually address this as 99.99% of all workload pods are 0 (or below zero for the easily evicted buffer pods and such)
Reason for Change:
Issue Fixed:
Credit Where Due:
Does this change contain code from or inspired by another project?
If "Yes," did you notify that project's maintainers and provide attribution?
Requirements:
Notes: