diff --git a/api/v1beta1/ovncontroller_types.go b/api/v1beta1/ovncontroller_types.go index 5e615a0b..cdb3fafd 100644 --- a/api/v1beta1/ovncontroller_types.go +++ b/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. diff --git a/api/v1beta1/ovndbcluster_types.go b/api/v1beta1/ovndbcluster_types.go index af8287cc..83ef7d48 100644 --- a/api/v1beta1/ovndbcluster_types.go +++ b/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 diff --git a/api/v1beta1/ovnnorthd_types.go b/api/v1beta1/ovnnorthd_types.go index 5222692d..54db7894 100644 --- a/api/v1beta1/ovnnorthd_types.go +++ b/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 diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 71d7a349..6f5b48a4 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -131,9 +131,13 @@ func (in *OVNControllerSpecCore) DeepCopyInto(out *OVNControllerSpecCore) { in.Resources.DeepCopyInto(&out.Resources) if in.NodeSelector != nil { in, out := &in.NodeSelector, &out.NodeSelector - *out = make(map[string]string, len(*in)) - for key, val := range *in { - (*out)[key] = val + *out = new(map[string]string) + if **in != nil { + in, out := *in, *out + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } } } in.TLS.DeepCopyInto(&out.TLS) @@ -313,9 +317,13 @@ func (in *OVNDBClusterSpecCore) DeepCopyInto(out *OVNDBClusterSpecCore) { } if in.NodeSelector != nil { in, out := &in.NodeSelector, &out.NodeSelector - *out = make(map[string]string, len(*in)) - for key, val := range *in { - (*out)[key] = val + *out = new(map[string]string) + if **in != nil { + in, out := *in, *out + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } } } in.Resources.DeepCopyInto(&out.Resources) @@ -477,9 +485,13 @@ func (in *OVNNorthdSpecCore) DeepCopyInto(out *OVNNorthdSpecCore) { } if in.NodeSelector != nil { in, out := &in.NodeSelector, &out.NodeSelector - *out = make(map[string]string, len(*in)) - for key, val := range *in { - (*out)[key] = val + *out = new(map[string]string) + if **in != nil { + in, out := *in, *out + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } } } in.Resources.DeepCopyInto(&out.Resources) diff --git a/pkg/ovncontroller/configjob.go b/pkg/ovncontroller/configjob.go index 6eb91e79..1914d55a 100644 --- a/pkg/ovncontroller/configjob.go +++ b/pkg/ovncontroller/configjob.go @@ -99,6 +99,7 @@ func ConfigJob( }, Volumes: GetOVNControllerVolumes(instance.Name, instance.Namespace), NodeName: ovnPod.Spec.NodeName, + // ^ NodeSelector not required }, }, }, diff --git a/pkg/ovncontroller/daemonset.go b/pkg/ovncontroller/daemonset.go index f3be1839..06ec0ccf 100644 --- a/pkg/ovncontroller/daemonset.go +++ b/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 { diff --git a/pkg/ovndbcluster/statefulset.go b/pkg/ovndbcluster/statefulset.go index 0e7df4f7..620c2972 100644 --- a/pkg/ovndbcluster/statefulset.go +++ b/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 diff --git a/pkg/ovnnorthd/deployment.go b/pkg/ovnnorthd/deployment.go index 2f20a7fa..a4a330b2 100644 --- a/pkg/ovnnorthd/deployment.go +++ b/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 diff --git a/tests/functional/ovncontroller_controller_test.go b/tests/functional/ovncontroller_controller_test.go index 47df4162..9dcabca8 100644 --- a/tests/functional/ovncontroller_controller_test.go +++ b/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()) + }) + }) }) diff --git a/tests/functional/ovndbcluster_controller_test.go b/tests/functional/ovndbcluster_controller_test.go index c6454d8c..90d2a251 100644 --- a/tests/functional/ovndbcluster_controller_test.go +++ b/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 diff --git a/tests/functional/ovnnorthd_controller_test.go b/tests/functional/ovnnorthd_controller_test.go index 94016e44..5bc90c60 100644 --- a/tests/functional/ovnnorthd_controller_test.go +++ b/tests/functional/ovnnorthd_controller_test.go @@ -129,6 +129,86 @@ var _ = Describe("OVNNorthd controller", func() { }) }) + When("OVNNorthd is created with nodeSelector", func() { + var ovnNorthdName types.NamespacedName + var deploymentName types.NamespacedName + + BeforeEach(func() { + dbs := CreateOVNDBClusters(namespace, map[string][]string{}, 1) + DeferCleanup(DeleteOVNDBClusters, dbs) + spec := GetDefaultOVNNorthdSpec() + nodeSelector := map[string]string{ + "foo": "bar", + } + spec.NodeSelector = &nodeSelector + ovnNorthdName = ovn.CreateOVNNorthd(namespace, spec) + DeferCleanup(ovn.DeleteOVNNorthd, ovnNorthdName) + deploymentName = types.NamespacedName{ + Namespace: namespace, + Name: "ovn-northd", + } + th.SimulateDeploymentReplicaReady(deploymentName) + }) + + It("sets nodeSelector in resource specs", func() { + Eventually(func(g Gomega) { + g.Expect(th.GetDeployment(deploymentName).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.GetDeployment(deploymentName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + northd := ovn.GetOVNNorthd(ovnNorthdName) + newNodeSelector := map[string]string{ + "foo2": "bar2", + } + northd.Spec.NodeSelector = &newNodeSelector + g.Expect(k8sClient.Update(ctx, northd)).Should(Succeed()) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + g.Expect(th.GetDeployment(deploymentName).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.GetDeployment(deploymentName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + northd := ovn.GetOVNNorthd(ovnNorthdName) + emptyNodeSelector := map[string]string{} + northd.Spec.NodeSelector = &emptyNodeSelector + g.Expect(k8sClient.Update(ctx, northd)).Should(Succeed()) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + g.Expect(th.GetDeployment(deploymentName).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.GetDeployment(deploymentName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + northd := ovn.GetOVNNorthd(ovnNorthdName) + northd.Spec.NodeSelector = nil + g.Expect(k8sClient.Update(ctx, northd)).Should(Succeed()) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + g.Expect(th.GetDeployment(deploymentName).Spec.Template.Spec.NodeSelector).To(BeNil()) + }, timeout, interval).Should(Succeed()) + }) + }) + When("OVNNorthd is created with TLS", func() { var ovnNorthdName types.NamespacedName