[fix] fix ds controller deletes pod when not match RequiredDuringSchedulingIgnoredDuringExecution

Signed-off-by: xyz-li <[email protected]>

datadog:patch

Author: xyz-li

pkg/controller/daemon/daemon_controller.go

@@ -1289,8 +1289,14 @@ func NodeShouldRunDaemonPod(node *v1.Node, ds *apps.DaemonSet) (bool, bool) {
 	}
 
 	taints := node.Spec.Taints
-	fitsNodeName, fitsNodeAffinity, fitsTaints := predicates(pod, node, taints)
-	if !fitsNodeName || !fitsNodeAffinity {
+	fitsNodeName := len(pod.Spec.NodeName) == 0 || pod.Spec.NodeName == node.Name
+	if !fitsNodeName {
+		return false, false
+	}
+
+	fitsNodeName, fitsNodeSelector, fitsNodeAffinity, fitsTaints := predicates(pod, node, taints)
+
+	if !fitsNodeName || !fitsNodeSelector {
 		return false, false
 	}
 
@@ -1302,14 +1308,35 @@ func NodeShouldRunDaemonPod(node *v1.Node, ds *apps.DaemonSet) (bool, bool) {
 		return false, !hasUntoleratedTaint
 	}
 
+	if !fitsNodeAffinity {
+		// IgnoredDuringExecution means that if the node labels change after Kubernetes schedules the Pod, the Pod continues to run.
+		return false, true
+	}
+
 	return true, true
 }
 
 // predicates checks if a DaemonSet's pod can run on a node.
-func predicates(pod *v1.Pod, node *v1.Node, taints []v1.Taint) (fitsNodeName, fitsNodeAffinity, fitsTaints bool) {
+func predicates(pod *v1.Pod, node *v1.Node, taints []v1.Taint) (fitsNodeName, fitsNodeSelector, fitsNodeAffinity, fitsTaints bool) {
 	fitsNodeName = len(pod.Spec.NodeName) == 0 || pod.Spec.NodeName == node.Name
+
+	if len(pod.Spec.NodeSelector) > 0 {
+		selector := labels.SelectorFromSet(pod.Spec.NodeSelector)
+		fitsNodeSelector = selector.Matches(labels.Set(node.Labels))
+	} else {
+		fitsNodeSelector = true
+	}
+
+	if pod.Spec.Affinity != nil &&
+		pod.Spec.Affinity.NodeAffinity != nil &&
+		pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution != nil {
+		affinity := nodeaffinity.NewLazyErrorNodeSelector(pod.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution)
+		fitsNodeAffinity, _ = affinity.Match(node)
+	} else {
+		fitsNodeAffinity = true
+	}
+
 	// Ignore parsing errors for backwards compatibility.
-	fitsNodeAffinity, _ = nodeaffinity.GetRequiredNodeAffinity(pod).Match(node)
 	_, hasUntoleratedTaint := v1helper.FindMatchingUntoleratedTaint(taints, pod.Spec.Tolerations, func(t *v1.Taint) bool {
 		return t.Effect == v1.TaintEffectNoExecute || t.Effect == v1.TaintEffectNoSchedule
 	})

pkg/controller/daemon/daemon_controller_test.go

@@ -1640,6 +1640,52 @@ func TestNodeAffinityDaemonLaunchesPods(t *testing.T) {
 	}
 }
 
+// RequiredDuringSchedulingIgnoredDuringExecution means that if the node labels change after Kubernetes schedules the Pod, the Pod continues to run.
+func TestNodeAffinityAndChangeNodeLabels(t *testing.T) {
+	logger, _ := ktesting.NewTestContext(t)
+	for _, strategy := range updateStrategies() {
+		daemon := newDaemonSet("foo")
+		daemon.Spec.UpdateStrategy = *strategy
+		daemon.Spec.Template.Spec.Affinity = &v1.Affinity{
+			NodeAffinity: &v1.NodeAffinity{
+				RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{
+					NodeSelectorTerms: []v1.NodeSelectorTerm{
+						{
+							MatchExpressions: []v1.NodeSelectorRequirement{
+								{
+									Key:      "color",
+									Operator: v1.NodeSelectorOpIn,
+									Values:   []string{simpleNodeLabel["color"]},
+								},
+							},
+						},
+					},
+				},
+			},
+		}
+		_, ctx := ktesting.NewTestContext(t)
+
+		manager, podControl, _, err := newTestController(ctx, daemon)
+		if err != nil {
+			t.Fatalf("error creating DaemonSetsController: %v", err)
+		}
+		node1 := newNode("node-1", simpleNodeLabel)
+		node2 := newNode("node-2", simpleNodeLabel)
+		manager.nodeStore.Add(node1)
+		manager.nodeStore.Add(node2)
+		err = manager.dsStore.Add(daemon)
+		if err != nil {
+			t.Fatal(err)
+		}
+		expectSyncDaemonSets(t, manager, daemon, podControl, 2, 0, 0)
+		oldNode := node1.DeepCopy()
+		node1.Labels = nil
+		manager.updateNode(logger, oldNode, node1)
+		manager.nodeStore.Add(newNode("node-3", nil))
+		expectSyncDaemonSets(t, manager, daemon, podControl, 2, 0, 0)
+	}
+}
+
 func TestNumberReadyStatus(t *testing.T) {
 	for _, strategy := range updateStrategies() {
 		ds := newDaemonSet("foo")
@@ -2284,7 +2330,7 @@ func TestNodeShouldRunDaemonPod(t *testing.T) {
 			shouldContinueRunning: true,
 		},
 		{
-			predicateName: "ErrPodAffinityNotMatch",
+			predicateName: "PodAffinityNotMatchDuringExecution",
 			ds: &apps.DaemonSet{
 				Spec: apps.DaemonSetSpec{
 					Selector: &metav1.LabelSelector{MatchLabels: simpleDaemonSetLabel},
@@ -2315,7 +2361,7 @@ func TestNodeShouldRunDaemonPod(t *testing.T) {
 				},
 			},
 			shouldRun:             false,
-			shouldContinueRunning: false,
+			shouldContinueRunning: true,
 		},
 		{
 			predicateName: "ShouldRunDaemonPod",
@@ -2476,6 +2522,36 @@ func TestUpdateNode(t *testing.T) {
 				return 1
 			},
 		},
+		{
+			test:    "Node labels changed, ds with NodeAffinity ",
+			oldNode: newNode("node1", simpleNodeLabel),
+			newNode: newNode("node1", simpleNodeLabel2),
+			ds: func() *apps.DaemonSet {
+				ds := newDaemonSet("ds")
+				ds.Spec.Template.Spec.Affinity = &v1.Affinity{
+					NodeAffinity: &v1.NodeAffinity{
+						RequiredDuringSchedulingIgnoredDuringExecution: &v1.NodeSelector{
+							NodeSelectorTerms: []v1.NodeSelectorTerm{
+								{
+									MatchExpressions: []v1.NodeSelectorRequirement{
+										{
+											Key:      "color",
+											Operator: v1.NodeSelectorOpIn,
+											Values:   []string{"blue"},
+										},
+									},
+								},
+							},
+						},
+					},
+				}
+				return ds
+			}(),
+			shouldEnqueue: true,
+			expectedCreates: func() int {
+				return 1
+			},
+		},
 	}
 	for _, c := range cases {
 		for _, strategy := range updateStrategies() {