Merge pull request #383 from olliewalsh/node_selectors

Ensure nodeSelector logic is consistent for all operators

api/v1beta1/ovncontroller_types.go

@@ -74,7 +74,7 @@ type OVNControllerSpecCore struct {
 
 	// +kubebuilder:validation:Optional
 	// NodeSelector to target subset of worker nodes running this service
-	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+	NodeSelector *map[string]string `json:"nodeSelector,omitempty"`
 
 	// +kubebuilder:validation:Optional
 	// NetworkAttachment is a NetworkAttachment resource name to expose the service to the given network.

api/v1beta1/ovndbcluster_types.go

@@ -79,7 +79,7 @@ type OVNDBClusterSpecCore struct {
 
 	// +kubebuilder:validation:Optional
 	// NodeSelector to target subset of worker nodes running this service
-	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+	NodeSelector *map[string]string `json:"nodeSelector,omitempty"`
 
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default=info

api/v1beta1/ovnnorthd_types.go

@@ -56,7 +56,7 @@ type OVNNorthdSpecCore struct {
 
 	// +kubebuilder:validation:Optional
 	// NodeSelector to target subset of worker nodes running this service
-	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+	NodeSelector *map[string]string `json:"nodeSelector,omitempty"`
 
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default=info

pkg/ovncontroller/configjob.go

@@ -99,6 +99,7 @@ func ConfigJob(
 							},
 							Volumes:  GetOVNControllerVolumes(instance.Name, instance.Namespace),
 							NodeName: ovnPod.Spec.NodeName,
+							// ^ NodeSelector not required
 						},
 					},
 				},

pkg/ovncontroller/daemonset.go

@@ -146,8 +146,8 @@ func CreateOVNDaemonSet(
 		},
 	}
 
-	if instance.Spec.NodeSelector != nil && len(instance.Spec.NodeSelector) > 0 {
-		daemonset.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector
+	if instance.Spec.NodeSelector != nil {
+		daemonset.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
 	}
 
 	return daemonset
@@ -317,8 +317,8 @@ func CreateOVSDaemonSet(
 		},
 	}
 
-	if instance.Spec.NodeSelector != nil && len(instance.Spec.NodeSelector) > 0 {
-		daemonset.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector
+	if instance.Spec.NodeSelector != nil {
+		daemonset.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
 	}
 
 	if len(annotations) > 0 {

pkg/ovndbcluster/statefulset.go

@@ -219,8 +219,8 @@ func StatefulSet(
 		},
 		corev1.LabelHostname,
 	)
-	if instance.Spec.NodeSelector != nil && len(instance.Spec.NodeSelector) > 0 {
-		statefulset.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector
+	if instance.Spec.NodeSelector != nil {
+		statefulset.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
 	}
 
 	return statefulset

pkg/ovnnorthd/deployment.go

@@ -150,8 +150,8 @@ func Deployment(
 		},
 		corev1.LabelHostname,
 	)
-	if instance.Spec.NodeSelector != nil && len(instance.Spec.NodeSelector) > 0 {
-		deployment.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector
+	if instance.Spec.NodeSelector != nil {
+		deployment.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
 	}
 
 	return deployment

tests/functional/ovncontroller_controller_test.go

@@ -1085,4 +1085,104 @@ var _ = Describe("OVNController controller", func() {
 			}, timeout, interval).Should(Succeed())
 		})
 	})
