Merge pull request #42414 from lukaszo/ds-taint

Automatic merge from submit-queue (batch tested with PRs 42456, 42457, 42414, 42480, 42370)

Enque DaemonSet sync when node taints changed

Fixes #42398

 @kargakis @janetkuo @mdshuai PTAL

pkg/controller/daemon/daemoncontroller.go

@@ -85,6 +85,8 @@ type DaemonSetsController struct {
 
 	// To allow injection of syncDaemonSet for testing.
 	syncHandler func(dsKey string) error
+	// used for unit testing
+	enqueueDaemonSet func(ds *extensions.DaemonSet)
 	// A TTLCache of pod creates/deletes each ds expects to see
 	expectations controller.ControllerExpectationsInterface
 	// dsLister can list/get daemonsets from the shared informer's store
@@ -181,6 +183,7 @@ func NewDaemonSetsController(daemonSetInformer extensionsinformers.DaemonSetInfo
 	dsc.nodeLister = nodeInformer.Lister()
 
 	dsc.syncHandler = dsc.syncDaemonSet
+	dsc.enqueueDaemonSet = dsc.enqueue
 	dsc.lookupCache = controller.NewMatchingCache(lookupCacheSize)
 	return dsc
 }
@@ -248,7 +251,7 @@ func (dsc *DaemonSetsController) processNextWorkItem() bool {
 	return true
 }
 
-func (dsc *DaemonSetsController) enqueueDaemonSet(ds *extensions.DaemonSet) {
+func (dsc *DaemonSetsController) enqueue(ds *extensions.DaemonSet) {
 	key, err := controller.KeyFunc(ds)
 	if err != nil {
 		utilruntime.HandleError(fmt.Errorf("Couldn't get key for object %#v: %v", ds, err))
@@ -432,8 +435,8 @@ func (dsc *DaemonSetsController) addNode(obj interface{}) {
 func (dsc *DaemonSetsController) updateNode(old, cur interface{}) {
 	oldNode := old.(*v1.Node)
 	curNode := cur.(*v1.Node)
-	if reflect.DeepEqual(oldNode.Labels, curNode.Labels) {
-		// If node labels didn't change, we can ignore this update.
+	if reflect.DeepEqual(oldNode.Labels, curNode.Labels) && reflect.DeepEqual(oldNode.Spec.Taints, curNode.Spec.Taints) {
+		// If node labels and taints didn't change, we can ignore this update.
 		return
 	}
 	dsList, err := dsc.dsLister.List(labels.Everything())

pkg/controller/daemon/daemoncontroller_test.go

@@ -1086,3 +1086,68 @@ func TestNodeShouldRunDaemonPod(t *testing.T) {
 		}
 	}
 }
+
+// DaemonSets should be resynced when node labels or taints changed
+func TestUpdateNode(t *testing.T) {
+	var enqueued bool
+
+	cases := []struct {
+		test          string
+		newNode       *v1.Node
+		oldNode       *v1.Node
+		ds            *extensions.DaemonSet
+		shouldEnqueue bool
+	}{
+		{
+			test:    "Nothing changed, should not enqueue",
+			oldNode: newNode("node1", nil),
+			newNode: newNode("node1", nil),
+			ds: func() *extensions.DaemonSet {
+				ds := newDaemonSet("ds")
+				ds.Spec.Template.Spec.NodeSelector = simpleNodeLabel
+				return ds
+			}(),
+			shouldEnqueue: false,
+		},
+		{
+			test:    "Node labels changed",
+			oldNode: newNode("node1", nil),
+			newNode: newNode("node1", simpleNodeLabel),
+			ds: func() *extensions.DaemonSet {
+				ds := newDaemonSet("ds")
+				ds.Spec.Template.Spec.NodeSelector = simpleNodeLabel
+				return ds
+			}(),
+			shouldEnqueue: true,
+		},
+		{
+			test: "Node taints changed",
+			oldNode: func() *v1.Node {
+				node := newNode("node1", nil)
+				setNodeTaint(node, noScheduleTaints)
+				return node
+			}(),
+			newNode:       newNode("node1", nil),
+			ds:            newDaemonSet("ds"),
+			shouldEnqueue: true,
+		},
+	}
+	for _, c := range cases {
+		manager, podControl, _ := newTestController()
+		manager.nodeStore.Add(c.oldNode)
+		manager.dsStore.Add(c.ds)
+		syncAndValidateDaemonSets(t, manager, c.ds, podControl, 0, 0)
+
+		manager.enqueueDaemonSet = func(ds *extensions.DaemonSet) {
+			if ds.Name == "ds" {
+				enqueued = true
+			}
+		}
+
+		enqueued = false
+		manager.updateNode(c.oldNode, c.newNode)
+		if enqueued != c.shouldEnqueue {
+			t.Errorf("Test case: '%s', expected: %t, got: %t", c.test, c.shouldEnqueue, enqueued)
+		}
+	}
+}