diff --git a/api/bases/cinder.openstack.org_cinderapis.yaml b/api/bases/cinder.openstack.org_cinderapis.yaml index 0fec1983..104c6d12 100644 --- a/api/bases/cinder.openstack.org_cinderapis.yaml +++ b/api/bases/cinder.openstack.org_cinderapis.yaml @@ -60,7 +60,7 @@ spec: description: |- 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 - to /etc//.conf.d directory as a custom config file. + the /etc//.conf.d directory as a custom config file. type: string customServiceConfigSecrets: description: |- diff --git a/api/bases/cinder.openstack.org_cinderbackups.yaml b/api/bases/cinder.openstack.org_cinderbackups.yaml index 63757d3e..3198f681 100644 --- a/api/bases/cinder.openstack.org_cinderbackups.yaml +++ b/api/bases/cinder.openstack.org_cinderbackups.yaml @@ -60,7 +60,7 @@ spec: description: |- 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 - to /etc//.conf.d directory as a custom config file. + the /etc//.conf.d directory as a custom config file. type: string customServiceConfigSecrets: description: |- diff --git a/api/bases/cinder.openstack.org_cinders.yaml b/api/bases/cinder.openstack.org_cinders.yaml index 7614b17e..bfa1b22f 100644 --- a/api/bases/cinder.openstack.org_cinders.yaml +++ b/api/bases/cinder.openstack.org_cinders.yaml @@ -65,7 +65,7 @@ spec: description: |- 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 - to /etc//.conf.d directory as a custom config file. + the /etc//.conf.d directory as a custom config file. type: string customServiceConfigSecrets: description: |- @@ -377,7 +377,7 @@ spec: description: |- 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 - to /etc//.conf.d directory as a custom config file. + the /etc//.conf.d directory as a custom config file. type: string customServiceConfigSecrets: description: |- @@ -488,6 +488,133 @@ spec: required: - containerImage type: object + cinderBackups: + additionalProperties: + description: CinderBackupTemplate defines the input parameters for + the Cinder Backup service + properties: + containerImage: + description: ContainerImage - Cinder Container Image URL (will + be set to environmental default if empty) + type: string + customServiceConfig: + description: |- + 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 + the /etc//.conf.d directory as a custom config file. + type: string + customServiceConfigSecrets: + description: |- + CustomServiceConfigSecrets - customize the service config using this parameter to specify Secrets + that contain sensitive service config data. The content of each Secret gets added to the + /etc//.conf.d directory as a custom config file. + items: + type: string + type: array + networkAttachments: + description: NetworkAttachments is a list of NetworkAttachment + resource names to expose the services to the given network + items: + type: string + type: array + nodeSelector: + additionalProperties: + type: string + description: |- + NodeSelector to target subset of worker nodes running this service. Setting here overrides + any global NodeSelector settings within the Cinder CR. + type: object + replicas: + default: 1 + description: Replicas - Cinder Backup Replicas + format: int32 + minimum: 0 + type: integer + resources: + description: |- + Resources - Compute Resources required by this service (Limits/Requests). + https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + properties: + claims: + description: |- + Claims lists the names of resources, defined in spec.resourceClaims, + that are used by this container. + + This is an alpha field and requires enabling the + DynamicResourceAllocation feature gate. + + This field is immutable. It can only be set for containers. + items: + description: ResourceClaim references one entry in PodSpec.ResourceClaims. + properties: + name: + description: |- + Name must match the name of one entry in pod.spec.resourceClaims of + the Pod where this field is used. It makes that resource available + inside a container. + type: string + request: + description: |- + Request is the name chosen for a request in the referenced claim. + If empty, everything from the claim is made available, otherwise + only the result of this request. + type: string + required: + - name + type: object + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Limits describes the maximum amount of compute resources allowed. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Requests describes the minimum amount of compute resources required. + If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + otherwise to an implementation-defined value. Requests cannot exceed Limits. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + type: object + topologyRef: + description: |- + TopologyRef to apply the Topology defined by the associated CR referenced + by name + properties: + name: + description: Name - The Topology CR name that the Service + references + type: string + namespace: + description: |- + Namespace - The Namespace to fetch the Topology CR referenced + NOTE: Namespace currently points by default to the same namespace where + the Service is deployed. Customizing the namespace is not supported and + webhooks prevent editing this field to a value different from the + current project + type: string + type: object + required: + - containerImage + type: object + description: CinderBackups - Spec definition for the Backup service + of this Cinder deployment + type: object cinderScheduler: description: CinderScheduler - Spec definition for the Scheduler service of this Cinder deployment @@ -500,7 +627,7 @@ spec: description: |- 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 - to /etc//.conf.d directory as a custom config file. + the /etc//.conf.d directory as a custom config file. type: string customServiceConfigSecrets: description: |- @@ -624,7 +751,7 @@ spec: description: |- 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 - to /etc//.conf.d directory as a custom config file. + the /etc//.conf.d directory as a custom config file. type: string customServiceConfigSecrets: description: |- @@ -1988,6 +2115,12 @@ spec: format: int32 minimum: 0 type: integer + cinderBackupsReadyCounts: + additionalProperties: + format: int32 + type: integer + description: ReadyCounts of Cinder Backup instances + type: object cinderSchedulerReadyCount: default: 0 description: ReadyCount of Cinder Scheduler instance diff --git a/api/bases/cinder.openstack.org_cinderschedulers.yaml b/api/bases/cinder.openstack.org_cinderschedulers.yaml index c72ab485..5f1b37ba 100644 --- a/api/bases/cinder.openstack.org_cinderschedulers.yaml +++ b/api/bases/cinder.openstack.org_cinderschedulers.yaml @@ -60,7 +60,7 @@ spec: description: |- 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 - to /etc//.conf.d directory as a custom config file. + the /etc//.conf.d directory as a custom config file. type: string customServiceConfigSecrets: description: |- diff --git a/api/bases/cinder.openstack.org_cindervolumes.yaml b/api/bases/cinder.openstack.org_cindervolumes.yaml index b68130af..79ee5ded 100644 --- a/api/bases/cinder.openstack.org_cindervolumes.yaml +++ b/api/bases/cinder.openstack.org_cindervolumes.yaml @@ -60,7 +60,7 @@ spec: description: |- 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 - to /etc//.conf.d directory as a custom config file. + the /etc//.conf.d directory as a custom config file. type: string customServiceConfigSecrets: description: |- diff --git a/api/v1beta1/cinder_types.go b/api/v1beta1/cinder_types.go index 90796cef..dbdc5426 100644 --- a/api/v1beta1/cinder_types.go +++ b/api/v1beta1/cinder_types.go @@ -128,10 +128,17 @@ type CinderSpecCore struct { // CinderScheduler - Spec definition for the Scheduler service of this Cinder deployment CinderScheduler CinderSchedulerTemplateCore `json:"cinderScheduler"` + // CinderBackup is DEPRECATED and will be removed in a future release. + // Use the separate CinderBackups resource instead. // +kubebuilder:validation:Optional - // CinderBackup - Spec definition for the Backup service of this Cinder deployment + // +kubebuilder:deprecated:true + // +kubebuilder:deprecatedversion:warning="Cinder.Spec.CinderBackup is deprecated, use CinderBackups instead" CinderBackup CinderBackupTemplateCore `json:"cinderBackup"` + // +kubebuilder:validation:Optional + // CinderBackups - Spec definition for the Backup service of this Cinder deployment + CinderBackups *map[string]CinderBackupTemplateCore `json:"cinderBackups,omitempty"` + // +kubebuilder:validation:Optional // CinderVolumes - Map of chosen names to spec definitions for the Volume(s) service(s) of this Cinder deployment CinderVolumes map[string]CinderVolumeTemplateCore `json:"cinderVolumes,omitempty"` @@ -153,6 +160,10 @@ type CinderSpec struct { // CinderBackup - Spec definition for the Backup service of this Cinder deployment CinderBackup CinderBackupTemplate `json:"cinderBackup"` + // +kubebuilder:validation:Optional + // CinderBackups - Spec definition for the Backup service of this Cinder deployment + CinderBackups *map[string]CinderBackupTemplate `json:"cinderBackups,omitempty"` + // +kubebuilder:validation:Optional // CinderVolumes - Map of chosen names to spec definitions for the Volume(s) service(s) of this Cinder deployment CinderVolumes map[string]CinderVolumeTemplate `json:"cinderVolumes,omitempty"` @@ -191,6 +202,9 @@ type CinderStatus struct { // +kubebuilder:default=0 CinderBackupReadyCount int32 `json:"cinderBackupReadyCount"` + // ReadyCounts of Cinder Backup instances + CinderBackupsReadyCounts map[string]int32 `json:"cinderBackupsReadyCounts,omitempty"` + // ReadyCount of Cinder Scheduler instance // +kubebuilder:validation:Minimum=0 // +kubebuilder:default=0 diff --git a/api/v1beta1/cinder_webhook.go b/api/v1beta1/cinder_webhook.go index 5a54dbc1..20e94d09 100644 --- a/api/v1beta1/cinder_webhook.go +++ b/api/v1beta1/cinder_webhook.go @@ -36,7 +36,6 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" "sigs.k8s.io/controller-runtime/pkg/webhook" "sigs.k8s.io/controller-runtime/pkg/webhook/admission" - common_webhook "github.com/openstack-k8s-operators/lib-common/modules/common/webhook" ) @@ -89,6 +88,17 @@ func (r *Cinder) Default() { r.Spec.CinderScheduler.ContainerImage = cinderDefaults.SchedulerContainerImageURL } + if r.Spec.CinderBackups != nil { + cinderBackupList := r.Spec.CinderBackups + for index, cinderBackup := range *cinderBackupList { + if cinderBackup.ContainerImage == "" { + cinderBackup.ContainerImage = cinderDefaults.BackupContainerImageURL + } + // This is required, as the loop variable is a by-value copy + (*cinderBackupList)[index] = cinderBackup + } + } + for index, cinderVolume := range r.Spec.CinderVolumes { if cinderVolume.ContainerImage == "" { cinderVolume.ContainerImage = cinderDefaults.VolumeContainerImageURL @@ -125,6 +135,8 @@ func (r *Cinder) ValidateCreate() (admission.Warnings, error) { cinderlog.Info("validate create", "name", r.Name) var allErrs field.ErrorList + var allWarns admission.Warnings + basePath := field.NewPath("spec") // Validate cinderVolume name is valid @@ -144,8 +156,9 @@ func (r *Cinder) ValidateCreate() (admission.Warnings, error) { GetCrMaxLengthCorrection(r.Name)) // omit issue with statefulset pod label "controller-revision-hash": "-" allErrs = append(allErrs, err...) - if err := r.Spec.ValidateCreate(basePath, r.Namespace); err != nil { - allErrs = append(allErrs, err...) + if warn, errs := r.Spec.ValidateCreate(basePath, r.Namespace); err != nil { + allErrs = append(allErrs, errs...) + allWarns = append(allWarns, warn...) } if len(allErrs) != 0 { @@ -159,20 +172,35 @@ func (r *Cinder) ValidateCreate() (admission.Warnings, error) { // ValidateCreate - Exported function wrapping non-exported validate functions, // this function can be called externally to validate an cinder spec. -func (spec *CinderSpec) ValidateCreate(basePath *field.Path, namespace string) field.ErrorList { +func (spec *CinderSpec) ValidateCreate( + basePath *field.Path, + namespace string, +) ([]string, field.ErrorList) { var allErrs field.ErrorList + var allWarns admission.Warnings // validate the service override key is valid allErrs = append(allErrs, service.ValidateRoutedOverrides( basePath.Child("cinderAPI").Child("override").Child("service"), spec.CinderAPI.Override.Service)...) + // Cinder Backup validation: it returns errors in case both CinderBackup + // and CinderBackups are used, and it throws a warning by default when the + // deprecated parameter is used (it is a non blocking issue though) allErrs = append(allErrs, spec.ValidateCinderTopology(basePath, namespace)...) - return allErrs + bkpWarns, bkpErrs := spec.ValidateCinderBackup(basePath) + allErrs = append(allErrs, bkpErrs...) + allWarns = append(allWarns, bkpWarns...) + + return allWarns, allErrs } -func (spec *CinderSpecCore) ValidateCreate(basePath *field.Path, namespace string) field.ErrorList { +func (spec *CinderSpecCore) ValidateCreate( + basePath *field.Path, + namespace string, +) ([]string, field.ErrorList){ var allErrs field.ErrorList + var allWarns admission.Warnings // validate the service override key is valid allErrs = append(allErrs, service.ValidateRoutedOverrides( @@ -180,7 +208,15 @@ func (spec *CinderSpecCore) ValidateCreate(basePath *field.Path, namespace strin spec.CinderAPI.Override.Service)...) allErrs = append(allErrs, spec.ValidateCinderTopology(basePath, namespace)...) - return allErrs + + // Cinder Backup validation: it returns errors in case both CinderBackup + // and CinderBackups are used, and it throws a warning by default when the + // deprecated parameter is used (it is a non blocking issue though) + bkpWarns, bkpErrs := spec.ValidateCinderBackup(basePath) + allErrs = append(allErrs, bkpErrs...) + allWarns = append(allWarns, bkpWarns...) + + return allWarns, allErrs } // ValidateUpdate implements webhook.Validator so a webhook will be registered for the type @@ -193,6 +229,8 @@ func (r *Cinder) ValidateUpdate(old runtime.Object) (admission.Warnings, error) } var allErrs field.ErrorList + var allWarns admission.Warnings + basePath := field.NewPath("spec") // Validate cinderVolume name is valid @@ -212,8 +250,9 @@ func (r *Cinder) ValidateUpdate(old runtime.Object) (admission.Warnings, error) GetCrMaxLengthCorrection(r.Name)) // omit issue with statefulset pod label "controller-revision-hash": "-" allErrs = append(allErrs, err...) - if err := r.Spec.ValidateUpdate(oldCinder.Spec, basePath, r.Namespace); err != nil { + if warns, err := r.Spec.ValidateUpdate(oldCinder.Spec, basePath, r.Namespace); err != nil { allErrs = append(allErrs, err...) + allWarns = append(allWarns, warns...) } if len(allErrs) != 0 { @@ -222,13 +261,19 @@ func (r *Cinder) ValidateUpdate(old runtime.Object) (admission.Warnings, error) r.Name, allErrs) } - return nil, nil + return allWarns, nil } // ValidateUpdate - Exported function wrapping non-exported validate functions, // this function can be called externally to validate an cinder spec. -func (spec *CinderSpec) ValidateUpdate(old CinderSpec, basePath *field.Path, namespace string) field.ErrorList { +func (spec *CinderSpec) ValidateUpdate( + old CinderSpec, + basePath *field.Path, + namespace string, +) ([]string, field.ErrorList) { + var allErrs field.ErrorList + var allWarns []string // validate the service override key is valid allErrs = append(allErrs, service.ValidateRoutedOverrides( @@ -236,11 +281,25 @@ func (spec *CinderSpec) ValidateUpdate(old CinderSpec, basePath *field.Path, nam spec.CinderAPI.Override.Service)...) allErrs = append(allErrs, spec.ValidateCinderTopology(basePath, namespace)...) - return allErrs + + // Cinder Backup validation: it returns errors in case both CinderBackup + // and CinderBackups are used, and it throws a warning by default when the + // deprecated parameter is used (it is a non blocking issue though) + bkpWarns, bkpErrs := spec.ValidateCinderBackup(basePath) + allErrs = append(allErrs, bkpErrs...) + allWarns = append(allWarns, bkpWarns...) + + return allWarns, allErrs } -func (spec *CinderSpecCore) ValidateUpdate(old CinderSpecCore, basePath *field.Path, namespace string) field.ErrorList { +func (spec *CinderSpecCore) ValidateUpdate( + old CinderSpecCore, + basePath *field.Path, + namespace string, +) ([]string, field.ErrorList) { + var allErrs field.ErrorList + var allWarns []string // validate the service override key is valid allErrs = append(allErrs, service.ValidateRoutedOverrides( @@ -248,7 +307,15 @@ func (spec *CinderSpecCore) ValidateUpdate(old CinderSpecCore, basePath *field.P spec.CinderAPI.Override.Service)...) allErrs = append(allErrs, spec.ValidateCinderTopology(basePath, namespace)...) - return allErrs + + // Cinder Backup validation: it returns errors in case both CinderBackup + // and CinderBackups are used, and it throws a warning by default when the + // deprecated parameter is used (it is a non blocking issue though) + bkpWarns, bkpErrs := spec.ValidateCinderBackup(basePath) + allErrs = append(allErrs, bkpErrs...) + allWarns = append(allWarns, bkpWarns...) + + return allWarns, allErrs } // ValidateDelete implements webhook.Validator so a webhook will be registered for the type @@ -326,6 +393,16 @@ func (spec *CinderSpecCore) ValidateCinderTopology(basePath *field.Path, namespa allErrs = append(allErrs, spec.CinderBackup.ValidateTopology(bkPath, namespace)...) + // When a TopologyRef CR is referenced with an override to an instance of + // CinderBackups, fail if a different Namespace is referenced because not + // supported + if spec.CinderBackups != nil { + for k, ms := range *spec.CinderBackups { + path := basePath.Child("cinderBackups").Key(k) + allErrs = append(allErrs, ms.ValidateTopology(path, namespace)...) + } + } + // When a TopologyRef CR is referenced with an override to an instance of // CinderVolumes, fail if a different Namespace is referenced because not // supported @@ -365,6 +442,16 @@ func (spec *CinderSpec) ValidateCinderTopology(basePath *field.Path, namespace s allErrs = append(allErrs, spec.CinderBackup.ValidateTopology(bkPath, namespace)...) + // When a TopologyRef CR is referenced with an override to an instance of + // CinderBackups, fail if a different Namespace is referenced because not + // supported + if spec.CinderBackups != nil { + for k, ms := range *spec.CinderBackups { + path := basePath.Child("cinderBackups").Key(k) + allErrs = append(allErrs, ms.ValidateTopology(path, namespace)...) + } + } + // When a TopologyRef CR is referenced with an override to an instance of // CinderVolumes, fail if a different Namespace is referenced because not // supported @@ -374,3 +461,50 @@ func (spec *CinderSpec) ValidateCinderTopology(basePath *field.Path, namespace s } return allErrs } + +// TODO: Remove this function when refactoring CinderSpec to include CinderSpecCore +func (spec *CinderSpec) ValidateCinderBackup(basePath *field.Path) ([]string, field.ErrorList) { + var allErrs field.ErrorList + var allWarns []string + + // Check if the deprecated field is configured + isDeprecatedUsed := spec.CinderBackup.Replicas != nil && *spec.CinderBackup.Replicas > 0 + // Check if CinderBackup list is configured + isCinderBackupListUsed := spec.CinderBackups != nil && len(*spec.CinderBackups) > 0 + + // Fail if both cinderBackup (the deprecated field) and cinderBackups (the new list) + // are used together + if isDeprecatedUsed && isCinderBackupListUsed { + errMsg := "Usage of the deprecated 'cinderBackup' is forbidden when 'cinderBackups' is defined." + allErrs = append(allErrs, field.Invalid(basePath, "cinderBackup", errMsg)) + } + // Append a warning depending on the cinderBackup field usage + if isDeprecatedUsed { + warningMsg := "The 'cinderBackup' field is deprecated and will be removed in a future release. Please migrate to 'cinderBackups'." + allWarns = append(allWarns, warningMsg) + } + return allWarns, allErrs +} + +func (spec *CinderSpecCore) ValidateCinderBackup(basePath *field.Path)([]string, field.ErrorList) { + var allErrs field.ErrorList + var allWarns []string + + // Check if the deprecated field is configured + isDeprecatedUsed := spec.CinderBackup.Replicas != nil && *spec.CinderBackup.Replicas > 0 + // Check if CinderBackup list is configured + isCinderBackupListUsed := spec.CinderBackups != nil && len(*spec.CinderBackups) > 0 + + // Fail if both cinderBackup (the deprecated field) and cinderBackups (the new list) + // are used together + if isDeprecatedUsed && isCinderBackupListUsed { + errMsg := "Usage of the deprecated 'cinderBackup' is forbidden when the new 'cinderBackups' API is used." + allErrs = append(allErrs, field.Invalid(basePath, "cinderBackup", errMsg)) + } + // Append a warning depending on the cinderBackup field usage + if isDeprecatedUsed { + warningMsg := "The 'cinderBackup' field is deprecated and will be removed in a future release. Please migrate to 'cinderBackups'." + allWarns = append(allWarns, warningMsg) + } + return allWarns, allErrs +} diff --git a/api/v1beta1/common_types.go b/api/v1beta1/common_types.go index 977f81f8..094468f2 100644 --- a/api/v1beta1/common_types.go +++ b/api/v1beta1/common_types.go @@ -56,7 +56,7 @@ type CinderServiceTemplate struct { // +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 - // to /etc//.conf.d directory as a custom config file. + // the /etc//.conf.d directory as a custom config file. CustomServiceConfig string `json:"customServiceConfig,omitempty"` // +kubebuilder:validation:Optional diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index fc8d7685..5cb37ef8 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -749,6 +749,17 @@ func (in *CinderSpec) DeepCopyInto(out *CinderSpec) { in.CinderAPI.DeepCopyInto(&out.CinderAPI) in.CinderScheduler.DeepCopyInto(&out.CinderScheduler) in.CinderBackup.DeepCopyInto(&out.CinderBackup) + if in.CinderBackups != nil { + in, out := &in.CinderBackups, &out.CinderBackups + *out = new(map[string]CinderBackupTemplate) + if **in != nil { + in, out := *in, *out + *out = make(map[string]CinderBackupTemplate, len(*in)) + for key, val := range *in { + (*out)[key] = *val.DeepCopy() + } + } + } if in.CinderVolumes != nil { in, out := &in.CinderVolumes, &out.CinderVolumes *out = make(map[string]CinderVolumeTemplate, len(*in)) @@ -820,6 +831,17 @@ func (in *CinderSpecCore) DeepCopyInto(out *CinderSpecCore) { in.CinderAPI.DeepCopyInto(&out.CinderAPI) in.CinderScheduler.DeepCopyInto(&out.CinderScheduler) in.CinderBackup.DeepCopyInto(&out.CinderBackup) + if in.CinderBackups != nil { + in, out := &in.CinderBackups, &out.CinderBackups + *out = new(map[string]CinderBackupTemplateCore) + if **in != nil { + in, out := *in, *out + *out = make(map[string]CinderBackupTemplateCore, len(*in)) + for key, val := range *in { + (*out)[key] = *val.DeepCopy() + } + } + } if in.CinderVolumes != nil { in, out := &in.CinderVolumes, &out.CinderVolumes *out = make(map[string]CinderVolumeTemplateCore, len(*in)) @@ -886,6 +908,13 @@ func (in *CinderStatus) DeepCopyInto(out *CinderStatus) { (*out)[key] = val } } + if in.CinderBackupsReadyCounts != nil { + in, out := &in.CinderBackupsReadyCounts, &out.CinderBackupsReadyCounts + *out = make(map[string]int32, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.CinderVolumesReadyCounts != nil { in, out := &in.CinderVolumesReadyCounts, &out.CinderVolumesReadyCounts *out = make(map[string]int32, len(*in)) diff --git a/config/crd/bases/cinder.openstack.org_cinderapis.yaml b/config/crd/bases/cinder.openstack.org_cinderapis.yaml index 0fec1983..104c6d12 100644 --- a/config/crd/bases/cinder.openstack.org_cinderapis.yaml +++ b/config/crd/bases/cinder.openstack.org_cinderapis.yaml @@ -60,7 +60,7 @@ spec: description: |- 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 - to /etc//.conf.d directory as a custom config file. + the /etc//.conf.d directory as a custom config file. type: string customServiceConfigSecrets: description: |- diff --git a/config/crd/bases/cinder.openstack.org_cinderbackups.yaml b/config/crd/bases/cinder.openstack.org_cinderbackups.yaml index 63757d3e..3198f681 100644 --- a/config/crd/bases/cinder.openstack.org_cinderbackups.yaml +++ b/config/crd/bases/cinder.openstack.org_cinderbackups.yaml @@ -60,7 +60,7 @@ spec: description: |- 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 - to /etc//.conf.d directory as a custom config file. + the /etc//.conf.d directory as a custom config file. type: string customServiceConfigSecrets: description: |- diff --git a/config/crd/bases/cinder.openstack.org_cinders.yaml b/config/crd/bases/cinder.openstack.org_cinders.yaml index 7614b17e..bfa1b22f 100644 --- a/config/crd/bases/cinder.openstack.org_cinders.yaml +++ b/config/crd/bases/cinder.openstack.org_cinders.yaml @@ -65,7 +65,7 @@ spec: description: |- 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 - to /etc//.conf.d directory as a custom config file. + the /etc//.conf.d directory as a custom config file. type: string customServiceConfigSecrets: description: |- @@ -377,7 +377,7 @@ spec: description: |- 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 - to /etc//.conf.d directory as a custom config file. + the /etc//.conf.d directory as a custom config file. type: string customServiceConfigSecrets: description: |- @@ -488,6 +488,133 @@ spec: required: - containerImage type: object + cinderBackups: + additionalProperties: + description: CinderBackupTemplate defines the input parameters for + the Cinder Backup service + properties: + containerImage: + description: ContainerImage - Cinder Container Image URL (will + be set to environmental default if empty) + type: string + customServiceConfig: + description: |- + 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 + the /etc//.conf.d directory as a custom config file. + type: string + customServiceConfigSecrets: + description: |- + CustomServiceConfigSecrets - customize the service config using this parameter to specify Secrets + that contain sensitive service config data. The content of each Secret gets added to the + /etc//.conf.d directory as a custom config file. + items: + type: string + type: array + networkAttachments: + description: NetworkAttachments is a list of NetworkAttachment + resource names to expose the services to the given network + items: + type: string + type: array + nodeSelector: + additionalProperties: + type: string + description: |- + NodeSelector to target subset of worker nodes running this service. Setting here overrides + any global NodeSelector settings within the Cinder CR. + type: object + replicas: + default: 1 + description: Replicas - Cinder Backup Replicas + format: int32 + minimum: 0 + type: integer + resources: + description: |- + Resources - Compute Resources required by this service (Limits/Requests). + https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + properties: + claims: + description: |- + Claims lists the names of resources, defined in spec.resourceClaims, + that are used by this container. + + This is an alpha field and requires enabling the + DynamicResourceAllocation feature gate. + + This field is immutable. It can only be set for containers. + items: + description: ResourceClaim references one entry in PodSpec.ResourceClaims. + properties: + name: + description: |- + Name must match the name of one entry in pod.spec.resourceClaims of + the Pod where this field is used. It makes that resource available + inside a container. + type: string + request: + description: |- + Request is the name chosen for a request in the referenced claim. + If empty, everything from the claim is made available, otherwise + only the result of this request. + type: string + required: + - name + type: object + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Limits describes the maximum amount of compute resources allowed. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + description: |- + Requests describes the minimum amount of compute resources required. + If Requests is omitted for a container, it defaults to Limits if that is explicitly specified, + otherwise to an implementation-defined value. Requests cannot exceed Limits. + More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + type: object + type: object + topologyRef: + description: |- + TopologyRef to apply the Topology defined by the associated CR referenced + by name + properties: + name: + description: Name - The Topology CR name that the Service + references + type: string + namespace: + description: |- + Namespace - The Namespace to fetch the Topology CR referenced + NOTE: Namespace currently points by default to the same namespace where + the Service is deployed. Customizing the namespace is not supported and + webhooks prevent editing this field to a value different from the + current project + type: string + type: object + required: + - containerImage + type: object + description: CinderBackups - Spec definition for the Backup service + of this Cinder deployment + type: object cinderScheduler: description: CinderScheduler - Spec definition for the Scheduler service of this Cinder deployment @@ -500,7 +627,7 @@ spec: description: |- 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 - to /etc//.conf.d directory as a custom config file. + the /etc//.conf.d directory as a custom config file. type: string customServiceConfigSecrets: description: |- @@ -624,7 +751,7 @@ spec: description: |- 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 - to /etc//.conf.d directory as a custom config file. + the /etc//.conf.d directory as a custom config file. type: string customServiceConfigSecrets: description: |- @@ -1988,6 +2115,12 @@ spec: format: int32 minimum: 0 type: integer + cinderBackupsReadyCounts: + additionalProperties: + format: int32 + type: integer + description: ReadyCounts of Cinder Backup instances + type: object cinderSchedulerReadyCount: default: 0 description: ReadyCount of Cinder Scheduler instance diff --git a/config/crd/bases/cinder.openstack.org_cinderschedulers.yaml b/config/crd/bases/cinder.openstack.org_cinderschedulers.yaml index c72ab485..5f1b37ba 100644 --- a/config/crd/bases/cinder.openstack.org_cinderschedulers.yaml +++ b/config/crd/bases/cinder.openstack.org_cinderschedulers.yaml @@ -60,7 +60,7 @@ spec: description: |- 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 - to /etc//.conf.d directory as a custom config file. + the /etc//.conf.d directory as a custom config file. type: string customServiceConfigSecrets: description: |- diff --git a/config/crd/bases/cinder.openstack.org_cindervolumes.yaml b/config/crd/bases/cinder.openstack.org_cindervolumes.yaml index b68130af..79ee5ded 100644 --- a/config/crd/bases/cinder.openstack.org_cindervolumes.yaml +++ b/config/crd/bases/cinder.openstack.org_cindervolumes.yaml @@ -60,7 +60,7 @@ spec: description: |- 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 - to /etc//.conf.d directory as a custom config file. + the /etc//.conf.d directory as a custom config file. type: string customServiceConfigSecrets: description: |- diff --git a/internal/controller/cinder_controller.go b/internal/controller/cinder_controller.go index cb26e377..7d166dfb 100644 --- a/internal/controller/cinder_controller.go +++ b/internal/controller/cinder_controller.go @@ -835,13 +835,19 @@ func (r *CinderReconciler) reconcileNormal(ctx context.Context, instance *cinder } } - // deploy cinder-backup, but only if necessary - // + // DEPRECATED: A new interface has been implemented to deploy Cinder Backup, allowing + // it to be configured as a list of deployable instances. This code represents the + // **legacy interface** for a single Cinder Backup instance. It is required for + // compatibility with **brownfield deployments** that rely on the previous configuration, + // but it is will be removed in an upcoming release (when the OpenStack control plane + // API rolls out a v2 version of the API) + + var backupCondition *condition.Condition + crName := fmt.Sprintf("%s-backup", instance.Name) // Many OpenStack deployments don't use the cinder-backup service (it's optional), // so there's no need to deploy it unless it's required. - var backupCondition *condition.Condition - if *instance.Spec.CinderBackup.Replicas > 0 { - cinderBackup, op, err := r.backupDeploymentCreateOrUpdate(ctx, instance) + if *instance.Spec.CinderBackup.Replicas > 0 && instance.Spec.CinderBackups == nil { + cinderBackup, op, err := r.backupDeploymentCreateOrUpdate(ctx, instance, crName, nil) if err != nil { instance.Status.Conditions.Set(condition.FalseCondition( cinderv1beta1.CinderBackupReadyCondition, @@ -854,7 +860,6 @@ func (r *CinderReconciler) reconcileNormal(ctx context.Context, instance *cinder if op != controllerutil.OperationResultNone { Log.Info(fmt.Sprintf("Backup CR for %s successfully %s", instance.Name, string(op))) } - // Mirror values when the data in the StatefulSet is for the current generation if cinderBackup.Generation == cinderBackup.Status.ObservedGeneration { // Mirror CinderBackup status' ReadyCount to this parent CR @@ -864,22 +869,78 @@ func (r *CinderReconciler) reconcileNormal(ctx context.Context, instance *cinder backupCondition = cinderBackup.Status.Conditions.Mirror(cinderv1beta1.CinderBackupReadyCondition) instance.Status.Conditions.Set(backupCondition) } - } else { // Clean up cinder-backup if there are no replicas - err = r.backupCleanupDeployment(ctx, instance) + err = r.backupCleanupDeployment(ctx, instance, crName) if err != nil { // Should we set the condition to False? return ctrl.Result{}, err } - // TODO: Wait for the deployment to actually disappear before setting the condition - // The CinderBackup is ready, even if the service wasn't deployed. // Using "condition.DeploymentReadyMessage" here because that is what gets mirrored // as the message for the other Cinder children when they are successfully-deployed instance.Status.Conditions.MarkTrue(cinderv1beta1.CinderBackupReadyCondition, condition.DeploymentReadyMessage) } + // Deploy a list of cinder-backups only if Spec.CinderBackup is not used + // Note: many openstack deployments don't use the cinder-backup service + // (it's optional), so there's no need to deploy it unless it's explicitly + // added. + if instance.Spec.CinderBackups != nil { + var bckCondition *condition.Condition + waitingBkpGenerationMatch := false + for name, backup := range *instance.Spec.CinderBackups { + crName := fmt.Sprintf("%s-backup-%s", instance.Name, name) + if *backup.Replicas > 0 { + cinderBackup, op, err := r.backupDeploymentCreateOrUpdate(ctx, instance, crName, &backup) + if err != nil { + instance.Status.Conditions.Set(condition.FalseCondition( + cinderv1beta1.CinderBackupReadyCondition, + condition.ErrorReason, + condition.SeverityWarning, + cinderv1beta1.CinderBackupReadyErrorMessage, + err.Error())) + return ctrl.Result{}, err + } + if op != controllerutil.OperationResultNone { + Log.Info(fmt.Sprintf("Backup CR for %s successfully %s", instance.Name, string(op))) + } + if cinderBackup.Generation != cinderBackup.Status.ObservedGeneration { + waitingBkpGenerationMatch = true + } else { + // Mirror CinderBackup status' ReadyCount to this parent CR + if instance.Status.CinderBackupsReadyCounts == nil { + instance.Status.CinderBackupsReadyCounts = map[string]int32{} + } + instance.Status.CinderBackupsReadyCounts[name] = cinderBackup.Status.ReadyCount + + // If this cinderBackup is not IsReady, mirror the condition to get the latest step it is in. + // Could also check the overall ReadyCondition of the cinderBackup. + if !cinderBackup.IsReady() { + c := cinderBackup.Status.Conditions.Mirror(cinderv1beta1.CinderBackupReadyCondition) + // Get the condition with higher priority for backupCondition. + bckCondition = condition.GetHigherPrioCondition(c, bckCondition).DeepCopy() + } + } + if bckCondition != nil { + // If there was a Status=False condition, set that as the CinderBackupReadyCondition + instance.Status.Conditions.Set(bckCondition) + } else if !waitingBkpGenerationMatch { + // The CinderBackups are ready. + // Using "condition.DeploymentReadyMessage" here because that is what gets mirrored + // as the message for the other Cinder children when they are successfully-deployed + instance.Status.Conditions.MarkTrue(cinderv1beta1.CinderBackupReadyCondition, condition.DeploymentReadyMessage) + } + } else { + err = r.backupCleanupDeployment(ctx, instance, crName) + if err != nil { + return ctrl.Result{}, err + } + } + } + } + // End list of cinder backup(s) + // deploy cinder-volumes var volumeCondition *condition.Condition waitingGenerationMatch := false @@ -1255,16 +1316,27 @@ func (r *CinderReconciler) schedulerDeploymentCreateOrUpdate(ctx context.Context return deployment, op, err } -func (r *CinderReconciler) backupDeploymentCreateOrUpdate(ctx context.Context, instance *cinderv1beta1.Cinder) (*cinderv1beta1.CinderBackup, controllerutil.OperationResult, error) { +func (r *CinderReconciler) backupDeploymentCreateOrUpdate( + ctx context.Context, + instance *cinderv1beta1.Cinder, + name string, + bkpTemplate *cinderv1beta1.CinderBackupTemplate, +) (*cinderv1beta1.CinderBackup, controllerutil.OperationResult, error) { + cinderBackupSpec := cinderv1beta1.CinderBackupSpec{ - CinderTemplate: instance.Spec.CinderTemplate, - CinderBackupTemplate: instance.Spec.CinderBackup, - ExtraMounts: instance.Spec.ExtraMounts, - DatabaseHostname: instance.Status.DatabaseHostname, - TransportURLSecret: instance.Status.TransportURLSecret, - ServiceAccount: instance.RbacResourceName(), - TLS: instance.Spec.CinderAPI.TLS.Ca, - MemcachedInstance: &instance.Spec.MemcachedInstance, + CinderTemplate: instance.Spec.CinderTemplate, + ExtraMounts: instance.Spec.ExtraMounts, + DatabaseHostname: instance.Status.DatabaseHostname, + TransportURLSecret: instance.Status.TransportURLSecret, + ServiceAccount: instance.RbacResourceName(), + TLS: instance.Spec.CinderAPI.TLS.Ca, + MemcachedInstance: &instance.Spec.MemcachedInstance, + } + + if bkpTemplate != nil { + cinderBackupSpec.CinderBackupTemplate = *bkpTemplate + } else { + cinderBackupSpec.CinderBackupTemplate = instance.Spec.CinderBackup } if cinderBackupSpec.NodeSelector == nil { @@ -1273,7 +1345,7 @@ func (r *CinderReconciler) backupDeploymentCreateOrUpdate(ctx context.Context, i deployment := &cinderv1beta1.CinderBackup{ ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("%s-backup", instance.Name), + Name: name, Namespace: instance.Namespace, }, } @@ -1296,10 +1368,15 @@ func (r *CinderReconciler) backupDeploymentCreateOrUpdate(ctx context.Context, i return deployment, op, err } -func (r *CinderReconciler) backupCleanupDeployment(ctx context.Context, instance *cinderv1beta1.Cinder) error { +func (r *CinderReconciler) backupCleanupDeployment( + ctx context.Context, + instance *cinderv1beta1.Cinder, + cbakInstanceName string, +) error { + deployment := &cinderv1beta1.CinderBackup{ ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("%s-backup", instance.Name), + Name: cbakInstanceName, Namespace: instance.Namespace, }, } diff --git a/test/functional/cinder_controller_test.go b/test/functional/cinder_controller_test.go index 0e7d6133..3c23ac91 100644 --- a/test/functional/cinder_controller_test.go +++ b/test/functional/cinder_controller_test.go @@ -94,6 +94,7 @@ var _ = Describe("Cinder controller", func() { Expect(Cinder.Status.TransportURLSecret).To(Equal("")) Expect(Cinder.Status.CinderAPIReadyCount).To(Equal(int32(0))) Expect(Cinder.Status.CinderBackupReadyCount).To(Equal(int32(0))) + Expect(Cinder.Status.CinderBackupsReadyCounts).To(BeNil()) Expect(Cinder.Status.CinderSchedulerReadyCount).To(Equal(int32(0))) Expect(Cinder.Status.CinderVolumesReadyCounts["volume1"]).To(Equal(int32(0))) Expect(Cinder.Status.CinderVolumesReadyCounts["volume2"]).To(Equal(int32(0))) @@ -1744,4 +1745,64 @@ var _ = Describe("Cinder Webhook", func() { return instance, fmt.Sprintf("cinderVolumes[%s].topologyRef", instance) }), ) + + It("webhooks reject the request - both cinderBackup interfaces are populated", func() { + spec := GetDefaultCinderSpec() + raw := map[string]any{ + "apiVersion": "cinder.openstack.org/v1beta1", + "kind": "Cinder", + "metadata": map[string]any{ + "name": cinderTest.Instance.Name, + "namespace": cinderTest.Instance.Namespace, + }, + "spec": spec, + } + cinderBackupList := map[string]any{ + "foo": map[string]any{ + "replicas": 1, + }, + } + cinderBackupDeprecated := map[string]any{ + "replicas": 1, + } + spec["cinderBackups"] = cinderBackupList + spec["cinderBackup"] = cinderBackupDeprecated + + unstructuredObj := &unstructured.Unstructured{Object: raw} + _, err := controllerutil.CreateOrPatch( + ctx, k8sClient, unstructuredObj, func() error { return nil }) + + Expect(err).Should(HaveOccurred()) + var statusError *k8s_errors.StatusError + Expect(errors.As(err, &statusError)).To(BeTrue()) + Expect(statusError.ErrStatus.Message).To( + ContainSubstring( + "Usage of the deprecated 'cinderBackup' is forbidden when 'cinderBackups' is defined."), + ) + }) + + It("webhooks still allow to deploy cinderBackup using the old interface", func() { + spec := GetDefaultCinderSpec() + raw := map[string]any{ + "apiVersion": "cinder.openstack.org/v1beta1", + "kind": "Cinder", + "metadata": map[string]any{ + "name": cinderTest.Instance.Name, + "namespace": cinderTest.Instance.Namespace, + }, + "spec": spec, + } + cinderBackupDeprecated := map[string]any{ + "replicas": 1, + } + spec["cinderBackup"] = cinderBackupDeprecated + + unstructuredObj := &unstructured.Unstructured{Object: raw} + _, err := controllerutil.CreateOrPatch( + ctx, k8sClient, unstructuredObj, func() error { return nil }) + + Expect(err).ShouldNot(HaveOccurred()) + var statusError *k8s_errors.StatusError + Expect(errors.As(err, &statusError)).To(BeFalse()) + }) })