Mark Static pods on the Master as critical

[dashpole]

fixes #47277.

A known issue with static pods is that they do not interact well with evictions. If a static pod is evicted or oom killed, then it will never be recreated. To mitigate this, we do not evict static pods that are critical. In addition, non-critical pods are candidates for preemption if a critical pod is scheduled to the node. If there are not enough allocatable resources on the node, this causes the static pod to be preempted.

This PR marks all static pods in the kube-system namspace as critical.

cc @vishh @dchen1107

[dashpole]

/release-note-none

[dashpole]

/assign @dchen1107 @vishh
/unassign @jbeda @roberthbailey

[vishh]

/lgtm
/approve

[vishh]

@dashpole should this be included in v1.7?

[dashpole]

I would say yes. I would consider not placing the "critical" annotation on critical system pods a bug.

[davidopp]

This PR is good but seems we should also add the three non-static pods?

event-exporter-v0.1.0-947590526-h897n (non-static)
l7-default-backend-4019876529-14krg (non-static)
monitoring-influxdb-grafana-v4-2dv01 (non-static)

[luxas]

@dashpole Please make the kubeadm static pods critical then as well (cmd/kubeadm/app/master/manifests.go)

[dashpole]

@luxas unless I am missing something, those are the static pods I am changing in this PR

[dashpole]

@davidopp I am not familiar with what those pods do. Making them critical allows them to preempt other pods in the system in order to schedule.
Is that behavior we want in all 3 of those pods?

[dashpole]

Another non-static system pod that is not currently marked as critical is the NPD. We plan to leave it non-critical for the time being, as it is not critical for the system to function, and will reschedule when there is enough allocatable resources.

[dashpole]

@k8s-bot pull-kubernetes-e2e-gce-etcd3 test this

[davidopp]

I am not familiar with what those pods do. Making them critical allows them to preempt other pods in the system in order to schedule.
Is that behavior we want in all 3 of those pods?

event-exporter is important for debugging so probably yes. l7-default-backend I don't know exactly what it does but it sounds from the name like it may be critical for Ingress so probably yes. monitoring-influxdb-grafana maybe could be skipped.

[dashpole]

alright, Ill add the event-exporter, and the L7-default-backend.

[roberthbailey]

l7-default-backend is (as the name sounds) the default backend for an ingress resource created in GCE. I believe it black-holes any traffic that you don't configure to hit one of your own backends. I'm not sure what happens if it isn't running, but @bowei or @dnardo would probably know.

[roberthbailey]

@luxas unless I am missing something, those are the static pods I am changing in this PR

You are changing the manifest files used by the kube-up.sh scripts, but not the ones used by kubeadm. See https://github.com/kubernetes/kubernetes/blob/master/cmd/kubeadm/app/master/manifests.go

[dashpole]

Ok, I have added the kubeadm static pods as well. This PR now makes all static system pods critical.

[dashpole]

based on #47277 (comment), I am going to leave out non-static pods.

[dchen1107]

I also added another comment on static pods at: #47277 (comment)

We need to answer that questions. Also release-note is required for this.

[luxas]

/retest

/approve
for the kubeadm change

[luxas]

PTAL @kubernetes/sig-scalability-pr-reviews @kubernetes/sig-node-pr-reviews @kubernetes/kubernetes-release-managers

This was already LGTM'd, can we get the label reapplied?

[luxas]

Damn, can't even tag the right sig, I meant @kubernetes/sig-scheduling-pr-reviews of course

[timothysc]

/lgtm but needs a different owner for final approval.

/cc @roberthbailey

[jbeda]

This is clearly in cluster lifecycle territory.

/approve

[roberthbailey]

/lgtm

[k8s-github-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dashpole, jbeda, luxas, roberthbailey, timothysc, vishh

Associated issue: 47277

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

• cluster/OWNERS [jbeda,roberthbailey]
• cmd/kubeadm/OWNERS [jbeda,luxas]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

[k8s-github-robot]

Automatic merge from submit-queue (batch tested with PRs 47669, 40284, 47356, 47458, 47701)