Merge pull request #316 from openshift-cherrypick-robot/cherry-pick-3…

…12-to-18.0-fr1

[18.0-fr1] Ensure nodeSelector logic is consistent for all operators

apis/bases/instanceha.openstack.org_instancehas.yaml

@@ -77,8 +77,7 @@ spec:
                 additionalProperties:
                   type: string
                 description: NodeSelector to target subset of worker nodes running
-                  control plane services (currently only applies to KeystoneAPI and
-                  PlacementAPI)
+                  control plane services
                 type: object
               openStackCloud:
                 default: default

apis/bases/redis.openstack.org_redises.yaml

@@ -39,6 +39,12 @@ spec:
                 description: Name of the redis container image to run (will be set
                   to environmental default if empty)
                 type: string
+              nodeSelector:
+                additionalProperties:
+                  type: string
+                description: NodeSelector to target subset of worker nodes running
+                  this service
+                type: object
               replicas:
                 default: 1
                 description: Size of the redis cluster

apis/instanceha/v1beta1/instanceha_types.go

@@ -28,7 +28,7 @@ const (
 
 	// InstanceHaContainerImage is the fall-back container image for InstanceHa
 	InstanceHaContainerImage = "quay.io/podified-antelope-centos9/openstack-openstackclient:current-podified"
-	OpenStackCloud          = "default"
+	OpenStackCloud           = "default"
 )
 
 // InstanceHaSpec defines the desired state of InstanceHa
@@ -38,10 +38,10 @@ type InstanceHaSpec struct {
 	// ContainerImage for the the InstanceHa container (will be set to environmental default if empty)
 	ContainerImage string `json:"containerImage"`
 
-        // +kubebuilder:validation:Required
-        // +kubebuilder:default="default"
-        // OpenStackClould is the name of the Cloud to use as per clouds.yaml (will be set to "default" if empty)
-        OpenStackCloud string `json:"openStackCloud"`
+	// +kubebuilder:validation:Required
+	// +kubebuilder:default="default"
+	// OpenStackClould is the name of the Cloud to use as per clouds.yaml (will be set to "default" if empty)
+	OpenStackCloud string `json:"openStackCloud"`
 
 	// +kubebuilder:validation:Required
 	// +kubebuilder:default=openstack-config
@@ -68,8 +68,8 @@ type InstanceHaSpec struct {
 	InstanceHaKdumpPort int32 `json:"instanceHaKdumpPort"`
 
 	// +kubebuilder:validation:Optional
-	// NodeSelector to target subset of worker nodes running control plane services (currently only applies to KeystoneAPI and PlacementAPI)
-	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+	// NodeSelector to target subset of worker nodes running control plane services
+	NodeSelector *map[string]string `json:"nodeSelector,omitempty"`
 
 	// +kubebuilder:validation:Optional
 	// NetworkAttachments is a list of NetworkAttachment resource names to expose

apis/memcached/v1beta1/memcached_types.go

@@ -54,7 +54,7 @@ type MemcachedSpecCore 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
 	// +operator-sdk:csv:customresourcedefinitions:type=spec

apis/network/v1beta1/dnsmasq_types.go

@@ -69,7 +69,7 @@ type DNSMasqSpecCore struct {
 	// NodeSelector to target subset of worker nodes running this service. Setting
 	// NodeSelector here acts as a default value and can be overridden by service
 	// specific NodeSelector Settings.
-	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+	NodeSelector *map[string]string `json:"nodeSelector,omitempty"`
 
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default="dnsdata"

apis/redis/v1beta1/redis_types.go

@@ -29,10 +29,10 @@ const (
 	// RedisContainerImage is the fall-back container image for Redis
 	RedisContainerImage = "quay.io/podified-antelope-centos9/openstack-redis:current-podified"
 
-        // CrMaxLengthCorrection - DNS1123LabelMaxLength (63) - CrMaxLengthCorrection used in validation to
-        // omit issue with statefulset pod label "controller-revision-hash": "<statefulset_name>-<hash>"
-        // Int32 is a 10 character + hyphen = 11
-        CrMaxLengthCorrection = 11
+	// CrMaxLengthCorrection - DNS1123LabelMaxLength (63) - CrMaxLengthCorrection used in validation to
+	// omit issue with statefulset pod label "controller-revision-hash": "<statefulset_name>-<hash>"
+	// Int32 is a 10 character + hyphen = 11
+	CrMaxLengthCorrection = 11
 )
 
 // RedisSpec defines the desired state of Redis
@@ -54,6 +54,9 @@ type RedisSpecCore struct {
 	// +operator-sdk:csv:customresourcedefinitions:type=spec
 	// TLS settings for Redis service and internal Redis replication
 	TLS tls.SimpleService `json:"tls,omitempty"`
+	// +kubebuilder:validation:Optional
+	// NodeSelector to target subset of worker nodes running this service
+	NodeSelector *map[string]string `json:"nodeSelector,omitempty"`
 }
 
 // RedisStatus defines the observed state of Redis

config/crd/bases/instanceha.openstack.org_instancehas.yaml

@@ -77,8 +77,7 @@ spec:
                 additionalProperties:
                   type: string
                 description: NodeSelector to target subset of worker nodes running
-                  control plane services (currently only applies to KeystoneAPI and
-                  PlacementAPI)
+                  control plane services
                 type: object
               openStackCloud:
                 default: default

config/crd/bases/redis.openstack.org_redises.yaml

@@ -39,6 +39,12 @@ spec:
                 description: Name of the redis container image to run (will be set
                   to environmental default if empty)
                 type: string
+              nodeSelector:
+                additionalProperties:
+                  type: string
+                description: NodeSelector to target subset of worker nodes running
+                  this service
+                type: object
               replicas:
                 default: 1
                 description: Size of the redis cluster

pkg/dnsmasq/deployment.go

@@ -173,8 +173,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

pkg/instanceha/funcs.go

@@ -82,7 +82,6 @@ func Deployment(
 					ServiceAccountName:            instance.RbacResourceName(),
 					Volumes:                       volumes,
 					TerminationGracePeriodSeconds: ptr.To[int64](0),
-					NodeSelector:                  instance.Spec.NodeSelector,
 					Containers: []corev1.Container{{
 						Name:    "instanceha",
 						Image:   containerImage,
@@ -112,6 +111,10 @@ func Deployment(
 		},
 	}
 
+	if instance.Spec.NodeSelector != nil {
+		dep.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
+	}
+
 	return dep
 }
 

pkg/memcached/statefulset.go

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

pkg/redis/statefulset.go

@@ -141,5 +141,9 @@ func StatefulSet(
 		},
 	}
 
+	if r.Spec.NodeSelector != nil {
+		sts.Spec.Template.Spec.NodeSelector = *r.Spec.NodeSelector
+	}
+
 	return sts
 }

tests/functional/dnsmasq_controller_test.go

@@ -266,4 +266,110 @@ var _ = Describe("DNSMasq controller", func() {
 			})
 		})
 	})
