diff --git a/pkg/controller/daemon/daemoncontroller.go b/pkg/controller/daemon/daemoncontroller.go index 9b154990d7f..9641cbde14c 100644 --- a/pkg/controller/daemon/daemoncontroller.go +++ b/pkg/controller/daemon/daemoncontroller.go @@ -1040,7 +1040,14 @@ func (dsc *DaemonSetsController) nodeShouldRunDaemonPod(node *v1.Node, ds *exten // Because these bools require an && of all their required conditions, we start // with all bools set to true and set a bool to false if a condition is not met. - // A bool should probably not be set to true after this line. + // A bool should probably not be set to true after this line. We can + // return early if we are: + // + // 1. return false, false, false, err + // 2. return false, false, false, nil + // + // Otherwise if a condition is not met, we should set one of these + // bools to false. wantToRun, shouldSchedule, shouldContinueRunning = true, true, true // If the daemon set specifies a node name, check that it matches with node.Name. if !(ds.Spec.Template.Spec.NodeName == "" || ds.Spec.Template.Spec.NodeName == node.Name) { @@ -1130,10 +1137,17 @@ func (dsc *DaemonSetsController) nodeShouldRunDaemonPod(node *v1.Node, ds *exten // this one is probably intentional since it's a workaround for not having // pod hard anti affinity. predicates.ErrPodNotFitsHostPorts: - wantToRun, shouldSchedule, shouldContinueRunning = false, false, false + return false, false, false, nil case predicates.ErrTaintsTolerationsNotMatch: // DaemonSet is expected to respect taints and tolerations - wantToRun, shouldSchedule, shouldContinueRunning = false, false, true + fitsNoExecute, _, err := predicates.PodToleratesNodeNoExecuteTaints(newPod, nil, nodeInfo) + if err != nil { + return false, false, false, err + } + if !fitsNoExecute { + return false, false, false, nil + } + wantToRun, shouldSchedule = false, false // unintentional case predicates.ErrDiskConflict, diff --git a/pkg/controller/daemon/daemoncontroller_test.go b/pkg/controller/daemon/daemoncontroller_test.go index 5a853ccd575..e65e40c94fb 100644 --- a/pkg/controller/daemon/daemoncontroller_test.go +++ b/pkg/controller/daemon/daemoncontroller_test.go @@ -60,6 +60,7 @@ var ( var ( noScheduleTolerations = []v1.Toleration{{Key: "dedicated", Value: "user1", Effect: "NoSchedule"}} noScheduleTaints = []v1.Taint{{Key: "dedicated", Value: "user1", Effect: "NoSchedule"}} + noExecuteTaints = []v1.Taint{{Key: "dedicated", Value: "user1", Effect: "NoExecute"}} ) var ( @@ -1079,10 +1080,46 @@ func TestDaemonKillFailedPods(t *testing.T) { } } -// DaemonSet should not launch a pod on a tainted node when the pod doesn't tolerate that taint. -func TestTaintedNodeDaemonDoesNotLaunchUntoleratePod(t *testing.T) { +// Daemonset should not remove a running pod from a node if the pod doesn't +// tolerate the nodes NoSchedule taint +func TestNoScheduleTaintedDoesntEvicitRunningIntolerantPod(t *testing.T) { for _, strategy := range updateStrategies() { - ds := newDaemonSet("untolerate") + ds := newDaemonSet("intolerant") + ds.Spec.UpdateStrategy = *strategy + manager, podControl, _ := newTestController(ds) + + node := newNode("tainted", nil) + manager.nodeStore.Add(node) + setNodeTaint(node, noScheduleTaints) + manager.podStore.Add(newPod("keep-running-me", "tainted", simpleDaemonSetLabel, ds)) + manager.dsStore.Add(ds) + + syncAndValidateDaemonSets(t, manager, ds, podControl, 0, 0) + } +} + +// Daemonset should remove a running pod from a node if the pod doesn't +// tolerate the nodes NoExecute taint +func TestNoExecuteTaintedDoesEvicitRunningIntolerantPod(t *testing.T) { + for _, strategy := range updateStrategies() { + ds := newDaemonSet("intolerant") + ds.Spec.UpdateStrategy = *strategy + manager, podControl, _ := newTestController(ds) + + node := newNode("tainted", nil) + manager.nodeStore.Add(node) + setNodeTaint(node, noExecuteTaints) + manager.podStore.Add(newPod("stop-running-me", "tainted", simpleDaemonSetLabel, ds)) + manager.dsStore.Add(ds) + + syncAndValidateDaemonSets(t, manager, ds, podControl, 0, 1) + } +} + +// DaemonSet should not launch a pod on a tainted node when the pod doesn't tolerate that taint. +func TestTaintedNodeDaemonDoesNotLaunchIntolerantPod(t *testing.T) { + for _, strategy := range updateStrategies() { + ds := newDaemonSet("intolerant") ds.Spec.UpdateStrategy = *strategy manager, podControl, _ := newTestController(ds) diff --git a/plugin/pkg/scheduler/algorithm/predicates/predicates.go b/plugin/pkg/scheduler/algorithm/predicates/predicates.go index dbb214c943d..99b7482d5cf 100644 --- a/plugin/pkg/scheduler/algorithm/predicates/predicates.go +++ b/plugin/pkg/scheduler/algorithm/predicates/predicates.go @@ -1247,15 +1247,26 @@ func (c *PodAffinityChecker) satisfiesPodsAffinityAntiAffinity(pod *v1.Pod, node // PodToleratesNodeTaints checks if a pod tolertaions can tolerate the node taints func PodToleratesNodeTaints(pod *v1.Pod, meta interface{}, nodeInfo *schedulercache.NodeInfo) (bool, []algorithm.PredicateFailureReason, error) { + return podToleratesNodeTaints(pod, nodeInfo, func(t *v1.Taint) bool { + // PodToleratesNodeTaints is only interested in NoSchedule and NoExecute taints. + return t.Effect == v1.TaintEffectNoSchedule || t.Effect == v1.TaintEffectNoExecute + }) +} + +// PodToleratesNodeNoExecuteTaints checks if a pod tolertaions can tolerate the node's NoExecute taints +func PodToleratesNodeNoExecuteTaints(pod *v1.Pod, meta interface{}, nodeInfo *schedulercache.NodeInfo) (bool, []algorithm.PredicateFailureReason, error) { + return podToleratesNodeTaints(pod, nodeInfo, func(t *v1.Taint) bool { + return t.Effect == v1.TaintEffectNoExecute + }) +} + +func podToleratesNodeTaints(pod *v1.Pod, nodeInfo *schedulercache.NodeInfo, filter func(t *v1.Taint) bool) (bool, []algorithm.PredicateFailureReason, error) { taints, err := nodeInfo.Taints() if err != nil { return false, nil, err } - if v1helper.TolerationsTolerateTaintsWithFilter(pod.Spec.Tolerations, taints, func(t *v1.Taint) bool { - // PodToleratesNodeTaints is only interested in NoSchedule and NoExecute taints. - return t.Effect == v1.TaintEffectNoSchedule || t.Effect == v1.TaintEffectNoExecute - }) { + if v1helper.TolerationsTolerateTaintsWithFilter(pod.Spec.Tolerations, taints, filter) { return true, nil, nil } return false, []algorithm.PredicateFailureReason{ErrTaintsTolerationsNotMatch}, nil