From 98f7e6ef9dd5b6eda31bcba801a0f85a263a1aae Mon Sep 17 00:00:00 2001 From: Oliver Walsh Date: Fri, 8 Nov 2024 23:42:33 +0000 Subject: [PATCH 1/3] Set nodeSelector on jobs and allow empty nodeSelector Switch to a pointer for nodeSelector to allow different logic for empty vs unset --- api/v1beta1/cinder_types.go | 2 +- api/v1beta1/common_types.go | 2 +- api/v1beta1/zz_generated.deepcopy.go | 20 ++++++++++++------ controllers/cinder_controller.go | 31 ++++++++++++++-------------- pkg/cinder/cronjob.go | 6 ++++-- pkg/cinder/dbsync.go | 4 ++++ pkg/cinderapi/statefuleset.go | 9 +++++--- pkg/cinderbackup/statefulset.go | 9 +++++--- pkg/cinderscheduler/statefulset.go | 9 +++++--- pkg/cindervolume/statefulset.go | 9 +++++--- 10 files changed, 64 insertions(+), 37 deletions(-) diff --git a/api/v1beta1/cinder_types.go b/api/v1beta1/cinder_types.go index 2a41b015..baac5880 100644 --- a/api/v1beta1/cinder_types.go +++ b/api/v1beta1/cinder_types.go @@ -93,7 +93,7 @@ type CinderSpecBase 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 // DBPurge parameters - diff --git a/api/v1beta1/common_types.go b/api/v1beta1/common_types.go index 0a122e03..7e99ae2e 100644 --- a/api/v1beta1/common_types.go +++ b/api/v1beta1/common_types.go @@ -49,7 +49,7 @@ type CinderServiceTemplate struct { // +kubebuilder:validation:Optional // NodeSelector to target subset of worker nodes running this service. Setting here overrides // any global NodeSelector settings within the Cinder 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, diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index e9fc1f73..2164887c 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -671,9 +671,13 @@ func (in *CinderServiceTemplate) DeepCopyInto(out *CinderServiceTemplate) { *out = *in 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.CustomServiceConfigSecrets != nil { @@ -738,9 +742,13 @@ func (in *CinderSpecBase) DeepCopyInto(out *CinderSpecBase) { } 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 + } } } out.DBPurge = in.DBPurge diff --git a/controllers/cinder_controller.go b/controllers/cinder_controller.go index 119da0c6..4b1a1bd2 100644 --- a/controllers/cinder_controller.go +++ b/controllers/cinder_controller.go @@ -1014,6 +1014,10 @@ func (r *CinderReconciler) apiDeploymentCreateOrUpdate(ctx context.Context, inst ServiceAccount: instance.RbacResourceName(), } + if cinderAPISpec.NodeSelector == nil { + cinderAPISpec.NodeSelector = instance.Spec.NodeSelector + } + deployment := &cinderv1beta1.CinderAPI{ ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("%s-api", instance.Name), @@ -1023,9 +1027,6 @@ func (r *CinderReconciler) apiDeploymentCreateOrUpdate(ctx context.Context, inst op, err := controllerutil.CreateOrUpdate(ctx, r.Client, deployment, func() error { deployment.Spec = cinderAPISpec - if len(deployment.Spec.NodeSelector) == 0 { - deployment.Spec.NodeSelector = instance.Spec.NodeSelector - } err := controllerutil.SetControllerReference(instance, deployment, r.Scheme) if err != nil { @@ -1049,6 +1050,10 @@ func (r *CinderReconciler) schedulerDeploymentCreateOrUpdate(ctx context.Context TLS: instance.Spec.CinderAPI.TLS.Ca, } + if cinderSchedulerSpec.NodeSelector == nil { + cinderSchedulerSpec.NodeSelector = instance.Spec.NodeSelector + } + deployment := &cinderv1beta1.CinderScheduler{ ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("%s-scheduler", instance.Name), @@ -1058,9 +1063,6 @@ func (r *CinderReconciler) schedulerDeploymentCreateOrUpdate(ctx context.Context op, err := controllerutil.CreateOrUpdate(ctx, r.Client, deployment, func() error { deployment.Spec = cinderSchedulerSpec - if len(deployment.Spec.NodeSelector) == 0 { - deployment.Spec.NodeSelector = instance.Spec.NodeSelector - } err := controllerutil.SetControllerReference(instance, deployment, r.Scheme) if err != nil { @@ -1084,6 +1086,10 @@ func (r *CinderReconciler) backupDeploymentCreateOrUpdate(ctx context.Context, i TLS: instance.Spec.CinderAPI.TLS.Ca, } + if cinderBackupSpec.NodeSelector == nil { + cinderBackupSpec.NodeSelector = instance.Spec.NodeSelector + } + deployment := &cinderv1beta1.CinderBackup{ ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("%s-backup", instance.Name), @@ -1093,9 +1099,6 @@ func (r *CinderReconciler) backupDeploymentCreateOrUpdate(ctx context.Context, i op, err := controllerutil.CreateOrUpdate(ctx, r.Client, deployment, func() error { deployment.Spec = cinderBackupSpec - if len(deployment.Spec.NodeSelector) == 0 { - deployment.Spec.NodeSelector = instance.Spec.NodeSelector - } err := controllerutil.SetControllerReference(instance, deployment, r.Scheme) if err != nil { @@ -1146,6 +1149,10 @@ func (r *CinderReconciler) volumeDeploymentCreateOrUpdate(ctx context.Context, i TLS: instance.Spec.CinderAPI.TLS.Ca, } + if cinderVolumeSpec.CinderVolumeTemplate.NodeSelector == nil { + cinderVolumeSpec.CinderVolumeTemplate.NodeSelector = instance.Spec.NodeSelector + } + deployment := &cinderv1beta1.CinderVolume{ ObjectMeta: metav1.ObjectMeta{ Name: fmt.Sprintf("%s-volume-%s", instance.Name, name), @@ -1157,12 +1164,6 @@ func (r *CinderReconciler) volumeDeploymentCreateOrUpdate(ctx context.Context, i op, err := controllerutil.CreateOrUpdate(ctx, r.Client, deployment, func() error { deployment.Spec = cinderVolumeSpec - // If NodeSelector is not specified in volumeTemplate, the current - // cinder-volume instance inherits the value from the top-level CR - if len(volTemplate.NodeSelector) == 0 { - deployment.Spec.NodeSelector = instance.Spec.NodeSelector - } - err := controllerutil.SetControllerReference(instance, deployment, r.Scheme) if err != nil { return err diff --git a/pkg/cinder/cronjob.go b/pkg/cinder/cronjob.go index c39bd504..e358cf77 100644 --- a/pkg/cinder/cronjob.go +++ b/pkg/cinder/cronjob.go @@ -134,8 +134,10 @@ func CronJob( }, }, } - if instance.Spec.NodeSelector != nil && len(instance.Spec.NodeSelector) > 0 { - cronjob.Spec.JobTemplate.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector + + if instance.Spec.NodeSelector != nil && len(*instance.Spec.NodeSelector) > 0 { + cronjob.Spec.JobTemplate.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector } + return cronjob } diff --git a/pkg/cinder/dbsync.go b/pkg/cinder/dbsync.go index 2844b188..b124324d 100644 --- a/pkg/cinder/dbsync.go +++ b/pkg/cinder/dbsync.go @@ -115,5 +115,9 @@ func DbSyncJob(instance *cinderv1beta1.Cinder, labels map[string]string, annotat }, } + if instance.Spec.NodeSelector != nil && len(*instance.Spec.NodeSelector) > 0 { + job.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector + } + return job } diff --git a/pkg/cinderapi/statefuleset.go b/pkg/cinderapi/statefuleset.go index bde646ca..69554b0d 100644 --- a/pkg/cinderapi/statefuleset.go +++ b/pkg/cinderapi/statefuleset.go @@ -165,13 +165,16 @@ func StatefulSet( LivenessProbe: livenessProbe, }, }, - Affinity: cinder.GetPodAffinity(ComponentName), - NodeSelector: instance.Spec.NodeSelector, - Volumes: volumes, + Affinity: cinder.GetPodAffinity(ComponentName), + Volumes: volumes, }, }, }, } + if instance.Spec.NodeSelector != nil && len(*instance.Spec.NodeSelector) > 0 { + statefulset.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector + } + return statefulset, nil } diff --git a/pkg/cinderbackup/statefulset.go b/pkg/cinderbackup/statefulset.go index dbe7830e..aca221d2 100644 --- a/pkg/cinderbackup/statefulset.go +++ b/pkg/cinderbackup/statefulset.go @@ -146,13 +146,16 @@ func StatefulSet( VolumeMounts: volumeMounts, }, }, - Affinity: cinder.GetPodAffinity(ComponentName), - NodeSelector: instance.Spec.NodeSelector, - Volumes: volumes, + Affinity: cinder.GetPodAffinity(ComponentName), + Volumes: volumes, }, }, }, } + if instance.Spec.NodeSelector != nil && len(*instance.Spec.NodeSelector) > 0 { + statefulset.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector + } + return statefulset } diff --git a/pkg/cinderscheduler/statefulset.go b/pkg/cinderscheduler/statefulset.go index eecd0b11..16c9c43a 100644 --- a/pkg/cinderscheduler/statefulset.go +++ b/pkg/cinderscheduler/statefulset.go @@ -131,13 +131,16 @@ func StatefulSet( VolumeMounts: volumeMounts, }, }, - Affinity: cinder.GetPodAffinity(ComponentName), - NodeSelector: instance.Spec.NodeSelector, - Volumes: volumes, + Affinity: cinder.GetPodAffinity(ComponentName), + Volumes: volumes, }, }, }, } + if instance.Spec.NodeSelector != nil && len(*instance.Spec.NodeSelector) > 0 { + statefulset.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector + } + return statefulset } diff --git a/pkg/cindervolume/statefulset.go b/pkg/cindervolume/statefulset.go index 193cf318..5f08bef3 100644 --- a/pkg/cindervolume/statefulset.go +++ b/pkg/cindervolume/statefulset.go @@ -153,13 +153,16 @@ func StatefulSet( VolumeMounts: volumeMounts, }, }, - Affinity: cinder.GetPodAffinity(ComponentName), - NodeSelector: instance.Spec.NodeSelector, - Volumes: volumes, + Affinity: cinder.GetPodAffinity(ComponentName), + Volumes: volumes, }, }, }, } + if instance.Spec.NodeSelector != nil && len(*instance.Spec.NodeSelector) > 0 { + statefulset.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector + } + return statefulset } From 62c0ff0f8d200252c060c3f61449e1a8397a891c Mon Sep 17 00:00:00 2001 From: Oliver Walsh Date: Wed, 13 Nov 2024 22:30:22 +0000 Subject: [PATCH 2/3] Add nodeSelector functional tests --- test/functional/base_test.go | 9 + test/functional/cinder_controller_test.go | 198 ++++++++++++++++++++++ test/functional/cinder_test_data.go | 5 + 3 files changed, 212 insertions(+) diff --git a/test/functional/base_test.go b/test/functional/base_test.go index 2d44dfd3..0654e58a 100644 --- a/test/functional/base_test.go +++ b/test/functional/base_test.go @@ -19,6 +19,7 @@ import ( "golang.org/x/exp/maps" . "github.com/onsi/gomega" //revive:disable:dot-imports + batchv1 "k8s.io/api/batch/v1" corev1 "k8s.io/api/core/v1" k8s_errors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -230,6 +231,14 @@ func GetCinderVolume(name types.NamespacedName) *cinderv1.CinderVolume { return instance } +func GetCronJob(name types.NamespacedName) *batchv1.CronJob { + instance := &batchv1.CronJob{} + Eventually(func(g Gomega) { + g.Expect(k8sClient.Get(ctx, name, instance)).Should(Succeed()) + }, timeout, interval).Should(Succeed()) + return instance +} + func CinderAPIConditionGetter(name types.NamespacedName) condition.Conditions { instance := GetCinderAPI(name) return instance.Status.Conditions diff --git a/test/functional/cinder_controller_test.go b/test/functional/cinder_controller_test.go index b07352d7..15da6f05 100644 --- a/test/functional/cinder_controller_test.go +++ b/test/functional/cinder_controller_test.go @@ -665,6 +665,204 @@ var _ = Describe("Cinder controller", func() { }) }) + When("A Cinder is created with nodeSelector", func() { + BeforeEach(func() { + spec := GetDefaultCinderSpec() + spec["nodeSelector"] = map[string]interface{}{ + "foo": "bar", + } + spec["cinderVolumes"] = map[string]interface{}{ + "volume1": map[string]interface{}{ + "containerImage": cinderv1.CinderVolumeContainerImage, + }, + } + DeferCleanup(th.DeleteInstance, CreateCinder(cinderTest.Instance, spec)) + DeferCleanup(k8sClient.Delete, ctx, CreateCinderMessageBusSecret(cinderTest.Instance.Namespace, cinderTest.RabbitmqSecretName)) + DeferCleanup( + mariadb.DeleteDBService, + mariadb.CreateDBService( + cinderTest.Instance.Namespace, + GetCinder(cinderName).Spec.DatabaseInstance, + corev1.ServiceSpec{ + Ports: []corev1.ServicePort{{Port: 3306}}, + }, + ), + ) + infra.SimulateTransportURLReady(cinderTest.CinderTransportURL) + DeferCleanup(infra.DeleteMemcached, infra.CreateMemcached(namespace, cinderTest.MemcachedInstance, memcachedSpec)) + infra.SimulateMemcachedReady(cinderTest.CinderMemcached) + DeferCleanup(keystone.DeleteKeystoneAPI, keystone.CreateKeystoneAPI(cinderTest.Instance.Namespace)) + mariadb.SimulateMariaDBAccountCompleted(cinderTest.Database) + mariadb.SimulateMariaDBDatabaseCompleted(cinderTest.Database) + th.SimulateJobSuccess(cinderTest.CinderDBSync) + keystone.SimulateKeystoneServiceReady(cinderTest.CinderKeystoneService) + keystone.SimulateKeystoneEndpointReady(cinderTest.CinderKeystoneEndpoint) + th.SimulateStatefulSetReplicaReady(cinderTest.CinderAPI) + th.SimulateStatefulSetReplicaReady(cinderTest.CinderScheduler) + th.SimulateStatefulSetReplicaReady(cinderTest.CinderVolumes[0]) + }) + + It("sets nodeSelector in resource specs", func() { + Eventually(func(g Gomega) { + g.Expect(th.GetStatefulSet(cinderTest.CinderAPI).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(th.GetStatefulSet(cinderTest.CinderScheduler).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(th.GetStatefulSet(cinderTest.CinderVolumes[0]).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(th.GetJob(cinderTest.CinderDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(GetCronJob(cinderTest.CinderDBPurge).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.GetStatefulSet(cinderTest.CinderAPI).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(th.GetStatefulSet(cinderTest.CinderScheduler).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(th.GetStatefulSet(cinderTest.CinderVolumes[0]).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(th.GetJob(cinderTest.CinderDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(GetCronJob(cinderTest.CinderDBPurge).Spec.JobTemplate.Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + cinder := GetCinder(cinderName) + newNodeSelector := map[string]string{ + "foo2": "bar2", + } + cinder.Spec.NodeSelector = &newNodeSelector + g.Expect(k8sClient.Update(ctx, cinder)).Should(Succeed()) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + th.SimulateJobSuccess(cinderTest.CinderDBSync) + g.Expect(th.GetStatefulSet(cinderTest.CinderAPI).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo2": "bar2"})) + g.Expect(th.GetStatefulSet(cinderTest.CinderScheduler).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo2": "bar2"})) + g.Expect(th.GetStatefulSet(cinderTest.CinderVolumes[0]).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo2": "bar2"})) + g.Expect(th.GetJob(cinderTest.CinderDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo2": "bar2"})) + g.Expect(GetCronJob(cinderTest.CinderDBPurge).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.GetStatefulSet(cinderTest.CinderAPI).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(th.GetStatefulSet(cinderTest.CinderScheduler).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(th.GetStatefulSet(cinderTest.CinderVolumes[0]).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(th.GetJob(cinderTest.CinderDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(GetCronJob(cinderTest.CinderDBPurge).Spec.JobTemplate.Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + cinder := GetCinder(cinderName) + emptyNodeSelector := map[string]string{} + cinder.Spec.NodeSelector = &emptyNodeSelector + g.Expect(k8sClient.Update(ctx, cinder)).Should(Succeed()) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + th.SimulateJobSuccess(cinderTest.CinderDBSync) + g.Expect(th.GetStatefulSet(cinderTest.CinderAPI).Spec.Template.Spec.NodeSelector).To(BeNil()) + g.Expect(th.GetStatefulSet(cinderTest.CinderScheduler).Spec.Template.Spec.NodeSelector).To(BeNil()) + g.Expect(th.GetStatefulSet(cinderTest.CinderVolumes[0]).Spec.Template.Spec.NodeSelector).To(BeNil()) + g.Expect(th.GetJob(cinderTest.CinderDBSync).Spec.Template.Spec.NodeSelector).To(BeNil()) + g.Expect(GetCronJob(cinderTest.CinderDBPurge).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.GetStatefulSet(cinderTest.CinderAPI).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(th.GetStatefulSet(cinderTest.CinderScheduler).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(th.GetStatefulSet(cinderTest.CinderVolumes[0]).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(th.GetJob(cinderTest.CinderDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(GetCronJob(cinderTest.CinderDBPurge).Spec.JobTemplate.Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + cinder := GetCinder(cinderName) + cinder.Spec.NodeSelector = nil + g.Expect(k8sClient.Update(ctx, cinder)).Should(Succeed()) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + th.SimulateJobSuccess(cinderTest.CinderDBSync) + g.Expect(th.GetStatefulSet(cinderTest.CinderAPI).Spec.Template.Spec.NodeSelector).To(BeNil()) + g.Expect(th.GetStatefulSet(cinderTest.CinderScheduler).Spec.Template.Spec.NodeSelector).To(BeNil()) + g.Expect(th.GetStatefulSet(cinderTest.CinderVolumes[0]).Spec.Template.Spec.NodeSelector).To(BeNil()) + g.Expect(th.GetJob(cinderTest.CinderDBSync).Spec.Template.Spec.NodeSelector).To(BeNil()) + g.Expect(GetCronJob(cinderTest.CinderDBPurge).Spec.JobTemplate.Spec.Template.Spec.NodeSelector).To(BeNil()) + }, timeout, interval).Should(Succeed()) + }) + + It("allows nodeSelector service override", func() { + Eventually(func(g Gomega) { + g.Expect(th.GetStatefulSet(cinderTest.CinderAPI).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(th.GetStatefulSet(cinderTest.CinderScheduler).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(th.GetStatefulSet(cinderTest.CinderVolumes[0]).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(th.GetJob(cinderTest.CinderDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(GetCronJob(cinderTest.CinderDBPurge).Spec.JobTemplate.Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + cinder := GetCinder(cinderName) + apiNodeSelector := map[string]string{ + "foo": "api", + } + cinder.Spec.CinderAPI.NodeSelector = &apiNodeSelector + schedulerNodeSelector := map[string]string{ + "foo": "scheduler", + } + cinder.Spec.CinderScheduler.NodeSelector = &schedulerNodeSelector + volumeNodeSelector := map[string]string{ + "foo": "volume", + } + volume := cinder.Spec.CinderVolumes["volume1"] + volume.NodeSelector = &volumeNodeSelector + cinder.Spec.CinderVolumes["volume1"] = volume + g.Expect(k8sClient.Update(ctx, cinder)).Should(Succeed()) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + th.SimulateJobSuccess(cinderTest.CinderDBSync) + g.Expect(th.GetStatefulSet(cinderTest.CinderAPI).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "api"})) + g.Expect(th.GetStatefulSet(cinderTest.CinderScheduler).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "scheduler"})) + g.Expect(th.GetStatefulSet(cinderTest.CinderVolumes[0]).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "volume"})) + g.Expect(th.GetJob(cinderTest.CinderDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(GetCronJob(cinderTest.CinderDBPurge).Spec.JobTemplate.Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + }, timeout, interval).Should(Succeed()) + }) + + It("allows nodeSelector service override to empty", func() { + Eventually(func(g Gomega) { + g.Expect(th.GetStatefulSet(cinderTest.CinderAPI).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(th.GetStatefulSet(cinderTest.CinderScheduler).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(th.GetStatefulSet(cinderTest.CinderVolumes[0]).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(th.GetJob(cinderTest.CinderDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(GetCronJob(cinderTest.CinderDBPurge).Spec.JobTemplate.Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + cinder := GetCinder(cinderName) + apiNodeSelector := map[string]string{} + cinder.Spec.CinderAPI.NodeSelector = &apiNodeSelector + schedulerNodeSelector := map[string]string{} + cinder.Spec.CinderScheduler.NodeSelector = &schedulerNodeSelector + volumeNodeSelector := map[string]string{} + volume := cinder.Spec.CinderVolumes["volume1"] + volume.NodeSelector = &volumeNodeSelector + cinder.Spec.CinderVolumes["volume1"] = volume + g.Expect(k8sClient.Update(ctx, cinder)).Should(Succeed()) + }, timeout, interval).Should(Succeed()) + + Eventually(func(g Gomega) { + th.SimulateJobSuccess(cinderTest.CinderDBSync) + g.Expect(th.GetStatefulSet(cinderTest.CinderAPI).Spec.Template.Spec.NodeSelector).To(BeNil()) + g.Expect(th.GetStatefulSet(cinderTest.CinderScheduler).Spec.Template.Spec.NodeSelector).To(BeNil()) + g.Expect(th.GetStatefulSet(cinderTest.CinderVolumes[0]).Spec.Template.Spec.NodeSelector).To(BeNil()) + g.Expect(th.GetJob(cinderTest.CinderDBSync).Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + g.Expect(GetCronJob(cinderTest.CinderDBPurge).Spec.JobTemplate.Spec.Template.Spec.NodeSelector).To(Equal(map[string]string{"foo": "bar"})) + }, 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. diff --git a/test/functional/cinder_test_data.go b/test/functional/cinder_test_data.go index 8ed28adf..16c671a7 100644 --- a/test/functional/cinder_test_data.go +++ b/test/functional/cinder_test_data.go @@ -51,6 +51,7 @@ type CinderTestData struct { CinderMemcached types.NamespacedName CinderSA types.NamespacedName CinderDBSync types.NamespacedName + CinderDBPurge types.NamespacedName CinderKeystoneService types.NamespacedName CinderKeystoneEndpoint types.NamespacedName CinderServicePublic types.NamespacedName @@ -85,6 +86,10 @@ func GetCinderTestData(cinderName types.NamespacedName) CinderTestData { Namespace: cinderName.Namespace, Name: fmt.Sprintf("%s-db-sync", cinderName.Name), }, + CinderDBPurge: types.NamespacedName{ + Namespace: cinderName.Namespace, + Name: "cinder-db-purge", + }, CinderAPI: types.NamespacedName{ Namespace: cinderName.Namespace, Name: fmt.Sprintf("%s-api", cinderName.Name), From 0fd9df228370efe6f2c4ea49f428989d1454abb4 Mon Sep 17 00:00:00 2001 From: Oliver Walsh Date: Tue, 19 Nov 2024 15:37:56 +0000 Subject: [PATCH 3/3] Pass-through empty nodeSelector to podSpecs There is no need drop empty nodeSelector maps, omitempty already does this --- pkg/cinder/cronjob.go | 2 +- pkg/cinder/dbsync.go | 2 +- pkg/cinderapi/statefuleset.go | 2 +- pkg/cinderbackup/statefulset.go | 2 +- pkg/cinderscheduler/statefulset.go | 2 +- pkg/cindervolume/statefulset.go | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/cinder/cronjob.go b/pkg/cinder/cronjob.go index e358cf77..059d72f7 100644 --- a/pkg/cinder/cronjob.go +++ b/pkg/cinder/cronjob.go @@ -135,7 +135,7 @@ func CronJob( }, } - if instance.Spec.NodeSelector != nil && len(*instance.Spec.NodeSelector) > 0 { + if instance.Spec.NodeSelector != nil { cronjob.Spec.JobTemplate.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector } diff --git a/pkg/cinder/dbsync.go b/pkg/cinder/dbsync.go index b124324d..351b2820 100644 --- a/pkg/cinder/dbsync.go +++ b/pkg/cinder/dbsync.go @@ -115,7 +115,7 @@ func DbSyncJob(instance *cinderv1beta1.Cinder, labels map[string]string, annotat }, } - if instance.Spec.NodeSelector != nil && len(*instance.Spec.NodeSelector) > 0 { + if instance.Spec.NodeSelector != nil { job.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector } diff --git a/pkg/cinderapi/statefuleset.go b/pkg/cinderapi/statefuleset.go index 69554b0d..05c9e6c1 100644 --- a/pkg/cinderapi/statefuleset.go +++ b/pkg/cinderapi/statefuleset.go @@ -172,7 +172,7 @@ func StatefulSet( }, } - if instance.Spec.NodeSelector != nil && len(*instance.Spec.NodeSelector) > 0 { + if instance.Spec.NodeSelector != nil { statefulset.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector } diff --git a/pkg/cinderbackup/statefulset.go b/pkg/cinderbackup/statefulset.go index aca221d2..95a9eb52 100644 --- a/pkg/cinderbackup/statefulset.go +++ b/pkg/cinderbackup/statefulset.go @@ -153,7 +153,7 @@ func StatefulSet( }, } - if instance.Spec.NodeSelector != nil && len(*instance.Spec.NodeSelector) > 0 { + if instance.Spec.NodeSelector != nil { statefulset.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector } diff --git a/pkg/cinderscheduler/statefulset.go b/pkg/cinderscheduler/statefulset.go index 16c9c43a..b5851873 100644 --- a/pkg/cinderscheduler/statefulset.go +++ b/pkg/cinderscheduler/statefulset.go @@ -138,7 +138,7 @@ func StatefulSet( }, } - if instance.Spec.NodeSelector != nil && len(*instance.Spec.NodeSelector) > 0 { + if instance.Spec.NodeSelector != nil { statefulset.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector } diff --git a/pkg/cindervolume/statefulset.go b/pkg/cindervolume/statefulset.go index 5f08bef3..e939488c 100644 --- a/pkg/cindervolume/statefulset.go +++ b/pkg/cindervolume/statefulset.go @@ -160,7 +160,7 @@ func StatefulSet( }, } - if instance.Spec.NodeSelector != nil && len(*instance.Spec.NodeSelector) > 0 { + if instance.Spec.NodeSelector != nil { statefulset.Spec.Template.Spec.NodeSelector = *instance.Spec.NodeSelector }