From cb441c23ae32d61adca0dc87f4006b2f091fda21 Mon Sep 17 00:00:00 2001 From: Oliver Walsh Date: Sun, 10 Nov 2024 00:41:18 +0000 Subject: [PATCH 1/3] Add redis NodeSelector --- apis/bases/redis.openstack.org_redises.yaml | 6 ++++++ apis/redis/v1beta1/redis_types.go | 3 +++ apis/redis/v1beta1/zz_generated.deepcopy.go | 7 +++++++ config/crd/bases/redis.openstack.org_redises.yaml | 6 ++++++ pkg/redis/statefulset.go | 4 ++++ 5 files changed, 26 insertions(+) diff --git a/apis/bases/redis.openstack.org_redises.yaml b/apis/bases/redis.openstack.org_redises.yaml index 13751669..7c70038b 100644 --- a/apis/bases/redis.openstack.org_redises.yaml +++ b/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 diff --git a/apis/redis/v1beta1/redis_types.go b/apis/redis/v1beta1/redis_types.go index 9b032efd..77d1348b 100644 --- a/apis/redis/v1beta1/redis_types.go +++ b/apis/redis/v1beta1/redis_types.go @@ -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 diff --git a/apis/redis/v1beta1/zz_generated.deepcopy.go b/apis/redis/v1beta1/zz_generated.deepcopy.go index edc770ca..70fb8039 100644 --- a/apis/redis/v1beta1/zz_generated.deepcopy.go +++ b/apis/redis/v1beta1/zz_generated.deepcopy.go @@ -125,6 +125,13 @@ func (in *RedisSpecCore) DeepCopyInto(out *RedisSpecCore) { **out = **in } in.TLS.DeepCopyInto(&out.TLS) + 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 + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RedisSpecCore. diff --git a/config/crd/bases/redis.openstack.org_redises.yaml b/config/crd/bases/redis.openstack.org_redises.yaml index 13751669..7c70038b 100644 --- a/config/crd/bases/redis.openstack.org_redises.yaml +++ b/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 diff --git a/pkg/redis/statefulset.go b/pkg/redis/statefulset.go index 1a99349f..92dc9ab3 100644 --- a/pkg/redis/statefulset.go +++ b/pkg/redis/statefulset.go @@ -141,5 +141,9 @@ func StatefulSet( }, } + if r.Spec.NodeSelector != nil && len(r.Spec.NodeSelector) > 0 { + sts.Spec.Template.Spec.NodeSelector = r.Spec.NodeSelector + } + return sts } From 0dea21140aeb5b6b9b306c8670235af7933292a8 Mon Sep 17 00:00:00 2001 From: Oliver Walsh Date: Wed, 20 Nov 2024 14:04:11 +0000 Subject: [PATCH 2/3] Allow empty nodeSelector Switch to a pointer for nodeSelector to allow different logic for empty vs unset --- .../instanceha.openstack.org_instancehas.yaml | 3 +-- apis/instanceha/v1beta1/instanceha_types.go | 14 +++++++------- apis/instanceha/v1beta1/zz_generated.deepcopy.go | 10 +++++++--- apis/memcached/v1beta1/memcached_types.go | 2 +- apis/memcached/v1beta1/zz_generated.deepcopy.go | 10 +++++++--- apis/network/v1beta1/dnsmasq_types.go | 2 +- apis/network/v1beta1/zz_generated.deepcopy.go | 10 +++++++--- apis/redis/v1beta1/redis_types.go | 14 +++++++------- apis/redis/v1beta1/zz_generated.deepcopy.go | 10 +++++++--- .../instanceha.openstack.org_instancehas.yaml | 3 +-- pkg/dnsmasq/deployment.go | 4 ++-- pkg/instanceha/funcs.go | 5 ++++- pkg/memcached/statefulset.go | 4 ++-- pkg/redis/statefulset.go | 4 ++-- 14 files changed, 56 insertions(+), 39 deletions(-) diff --git a/apis/bases/instanceha.openstack.org_instancehas.yaml b/apis/bases/instanceha.openstack.org_instancehas.yaml index 50cfa175..81a23d8b 100644 --- a/apis/bases/instanceha.openstack.org_instancehas.yaml +++ b/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 diff --git a/apis/instanceha/v1beta1/instanceha_types.go b/apis/instanceha/v1beta1/instanceha_types.go index b1041b3f..2af89735 100644 --- a/apis/instanceha/v1beta1/instanceha_types.go +++ b/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 diff --git a/apis/instanceha/v1beta1/zz_generated.deepcopy.go b/apis/instanceha/v1beta1/zz_generated.deepcopy.go index 121d26a7..eac32996 100644 --- a/apis/instanceha/v1beta1/zz_generated.deepcopy.go +++ b/apis/instanceha/v1beta1/zz_generated.deepcopy.go @@ -105,9 +105,13 @@ func (in *InstanceHaSpec) DeepCopyInto(out *InstanceHaSpec) { *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.NetworkAttachments != nil { diff --git a/apis/memcached/v1beta1/memcached_types.go b/apis/memcached/v1beta1/memcached_types.go index 87b146cf..d84ae7ed 100644 --- a/apis/memcached/v1beta1/memcached_types.go +++ b/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 diff --git a/apis/memcached/v1beta1/zz_generated.deepcopy.go b/apis/memcached/v1beta1/zz_generated.deepcopy.go index bebc4aca..c6efadea 100644 --- a/apis/memcached/v1beta1/zz_generated.deepcopy.go +++ b/apis/memcached/v1beta1/zz_generated.deepcopy.go @@ -126,9 +126,13 @@ func (in *MemcachedSpecCore) DeepCopyInto(out *MemcachedSpecCore) { } 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) diff --git a/apis/network/v1beta1/dnsmasq_types.go b/apis/network/v1beta1/dnsmasq_types.go index dfa0bb04..c3785d55 100644 --- a/apis/network/v1beta1/dnsmasq_types.go +++ b/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" diff --git a/apis/network/v1beta1/zz_generated.deepcopy.go b/apis/network/v1beta1/zz_generated.deepcopy.go index 1b53b355..33373be7 100644 --- a/apis/network/v1beta1/zz_generated.deepcopy.go +++ b/apis/network/v1beta1/zz_generated.deepcopy.go @@ -312,9 +312,13 @@ func (in *DNSMasqSpecCore) DeepCopyInto(out *DNSMasqSpecCore) { } 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.Override.DeepCopyInto(&out.Override) diff --git a/apis/redis/v1beta1/redis_types.go b/apis/redis/v1beta1/redis_types.go index 77d1348b..ebf907da 100644 --- a/apis/redis/v1beta1/redis_types.go +++ b/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": "-" - // 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": "-" + // Int32 is a 10 character + hyphen = 11 + CrMaxLengthCorrection = 11 ) // RedisSpec defines the desired state of Redis @@ -54,9 +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"` + // +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 diff --git a/apis/redis/v1beta1/zz_generated.deepcopy.go b/apis/redis/v1beta1/zz_generated.deepcopy.go index 70fb8039..dd4ed45c 100644 --- a/apis/redis/v1beta1/zz_generated.deepcopy.go +++ b/apis/redis/v1beta1/zz_generated.deepcopy.go @@ -127,9 +127,13 @@ func (in *RedisSpecCore) DeepCopyInto(out *RedisSpecCore) { in.TLS.DeepCopyInto(&out.TLS) 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 + } } } } diff --git a/config/crd/bases/instanceha.openstack.org_instancehas.yaml b/config/crd/bases/instanceha.openstack.org_instancehas.yaml index 50cfa175..81a23d8b 100644 --- a/config/crd/bases/instanceha.openstack.org_instancehas.yaml +++ b/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 diff --git a/pkg/dnsmasq/deployment.go b/pkg/dnsmasq/deployment.go index 78e44cda..57ddf5df 100644 --- a/pkg/dnsmasq/deployment.go +++ b/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 diff --git a/pkg/instanceha/funcs.go b/pkg/instanceha/funcs.go index 6df397b0..25ffefaf 100644 --- a/pkg/instanceha/funcs.go +++ b/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 } diff --git a/pkg/memcached/statefulset.go b/pkg/memcached/statefulset.go index 7635cb24..fac9579f 100644 --- a/pkg/memcached/statefulset.go +++ b/pkg/memcached/statefulset.go @@ -110,8 +110,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 diff --git a/pkg/redis/statefulset.go b/pkg/redis/statefulset.go index 92dc9ab3..f87263c0 100644 --- a/pkg/redis/statefulset.go +++ b/pkg/redis/statefulset.go @@ -141,8 +141,8 @@ func StatefulSet( }, } - if r.Spec.NodeSelector != nil && len(r.Spec.NodeSelector) > 0 { - sts.Spec.Template.Spec.NodeSelector = r.Spec.NodeSelector + if r.Spec.NodeSelector != nil { + sts.Spec.Template.Spec.NodeSelector = *r.Spec.NodeSelector } return sts From ca8d5698875b3e9ae17efa727e5d0a1b6bd517dc Mon Sep 17 00:00:00 2001 From: Oliver Walsh Date: Wed, 20 Nov 2024 14:17:32 +0000 Subject: [PATCH 3/3] Add dnsmasq nodeSelector tests --- tests/functional/dnsmasq_controller_test.go | 106 ++++++++++++++++++++ 1 file changed, 106 insertions(+) diff --git a/tests/functional/dnsmasq_controller_test.go b/tests/functional/dnsmasq_controller_test.go index 3bd905db..23ba61ce 100644 --- a/tests/functional/dnsmasq_controller_test.go +++ b/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()) + }) + }) })