Remove critical-pod annotations from scheduler & controller-manager

[aaronlevy]

If we mark the scheduler as critical, for example, then it happens to get in a state where a pod is unscheduled because the scheduler is down (e.g. #112) -- will the rescheduler start evicting other workloads?

This would be pretty dangerous as it would likely just keep evicting workloads even though it will never be scheduled (no scheduler to schedule it).

Ideally the rescheduler would only evict based on "known" scheduling failures (e.g. no more allocatable CPU), not just on "can't schedule" -- but I'm unsure of the exact behavior.

We don't currently deploy the rescheduler, but we should evaluate this behavior before doing so (possibly remove the critical pod annotation from controller-manager / scheduler).

xref: #474

[luxas]

cc @davidopp @bsalamat @timothysc

[bsalamat]

When a critical pod cannot be scheduled, rescheduler tries to find a node for it and removes enough pods from the node to allow scheduling of the critical pod, but once it does so, it adds the pod to a list of processed pods. Once the pod is added to the list, rescheduler will not remove more workloads from other nodes to make it schedule.
That said, after we add priority and preemption to Kubernetes, we will get rid of the way critical pods are annotated today and will probably retire rescheduler. In that world, scheduler and other critical system pods will get very high priority and won't get evicted/preempted.

[bsalamat]

Actually, when I looked at the code again, I noticed that if the processed pod is not scheduled within a certain amount of time, it is removed from the processed list and, therefore, is processed again, which means that more pods may be removed to let it schedule. So, the scenario described in the issue can happen and is dangerous.
As explained above, the priority and preemption work will allow us to retire the existing rescheduler.

[dghubble]

This is confusing. #728 (comment) states the annotations are only for the rescheduler. #474 (comment) states the annotations aren't safe if we do run the rescheduler. Since we don't run the rescheduler, it seems like these annotations are useless rather than dangerous currently? Upstream salt templates still includes the critical-pod annotation on kube-system components, Tectonic does not. @diegs @aaronlevy

Can we come to a decision to merge or close #728 (@squat) and plan whether we ever want the rescheduler or no if its being retired. #474

[diegs]

I just closed #474. We should merge #728. Upstream is out of date by the sounds of it. The rescheduler is deprecated in favor of priorities.

[diegs]

Fixed in #777