NodeController should add NoSchedule taints and we should get rid of getNodeConditionPredicate()

[davidopp]

getNodeConditionPredicate() in plugin/pkg/scheduler/factory/factory.go makes our code hard to understand because it hides the node-condition-based filtering in the node lister, which is totally non-obvious.

We should get rid of this function and have NodeController add NoSchedule taints for these situations instead. (Alas, we might not actually be able to get rid of getNodeConditionPredicate() completely until we get rid of the Unschedulable field of PodSpec, but at least we can get rid of all the other code in this function.) If there are pods that should still be able to schedule in any of these situations (e.g. DaemonSet pods?) we should add tolerations in admission control for them (e.g. see pkg/controller/daemon/daemoncontroller.go).

cc/ @kubernetes/sig-scheduling-misc @kubernetes/sig-cluster-lifecycle-misc
cc/ @gmarek @kevin-wangzefeng

Sub-tasks according to the design doc:

• Add node taints label and feature flag (Task 0: Added node taints labels and feature flags #49547)
• In the node controller, taint Nodes according to the Node Condition (Task 1: Tainted node by condition. #49257)
• In DaemonSet, update DaemonSetController to Tolerant new Taints (Task 2: Added toleration to DaemonSet pods for node condition taints #50186)
• In admissionController, add MemoryPressure/DiskPressure toleration for no BestEffort pod (Task 3: Add MemoryPressure toleration for no BestEffort pod. #50180)
• In scheduler, disable the current behaviour of filtering out Nodes. Instead, pods will not be scheduled to tainted nodes if a toleration does not exist in its PodSpec (Task 4: Ignored node condition predicates if TaintsByCondition enabled. #50185)
• Add e2e test for this feature (Apply algorithm in scheduler by feature gates. #52723)
• Update doc for this feature (Add documentation for TaintNodesByCondition website#5352)

[resouer]

until we get rid of the Unschedulable field of PodSpec

I think you are referring Unschedulable field of NodeSpec?

[davidopp]

I think you are referring Unschedulable field of NodeSpec?

Yes sorry, that was a typo.

[gmarek]

@resouer - please CC me to all PRs

[bgrant0607]

See also #29178

[davidopp]

Note that memory and disk pressure are checked in separate predicates, not in the NodeConditionPredicate. These should be represented as NoSchedule taints too.

kubernetes/plugin/pkg/scheduler/algorithm/predicates/predicates.go

Line 1239 in e7dfdd5

func CheckNodeMemoryPressurePredicate(pod *v1.Pod, meta interface{}, nodeInfo *schedulercache.NodeInfo) (bool, []algorithm.PredicateFailureReason, error) {

[davidopp]

Setting NoSchedule taints corresponding to the node conditions that today cause pods not to schedule onto a node, should also allow us to use tolerations rather than special-case code in the DaemonSet controller (and thereby make it easier to have DaemonSet pods be scheduled by the default scheduler.)

[davidopp]

ref/ #42002