+
+	When("OVNController is created with nodeSelector", func() {
+		var ovnControllerName types.NamespacedName
+		var daemonSetName types.NamespacedName
+		var daemonSetNameOVS types.NamespacedName
+
+		BeforeEach(func() {
+			dbs := CreateOVNDBClusters(namespace, map[string][]string{}, 1)
+			DeferCleanup(DeleteOVNDBClusters, dbs)
+
+			spec := GetDefaultOVNControllerSpec()
+			nodeSelector := map[string]string{
+				"foo": "bar",
+			}
+			spec.NodeSelector = &nodeSelector
+			instance := CreateOVNController(namespace, spec)
+			DeferCleanup(th.DeleteInstance, instance)
+
+			ovnControllerName = types.NamespacedName{Name: instance.GetName(), Namespace: instance.GetNamespace()}
+
+			daemonSetName = types.NamespacedName{
+				Namespace: namespace,
+				Name:      "ovn-controller",
+			}
+
+			SimulateDaemonsetNumberReady(daemonSetName)
+
+			daemonSetNameOVS = types.NamespacedName{
+				Namespace: namespace,
+				Name:      "ovn-controller-ovs",
+			}
+
+			SimulateDaemonsetNumberReady(daemonSetNameOVS)
+		})
+
+		It("sets nodeSelector in resource specs", func() {
+			Eventually(func(g Gomega) {
+				g.Expect(GetDaemonSet(daemonSetName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+				g.Expect(GetDaemonSet(daemonSetNameOVS).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+			}, timeout, interval).Should(Succeed())
+		})
+
+		It("updates nodeSelector in resource specs when changed", func() {
+			Eventually(func(g Gomega) {
+				g.Expect(GetDaemonSet(daemonSetName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+				g.Expect(GetDaemonSet(daemonSetNameOVS).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				ovnController := GetOVNController(ovnControllerName)
+				newNodeSelector := map[string]string{
+					"foo2": "bar2",
+				}
+				ovnController.Spec.NodeSelector = &newNodeSelector
+				g.Expect(k8sClient.Update(ctx, ovnController)).Should(Succeed())
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				g.Expect(GetDaemonSet(daemonSetName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo2": "bar2"}))
+				g.Expect(GetDaemonSet(daemonSetNameOVS).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo2": "bar2"}))
+			}, timeout, interval).Should(Succeed())
+		})
+
+		It("removes nodeSelector from resource specs when cleared", func() {
+			Eventually(func(g Gomega) {
+				g.Expect(GetDaemonSet(daemonSetName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+				g.Expect(GetDaemonSet(daemonSetNameOVS).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				ovnController := GetOVNController(ovnControllerName)
+				emptyNodeSelector := map[string]string{}
+				ovnController.Spec.NodeSelector = &emptyNodeSelector
+				g.Expect(k8sClient.Update(ctx, ovnController)).Should(Succeed())
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				g.Expect(GetDaemonSet(daemonSetName).Spec.Template.Spec.NodeSelector).To(BeNil())
+				g.Expect(GetDaemonSet(daemonSetNameOVS).Spec.Template.Spec.NodeSelector).To(BeNil())
+			}, timeout, interval).Should(Succeed())
+		})
+
+		It("removes nodeSelector from resource specs when nilled", func() {
+			Eventually(func(g Gomega) {
+				g.Expect(GetDaemonSet(daemonSetName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+				g.Expect(GetDaemonSet(daemonSetNameOVS).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				ovnController := GetOVNController(ovnControllerName)
+				ovnController.Spec.NodeSelector = nil
+				g.Expect(k8sClient.Update(ctx, ovnController)).Should(Succeed())
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				g.Expect(GetDaemonSet(daemonSetName).Spec.Template.Spec.NodeSelector).To(BeNil())
+				g.Expect(GetDaemonSet(daemonSetNameOVS).Spec.Template.Spec.NodeSelector).To(BeNil())
+			}, timeout, interval).Should(Succeed())
+		})
+	})
 })

tests/functional/ovndbcluster_controller_test.go

@@ -222,6 +222,86 @@ var _ = Describe("OVNDBCluster controller", func() {
 		})
 	})
 
+	When("A OVNDBCluster instance is created with nodeSelector", func() {
+		var OVNDBClusterName types.NamespacedName
+		var statefulSetName types.NamespacedName
+
+		BeforeEach(func() {
+			spec := GetDefaultOVNDBClusterSpec()
+			nodeSelector := map[string]string{
+				"foo": "bar",
+			}
+			spec.NodeSelector = &nodeSelector
+			instance := CreateOVNDBCluster(namespace, spec)
+			OVNDBClusterName = types.NamespacedName{Name: instance.GetName(), Namespace: instance.GetNamespace()}
+			DeferCleanup(th.DeleteInstance, instance)
+
+			statefulSetName = types.NamespacedName{
+				Namespace: namespace,
+				Name:      "ovsdbserver-nb",
+			}
+			th.SimulateStatefulSetReplicaReady(statefulSetName)
+		})
+
+		It("sets nodeSelector in resource specs", func() {
+			Eventually(func(g Gomega) {
+				g.Expect(th.GetStatefulSet(statefulSetName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+			}, timeout, interval).Should(Succeed())
+		})
+
+		It("updates nodeSelector in resource specs when changed", func() {
+			Eventually(func(g Gomega) {
+				g.Expect(th.GetStatefulSet(statefulSetName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				dbCluster := GetOVNDBCluster(OVNDBClusterName)
+				newNodeSelector := map[string]string{
+					"foo2": "bar2",
+				}
+				dbCluster.Spec.NodeSelector = &newNodeSelector
+				g.Expect(k8sClient.Update(ctx, dbCluster)).Should(Succeed())
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				g.Expect(th.GetStatefulSet(statefulSetName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo2": "bar2"}))
+			}, timeout, interval).Should(Succeed())
+		})
+
+		It("removes nodeSelector from resource specs when cleared", func() {
+			Eventually(func(g Gomega) {
+				g.Expect(th.GetStatefulSet(statefulSetName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				dbCluster := GetOVNDBCluster(OVNDBClusterName)
+				emptyNodeSelector := map[string]string{}
+				dbCluster.Spec.NodeSelector = &emptyNodeSelector
+				g.Expect(k8sClient.Update(ctx, dbCluster)).Should(Succeed())
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				g.Expect(th.GetStatefulSet(statefulSetName).Spec.Template.Spec.NodeSelector).To(BeNil())
+			}, timeout, interval).Should(Succeed())
+		})
+
+		It("removes nodeSelector from resource specs when nilled", func() {
+			Eventually(func(g Gomega) {
+				g.Expect(th.GetStatefulSet(statefulSetName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				dbCluster := GetOVNDBCluster(OVNDBClusterName)
+				dbCluster.Spec.NodeSelector = nil
+				g.Expect(k8sClient.Update(ctx, dbCluster)).Should(Succeed())
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				g.Expect(th.GetStatefulSet(statefulSetName).Spec.Template.Spec.NodeSelector).To(BeNil())
+			}, timeout, interval).Should(Succeed())
+		})
+	})
+
 	When("OVNDBClusters are created with networkAttachments", func() {
 		It("does not break if pods are not created yet", func() {
 			// Create OVNDBCluster with 1 replica