From deb6d854701c951bf028934eadffce4f0e4f028b Mon Sep 17 00:00:00 2001
From: Wei Huang <wei.huang1@ibm.com>
Date: Thu, 2 Aug 2018 17:51:06 -0700
Subject: [PATCH] ensure ScheduleDSPods respects tainted nodes

- add PodToleratesNodeTaints to nodeSelectionPredicates()
- add integration testcases
---
 pkg/controller/daemon/daemon_controller.go   | 15 ++++-
 test/integration/daemonset/daemonset_test.go | 66 ++++++++++++++++++++
 2 files changed, 78 insertions(+), 3 deletions(-)

diff --git a/pkg/controller/daemon/daemon_controller.go b/pkg/controller/daemon/daemon_controller.go
index b29977861f3..fde69a1f397 100644
--- a/pkg/controller/daemon/daemon_controller.go
+++ b/pkg/controller/daemon/daemon_controller.go
@@ -1423,11 +1423,12 @@ func NewPod(ds *apps.DaemonSet, nodeName string) *v1.Pod {
 	return newPod
 }
 
-// nodeSelectionPredicates runs a set of predicates that select candidate nodes for the DaemonSet;
+// checkNodeFitness runs a set of predicates that select candidate nodes for the DaemonSet;
 // the predicates include:
 //   - PodFitsHost: checks pod's NodeName against node
 //   - PodMatchNodeSelector: checks pod's NodeSelector and NodeAffinity against node
-func nodeSelectionPredicates(pod *v1.Pod, meta algorithm.PredicateMetadata, nodeInfo *schedulercache.NodeInfo) (bool, []algorithm.PredicateFailureReason, error) {
+//   - PodToleratesNodeTaints: exclude tainted node unless pod has specific toleration
+func checkNodeFitness(pod *v1.Pod, meta algorithm.PredicateMetadata, nodeInfo *schedulercache.NodeInfo) (bool, []algorithm.PredicateFailureReason, error) {
 	var predicateFails []algorithm.PredicateFailureReason
 	fit, reasons, err := predicates.PodFitsHost(pod, meta, nodeInfo)
 	if err != nil {
@@ -1444,6 +1445,14 @@ func nodeSelectionPredicates(pod *v1.Pod, meta algorithm.PredicateMetadata, node
 	if !fit {
 		predicateFails = append(predicateFails, reasons...)
 	}
+
+	fit, reasons, err = predicates.PodToleratesNodeTaints(pod, nil, nodeInfo)
+	if err != nil {
+		return false, predicateFails, err
+	}
+	if !fit {
+		predicateFails = append(predicateFails, reasons...)
+	}
 	return len(predicateFails) == 0, predicateFails, nil
 }
 
@@ -1454,7 +1463,7 @@ func Predicates(pod *v1.Pod, nodeInfo *schedulercache.NodeInfo) (bool, []algorit
 
 	// If ScheduleDaemonSetPods is enabled, only check nodeSelector and nodeAffinity.
 	if utilfeature.DefaultFeatureGate.Enabled(features.ScheduleDaemonSetPods) {
-		fit, reasons, err := nodeSelectionPredicates(pod, nil, nodeInfo)
+		fit, reasons, err := checkNodeFitness(pod, nil, nodeInfo)
 		if err != nil {
 			return false, predicateFails, err
 		}
diff --git a/test/integration/daemonset/daemonset_test.go b/test/integration/daemonset/daemonset_test.go
index fe63695de13..5ab3b0d63e9 100644
--- a/test/integration/daemonset/daemonset_test.go
+++ b/test/integration/daemonset/daemonset_test.go
@@ -897,3 +897,69 @@ func TestLaunchWithHashCollision(t *testing.T) {
 
 	validateDaemonSetCollisionCount(dsClient, ds.Name, orgCollisionCount+1, t)
 }
+
+// TestTaintedNode tests that no matter "ScheduleDaemonSetPods" feature is enabled or not
+// tainted node isn't expected to have pod scheduled
+func TestTaintedNode(t *testing.T) {
+	forEachFeatureGate(t, func(t *testing.T) {
+		forEachStrategy(t, func(t *testing.T, strategy *apps.DaemonSetUpdateStrategy) {
+			server, closeFn, dc, informers, clientset := setup(t)
+			defer closeFn()
+			ns := framework.CreateTestingNamespace("tainted-node", server, t)
+			defer framework.DeleteTestingNamespace(ns, server, t)
+
+			dsClient := clientset.AppsV1().DaemonSets(ns.Name)
+			podClient := clientset.CoreV1().Pods(ns.Name)
+			podInformer := informers.Core().V1().Pods().Informer()
+			nodeClient := clientset.CoreV1().Nodes()
+			stopCh := make(chan struct{})
+			defer close(stopCh)
+
+			informers.Start(stopCh)
+			go dc.Run(5, stopCh)
+
+			// Start Scheduler
+			setupScheduler(t, clientset, informers, stopCh)
+
+			ds := newDaemonSet("foo", ns.Name)
+			ds.Spec.UpdateStrategy = *strategy
+			ds, err := dsClient.Create(ds)
+			if err != nil {
+				t.Fatalf("Failed to create DaemonSet: %v", err)
+			}
+
+			defer cleanupDaemonSets(t, clientset, ds)
+
+			nodeWithTaint := newNode("node-with-taint", nil)
+			nodeWithTaint.Spec.Taints = []v1.Taint{{Key: "key1", Value: "val1", Effect: "NoSchedule"}}
+			_, err = nodeClient.Create(nodeWithTaint)
+			if err != nil {
+				t.Fatalf("Failed to create nodeWithTaint: %v", err)
+			}
+
+			nodeWithoutTaint := newNode("node-without-taint", nil)
+			_, err = nodeClient.Create(nodeWithoutTaint)
+			if err != nil {
+				t.Fatalf("Failed to create nodeWithoutTaint: %v", err)
+			}
+
+			validateDaemonSetPodsAndMarkReady(podClient, podInformer, 1, t)
+			validateDaemonSetStatus(dsClient, ds.Name, 1, t)
+
+			// remove taint from nodeWithTaint
+			nodeWithTaint, err = nodeClient.Get("node-with-taint", metav1.GetOptions{})
+			if err != nil {
+				t.Fatalf("Failed to retrieve nodeWithTaint: %v", err)
+			}
+			nodeWithTaintCopy := nodeWithTaint.DeepCopy()
+			nodeWithTaintCopy.Spec.Taints = []v1.Taint{}
+			_, err = nodeClient.Update(nodeWithTaintCopy)
+			if err != nil {
+				t.Fatalf("Failed to update nodeWithTaint: %v", err)
+			}
+
+			validateDaemonSetPodsAndMarkReady(podClient, podInformer, 2, t)
+			validateDaemonSetStatus(dsClient, ds.Name, 2, t)
+		})
+	})
+}