From fce29510b685afce2e8fbbd01a93232ff2a284aa Mon Sep 17 00:00:00 2001 From: Francesco Pantano Date: Tue, 19 Nov 2024 21:55:00 +0100 Subject: [PATCH] Add TopologySpreadConstraint to GlanceAPI statefulset For each GlanceAPI, as part of the StatefulSet generated by GlanceAPITemplate, it is possible to specify a TopologySpreadConstraint, which is now included in the API. However, if not specified, it can be inherited from the top-level Glance CR. Signed-off-by: Francesco Pantano --- .../glance.openstack.org_glanceapis.yaml | 52 +++++++++ api/bases/glance.openstack.org_glances.yaml | 104 ++++++++++++++++++ api/v1beta1/common_types.go | 22 +++- api/v1beta1/conditions.go | 2 +- api/v1beta1/glance_types.go | 9 +- api/v1beta1/glanceapi_webhook.go | 6 +- api/v1beta1/zz_generated.deepcopy.go | 50 ++++++--- .../glance.openstack.org_glanceapis.yaml | 52 +++++++++ .../bases/glance.openstack.org_glances.yaml | 104 ++++++++++++++++++ pkg/glanceapi/statefulset.go | 3 + test/functional/glance_controller_test.go | 65 +++++++++++ 11 files changed, 443 insertions(+), 26 deletions(-) diff --git a/api/bases/glance.openstack.org_glanceapis.yaml b/api/bases/glance.openstack.org_glanceapis.yaml index 4720440c..3f305081 100644 --- a/api/bases/glance.openstack.org_glanceapis.yaml +++ b/api/bases/glance.openstack.org_glanceapis.yaml @@ -705,6 +705,58 @@ spec: caBundleSecretName: type: string type: object + topologySpreadConstraint: + items: + properties: + labelSelector: + properties: + matchExpressions: + items: + properties: + key: + type: string + operator: + type: string + values: + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + type: object + type: object + x-kubernetes-map-type: atomic + matchLabelKeys: + items: + type: string + type: array + x-kubernetes-list-type: atomic + maxSkew: + format: int32 + type: integer + minDomains: + format: int32 + type: integer + nodeAffinityPolicy: + type: string + nodeTaintsPolicy: + type: string + topologyKey: + type: string + whenUnsatisfiable: + type: string + required: + - maxSkew + - topologyKey + - whenUnsatisfiable + type: object + type: array type: default: split enum: diff --git a/api/bases/glance.openstack.org_glances.yaml b/api/bases/glance.openstack.org_glances.yaml index bf0e7271..74e84113 100644 --- a/api/bases/glance.openstack.org_glances.yaml +++ b/api/bases/glance.openstack.org_glances.yaml @@ -697,6 +697,58 @@ spec: caBundleSecretName: type: string type: object + topologySpreadConstraint: + items: + properties: + labelSelector: + properties: + matchExpressions: + items: + properties: + key: + type: string + operator: + type: string + values: + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + type: object + type: object + x-kubernetes-map-type: atomic + matchLabelKeys: + items: + type: string + type: array + x-kubernetes-list-type: atomic + maxSkew: + format: int32 + type: integer + minDomains: + format: int32 + type: integer + nodeAffinityPolicy: + type: string + nodeTaintsPolicy: + type: string + topologyKey: + type: string + whenUnsatisfiable: + type: string + required: + - maxSkew + - topologyKey + - whenUnsatisfiable + type: object + type: array type: default: split enum: @@ -777,6 +829,58 @@ spec: storageRequest: type: string type: object + topologySpreadConstraint: + items: + properties: + labelSelector: + properties: + matchExpressions: + items: + properties: + key: + type: string + operator: + type: string + values: + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + type: object + type: object + x-kubernetes-map-type: atomic + matchLabelKeys: + items: + type: string + type: array + x-kubernetes-list-type: atomic + maxSkew: + format: int32 + type: integer + minDomains: + format: int32 + type: integer + nodeAffinityPolicy: + type: string + nodeTaintsPolicy: + type: string + topologyKey: + type: string + whenUnsatisfiable: + type: string + required: + - maxSkew + - topologyKey + - whenUnsatisfiable + type: object + type: array required: - containerImage - databaseInstance diff --git a/api/v1beta1/common_types.go b/api/v1beta1/common_types.go index 424090a5..e49c6411 100644 --- a/api/v1beta1/common_types.go +++ b/api/v1beta1/common_types.go @@ -56,10 +56,6 @@ type GlanceAPITemplate struct { // Glance Container Image URL (will be set to environmental default if empty) ContainerImage string `json:"containerImage"` - // +kubebuilder:validation:Optional - // NodeSelector to target subset of worker nodes running this service - NodeSelector map[string]string `json:"nodeSelector,omitempty"` - // +kubebuilder:validation:Optional // CustomServiceConfig - customize the service config using this parameter to change service defaults, // or overwrite rendered information using raw OpenStack config format. The content gets added to @@ -106,6 +102,24 @@ type GlanceAPITemplate struct { // +kubebuilder:validation:Minimum=1 // APITimeout for HAProxy and Apache defaults to GlanceSpecCore APITimeout APITimeout int `json:"apiTimeout,omitempty"` + + // +kubebuilder:validation:Optional + // Topology - The struct that contains the Topology related information to + // provide hints to k8s scheduler + Topology `json:",inline"` +} + +// Topology defines the information exposed in the API to provide hints to k8s +// scheduler +type Topology struct { + // +kubebuilder:validation:Optional + // NodeSelector to target subset of worker nodes running this service + NodeSelector map[string]string `json:"nodeSelector,omitempty"` + + // +kubebuilder:validation:Optional + // APITopologySpreadConstraint exposes topologySpreadConstraint that are applied + // to the StatefulSet + TopologySpreadConstraint *[]corev1.TopologySpreadConstraint `json:"topologySpreadConstraint,omitempty"` } // Storage - diff --git a/api/v1beta1/conditions.go b/api/v1beta1/conditions.go index 6977fcd6..fa71b9c0 100644 --- a/api/v1beta1/conditions.go +++ b/api/v1beta1/conditions.go @@ -32,7 +32,7 @@ const ( // GlanceAPIReadyCondition Status=True condition which indicates if the GlanceAPI is configured and operational GlanceAPIReadyCondition condition.Type = "GlanceAPIReady" // CinderCondition - CinderCondition= "CinderReady" + CinderCondition = "CinderReady" // GlanceLayoutUpdateErrorMessage GlanceLayoutUpdateErrorMessage = "The GlanceAPI layout (type) cannot be modified. To proceed, please add a new API with the desired layout and then decommission the previous API" // KeystoneEndpointErrorMessage diff --git a/api/v1beta1/glance_types.go b/api/v1beta1/glance_types.go index c9e8b1c7..2bff64ec 100644 --- a/api/v1beta1/glance_types.go +++ b/api/v1beta1/glance_types.go @@ -68,10 +68,6 @@ type GlanceSpecCore struct { // PasswordSelectors - Selectors to identify the DB and ServiceUser password from the Secret PasswordSelectors PasswordSelector `json:"passwordSelectors"` - // +kubebuilder:validation:Optional - // NodeSelector to target subset of worker nodes running this service - NodeSelector map[string]string `json:"nodeSelector,omitempty"` - // +kubebuilder:validation:Optional // +kubebuilder:default=false // PreserveJobs - do not delete jobs after they finished e.g. to check logs @@ -126,6 +122,11 @@ type GlanceSpecCore struct { // +kubebuilder:validation:Minimum=1 // Default APITimeout for HAProxy and Apache, defaults to 60 seconds APITimeout int `json:"apiTimeout"` + + // +kubebuilder:validation:Optional + // Topology - The struct that contains the Topology related information to + // provide hints to k8s scheduler + Topology `json:",inline"` } // GlanceSpec defines the desired state of Glance diff --git a/api/v1beta1/glanceapi_webhook.go b/api/v1beta1/glanceapi_webhook.go index 2a522234..ac4abb5f 100644 --- a/api/v1beta1/glanceapi_webhook.go +++ b/api/v1beta1/glanceapi_webhook.go @@ -17,14 +17,14 @@ limitations under the License. package v1beta1 import ( + "fmt" + "github.com/google/go-cmp/cmp" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime" ctrl "sigs.k8s.io/controller-runtime" logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook" - "fmt" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - "github.com/google/go-cmp/cmp" - apierrors "k8s.io/apimachinery/pkg/api/errors" ) // GlanceAPIDefaults - diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 42db9e29..5cfe88d3 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -25,6 +25,7 @@ import ( "github.com/openstack-k8s-operators/lib-common/modules/common/condition" "github.com/openstack-k8s-operators/lib-common/modules/common/service" "github.com/openstack-k8s-operators/lib-common/modules/storage" + "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" ) @@ -249,13 +250,6 @@ func (in *GlanceAPITemplate) DeepCopyInto(out *GlanceAPITemplate) { *out = new(int32) **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 - } - } if in.CustomServiceConfigSecrets != nil { in, out := &in.CustomServiceConfigSecrets, &out.CustomServiceConfigSecrets *out = make([]string, len(*in)) @@ -271,6 +265,7 @@ func (in *GlanceAPITemplate) DeepCopyInto(out *GlanceAPITemplate) { out.Storage = in.Storage in.TLS.DeepCopyInto(&out.TLS) out.ImageCache = in.ImageCache + in.Topology.DeepCopyInto(&out.Topology) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GlanceAPITemplate. @@ -372,13 +367,6 @@ func (in *GlanceSpec) DeepCopy() *GlanceSpec { func (in *GlanceSpecCore) DeepCopyInto(out *GlanceSpecCore) { *out = *in 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 - } - } if in.CustomServiceConfigSecrets != nil { in, out := &in.CustomServiceConfigSecrets, &out.CustomServiceConfigSecrets *out = make([]string, len(*in)) @@ -402,6 +390,7 @@ func (in *GlanceSpecCore) DeepCopyInto(out *GlanceSpecCore) { out.Quotas = in.Quotas out.ImageCache = in.ImageCache out.DBPurge = in.DBPurge + in.Topology.DeepCopyInto(&out.Topology) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new GlanceSpecCore. @@ -516,3 +505,36 @@ func (in *Storage) DeepCopy() *Storage { in.DeepCopyInto(out) return out } + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Topology) DeepCopyInto(out *Topology) { + *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 + } + } + if in.TopologySpreadConstraint != nil { + in, out := &in.TopologySpreadConstraint, &out.TopologySpreadConstraint + *out = new([]v1.TopologySpreadConstraint) + if **in != nil { + in, out := *in, *out + *out = make([]v1.TopologySpreadConstraint, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Topology. +func (in *Topology) DeepCopy() *Topology { + if in == nil { + return nil + } + out := new(Topology) + in.DeepCopyInto(out) + return out +} diff --git a/config/crd/bases/glance.openstack.org_glanceapis.yaml b/config/crd/bases/glance.openstack.org_glanceapis.yaml index 4720440c..3f305081 100644 --- a/config/crd/bases/glance.openstack.org_glanceapis.yaml +++ b/config/crd/bases/glance.openstack.org_glanceapis.yaml @@ -705,6 +705,58 @@ spec: caBundleSecretName: type: string type: object + topologySpreadConstraint: + items: + properties: + labelSelector: + properties: + matchExpressions: + items: + properties: + key: + type: string + operator: + type: string + values: + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + type: object + type: object + x-kubernetes-map-type: atomic + matchLabelKeys: + items: + type: string + type: array + x-kubernetes-list-type: atomic + maxSkew: + format: int32 + type: integer + minDomains: + format: int32 + type: integer + nodeAffinityPolicy: + type: string + nodeTaintsPolicy: + type: string + topologyKey: + type: string + whenUnsatisfiable: + type: string + required: + - maxSkew + - topologyKey + - whenUnsatisfiable + type: object + type: array type: default: split enum: diff --git a/config/crd/bases/glance.openstack.org_glances.yaml b/config/crd/bases/glance.openstack.org_glances.yaml index bf0e7271..74e84113 100644 --- a/config/crd/bases/glance.openstack.org_glances.yaml +++ b/config/crd/bases/glance.openstack.org_glances.yaml @@ -697,6 +697,58 @@ spec: caBundleSecretName: type: string type: object + topologySpreadConstraint: + items: + properties: + labelSelector: + properties: + matchExpressions: + items: + properties: + key: + type: string + operator: + type: string + values: + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + type: object + type: object + x-kubernetes-map-type: atomic + matchLabelKeys: + items: + type: string + type: array + x-kubernetes-list-type: atomic + maxSkew: + format: int32 + type: integer + minDomains: + format: int32 + type: integer + nodeAffinityPolicy: + type: string + nodeTaintsPolicy: + type: string + topologyKey: + type: string + whenUnsatisfiable: + type: string + required: + - maxSkew + - topologyKey + - whenUnsatisfiable + type: object + type: array type: default: split enum: @@ -777,6 +829,58 @@ spec: storageRequest: type: string type: object + topologySpreadConstraint: + items: + properties: + labelSelector: + properties: + matchExpressions: + items: + properties: + key: + type: string + operator: + type: string + values: + items: + type: string + type: array + required: + - key + - operator + type: object + type: array + matchLabels: + additionalProperties: + type: string + type: object + type: object + x-kubernetes-map-type: atomic + matchLabelKeys: + items: + type: string + type: array + x-kubernetes-list-type: atomic + maxSkew: + format: int32 + type: integer + minDomains: + format: int32 + type: integer + nodeAffinityPolicy: + type: string + nodeTaintsPolicy: + type: string + topologyKey: + type: string + whenUnsatisfiable: + type: string + required: + - maxSkew + - topologyKey + - whenUnsatisfiable + type: object + type: array required: - containerImage - databaseInstance diff --git a/pkg/glanceapi/statefulset.go b/pkg/glanceapi/statefulset.go index 325aa2ac..039327f8 100644 --- a/pkg/glanceapi/statefulset.go +++ b/pkg/glanceapi/statefulset.go @@ -304,5 +304,8 @@ func StatefulSet( statefulset.Spec.Template.Spec.NodeSelector = instance.Spec.NodeSelector } + if instance.Spec.TopologySpreadConstraint != nil { + statefulset.Spec.Template.Spec.TopologySpreadConstraints = *instance.Spec.TopologySpreadConstraint + } return statefulset, err } diff --git a/test/functional/glance_controller_test.go b/test/functional/glance_controller_test.go index 302707fe..65bdf2b5 100644 --- a/test/functional/glance_controller_test.go +++ b/test/functional/glance_controller_test.go @@ -438,6 +438,71 @@ var _ = Describe("Glance controller", func() { }) }) + When("Glance CR instance is built with TopologySpreadConstraints", func() { + BeforeEach(func() { + DeferCleanup(infra.DeleteMemcached, infra.CreateMemcached(namespace, glanceTest.MemcachedInstance, memcachedSpec)) + infra.SimulateMemcachedReady(glanceTest.GlanceMemcached) + nad := th.CreateNetworkAttachmentDefinition(glanceTest.InternalAPINAD) + DeferCleanup(th.DeleteInstance, nad) + + topologySpreadConstraints := []map[string]interface{}{ + { + "maxSkew": 1, + "topologyKey": "topology.kubernetes.io/zone", + "whenUnsatisfiable": "DoNotSchedule", + }, + } + + rawSpec := map[string]interface{}{ + "storage": map[string]interface{}{ + "storageRequest": glanceTest.GlancePVCSize, + }, + "storageRequest": glanceTest.GlancePVCSize, + "secret": SecretName, + "databaseInstance": "openstack", + "databaseAccount": glanceTest.GlanceDatabaseAccount.Name, + "keystoneEndpoint": "default", + "customServiceConfig": GlanceDummyBackend, + "topologySpreadConstraints": topologySpreadConstraints, + "glanceAPIs": map[string]interface{}{ + "default": map[string]interface{}{ + "containerImage": glancev1.GlanceAPIContainerImage, + "networkAttachments": []string{"internalapi"}, + }, + }, + } + DeferCleanup(th.DeleteInstance, CreateGlance(glanceTest.Instance, rawSpec)) + DeferCleanup( + mariadb.DeleteDBService, + mariadb.CreateDBService( + glanceTest.Instance.Namespace, + GetGlance(glanceName).Spec.DatabaseInstance, + corev1.ServiceSpec{ + Ports: []corev1.ServicePort{{Port: 3306}}, + }, + ), + ) + mariadb.SimulateMariaDBDatabaseCompleted(glanceTest.GlanceDatabaseName) + mariadb.SimulateMariaDBAccountCompleted(glanceTest.GlanceDatabaseAccount) + th.SimulateJobSuccess(glanceTest.GlanceDBSync) + keystoneAPI := keystone.CreateKeystoneAPI(glanceTest.Instance.Namespace) + DeferCleanup(keystone.DeleteKeystoneAPI, keystoneAPI) + }) + It("Check the topologySpreadConstraints are propagated to the generated sub-CRs", func() { + th.SimulateStatefulSetReplicaReady(glanceTest.GlanceInternalStatefulSet) + th.SimulateStatefulSetReplicaReady(glanceTest.GlanceExternalStatefulSet) + // Retrieve the generated resources and the two internal/external + // instances that are split behind the scenes + glance := GetGlance(glanceTest.Instance) + internalAPI := GetGlanceAPI(glanceTest.GlanceInternal) + externalAPI := GetGlanceAPI(glanceTest.GlanceExternal) + + for _, glanceAPI := range glance.Spec.GlanceAPIs { + Expect(internalAPI.Spec.TopologySpreadConstraint).To(Equal(glanceAPI.TopologySpreadConstraint)) + Expect(externalAPI.Spec.TopologySpreadConstraint).To(Equal(glanceAPI.TopologySpreadConstraint)) + } + }) + }) // 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.