support NoExecute and NoSchedule taints correctly in DaemonSet controller

And add some unit tests.

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,

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.
+// Daemonset should not remove a running pod from a node if the pod doesn't
-func TestTaintedNodeDaemonDoesNotLaunchUntoleratePod(t *testing.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)
 

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 {
+	if v1helper.TolerationsTolerateTaintsWithFilter(pod.Spec.Tolerations, taints, filter) {
-		// PodToleratesNodeTaints is only interested in NoSchedule and NoExecute taints.
-		return t.Effect == v1.TaintEffectNoSchedule || t.Effect == v1.TaintEffectNoExecute
-	}) {
 		return true, nil, nil
 	}
 	return false, []algorithm.PredicateFailureReason{ErrTaintsTolerationsNotMatch}, nil