diff --git a/api/v1beta1/keystoneapi_types.go b/api/v1beta1/keystoneapi_types.go index a4c9b3cc..41a46ddc 100644 --- a/api/v1beta1/keystoneapi_types.go +++ b/api/v1beta1/keystoneapi_types.go @@ -138,7 +138,7 @@ type KeystoneAPISpecCore 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=false diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 7ee47492..98047671 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -160,9 +160,13 @@ func (in *KeystoneAPISpecCore) DeepCopyInto(out *KeystoneAPISpecCore) { out.PasswordSelectors = in.PasswordSelectors 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 + } } } if in.DefaultConfigOverwrite != nil { diff --git a/pkg/keystone/bootstrap.go b/pkg/keystone/bootstrap.go index 8748673d..ae99440d 100644 --- a/pkg/keystone/bootstrap.go +++ b/pkg/keystone/bootstrap.go @@ -117,8 +117,8 @@ func BootstrapJob( } job.Spec.Template.Spec.Containers[0].Env = env.MergeEnvs(job.Spec.Template.Spec.Containers[0].Env, envVars) - if len(instance.Spec.NodeSelector) > 0 { - job.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector + if instance.Spec.NodeSelector != nil { + job.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector } return job diff --git a/pkg/keystone/cronjob.go b/pkg/keystone/cronjob.go index 38a1051b..7036bac6 100644 --- a/pkg/keystone/cronjob.go +++ b/pkg/keystone/cronjob.go @@ -98,9 +98,8 @@ func CronJob( }, }, } - if len(instance.Spec.NodeSelector) > 0 { - cronjob.Spec.JobTemplate.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector + if instance.Spec.NodeSelector != nil { + cronjob.Spec.JobTemplate.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector } - return cronjob } diff --git a/pkg/keystone/dbsync.go b/pkg/keystone/dbsync.go index e760b4d6..cacfbb79 100644 --- a/pkg/keystone/dbsync.go +++ b/pkg/keystone/dbsync.go @@ -90,8 +90,8 @@ func DbSyncJob( }, } - if len(instance.Spec.NodeSelector) > 0 { - job.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector + if instance.Spec.NodeSelector != nil { + job.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector } return job diff --git a/pkg/keystone/deployment.go b/pkg/keystone/deployment.go index 00c418f4..c5fdecfd 100644 --- a/pkg/keystone/deployment.go +++ b/pkg/keystone/deployment.go @@ -158,8 +158,9 @@ func Deployment( }, corev1.LabelHostname, ) - if 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, nil diff --git a/tests/functional/keystoneapi_controller_test.go b/tests/functional/keystoneapi_controller_test.go index 905b0341..f236ba86 100644 --- a/tests/functional/keystoneapi_controller_test.go +++ b/tests/functional/keystoneapi_controller_test.go @@ -51,6 +51,7 @@ var _ = Describe("Keystone controller", func() { var internalCertSecretName types.NamespacedName var publicCertSecretName types.NamespacedName var memcachedSpec memcachedv1.MemcachedSpec + var cronJobName types.NamespacedName BeforeEach(func() { @@ -99,6 +100,10 @@ var _ = Describe("Keystone controller", func() { Replicas: ptr.To(int32(3)), }, } + cronJobName = types.NamespacedName{ + Namespace: keystoneAPIName.Namespace, + Name: "keystone-cron", + } err := os.Setenv("OPERATOR_TEMPLATES", "../../templates") Expect(err).NotTo(HaveOccurred()) @@ -609,11 +614,7 @@ var _ = Describe("Keystone controller", func() { }) It("should create a CronJob for trust flush", func() { - cronJob := types.NamespacedName{ - Namespace: keystoneAPIName.Namespace, - Name: fmt.Sprintf("%s-%s", keystoneAPIName.Name, "cron"), - } - GetCronJob(cronJob) + GetCronJob(cronJobName) }) It("should create a ConfigMap and Secret for client config", func() { @@ -1189,6 +1190,133 @@ var _ = Describe("Keystone controller", func() { }) }) + When("A KeystoneAPI is created with nodeSelector", func() { + BeforeEach(func() { + spec := GetDefaultKeystoneAPISpec() + spec["nodeSelector"] = map[string]interface{}{ + "foo": "bar", + } + DeferCleanup( + k8sClient.Delete, ctx, CreateKeystoneMessageBusSecret(namespace, "rabbitmq-secret")) + keystone := CreateKeystoneAPI(keystoneAPIName, spec) + DeferCleanup(th.DeleteInstance, keystone) + DeferCleanup( + k8sClient.Delete, ctx, CreateKeystoneAPISecret(namespace, SecretName)) + DeferCleanup(infra.DeleteMemcached, infra.CreateMemcached(namespace, "memcached", memcachedSpec)) + DeferCleanup( + mariadb.DeleteDBService, + mariadb.CreateDBService( + namespace, + GetKeystoneAPI(keystoneAPIName).Spec.DatabaseInstance, + corev1.ServiceSpec{ + Ports: []corev1.ServicePort{{Port: 3306}}, + }, + ), + ) + mariadb.SimulateMariaDBAccountCompleted(keystoneAccountName) + mariadb.SimulateMariaDBDatabaseCompleted(keystoneDatabaseName) + infra.SimulateTransportURLReady(types.NamespacedName{ + Name: fmt.Sprintf("%s-keystone-transport", keystoneAPIName.Name), + Namespace: namespace, + }) + infra.SimulateMemcachedReady(types.NamespacedName{ + Name: "memcached", + Namespace: namespace, + }) + th.SimulateJobSuccess(dbSyncJobName) + th.SimulateJobSuccess(bootstrapJobName) + 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"})) + g.Expect(th.GetJob(bootstrapJobName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(th.GetJob(dbSyncJobName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(GetCronJob(cronJobName).Spec.JobTemplate.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"})) + g.Expect(th.GetJob(bootstrapJobName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(th.GetJob(dbSyncJobName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(GetCronJob(cronJobName).Spec.JobTemplate.Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + keystone := GetKeystoneAPI(keystoneAPIName) + newNodeSelector := map[string]string{ + "foo2": "bar2", + } + keystone.Spec.NodeSelector = &newNodeSelector + g.Expect(k8sClient.Update(ctx, keystone)).Should(Succeed()) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + th.SimulateJobSuccess(dbSyncJobName) + th.SimulateJobSuccess(bootstrapJobName) + th.SimulateDeploymentReplicaReady(deploymentName) + g.Expect(th.GetDeployment(deploymentName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo2": "bar2"})) + g.Expect(th.GetJob(bootstrapJobName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo2": "bar2"})) + g.Expect(th.GetJob(dbSyncJobName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo2": "bar2"})) + g.Expect(GetCronJob(cronJobName).Spec.JobTemplate.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"})) + g.Expect(th.GetJob(bootstrapJobName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(th.GetJob(dbSyncJobName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(GetCronJob(cronJobName).Spec.JobTemplate.Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + keystone := GetKeystoneAPI(keystoneAPIName) + emptyNodeSelector := map[string]string{} + keystone.Spec.NodeSelector = &emptyNodeSelector + g.Expect(k8sClient.Update(ctx, keystone)).Should(Succeed()) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + th.SimulateJobSuccess(dbSyncJobName) + th.SimulateJobSuccess(bootstrapJobName) + th.SimulateDeploymentReplicaReady(deploymentName) + g.Expect(th.GetDeployment(deploymentName).Spec.Template.Spec.NodeSelector).To(BeNil()) + g.Expect(th.GetJob(bootstrapJobName).Spec.Template.Spec.NodeSelector).To(BeNil()) + g.Expect(th.GetJob(dbSyncJobName).Spec.Template.Spec.NodeSelector).To(BeNil()) + g.Expect(GetCronJob(cronJobName).Spec.JobTemplate.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"})) + g.Expect(th.GetJob(bootstrapJobName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(th.GetJob(dbSyncJobName).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(GetCronJob(cronJobName).Spec.JobTemplate.Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + keystone := GetKeystoneAPI(keystoneAPIName) + keystone.Spec.NodeSelector = nil + g.Expect(k8sClient.Update(ctx, keystone)).Should(Succeed()) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + th.SimulateJobSuccess(dbSyncJobName) + th.SimulateJobSuccess(bootstrapJobName) + th.SimulateDeploymentReplicaReady(deploymentName) + g.Expect(th.GetDeployment(deploymentName).Spec.Template.Spec.NodeSelector).To(BeNil()) + g.Expect(th.GetJob(bootstrapJobName).Spec.Template.Spec.NodeSelector).To(BeNil()) + g.Expect(th.GetJob(dbSyncJobName).Spec.Template.Spec.NodeSelector).To(BeNil()) + g.Expect(GetCronJob(cronJobName).Spec.JobTemplate.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.