+
+	When("A DNSMasq is created with nodeSelector", func() {
+		BeforeEach(func() {
+			spec := GetDefaultDNSMasqSpec()
+			spec["nodeSelector"] = map[string]interface{}{
+				"foo": "bar",
+			}
+			instance := CreateDNSMasq(namespace, spec)
+			dnsMasqName = types.NamespacedName{
+				Name:      instance.GetName(),
+				Namespace: namespace,
+			}
+			dnsMasqServiceAccountName = types.NamespacedName{
+				Namespace: namespace,
+				Name:      "dnsmasq-" + dnsMasqName.Name,
+			}
+			dnsMasqRoleName = types.NamespacedName{
+				Namespace: namespace,
+				Name:      dnsMasqServiceAccountName.Name + "-role",
+			}
+			dnsMasqRoleBindingName = types.NamespacedName{
+				Namespace: namespace,
+				Name:      dnsMasqServiceAccountName.Name + "-rolebinding",
+			}
+			deploymentName = types.NamespacedName{
+				Namespace: namespace,
+				Name:      "dnsmasq-" + dnsMasqName.Name,
+			}
+
+			dnsDataCM = types.NamespacedName{
+				Namespace: namespace,
+				Name:      "some-dnsdata",
+			}
+
+			th.CreateConfigMap(dnsDataCM, map[string]interface{}{
+				dnsDataCM.Name: "172.20.0.80 keystone-internal.openstack.svc",
+			})
+			cm := th.GetConfigMap(dnsDataCM)
+			cm.Labels = util.MergeStringMaps(cm.Labels, map[string]string{
+				"dnsmasqhosts": "dnsdata",
+			})
+			Expect(th.K8sClient.Update(ctx, cm)).Should(Succeed())
+
+			DeferCleanup(th.DeleteConfigMap, dnsDataCM)
+			DeferCleanup(th.DeleteInstance, instance)
+			th.SimulateLoadBalancerServiceIP(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) {
+				dnsmasq := GetDNSMasq(dnsMasqName)
+				newNodeSelector := map[string]string{
+					"foo2": "bar2",
+				}
+				dnsmasq.Spec.NodeSelector = &newNodeSelector
+				g.Expect(k8sClient.Update(ctx, dnsmasq)).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) {
+				dnsmasq := GetDNSMasq(dnsMasqName)
+				emptyNodeSelector := map[string]string{}
+				dnsmasq.Spec.NodeSelector = &emptyNodeSelector
+				g.Expect(k8sClient.Update(ctx, dnsmasq)).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) {
+				dnsmasq := GetDNSMasq(dnsMasqName)
+				dnsmasq.Spec.NodeSelector = nil
+				g.Expect(k8sClient.Update(ctx, dnsmasq)).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())
+		})
+	})
 })