NodeSelector change does not trigger reconcile (#186)

* NodeSelector change does not trigger reconcile

When `nodeSelector` is changed and it matches a new node, this node ignores it.

This issue was caused by reconcile predicate `UpdateFunc` callback depending on
matching the node with both the new and the old policy `nodeSelector`. This
caused `UpdateFunc` to not work properly.

Now we check our node only against the new (updated) policy `nodeSelector`.

Signed-off-by: HlinaCZ <[email protected]>
Signed-off-by: Petr Horacek <[email protected]>

pkg/controller/nodenetworkconfigurationpolicy/nodenetworkconfigurationpolicy_controller.go

@@ -76,8 +76,7 @@ func forThisNodePredicate(cl client.Client) predicate.Funcs {
 			return nodeSelectorMatchesThisNode(cl, deleteEvent.Object)
 		},
 		UpdateFunc: func(updateEvent event.UpdateEvent) bool {
-			return nodeSelectorMatchesThisNode(cl, updateEvent.ObjectOld) &&
-				nodeSelectorMatchesThisNode(cl, updateEvent.ObjectNew)
+			return nodeSelectorMatchesThisNode(cl, updateEvent.ObjectNew)
 		},
 		GenericFunc: func(genericEvent event.GenericEvent) bool {
 			return nodeSelectorMatchesThisNode(cl, genericEvent.Object)

test/e2e/node_selector_test.go

@@ -0,0 +1,61 @@
+package e2e
+
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	nmstatev1alpha1 "github.com/nmstate/kubernetes-nmstate/pkg/apis/nmstate/v1alpha1"
+)
+
+var _ = Describe("NodeSelector", func() {
+	br1Up := nmstatev1alpha1.State(`interfaces:
+  - name: br1
+    type: linux-bridge
+    state: up
+    bridge:
+      options:
+        stp:
+          enabled: false
+      port:
+      - name: eth1
+`)
+	br1Absent := nmstatev1alpha1.State(`interfaces:
+- name: br1
+  type: linux-bridge
+  state: absent
+`)
+	nonexistentNodeSelector := map[string]string{"nonexistentKey": "nonexistentValue"}
+
+	Context("when policy is set with node selector not matching any nodes", func() {
+		BeforeEach(func() {
+			setDesiredStateWithPolicyAndNodeSelector("br1", br1Up, nonexistentNodeSelector)
+		})
+
+		AfterEach(func() {
+			setDesiredStateWithPolicy("br1", br1Absent)
+			for _, node := range nodes {
+				interfacesNameForNode(node).ShouldNot(ContainElement("br1"))
+			}
+
+			deletePolicy("br1")
+		})
+
+		It("should not update any nodes", func() {
+			for _, node := range nodes {
+				interfacesNameForNode(node).ShouldNot(ContainElement("br1"))
+			}
+		})
+
+		Context("and we remove the node selector", func() {
+			BeforeEach(func() {
+				setDesiredStateWithPolicyAndNodeSelector("br1", br1Up, map[string]string{})
+			})
+
+			It("should update all nodes", func() {
+				for _, node := range nodes {
+					interfacesNameForNode(node).Should(ContainElement("br1"))
+				}
+			})
+		})
+	})
+})

test/e2e/utils.go

@@ -141,13 +141,14 @@ func waitForDaemonSet(t *testing.T, kubeclient kubernetes.Interface, namespace,
 	return nil
 }
 
-func setDesiredStateWithPolicy(name string, desiredState nmstatev1alpha1.State) {
+func setDesiredStateWithPolicyAndNodeSelector(name string, desiredState nmstatev1alpha1.State, nodeSelector map[string]string) {
 	policy := nmstatev1alpha1.NodeNetworkConfigurationPolicy{}
 	policy.Name = name
 	key := types.NamespacedName{Name: name}
 	Eventually(func() error {
 		err := framework.Global.Client.Get(context.TODO(), key, &policy)
 		policy.Spec.DesiredState = desiredState
+		policy.Spec.NodeSelector = nodeSelector
 		if err != nil {
 			if apierrors.IsNotFound(err) {
 				return framework.Global.Client.Create(context.TODO(), &policy, &framework.CleanupOptions{})
@@ -158,6 +159,10 @@ func setDesiredStateWithPolicy(name string, desiredState nmstatev1alpha1.State)
 	}, ReadTimeout, ReadInterval).ShouldNot(HaveOccurred())
 }
 
+func setDesiredStateWithPolicy(name string, desiredState nmstatev1alpha1.State) {
+	setDesiredStateWithPolicyAndNodeSelector(name, desiredState, map[string]string{})
+}
+
 func updateDesiredStateAtNode(node string, desiredState nmstatev1alpha1.State) {
 	key := types.NamespacedName{Name: node}
 	state := nmstatev1alpha1.NodeNetworkState{}