chore: add priorityClassName: system-node-critical to kube-system,… #555
chore: add priorityClassName: system-node-critical to kube-system,… #555
Conversation
… master-schedulable components
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jackfrancis 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 |
Codecov Report
@@ Coverage Diff @@
## master #555 +/- ##
==========================================
+ Coverage 54.75% 54.79% +0.03%
==========================================
Files 97 97
Lines 14708 14716 +8
==========================================
+ Hits 8054 8064 +10
+ Misses 5980 5979 -1
+ Partials 674 673 -1 |
lgtm |
parts/k8s/addons/kubernetesmasteraddons-azure-npm-daemonset.yaml
Outdated
Show resolved
Hide resolved
omsagent cluster scenario manually tested |
flannel scenario manually tested |
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 see you used system-cluster-critical and not system-node-critical any reason for that?
Also to fix the full problem scope I believe we still need a wildcard toleration for kube-proxy at least.
- operator: "Exists"
effect: "NoExecute"
- operator: "Exists"
effect: "NoSchedule"
Add-on for the reasons: Adding priorityClass to ensure that kube-system components are schedulable when node taints are customized and added to any node. Fixes #573 |
@palma21 does the most recent commit address the desired add'l kube-proxy tolerations? |
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 believe there's a bug here with the indentation that is making the tests fail
- operator: "Exists" | ||
effect: NoExecute | ||
- operator: "Exists" | ||
effect: NoSchedule | ||
containers: |
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.
This is to ensure that we never schedule kube-proxy on master, correct?
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.
It's the opposite: it ensures that kube-proxy will always be able to be scheduled on any node, regardless of taint.
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
… master-schedulable components
Reason for Change:
For
kube-system
components that are declared to tolerate master node taints, we want to addpriorityClassName: system-node-critical
as well to doubly ensure that they are schedulable, even if the master node taints are customized. Otherwise, master node taint customization is liable to make system-critical components unschedulable.Included all components that fit the above description for k8s 1.9 and above.
Reference: https://kubernetes.io/docs/tasks/administer-cluster/guaranteed-scheduling-critical-addon-pods/
Issue Fixed:
Fixes #573
Requirements:
Notes: