Change logic to prevent device-plugin daemonset spec change

Instead of adding a NodeSelectorRequirement per NodePolicy we add all the NodeSelectors to a single NodeSelectorRequirent so they can all be sorted. This will prevent that the daemonset's node selector changes when there are multiple NodePolicies applied. Signed-off-by: Alejandro Macedo <alex.macedopereira@gmail.com>
k8snetworkplumbingwg · Jul 19, 2024 · 79ff9d2 · 79ff9d2
1 parent 7e44c75
commit 79ff9d2
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 32 deletions.
diff --git a/controllers/helper.go b/controllers/helper.go
@@ -281,7 +281,7 @@ func setDsNodeAffinity(pl *sriovnetworkv1.SriovNetworkNodePolicyList, ds *appsv1
 }
 
 func nodeSelectorTermsForPolicyList(policies []sriovnetworkv1.SriovNetworkNodePolicy) []corev1.NodeSelectorTerm {
-	terms := []corev1.NodeSelectorTerm{}
+	expressions := []corev1.NodeSelectorRequirement{}
 	for _, p := range policies {
 		// Note(adrianc): default policy is deprecated and ignored.
 		if p.Name == constants.DefaultPolicyName {
@@ -291,7 +291,7 @@ func nodeSelectorTermsForPolicyList(policies []sriovnetworkv1.SriovNetworkNodePo
 		if len(p.Spec.NodeSelector) == 0 {
 			continue
 		}
-		expressions := []corev1.NodeSelectorRequirement{}
+
 		for k, v := range p.Spec.NodeSelector {
 			exp := corev1.NodeSelectorRequirement{
 				Operator: corev1.NodeSelectorOpIn,
@@ -300,15 +300,20 @@ func nodeSelectorTermsForPolicyList(policies []sriovnetworkv1.SriovNetworkNodePo
 			}
 			expressions = append(expressions, exp)
 		}
-		// sorting is needed to keep the daemon spec stable.
-		// the items are popped in a random order from the map
-		sort.Slice(expressions, func(i, j int) bool {
-			return expressions[i].Key < expressions[j].Key
-		})
-		nodeSelector := corev1.NodeSelectorTerm{
+	}
+
+	// sorting is needed to keep the daemon spec stable.
+	// the items are popped in a random order from the map
+	sort.Slice(expressions, func(i, j int) bool {
+		return expressions[i].Key < expressions[j].Key
+	})
+
+	terms := []corev1.NodeSelectorTerm{}
+	if len(expressions) > 0 {
+		term := corev1.NodeSelectorTerm{
 			MatchExpressions: expressions,
 		}
-		terms = append(terms, nodeSelector)
+		terms = append(terms, term)
 	}
 
 	return terms

diff --git a/controllers/helper_test.go b/controllers/helper_test.go
@@ -64,17 +64,13 @@ func TestNodeSelectorMerge(t *testing.T) {
 					MatchExpressions: []corev1.NodeSelectorRequirement{
 						{
 							Operator: corev1.NodeSelectorOpIn,
-							Key:      "foo",
-							Values:   []string{"bar"},
+							Key:      "bb",
+							Values:   []string{"cc"},
 						},
-					},
-				},
-				{
-					MatchExpressions: []corev1.NodeSelectorRequirement{
 						{
 							Operator: corev1.NodeSelectorOpIn,
-							Key:      "bb",
-							Values:   []string{"cc"},
+							Key:      "foo",
+							Values:   []string{"bar"},
 						},
 					},
 				},
@@ -102,20 +98,6 @@ func TestNodeSelectorMerge(t *testing.T) {
 				},
 			},
 			expected: []corev1.NodeSelectorTerm{
-				{
-					MatchExpressions: []corev1.NodeSelectorRequirement{
-						{
-							Operator: corev1.NodeSelectorOpIn,
-							Key:      "foo",
-							Values:   []string{"bar"},
-						},
-						{
-							Operator: corev1.NodeSelectorOpIn,
-							Key:      "foo1",
-							Values:   []string{"bar1"},
-						},
-					},
-				},
 				{
 					MatchExpressions: []corev1.NodeSelectorRequirement{
 						{
@@ -133,6 +115,16 @@ func TestNodeSelectorMerge(t *testing.T) {
 							Key:      "bb2",
 							Values:   []string{"cc2"},
 						},
+						{
+							Operator: corev1.NodeSelectorOpIn,
+							Key:      "foo",
+							Values:   []string{"bar"},
+						},
+						{
+							Operator: corev1.NodeSelectorOpIn,
+							Key:      "foo1",
+							Values:   []string{"bar1"},
+						},
 					},
 				},
 			},
@@ -254,6 +246,7 @@ var _ = Describe("Helper Validation", Ordered, func() {
 			Expect(in.Spec.Template.Spec.Affinity.NodeAffinity).ToNot(BeNil())
 			Expect(in.Spec.Template.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution).ToNot(BeNil())
 			Expect(len(in.Spec.Template.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms)).To(Equal(1))
+			Expect(len(in.Spec.Template.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions)).To(Equal(1))
 		})
 		It("should update affinity with multiple", func() {
 			pl := &sriovnetworkv1.SriovNetworkNodePolicyList{Items: []sriovnetworkv1.SriovNetworkNodePolicy{
@@ -285,7 +278,8 @@ var _ = Describe("Helper Validation", Ordered, func() {
 			Expect(in.Spec.Template.Spec.Affinity).ToNot(BeNil())
 			Expect(in.Spec.Template.Spec.Affinity.NodeAffinity).ToNot(BeNil())
 			Expect(in.Spec.Template.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution).ToNot(BeNil())
-			Expect(len(in.Spec.Template.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms)).To(Equal(2))
+			Expect(len(in.Spec.Template.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms)).To(Equal(1))
+			Expect(len(in.Spec.Template.Spec.Affinity.NodeAffinity.RequiredDuringSchedulingIgnoredDuringExecution.NodeSelectorTerms[0].MatchExpressions)).To(Equal(2))
 		})
 		It("should switch affinity", func() {
 			pl := &sriovnetworkv1.SriovNetworkNodePolicyList{Items: []sriovnetworkv1.SriovNetworkNodePolicy{