Merge pull request #187 from openshift-cherrypick-robot/cherry-pick-1…

…86-to-18.0-fr1

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

api/v1beta1/barbican_types.go

@@ -72,7 +72,7 @@ type BarbicanSpecBase struct {
 	// +kubebuilder:validation:Optional
 	// NodeSelector to target subset of worker nodes running this component. Setting here overrides
 	// any global NodeSelector settings within the Barbican CR.
-	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+	NodeSelector *map[string]string `json:"nodeSelector,omitempty"`
 
 	// +kubebuilder:validation:Optional
 	// CustomServiceConfig - customize the service config using this parameter to change service defaults,

api/v1beta1/common_types.go

@@ -58,7 +58,7 @@ type BarbicanComponentTemplate struct {
 	// +kubebuilder:validation:Optional
 	// NodeSelector to target subset of worker nodes running this component. Setting here overrides
 	// any global NodeSelector settings within the Barbican CR.
-	NodeSelector map[string]string `json:"nodeSelector,omitempty"`
+	NodeSelector *map[string]string `json:"nodeSelector,omitempty"`
 
 	// +kubebuilder:validation:Optional
 	// +kubebuilder:default=1

controllers/barbican_controller.go

@@ -681,8 +681,8 @@ func (r *BarbicanReconciler) apiDeploymentCreateOrUpdate(ctx context.Context, in
 
 	// If NodeSelector is not specified in BarbicanAPITemplate, the current
 	// API instance inherits the value from the top-level CR.
-	if apiSpec.BarbicanAPITemplate.BarbicanAPITemplateCore.BarbicanComponentTemplate.NodeSelector == nil {
-		apiSpec.BarbicanAPITemplate.BarbicanAPITemplateCore.BarbicanComponentTemplate.NodeSelector = instance.Spec.BarbicanSpecBase.NodeSelector
+	if apiSpec.NodeSelector == nil {
+		apiSpec.NodeSelector = instance.Spec.NodeSelector
 	}
 
 	deployment := &barbicanv1beta1.BarbicanAPI{
@@ -725,8 +725,8 @@ func (r *BarbicanReconciler) workerDeploymentCreateOrUpdate(ctx context.Context,
 
 	// If NodeSelector is not specified in BarbicanWorkerTemplate, the current
 	// Worker instance inherits the value from the top-level CR.
-	if workerSpec.BarbicanWorkerTemplate.BarbicanWorkerTemplateCore.BarbicanComponentTemplate.NodeSelector == nil {
-		workerSpec.BarbicanWorkerTemplate.BarbicanWorkerTemplateCore.BarbicanComponentTemplate.NodeSelector = instance.Spec.BarbicanSpecBase.NodeSelector
+	if workerSpec.NodeSelector == nil {
+		workerSpec.NodeSelector = instance.Spec.NodeSelector
 	}
 
 	deployment := &barbicanv1beta1.BarbicanWorker{
@@ -768,8 +768,8 @@ func (r *BarbicanReconciler) keystoneListenerDeploymentCreateOrUpdate(ctx contex
 
 	// If NodeSelector is not specified in BarbicanKeystoneListenerTemplate, the current
 	// KeystoneListener instance inherits the value from the top-level CR.
-	if keystoneListenerSpec.BarbicanKeystoneListenerTemplate.BarbicanKeystoneListenerTemplateCore.BarbicanComponentTemplate.NodeSelector == nil {
-		keystoneListenerSpec.BarbicanKeystoneListenerTemplate.BarbicanKeystoneListenerTemplateCore.BarbicanComponentTemplate.NodeSelector = instance.Spec.BarbicanSpecBase.NodeSelector
+	if keystoneListenerSpec.NodeSelector == nil {
+		keystoneListenerSpec.NodeSelector = instance.Spec.NodeSelector
 	}
 
 	deployment := &barbicanv1beta1.BarbicanKeystoneListener{

pkg/barbican/dbsync.go

@@ -114,5 +114,9 @@ func DbSyncJob(instance *barbicanv1beta1.Barbican, labels map[string]string, ann
 		dbSyncVolume...,
 	)
 
+	if instance.Spec.NodeSelector != nil {
+		job.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
+	}
+
 	return job
 }

pkg/barbicanapi/deployment.go

@@ -128,7 +128,6 @@ func Deployment(
 				},
 				Spec: corev1.PodSpec{
 					ServiceAccountName: instance.Spec.ServiceAccount,
-					NodeSelector:       instance.Spec.NodeSelector,
 					Containers: []corev1.Container{
 						{
 							Name: instance.Name + "-log",
@@ -183,5 +182,9 @@ func Deployment(
 		instance.Spec.CustomServiceConfigSecrets),
 		apiVolumes...)
 
+	if instance.Spec.NodeSelector != nil {
+		deployment.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
+	}
+
 	return deployment, nil
 }

pkg/barbicankeystonelistener/deployment.go

@@ -80,7 +80,6 @@ func Deployment(
 				},
 				Spec: corev1.PodSpec{
 					ServiceAccountName: instance.Spec.ServiceAccount,
-					NodeSelector:       instance.Spec.NodeSelector,
 					Containers: []corev1.Container{
 						{
 							Name: instance.Name + "-log",
@@ -129,5 +128,10 @@ func Deployment(
 		instance.Name,
 		instance.Spec.CustomServiceConfigSecrets),
 		keystoneListenerVolumes...)
+
+	if instance.Spec.NodeSelector != nil {
+		deployment.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
+	}
+
 	return deployment
 }

pkg/barbicanworker/deployment.go

@@ -104,7 +104,6 @@ func Deployment(
 				},
 				Spec: corev1.PodSpec{
 					ServiceAccountName: instance.Spec.ServiceAccount,
-					NodeSelector:       instance.Spec.NodeSelector,
 					Containers: []corev1.Container{
 						{
 							Name: instance.Name + "-log",
@@ -157,5 +156,10 @@ func Deployment(
 		instance.Name,
 		instance.Spec.CustomServiceConfigSecrets),
 		workerVolumes...)
+
+	if instance.Spec.NodeSelector != nil {
+		deployment.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector
+	}
+
 	return deployment
 }

tests/functional/barbican_controller_test.go

@@ -280,6 +280,148 @@ var _ = Describe("Barbican controller", func() {
 		})
 	})
 
+	When("A Barbican with nodeSelector is created", func() {
+		BeforeEach(func() {
+			spec := GetDefaultBarbicanSpec()
+			spec["nodeSelector"] = map[string]interface{}{
+				"foo": "bar",
+			}
+			spec["barbicanAPI"] = GetDefaultBarbicanAPISpec()
+			DeferCleanup(k8sClient.Delete, ctx, CreateBarbicanMessageBusSecret(barbicanTest.Instance.Namespace, "rabbitmq-secret"))
+			DeferCleanup(th.DeleteInstance, CreateBarbican(barbicanTest.Instance, spec))
+			DeferCleanup(k8sClient.Delete, ctx, CreateKeystoneAPISecret(barbicanTest.Instance.Namespace, SecretName))
+
+			DeferCleanup(
+				k8sClient.Delete, ctx, CreateBarbicanSecret(barbicanTest.Instance.Namespace, "test-osp-secret-barbican"))
+
+			DeferCleanup(
+				mariadb.DeleteDBService,
+				mariadb.CreateDBService(
+					barbicanTest.Instance.Namespace,
+					GetBarbican(barbicanTest.Instance).Spec.DatabaseInstance,
+					corev1.ServiceSpec{
+						Ports: []corev1.ServicePort{{Port: 3306}},
+					},
+				),
+			)
+			infra.SimulateTransportURLReady(barbicanTest.BarbicanTransportURL)
+			DeferCleanup(keystone.DeleteKeystoneAPI, keystone.CreateKeystoneAPI(barbicanTest.Instance.Namespace))
+			mariadb.SimulateMariaDBAccountCompleted(barbicanTest.BarbicanDatabaseAccount)
+			mariadb.SimulateMariaDBDatabaseCompleted(barbicanTest.BarbicanDatabaseName)
+			th.SimulateJobSuccess(barbicanTest.BarbicanDBSync)
+		})
+
+		It("sets nodeSelector in resource specs", func() {
+			Eventually(func(g Gomega) {
+				g.Expect(th.GetJob(barbicanTest.BarbicanDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+				g.Expect(th.GetDeployment(barbicanTest.BarbicanAPI).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.GetJob(barbicanTest.BarbicanDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+				g.Expect(th.GetDeployment(barbicanTest.BarbicanAPI).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				barbican := GetBarbican(barbicanName)
+				newNodeSelector := map[string]string{
+					"foo2": "bar2",
+				}
+				barbican.Spec.NodeSelector = &newNodeSelector
+				g.Expect(k8sClient.Update(ctx, barbican)).Should(Succeed())
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				th.SimulateJobSuccess(barbicanTest.BarbicanDBSync)
+				g.Expect(th.GetJob(barbicanTest.BarbicanDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo2": "bar2"}))
+				g.Expect(th.GetDeployment(barbicanTest.BarbicanAPI).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.GetJob(barbicanTest.BarbicanDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+				g.Expect(th.GetDeployment(barbicanTest.BarbicanAPI).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				barbican := GetBarbican(barbicanName)
+				emptyNodeSelector := map[string]string{}
+				barbican.Spec.NodeSelector = &emptyNodeSelector
+				g.Expect(k8sClient.Update(ctx, barbican)).Should(Succeed())
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				th.SimulateJobSuccess(barbicanTest.BarbicanDBSync)
+				g.Expect(th.GetJob(barbicanTest.BarbicanDBSync).Spec.Template.Spec.NodeSelector).To(BeNil())
+				g.Expect(th.GetDeployment(barbicanTest.BarbicanAPI).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.GetJob(barbicanTest.BarbicanDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+				g.Expect(th.GetDeployment(barbicanTest.BarbicanAPI).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				barbican := GetBarbican(barbicanName)
+				barbican.Spec.NodeSelector = nil
+				g.Expect(k8sClient.Update(ctx, barbican)).Should(Succeed())
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				th.SimulateJobSuccess(barbicanTest.BarbicanDBSync)
+				g.Expect(th.GetJob(barbicanTest.BarbicanDBSync).Spec.Template.Spec.NodeSelector).To(BeNil())
+				g.Expect(th.GetDeployment(barbicanTest.BarbicanAPI).Spec.Template.Spec.NodeSelector).To(BeNil())
+			}, timeout, interval).Should(Succeed())
+		})
+
+		It("allows nodeSelector service override", func() {
+			Eventually(func(g Gomega) {
+				g.Expect(th.GetJob(barbicanTest.BarbicanDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+				g.Expect(th.GetDeployment(barbicanTest.BarbicanAPI).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				barbican := GetBarbican(barbicanName)
+				apiNodeSelector := map[string]string{
+					"foo": "api",
+				}
+				barbican.Spec.BarbicanAPI.NodeSelector = &apiNodeSelector
+				g.Expect(k8sClient.Update(ctx, barbican)).Should(Succeed())
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				th.SimulateJobSuccess(barbicanTest.BarbicanDBSync)
+				g.Expect(th.GetJob(barbicanTest.BarbicanDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+				g.Expect(th.GetDeployment(barbicanTest.BarbicanAPI).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "api"}))
+			}, timeout, interval).Should(Succeed())
+		})
+
+		It("allows nodeSelector service override to empty", func() {
+			Eventually(func(g Gomega) {
+				g.Expect(th.GetJob(barbicanTest.BarbicanDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+				g.Expect(th.GetDeployment(barbicanTest.BarbicanAPI).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				barbican := GetBarbican(barbicanName)
+				emptyNodeSelector := map[string]string{}
+				barbican.Spec.BarbicanAPI.NodeSelector = &emptyNodeSelector
+				g.Expect(k8sClient.Update(ctx, barbican)).Should(Succeed())
+			}, timeout, interval).Should(Succeed())
+
+			Eventually(func(g Gomega) {
+				th.SimulateJobSuccess(barbicanTest.BarbicanDBSync)
+				g.Expect(th.GetJob(barbicanTest.BarbicanDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"}))
+				g.Expect(th.GetDeployment(barbicanTest.BarbicanAPI).Spec.Template.Spec.NodeSelector).To(BeNil())
+			}, timeout, interval).Should(Succeed())
+		})
+	})
+
 	// Run MariaDBAccount suite tests.  these are pre-packaged ginkgo tests
 	// that exercise standard account create / update patterns that should be
 	// common to all controllers that ensure MariaDBAccount CRs.