From e97685d72f7039eefc4cbfd378271c41d23a733a Mon Sep 17 00:00:00 2001 From: Dan Prince Date: Mon, 18 Aug 2025 12:20:46 -0400 Subject: [PATCH] Track CustomContainerImages and prevent minor updates without image changes Add tracking and validation to ensure CustomContainerImages are updated when changing targetVersion during minor updates: - Add TrackedCustomImages field to OpenStackVersionStatus to track CustomContainerImages used for each version - Implement validation webhook logic to prevent targetVersion updates when CustomContainerImages remain unchanged from previous version - Add helper functions for deep comparison of CustomContainerImages - Update controller to automatically track CustomContainerImages for each processed target version - Generate updated CRD manifests with new tracking field This prevents inconsistent deployments where users update the target version but forget to update associated custom container images, ensuring proper version tracking and validation during minor updates. Users can skip all validations related to this commit by setting the core.openstack.org/skip-custom-images-validation metadata annotations. Co-Authored-By: Claude Jira: OSPRH-19183 --- .../core.openstack.org_openstackversions.yaml | 197 +++++++ apis/core/v1beta1/openstackversion_types.go | 92 +++ apis/core/v1beta1/openstackversion_webhook.go | 80 +++ .../v1beta1/openstackversion_webhook_test.go | 182 ++++++ apis/core/v1beta1/version_test.go | 404 +++++++++++++ apis/core/v1beta1/zz_generated.deepcopy.go | 7 + bindata/crds/crds.yaml | 197 +++++++ .../core.openstack.org_openstackversions.yaml | 197 +++++++ .../core/openstackversion_controller.go | 14 + .../openstackversion_controller_test.go | 546 ++++++++++++++++++ 10 files changed, 1916 insertions(+) create mode 100644 apis/core/v1beta1/openstackversion_webhook_test.go diff --git a/apis/bases/core.openstack.org_openstackversions.yaml b/apis/bases/core.openstack.org_openstackversions.yaml index 3020e010e1..6389e8bca4 100644 --- a/apis/bases/core.openstack.org_openstackversions.yaml +++ b/apis/bases/core.openstack.org_openstackversions.yaml @@ -674,6 +674,203 @@ spec: glanceWsgi: type: string type: object + trackedCustomImages: + additionalProperties: + properties: + agentImage: + type: string + ansibleeeImage: + type: string + aodhAPIImage: + type: string + aodhEvaluatorImage: + type: string + aodhListenerImage: + type: string + aodhNotifierImage: + type: string + apacheImage: + type: string + barbicanAPIImage: + type: string + barbicanKeystoneListenerImage: + type: string + barbicanWorkerImage: + type: string + ceilometerCentralImage: + type: string + ceilometerComputeImage: + type: string + ceilometerIpmiImage: + type: string + ceilometerMysqldExporterImage: + type: string + ceilometerNotificationImage: + type: string + ceilometerSgcoreImage: + type: string + cinderAPIImage: + type: string + cinderBackupImage: + type: string + cinderSchedulerImage: + type: string + cinderVolumeImages: + additionalProperties: + type: string + type: object + designateAPIImage: + type: string + designateBackendbind9Image: + type: string + designateCentralImage: + type: string + designateMdnsImage: + type: string + designateProducerImage: + type: string + designateUnboundImage: + type: string + designateWorkerImage: + type: string + edpmFrrImage: + type: string + edpmIscsidImage: + type: string + edpmKeplerImage: + type: string + edpmLogrotateCrondImage: + type: string + edpmMultipathdImage: + type: string + edpmNeutronDhcpAgentImage: + type: string + edpmNeutronMetadataAgentImage: + type: string + edpmNeutronOvnAgentImage: + type: string + edpmNeutronSriovAgentImage: + type: string + edpmNodeExporterImage: + type: string + edpmOpenstackNetworkExporterImage: + type: string + edpmOvnBgpAgentImage: + type: string + edpmPodmanExporterImage: + type: string + glanceAPIImage: + type: string + heatAPIImage: + type: string + heatCfnapiImage: + type: string + heatEngineImage: + type: string + horizonImage: + type: string + infraDnsmasqImage: + type: string + infraMemcachedImage: + type: string + infraRedisImage: + type: string + ironicAPIImage: + type: string + ironicConductorImage: + type: string + ironicInspectorImage: + type: string + ironicNeutronAgentImage: + type: string + ironicPxeImage: + type: string + ironicPythonAgentImage: + type: string + keystoneAPIImage: + type: string + ksmImage: + type: string + manilaAPIImage: + type: string + manilaSchedulerImage: + type: string + manilaShareImages: + additionalProperties: + type: string + type: object + mariadbImage: + type: string + netUtilsImage: + type: string + neutronAPIImage: + type: string + novaAPIImage: + type: string + novaComputeImage: + type: string + novaConductorImage: + type: string + novaNovncImage: + type: string + novaSchedulerImage: + type: string + octaviaAPIImage: + type: string + octaviaHealthmanagerImage: + type: string + octaviaHousekeepingImage: + type: string + octaviaRsyslogImage: + type: string + octaviaWorkerImage: + type: string + openstackClientImage: + type: string + openstackNetworkExporterImage: + type: string + osContainerImage: + type: string + ovnControllerImage: + type: string + ovnControllerOvsImage: + type: string + ovnNbDbclusterImage: + type: string + ovnNorthdImage: + type: string + ovnSbDbclusterImage: + type: string + placementAPIImage: + type: string + rabbitmqImage: + type: string + swiftAccountImage: + type: string + swiftContainerImage: + type: string + swiftObjectImage: + type: string + swiftProxyImage: + type: string + telemetryNodeExporterImage: + type: string + testAnsibletestImage: + type: string + testHorizontestImage: + type: string + testTempestImage: + type: string + testTobikoImage: + type: string + watcherAPIImage: + type: string + watcherApplierImage: + type: string + watcherDecisionEngineImage: + type: string + type: object + type: object type: object type: object served: true diff --git a/apis/core/v1beta1/openstackversion_types.go b/apis/core/v1beta1/openstackversion_types.go index 3d5828e376..7c3f9a3b9b 100644 --- a/apis/core/v1beta1/openstackversion_types.go +++ b/apis/core/v1beta1/openstackversion_types.go @@ -18,6 +18,7 @@ package v1beta1 import ( "context" + "reflect" "regexp" condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition" @@ -198,6 +199,9 @@ type OpenStackVersionStatus struct { // ServiceDefaults - struct that contains current defaults for OSP services ServiceDefaults ServiceDefaults `json:"serviceDefaults,omitempty"` + // TrackedCustomImages tracks CustomContainerImages used for each version to detect changes + TrackedCustomImages map[string]CustomContainerImages `json:"trackedCustomImages,omitempty"` + //ObservedGeneration - the most recent generation observed for this object. ObservedGeneration int64 `json:"observedGeneration,omitempty"` } @@ -288,3 +292,91 @@ func GetOpenStackVersions(namespace string, k8sClient client.Client) (*OpenStack return versionList, nil } + +// isContainerTemplateEmpty checks if all fields in a ContainerTemplate are nil +func isContainerTemplateEmpty(ct ContainerTemplate) bool { + v := reflect.ValueOf(ct) + numFields := v.NumField() + for i := 0; i < numFields; i++ { + field := v.Field(i) + // Check if field is a pointer and not nil + if field.Kind() == reflect.Ptr && !field.IsNil() { + return false + } + } + return true +} + +// customContainerImagesModified compares two CustomContainerImages and returns true if they are different +func customContainerImagesAllModified(a, b CustomContainerImages) bool { + if !containerTemplateEqual(a.ContainerTemplate, b.ContainerTemplate) { + return true + } + + if !stringMapEqual(a.CinderVolumeImages, b.CinderVolumeImages) { + return true + } + + if !stringMapEqual(a.ManilaShareImages, b.ManilaShareImages) { + return true + } + + // If all fields are equal, return false (not modified) + return false +} + +// containerTemplateEqual compares two ContainerTemplate structs for equality using reflection +func containerTemplateEqual(a, b ContainerTemplate) bool { + va := reflect.ValueOf(a) + vb := reflect.ValueOf(b) + + numFields := va.NumField() + for i := 0; i < numFields; i++ { + fieldA := va.Field(i) + fieldB := vb.Field(i) + + // Both fields should be *string type + if fieldA.Kind() != reflect.Ptr || fieldB.Kind() != reflect.Ptr { + continue + } + + if fieldA.IsNil() && fieldB.IsNil() { + continue + } + if fieldA.IsNil() || fieldB.IsNil() { + return false + } + if fieldA.Elem().String() != fieldB.Elem().String() { + return false + } + } + + return true +} + +// stringPtrEqual compares two string pointers for equality +func stringPtrEqual(a, b *string) bool { + if a == nil && b == nil { + return true + } + if a == nil || b == nil { + return false + } + return *a == *b +} + +// stringMapEqual compares two string maps for equality +func stringMapEqual(a, b map[string]*string) bool { + if len(a) != len(b) { + return false + } + + for key, valueA := range a { + valueB, exists := b[key] + if !exists || !stringPtrEqual(valueA, valueB) { + return false + } + } + + return true +} diff --git a/apis/core/v1beta1/openstackversion_webhook.go b/apis/core/v1beta1/openstackversion_webhook.go index e0fba1fbb1..6fa642a1b8 100644 --- a/apis/core/v1beta1/openstackversion_webhook.go +++ b/apis/core/v1beta1/openstackversion_webhook.go @@ -17,7 +17,9 @@ limitations under the License. package v1beta1 import ( + "fmt" "os" + "reflect" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/runtime/schema" @@ -153,9 +155,87 @@ func (r *OpenStackVersion) ValidateUpdate(old runtime.Object) (admission.Warning ) } + // Validate CustomContainerImages changes during version updates + oldVersion, ok := old.(*OpenStackVersion) + if !ok { + return nil, apierrors.NewInternalError(fmt.Errorf("failed to convert old object to OpenStackVersion")) + } + + // Check if targetVersion is changing and this is a minor update + if oldVersion.Spec.TargetVersion != r.Spec.TargetVersion && oldVersion.Status.DeployedVersion != nil { + // Check if the skip annotation is present + skipValidation := false + if r.Annotations != nil { + if _, exists := r.Annotations["core.openstack.org/skip-custom-images-validation"]; exists { + skipValidation = true + } + } + + // When changing target version during a minor update, ensure CustomContainerImages are also updated + // unless skip annotation is present + if !skipValidation && hasAnyCustomImage(r.Spec.CustomContainerImages) { + + // Get the tracked custom images for the previous version + if r.Status.TrackedCustomImages != nil { + if trackedImages, exists := r.Status.TrackedCustomImages[oldVersion.Spec.TargetVersion]; exists { + // Compare current CustomContainerImages with tracked ones + if !customContainerImagesAllModified(r.Spec.CustomContainerImages, trackedImages) { + return nil, apierrors.NewForbidden( + schema.GroupResource{ + Group: GroupVersion.WithKind("OpenStackVersion").Group, + Resource: GroupVersion.WithKind("OpenStackVersion").Kind, + }, r.GetName(), &field.Error{ + Type: field.ErrorTypeForbidden, + Field: "spec.customContainerImages", + BadValue: r.Spec.TargetVersion, + Detail: "CustomContainerImages must be updated when changing targetVersion. The current CustomContainerImages are identical to those used in the previous version (" + oldVersion.Spec.TargetVersion + "), which prevents proper version tracking and validation.", + }, + ) + } + } + } + } + } + return nil, nil } +// hasAnyCustomImage checks if any image field in CustomContainerImages is set +func hasAnyCustomImage(images CustomContainerImages) bool { + // Check CinderVolumeImages map + for _, img := range images.CinderVolumeImages { + if img != nil { + return true + } + } + + // Check ManilaShareImages map + for _, img := range images.ManilaShareImages { + if img != nil { + return true + } + } + + // Check ContainerTemplate fields using reflection + v := reflect.ValueOf(images.ContainerTemplate) + t := reflect.TypeOf(images.ContainerTemplate) + + for i := 0; i < v.NumField(); i++ { + field := v.Field(i) + fieldType := t.Field(i) + + // Check if field is a pointer and not nil + if field.Kind() == reflect.Ptr && !field.IsNil() { + // Additional check to ensure it's a string pointer (image fields) + if fieldType.Type.Elem().Kind() == reflect.String { + return true + } + } + } + + return false +} + // ValidateDelete implements webhook.Validator so a webhook will be registered for the type func (r *OpenStackVersion) ValidateDelete() (admission.Warnings, error) { openstackversionlog.Info("validate delete", "name", r.Name) diff --git a/apis/core/v1beta1/openstackversion_webhook_test.go b/apis/core/v1beta1/openstackversion_webhook_test.go new file mode 100644 index 0000000000..915e2a9a6e --- /dev/null +++ b/apis/core/v1beta1/openstackversion_webhook_test.go @@ -0,0 +1,182 @@ +package v1beta1 + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" +) + +// DummyObject is a mock for testing with objects that are not OpenStackVersion +type DummyObject struct { + metav1.TypeMeta `json:",inline"` + metav1.ObjectMeta `json:"metadata,omitempty"` +} + +// DeepCopyObject implements runtime.Object +func (d *DummyObject) DeepCopyObject() runtime.Object { + copy := *d + return © +} + +var _ = Describe("OpenStackVersion Webhook", func() { + + Context("ValidateUpdate with annotation support", func() { + + var ( + oldVersion *OpenStackVersion + newVersion *OpenStackVersion + ) + + BeforeEach(func() { + // Set up test defaults to make tests work + SetupOpenStackVersionDefaults(OpenStackVersionDefaults{ + AvailableVersion: "1.1.0", + }) + + // Setup a base version with deployed status and custom images + cinderImage := "registry.example.com/cinder-volume:backend1-v1.0.0" + keystoneImage := "registry.example.com/keystone:v1.0.0" + + oldVersion = &OpenStackVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-version", + Namespace: "test-namespace", + }, + Spec: OpenStackVersionSpec{ + TargetVersion: "1.0.0", + CustomContainerImages: CustomContainerImages{ + ContainerTemplate: ContainerTemplate{ + KeystoneAPIImage: &keystoneImage, + }, + CinderVolumeImages: map[string]*string{ + "backend1": &cinderImage, + }, + }, + }, + Status: OpenStackVersionStatus{ + DeployedVersion: &[]string{"1.0.0"}[0], + ContainerImageVersionDefaults: map[string]*ContainerDefaults{ + "1.0.0": &ContainerDefaults{}, + "1.1.0": &ContainerDefaults{}, + }, + TrackedCustomImages: map[string]CustomContainerImages{ + "1.0.0": { + ContainerTemplate: ContainerTemplate{ + KeystoneAPIImage: &keystoneImage, + }, + CinderVolumeImages: map[string]*string{ + "backend1": &cinderImage, + }, + }, + }, + }, + } + + // Setup new version with changed target version but same custom images + newVersion = &OpenStackVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-version", + Namespace: "test-namespace", + }, + Spec: OpenStackVersionSpec{ + TargetVersion: "1.1.0", + CustomContainerImages: CustomContainerImages{ + ContainerTemplate: ContainerTemplate{ + KeystoneAPIImage: &keystoneImage, + }, + CinderVolumeImages: map[string]*string{ + "backend1": &cinderImage, + }, + }, + }, + Status: OpenStackVersionStatus{ + DeployedVersion: &[]string{"1.0.0"}[0], + ContainerImageVersionDefaults: map[string]*ContainerDefaults{ + "1.0.0": &ContainerDefaults{}, + "1.1.0": &ContainerDefaults{}, + }, + TrackedCustomImages: map[string]CustomContainerImages{ + "1.0.0": { + ContainerTemplate: ContainerTemplate{ + KeystoneAPIImage: &keystoneImage, + }, + CinderVolumeImages: map[string]*string{ + "backend1": &cinderImage, + }, + }, + }, + }, + } + }) + + It("should reject update when CustomContainerImages are unchanged and no skip annotation", func() { + _, err := newVersion.ValidateUpdate(oldVersion) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("CustomContainerImages must be updated when changing targetVersion")) + }) + + It("should allow update when skip annotation is present", func() { + newVersion.Annotations = map[string]string{ + "core.openstack.org/skip-custom-images-validation": "true", + } + _, err := newVersion.ValidateUpdate(oldVersion) + Expect(err).ToNot(HaveOccurred()) + }) + + It("should allow update when skip annotation exists with any value", func() { + newVersion.Annotations = map[string]string{ + "core.openstack.org/skip-custom-images-validation": "", + } + _, err := newVersion.ValidateUpdate(oldVersion) + Expect(err).ToNot(HaveOccurred()) + }) + + It("should allow update when CustomContainerImages are actually changed", func() { + newKeystoneImage := "registry.example.com/keystone:v2.0.0" + newVersion.Spec.CustomContainerImages.ContainerTemplate.KeystoneAPIImage = &newKeystoneImage + + _, err := newVersion.ValidateUpdate(oldVersion) + Expect(err).ToNot(HaveOccurred()) + }) + + It("should allow update when there are no custom images configured", func() { + newVersion.Spec.CustomContainerImages = CustomContainerImages{} + _, err := newVersion.ValidateUpdate(oldVersion) + Expect(err).ToNot(HaveOccurred()) + }) + + It("should allow update when DeployedVersion is nil (fresh install)", func() { + oldVersion.Status.DeployedVersion = nil + _, err := newVersion.ValidateUpdate(oldVersion) + Expect(err).ToNot(HaveOccurred()) + }) + + It("should allow update when target version is not changing", func() { + newVersion.Spec.TargetVersion = "1.0.0" // Same as old version + _, err := newVersion.ValidateUpdate(oldVersion) + Expect(err).ToNot(HaveOccurred()) + }) + + It("should handle edge case where TrackedCustomImages is nil", func() { + newVersion.Status.TrackedCustomImages = nil + _, err := newVersion.ValidateUpdate(oldVersion) + Expect(err).ToNot(HaveOccurred()) + }) + + It("should handle edge case where previous version not found in TrackedCustomImages", func() { + newVersion.Status.TrackedCustomImages = map[string]CustomContainerImages{ + "0.9.0": {}, // Different version than oldVersion.Spec.TargetVersion + } + _, err := newVersion.ValidateUpdate(oldVersion) + Expect(err).ToNot(HaveOccurred()) + }) + + It("should handle invalid old object type gracefully", func() { + invalidOld := &DummyObject{} // Wrong type + _, err := newVersion.ValidateUpdate(invalidOld) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("failed to convert old object to OpenStackVersion")) + }) + }) +}) diff --git a/apis/core/v1beta1/version_test.go b/apis/core/v1beta1/version_test.go index 14e5be4828..0c3b72bf4f 100644 --- a/apis/core/v1beta1/version_test.go +++ b/apis/core/v1beta1/version_test.go @@ -38,3 +38,407 @@ var _ = Describe("OpenStackReleaseVersion", func() { }) }) + +var _ = Describe("customContainerImagesModified", func() { + + Context("CinderVolumeImages comparison", func() { + + It("should return true when both CinderVolumeImages are nil", func() { + a := CustomContainerImages{} + b := CustomContainerImages{} + Expect(customContainerImagesAllModified(a, b)).To(BeFalse()) + }) + + It("should return true when both CinderVolumeImages are empty", func() { + a := CustomContainerImages{ + CinderVolumeImages: map[string]*string{}, + } + b := CustomContainerImages{ + CinderVolumeImages: map[string]*string{}, + } + Expect(customContainerImagesAllModified(a, b)).To(BeFalse()) + }) + + It("should return true when CinderVolumeImages have same key-value pairs", func() { + backend1Image := "registry.example.com/cinder-volume:backend1-v1.0.0" + backend2Image := "registry.example.com/cinder-volume:backend2-v1.0.0" + + a := CustomContainerImages{ + CinderVolumeImages: map[string]*string{ + "backend1": &backend1Image, + "backend2": &backend2Image, + }, + } + b := CustomContainerImages{ + CinderVolumeImages: map[string]*string{ + "backend1": &backend1Image, + "backend2": &backend2Image, + }, + } + Expect(customContainerImagesAllModified(a, b)).To(BeFalse()) + }) + + It("should return false when one CinderVolumeImages is nil and other is not", func() { + backend1Image := "registry.example.com/cinder-volume:backend1-v1.0.0" + + a := CustomContainerImages{} + b := CustomContainerImages{ + CinderVolumeImages: map[string]*string{ + "backend1": &backend1Image, + }, + } + Expect(customContainerImagesAllModified(a, b)).To(BeTrue()) + }) + + It("should return false when CinderVolumeImages have different number of backends", func() { + backend1Image := "registry.example.com/cinder-volume:backend1-v1.0.0" + backend2Image := "registry.example.com/cinder-volume:backend2-v1.0.0" + + a := CustomContainerImages{ + CinderVolumeImages: map[string]*string{ + "backend1": &backend1Image, + }, + } + b := CustomContainerImages{ + CinderVolumeImages: map[string]*string{ + "backend1": &backend1Image, + "backend2": &backend2Image, + }, + } + Expect(customContainerImagesAllModified(a, b)).To(BeTrue()) + }) + + It("should return false when CinderVolumeImages have different backend names", func() { + backend1Image := "registry.example.com/cinder-volume:backend1-v1.0.0" + + a := CustomContainerImages{ + CinderVolumeImages: map[string]*string{ + "backend1": &backend1Image, + }, + } + b := CustomContainerImages{ + CinderVolumeImages: map[string]*string{ + "backend2": &backend1Image, + }, + } + Expect(customContainerImagesAllModified(a, b)).To(BeTrue()) + }) + + It("should return false when CinderVolumeImages have different image values", func() { + backend1ImageV1 := "registry.example.com/cinder-volume:backend1-v1.0.0" + backend1ImageV2 := "registry.example.com/cinder-volume:backend1-v2.0.0" + + a := CustomContainerImages{ + CinderVolumeImages: map[string]*string{ + "backend1": &backend1ImageV1, + }, + } + b := CustomContainerImages{ + CinderVolumeImages: map[string]*string{ + "backend1": &backend1ImageV2, + }, + } + Expect(customContainerImagesAllModified(a, b)).To(BeTrue()) + }) + + It("should return false when one backend has nil value and other has a value", func() { + backend1Image := "registry.example.com/cinder-volume:backend1-v1.0.0" + + a := CustomContainerImages{ + CinderVolumeImages: map[string]*string{ + "backend1": nil, + }, + } + b := CustomContainerImages{ + CinderVolumeImages: map[string]*string{ + "backend1": &backend1Image, + }, + } + Expect(customContainerImagesAllModified(a, b)).To(BeTrue()) + }) + + It("should return true when both backends have nil values", func() { + a := CustomContainerImages{ + CinderVolumeImages: map[string]*string{ + "backend1": nil, + }, + } + b := CustomContainerImages{ + CinderVolumeImages: map[string]*string{ + "backend1": nil, + }, + } + Expect(customContainerImagesAllModified(a, b)).To(BeFalse()) + }) + + }) + + Context("ManilaShareImages comparison", func() { + + It("should return true when both ManilaShareImages are nil", func() { + a := CustomContainerImages{} + b := CustomContainerImages{} + Expect(customContainerImagesAllModified(a, b)).To(BeFalse()) + }) + + It("should return true when both ManilaShareImages are empty", func() { + a := CustomContainerImages{ + ManilaShareImages: map[string]*string{}, + } + b := CustomContainerImages{ + ManilaShareImages: map[string]*string{}, + } + Expect(customContainerImagesAllModified(a, b)).To(BeFalse()) + }) + + It("should return true when ManilaShareImages have same key-value pairs", func() { + nfsImage := "registry.example.com/manila-share:nfs-v1.0.0" + cephfsImage := "registry.example.com/manila-share:cephfs-v1.0.0" + + a := CustomContainerImages{ + ManilaShareImages: map[string]*string{ + "nfs": &nfsImage, + "cephfs": &cephfsImage, + }, + } + b := CustomContainerImages{ + ManilaShareImages: map[string]*string{ + "nfs": &nfsImage, + "cephfs": &cephfsImage, + }, + } + Expect(customContainerImagesAllModified(a, b)).To(BeFalse()) + }) + + It("should return false when one ManilaShareImages is nil and other is not", func() { + nfsImage := "registry.example.com/manila-share:nfs-v1.0.0" + + a := CustomContainerImages{} + b := CustomContainerImages{ + ManilaShareImages: map[string]*string{ + "nfs": &nfsImage, + }, + } + Expect(customContainerImagesAllModified(a, b)).To(BeTrue()) + }) + + It("should return false when ManilaShareImages have different number of share backends", func() { + nfsImage := "registry.example.com/manila-share:nfs-v1.0.0" + cephfsImage := "registry.example.com/manila-share:cephfs-v1.0.0" + + a := CustomContainerImages{ + ManilaShareImages: map[string]*string{ + "nfs": &nfsImage, + }, + } + b := CustomContainerImages{ + ManilaShareImages: map[string]*string{ + "nfs": &nfsImage, + "cephfs": &cephfsImage, + }, + } + Expect(customContainerImagesAllModified(a, b)).To(BeTrue()) + }) + + It("should return false when ManilaShareImages have different share backend names", func() { + shareImage := "registry.example.com/manila-share:v1.0.0" + + a := CustomContainerImages{ + ManilaShareImages: map[string]*string{ + "nfs": &shareImage, + }, + } + b := CustomContainerImages{ + ManilaShareImages: map[string]*string{ + "cephfs": &shareImage, + }, + } + Expect(customContainerImagesAllModified(a, b)).To(BeTrue()) + }) + + It("should return false when ManilaShareImages have different image values", func() { + nfsImageV1 := "registry.example.com/manila-share:nfs-v1.0.0" + nfsImageV2 := "registry.example.com/manila-share:nfs-v2.0.0" + + a := CustomContainerImages{ + ManilaShareImages: map[string]*string{ + "nfs": &nfsImageV1, + }, + } + b := CustomContainerImages{ + ManilaShareImages: map[string]*string{ + "nfs": &nfsImageV2, + }, + } + Expect(customContainerImagesAllModified(a, b)).To(BeTrue()) + }) + + It("should return false when one share backend has nil value and other has a value", func() { + nfsImage := "registry.example.com/manila-share:nfs-v1.0.0" + + a := CustomContainerImages{ + ManilaShareImages: map[string]*string{ + "nfs": nil, + }, + } + b := CustomContainerImages{ + ManilaShareImages: map[string]*string{ + "nfs": &nfsImage, + }, + } + Expect(customContainerImagesAllModified(a, b)).To(BeTrue()) + }) + + It("should return true when both share backends have nil values", func() { + a := CustomContainerImages{ + ManilaShareImages: map[string]*string{ + "nfs": nil, + }, + } + b := CustomContainerImages{ + ManilaShareImages: map[string]*string{ + "nfs": nil, + }, + } + Expect(customContainerImagesAllModified(a, b)).To(BeFalse()) + }) + + }) + + Context("Combined CinderVolumeImages and ManilaShareImages comparison", func() { + + It("should return true when both CinderVolumeImages and ManilaShareImages are identical", func() { + cinderImage := "registry.example.com/cinder-volume:backend1-v1.0.0" + manilaImage := "registry.example.com/manila-share:nfs-v1.0.0" + + a := CustomContainerImages{ + CinderVolumeImages: map[string]*string{ + "backend1": &cinderImage, + }, + ManilaShareImages: map[string]*string{ + "nfs": &manilaImage, + }, + } + b := CustomContainerImages{ + CinderVolumeImages: map[string]*string{ + "backend1": &cinderImage, + }, + ManilaShareImages: map[string]*string{ + "nfs": &manilaImage, + }, + } + Expect(customContainerImagesAllModified(a, b)).To(BeFalse()) + }) + + It("should return false when CinderVolumeImages match but ManilaShareImages differ", func() { + cinderImage := "registry.example.com/cinder-volume:backend1-v1.0.0" + manilaImageV1 := "registry.example.com/manila-share:nfs-v1.0.0" + manilaImageV2 := "registry.example.com/manila-share:nfs-v2.0.0" + + a := CustomContainerImages{ + CinderVolumeImages: map[string]*string{ + "backend1": &cinderImage, + }, + ManilaShareImages: map[string]*string{ + "nfs": &manilaImageV1, + }, + } + b := CustomContainerImages{ + CinderVolumeImages: map[string]*string{ + "backend1": &cinderImage, + }, + ManilaShareImages: map[string]*string{ + "nfs": &manilaImageV2, + }, + } + Expect(customContainerImagesAllModified(a, b)).To(BeTrue()) + }) + + It("should return false when ManilaShareImages match but CinderVolumeImages differ", func() { + cinderImageV1 := "registry.example.com/cinder-volume:backend1-v1.0.0" + cinderImageV2 := "registry.example.com/cinder-volume:backend1-v2.0.0" + manilaImage := "registry.example.com/manila-share:nfs-v1.0.0" + + a := CustomContainerImages{ + CinderVolumeImages: map[string]*string{ + "backend1": &cinderImageV1, + }, + ManilaShareImages: map[string]*string{ + "nfs": &manilaImage, + }, + } + b := CustomContainerImages{ + CinderVolumeImages: map[string]*string{ + "backend1": &cinderImageV2, + }, + ManilaShareImages: map[string]*string{ + "nfs": &manilaImage, + }, + } + Expect(customContainerImagesAllModified(a, b)).To(BeTrue()) + }) + + }) + + Context("ContainerTemplate fields comparison", func() { + + It("should return false when ContainerTemplate fields differ but CinderVolume/Manila fields match", func() { + keystoneImageV1 := "registry.example.com/keystone:v1.0.0" + keystoneImageV2 := "registry.example.com/keystone:v2.0.0" + cinderImage := "registry.example.com/cinder-volume:backend1-v1.0.0" + + a := CustomContainerImages{ + ContainerTemplate: ContainerTemplate{ + KeystoneAPIImage: &keystoneImageV1, + }, + CinderVolumeImages: map[string]*string{ + "backend1": &cinderImage, + }, + } + b := CustomContainerImages{ + ContainerTemplate: ContainerTemplate{ + KeystoneAPIImage: &keystoneImageV2, + }, + CinderVolumeImages: map[string]*string{ + "backend1": &cinderImage, + }, + } + Expect(customContainerImagesAllModified(a, b)).To(BeTrue()) + }) + + It("should return true when all fields including ContainerTemplate match", func() { + keystoneImage := "registry.example.com/keystone:v1.0.0" + novaImage := "registry.example.com/nova:v1.0.0" + cinderImage := "registry.example.com/cinder-volume:backend1-v1.0.0" + manilaImage := "registry.example.com/manila-share:nfs-v1.0.0" + + a := CustomContainerImages{ + ContainerTemplate: ContainerTemplate{ + KeystoneAPIImage: &keystoneImage, + NovaAPIImage: &novaImage, + }, + CinderVolumeImages: map[string]*string{ + "backend1": &cinderImage, + }, + ManilaShareImages: map[string]*string{ + "nfs": &manilaImage, + }, + } + b := CustomContainerImages{ + ContainerTemplate: ContainerTemplate{ + KeystoneAPIImage: &keystoneImage, + NovaAPIImage: &novaImage, + }, + CinderVolumeImages: map[string]*string{ + "backend1": &cinderImage, + }, + ManilaShareImages: map[string]*string{ + "nfs": &manilaImage, + }, + } + Expect(customContainerImagesAllModified(a, b)).To(BeFalse()) + }) + + }) + +}) diff --git a/apis/core/v1beta1/zz_generated.deepcopy.go b/apis/core/v1beta1/zz_generated.deepcopy.go index 1c6212861d..470654df30 100644 --- a/apis/core/v1beta1/zz_generated.deepcopy.go +++ b/apis/core/v1beta1/zz_generated.deepcopy.go @@ -1418,6 +1418,13 @@ func (in *OpenStackVersionStatus) DeepCopyInto(out *OpenStackVersionStatus) { } } in.ServiceDefaults.DeepCopyInto(&out.ServiceDefaults) + if in.TrackedCustomImages != nil { + in, out := &in.TrackedCustomImages, &out.TrackedCustomImages + *out = make(map[string]CustomContainerImages, len(*in)) + for key, val := range *in { + (*out)[key] = *val.DeepCopy() + } + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OpenStackVersionStatus. diff --git a/bindata/crds/crds.yaml b/bindata/crds/crds.yaml index 8c349f319a..32b7b09cc8 100644 --- a/bindata/crds/crds.yaml +++ b/bindata/crds/crds.yaml @@ -18033,6 +18033,203 @@ spec: glanceWsgi: type: string type: object + trackedCustomImages: + additionalProperties: + properties: + agentImage: + type: string + ansibleeeImage: + type: string + aodhAPIImage: + type: string + aodhEvaluatorImage: + type: string + aodhListenerImage: + type: string + aodhNotifierImage: + type: string + apacheImage: + type: string + barbicanAPIImage: + type: string + barbicanKeystoneListenerImage: + type: string + barbicanWorkerImage: + type: string + ceilometerCentralImage: + type: string + ceilometerComputeImage: + type: string + ceilometerIpmiImage: + type: string + ceilometerMysqldExporterImage: + type: string + ceilometerNotificationImage: + type: string + ceilometerSgcoreImage: + type: string + cinderAPIImage: + type: string + cinderBackupImage: + type: string + cinderSchedulerImage: + type: string + cinderVolumeImages: + additionalProperties: + type: string + type: object + designateAPIImage: + type: string + designateBackendbind9Image: + type: string + designateCentralImage: + type: string + designateMdnsImage: + type: string + designateProducerImage: + type: string + designateUnboundImage: + type: string + designateWorkerImage: + type: string + edpmFrrImage: + type: string + edpmIscsidImage: + type: string + edpmKeplerImage: + type: string + edpmLogrotateCrondImage: + type: string + edpmMultipathdImage: + type: string + edpmNeutronDhcpAgentImage: + type: string + edpmNeutronMetadataAgentImage: + type: string + edpmNeutronOvnAgentImage: + type: string + edpmNeutronSriovAgentImage: + type: string + edpmNodeExporterImage: + type: string + edpmOpenstackNetworkExporterImage: + type: string + edpmOvnBgpAgentImage: + type: string + edpmPodmanExporterImage: + type: string + glanceAPIImage: + type: string + heatAPIImage: + type: string + heatCfnapiImage: + type: string + heatEngineImage: + type: string + horizonImage: + type: string + infraDnsmasqImage: + type: string + infraMemcachedImage: + type: string + infraRedisImage: + type: string + ironicAPIImage: + type: string + ironicConductorImage: + type: string + ironicInspectorImage: + type: string + ironicNeutronAgentImage: + type: string + ironicPxeImage: + type: string + ironicPythonAgentImage: + type: string + keystoneAPIImage: + type: string + ksmImage: + type: string + manilaAPIImage: + type: string + manilaSchedulerImage: + type: string + manilaShareImages: + additionalProperties: + type: string + type: object + mariadbImage: + type: string + netUtilsImage: + type: string + neutronAPIImage: + type: string + novaAPIImage: + type: string + novaComputeImage: + type: string + novaConductorImage: + type: string + novaNovncImage: + type: string + novaSchedulerImage: + type: string + octaviaAPIImage: + type: string + octaviaHealthmanagerImage: + type: string + octaviaHousekeepingImage: + type: string + octaviaRsyslogImage: + type: string + octaviaWorkerImage: + type: string + openstackClientImage: + type: string + openstackNetworkExporterImage: + type: string + osContainerImage: + type: string + ovnControllerImage: + type: string + ovnControllerOvsImage: + type: string + ovnNbDbclusterImage: + type: string + ovnNorthdImage: + type: string + ovnSbDbclusterImage: + type: string + placementAPIImage: + type: string + rabbitmqImage: + type: string + swiftAccountImage: + type: string + swiftContainerImage: + type: string + swiftObjectImage: + type: string + swiftProxyImage: + type: string + telemetryNodeExporterImage: + type: string + testAnsibletestImage: + type: string + testHorizontestImage: + type: string + testTempestImage: + type: string + testTobikoImage: + type: string + watcherAPIImage: + type: string + watcherApplierImage: + type: string + watcherDecisionEngineImage: + type: string + type: object + type: object type: object type: object served: true diff --git a/config/crd/bases/core.openstack.org_openstackversions.yaml b/config/crd/bases/core.openstack.org_openstackversions.yaml index 3020e010e1..6389e8bca4 100644 --- a/config/crd/bases/core.openstack.org_openstackversions.yaml +++ b/config/crd/bases/core.openstack.org_openstackversions.yaml @@ -674,6 +674,203 @@ spec: glanceWsgi: type: string type: object + trackedCustomImages: + additionalProperties: + properties: + agentImage: + type: string + ansibleeeImage: + type: string + aodhAPIImage: + type: string + aodhEvaluatorImage: + type: string + aodhListenerImage: + type: string + aodhNotifierImage: + type: string + apacheImage: + type: string + barbicanAPIImage: + type: string + barbicanKeystoneListenerImage: + type: string + barbicanWorkerImage: + type: string + ceilometerCentralImage: + type: string + ceilometerComputeImage: + type: string + ceilometerIpmiImage: + type: string + ceilometerMysqldExporterImage: + type: string + ceilometerNotificationImage: + type: string + ceilometerSgcoreImage: + type: string + cinderAPIImage: + type: string + cinderBackupImage: + type: string + cinderSchedulerImage: + type: string + cinderVolumeImages: + additionalProperties: + type: string + type: object + designateAPIImage: + type: string + designateBackendbind9Image: + type: string + designateCentralImage: + type: string + designateMdnsImage: + type: string + designateProducerImage: + type: string + designateUnboundImage: + type: string + designateWorkerImage: + type: string + edpmFrrImage: + type: string + edpmIscsidImage: + type: string + edpmKeplerImage: + type: string + edpmLogrotateCrondImage: + type: string + edpmMultipathdImage: + type: string + edpmNeutronDhcpAgentImage: + type: string + edpmNeutronMetadataAgentImage: + type: string + edpmNeutronOvnAgentImage: + type: string + edpmNeutronSriovAgentImage: + type: string + edpmNodeExporterImage: + type: string + edpmOpenstackNetworkExporterImage: + type: string + edpmOvnBgpAgentImage: + type: string + edpmPodmanExporterImage: + type: string + glanceAPIImage: + type: string + heatAPIImage: + type: string + heatCfnapiImage: + type: string + heatEngineImage: + type: string + horizonImage: + type: string + infraDnsmasqImage: + type: string + infraMemcachedImage: + type: string + infraRedisImage: + type: string + ironicAPIImage: + type: string + ironicConductorImage: + type: string + ironicInspectorImage: + type: string + ironicNeutronAgentImage: + type: string + ironicPxeImage: + type: string + ironicPythonAgentImage: + type: string + keystoneAPIImage: + type: string + ksmImage: + type: string + manilaAPIImage: + type: string + manilaSchedulerImage: + type: string + manilaShareImages: + additionalProperties: + type: string + type: object + mariadbImage: + type: string + netUtilsImage: + type: string + neutronAPIImage: + type: string + novaAPIImage: + type: string + novaComputeImage: + type: string + novaConductorImage: + type: string + novaNovncImage: + type: string + novaSchedulerImage: + type: string + octaviaAPIImage: + type: string + octaviaHealthmanagerImage: + type: string + octaviaHousekeepingImage: + type: string + octaviaRsyslogImage: + type: string + octaviaWorkerImage: + type: string + openstackClientImage: + type: string + openstackNetworkExporterImage: + type: string + osContainerImage: + type: string + ovnControllerImage: + type: string + ovnControllerOvsImage: + type: string + ovnNbDbclusterImage: + type: string + ovnNorthdImage: + type: string + ovnSbDbclusterImage: + type: string + placementAPIImage: + type: string + rabbitmqImage: + type: string + swiftAccountImage: + type: string + swiftContainerImage: + type: string + swiftObjectImage: + type: string + swiftProxyImage: + type: string + telemetryNodeExporterImage: + type: string + testAnsibletestImage: + type: string + testHorizontestImage: + type: string + testTempestImage: + type: string + testTobikoImage: + type: string + watcherAPIImage: + type: string + watcherApplierImage: + type: string + watcherDecisionEngineImage: + type: string + type: object + type: object type: object type: object served: true diff --git a/controllers/core/openstackversion_controller.go b/controllers/core/openstackversion_controller.go index 86393782b6..fccecf839c 100644 --- a/controllers/core/openstackversion_controller.go +++ b/controllers/core/openstackversion_controller.go @@ -206,6 +206,12 @@ func (r *OpenStackVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req } instance.Status.ContainerImages = openstack.GetContainerImages(val, *instance) + // Track CustomContainerImages for this version + if instance.Status.TrackedCustomImages == nil { + instance.Status.TrackedCustomImages = make(map[string]corev1beta1.CustomContainerImages) + } + instance.Status.TrackedCustomImages[instance.Spec.TargetVersion] = instance.Spec.CustomContainerImages + // initialize service defaults serviceDefaults := openstack.InitializeOpenStackVersionServiceDefaults(ctx) if instance.Status.AvailableServiceDefaults == nil { @@ -394,6 +400,14 @@ func (r *OpenStackVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req if controlPlane.IsReady() { Log.Info("Setting DeployedVersion") instance.Status.DeployedVersion = &instance.Spec.TargetVersion + + // Remove skip-custom-images-validation annotation after minor update completion + if instance.Annotations != nil { + if _, exists := instance.Annotations["core.openstack.org/skip-custom-images-validation"]; exists { + delete(instance.Annotations, "core.openstack.org/skip-custom-images-validation") + Log.Info("Removed skip-custom-images-validation annotation after minor update completion") + } + } } if instance.Status.DeployedVersion != nil && *instance.Status.AvailableVersion != *instance.Status.DeployedVersion { diff --git a/tests/functional/ctlplane/openstackversion_controller_test.go b/tests/functional/ctlplane/openstackversion_controller_test.go index b55a8be27b..ac55de9162 100644 --- a/tests/functional/ctlplane/openstackversion_controller_test.go +++ b/tests/functional/ctlplane/openstackversion_controller_test.go @@ -19,6 +19,7 @@ package functional_test import ( "errors" "os" + "strings" . "github.com/onsi/ginkgo/v2" //revive:disable:dot-imports . "github.com/onsi/gomega" //revive:disable:dot-imports @@ -685,4 +686,549 @@ var _ = Describe("OpenStackOperator controller", func() { }) + When("CustomContainerImages are set", func() { + var ( + initialVersion = "0.0.1" + updatedVersion = "0.0.2" + ) + + When("KeystoneAPI custom images", func() { + var customKeystoneImage = "custom.registry/keystone:custom-tag" + + BeforeEach(func() { + // Create OpenStackVersion with custom container images + spec := GetDefaultOpenStackVersionSpec() + spec["customContainerImages"] = map[string]interface{}{ + "keystoneAPIImage": customKeystoneImage, + } + + DeferCleanup( + th.DeleteInstance, + CreateOpenStackVersion(names.OpenStackVersionName, spec), + ) + + // Wait for initial version to be created and initialized + Eventually(func(g Gomega) { + th.ExpectCondition( + names.OpenStackVersionName, + ConditionGetterFunc(OpenStackVersionConditionGetter), + corev1.OpenStackVersionInitialized, + k8s_corev1.ConditionTrue, + ) + + version := GetOpenStackVersion(names.OpenStackVersionName) + g.Expect(version).Should(Not(BeNil())) + g.Expect(*version.Status.AvailableVersion).Should(ContainSubstring(initialVersion)) + g.Expect(version.Spec.TargetVersion).Should(ContainSubstring(initialVersion)) + }, timeout, interval).Should(Succeed()) + + // Inject a newer version for testing minor updates + Eventually(func(g Gomega) { + version := GetOpenStackVersion(names.OpenStackVersionName) + // Create container defaults for the updated version + if version.Status.ContainerImageVersionDefaults == nil { + version.Status.ContainerImageVersionDefaults = make(map[string]*corev1.ContainerDefaults) + } + version.Status.ContainerImageVersionDefaults[updatedVersion] = &corev1.ContainerDefaults{ + ContainerTemplate: corev1.ContainerTemplate{ + KeystoneAPIImage: &customKeystoneImage, + }, + } + g.Expect(th.K8sClient.Status().Update(th.Ctx, version)).To(Succeed()) + }, timeout, interval).Should(Succeed()) + + // Simulate deployment by setting DeployedVersion + Eventually(func(g Gomega) { + version := GetOpenStackVersion(names.OpenStackVersionName) + version.Status.DeployedVersion = &initialVersion + + // Track the custom images for the initial version + if version.Status.TrackedCustomImages == nil { + version.Status.TrackedCustomImages = make(map[string]corev1.CustomContainerImages) + } + version.Status.TrackedCustomImages[initialVersion] = corev1.CustomContainerImages{ + ContainerTemplate: corev1.ContainerTemplate{ + KeystoneAPIImage: &customKeystoneImage, + }, + } + + g.Expect(th.K8sClient.Status().Update(th.Ctx, version)).To(Succeed()) + }, timeout, interval).Should(Succeed()) + + // Remove finalizer as needed for cleanup + DeferCleanup( + OpenStackVersionRemoveFinalizer, + ctx, + names.OpenStackVersionName, + ) + }) + + It("should prevent targetVersion modification when CustomContainerImages are unchanged", func() { + // Attempt to update targetVersion without changing CustomContainerImages + // This should be rejected by the webhook validation + Eventually(func(g Gomega) { + version := GetOpenStackVersion(names.OpenStackVersionName) + + // Try to update to the new version without changing custom images + version.Spec.TargetVersion = updatedVersion + // Keep the same custom container images (this should trigger validation error) + version.Spec.CustomContainerImages = corev1.CustomContainerImages{ + ContainerTemplate: corev1.ContainerTemplate{ + KeystoneAPIImage: &customKeystoneImage, + }, + } + + err := k8sClient.Update(ctx, version) + g.Expect(err).Should(HaveOccurred()) + g.Expect(err.Error()).Should(ContainSubstring("CustomContainerImages must be updated when changing targetVersion")) + g.Expect(err.Error()).Should(ContainSubstring("prevents proper version tracking and validation")) + }, timeout, interval).Should(Succeed()) + }) + + It("should allow targetVersion modification when CustomContainerImages are updated", func() { + // Update CustomContainerImages along with targetVersion + newCustomKeystoneImage := "custom.registry/keystone:new-custom-tag" + + // Use Eventually to handle potential conflicts from controller updates + Eventually(func(g Gomega) { + version := GetOpenStackVersion(names.OpenStackVersionName) + version.Spec.TargetVersion = updatedVersion + // Update the custom container images (this should be allowed) + version.Spec.CustomContainerImages = corev1.CustomContainerImages{ + ContainerTemplate: corev1.ContainerTemplate{ + KeystoneAPIImage: &newCustomKeystoneImage, + }, + } + + g.Expect(k8sClient.Update(ctx, version)).To(Succeed()) + }, timeout, interval).Should(Succeed()) + + // Verify the update was successful + Eventually(func(g Gomega) { + updatedVersionObj := GetOpenStackVersion(names.OpenStackVersionName) + g.Expect(updatedVersionObj.Spec.TargetVersion).Should(Equal(updatedVersion)) + g.Expect(*updatedVersionObj.Spec.CustomContainerImages.KeystoneAPIImage).Should(Equal(newCustomKeystoneImage)) + }, timeout, interval).Should(Succeed()) + }) + + It("should allow targetVersion modification when no CustomContainerImages are set", func() { + // First, remove custom container images + Eventually(func(g Gomega) { + version := GetOpenStackVersion(names.OpenStackVersionName) + version.Spec.CustomContainerImages = corev1.CustomContainerImages{} + g.Expect(k8sClient.Update(ctx, version)).To(Succeed()) + }, timeout, interval).Should(Succeed()) + + // Now update targetVersion (this should be allowed since no custom images) + Eventually(func(g Gomega) { + version := GetOpenStackVersion(names.OpenStackVersionName) + version.Spec.TargetVersion = updatedVersion + g.Expect(k8sClient.Update(ctx, version)).To(Succeed()) + }, timeout, interval).Should(Succeed()) + + // Verify the update was successful + Eventually(func(g Gomega) { + versionObj := GetOpenStackVersion(names.OpenStackVersionName) + g.Expect(versionObj.Spec.TargetVersion).Should(Equal(updatedVersion)) + }, timeout, interval).Should(Succeed()) + }) + }) + + When("CinderVolume custom images", func() { + var customCinderVolumeImage = "custom.registry/cinder-volume:custom-tag" + + BeforeEach(func() { + // Create OpenStackVersion with custom CinderVolumeImages + spec := GetDefaultOpenStackVersionSpec() + spec["customContainerImages"] = map[string]interface{}{ + "cinderVolumeImages": map[string]string{ + "backend1": customCinderVolumeImage, + }, + } + + DeferCleanup( + th.DeleteInstance, + CreateOpenStackVersion(names.OpenStackVersionName, spec), + ) + + // Wait for initial version to be created and initialized + Eventually(func(g Gomega) { + th.ExpectCondition( + names.OpenStackVersionName, + ConditionGetterFunc(OpenStackVersionConditionGetter), + corev1.OpenStackVersionInitialized, + k8s_corev1.ConditionTrue, + ) + + version := GetOpenStackVersion(names.OpenStackVersionName) + g.Expect(version).Should(Not(BeNil())) + g.Expect(*version.Status.AvailableVersion).Should(ContainSubstring(initialVersion)) + g.Expect(version.Spec.TargetVersion).Should(ContainSubstring(initialVersion)) + }, timeout, interval).Should(Succeed()) + + // Inject a newer version for testing minor updates + Eventually(func(g Gomega) { + version := GetOpenStackVersion(names.OpenStackVersionName) + // Create container defaults for the updated version + if version.Status.ContainerImageVersionDefaults == nil { + version.Status.ContainerImageVersionDefaults = make(map[string]*corev1.ContainerDefaults) + } + version.Status.ContainerImageVersionDefaults[updatedVersion] = &corev1.ContainerDefaults{} + g.Expect(th.K8sClient.Status().Update(th.Ctx, version)).To(Succeed()) + }, timeout, interval).Should(Succeed()) + + // Simulate deployment by setting DeployedVersion + Eventually(func(g Gomega) { + version := GetOpenStackVersion(names.OpenStackVersionName) + version.Status.DeployedVersion = &initialVersion + + // Track the custom images for the initial version + if version.Status.TrackedCustomImages == nil { + version.Status.TrackedCustomImages = make(map[string]corev1.CustomContainerImages) + } + version.Status.TrackedCustomImages[initialVersion] = corev1.CustomContainerImages{ + CinderVolumeImages: map[string]*string{ + "backend1": &customCinderVolumeImage, + }, + } + + g.Expect(th.K8sClient.Status().Update(th.Ctx, version)).To(Succeed()) + }, timeout, interval).Should(Succeed()) + + // Remove finalizer as needed for cleanup + DeferCleanup( + OpenStackVersionRemoveFinalizer, + ctx, + names.OpenStackVersionName, + ) + }) + + It("should prevent targetVersion modification when CinderVolumeImages are unchanged", func() { + // Attempt to update targetVersion without changing CinderVolumeImages + Eventually(func(g Gomega) { + version := GetOpenStackVersion(names.OpenStackVersionName) + version.Spec.TargetVersion = updatedVersion + // Keep the same CinderVolumeImages (this should trigger validation error) + version.Spec.CustomContainerImages = corev1.CustomContainerImages{ + CinderVolumeImages: map[string]*string{ + "backend1": &customCinderVolumeImage, + }, + } + + err := k8sClient.Update(ctx, version) + if err != nil && strings.Contains(err.Error(), "the object has been modified") { + // Retry on conflict errors + return + } + g.Expect(err).Should(HaveOccurred()) + g.Expect(err.Error()).Should(ContainSubstring("CustomContainerImages must be updated when changing targetVersion")) + }, timeout, interval).Should(Succeed()) + }) + + It("should allow targetVersion modification when CinderVolumeImages are updated", func() { + newCustomCinderVolumeImage := "custom.registry/cinder-volume:new-custom-tag" + + // Update CinderVolumeImages along with targetVersion + Eventually(func(g Gomega) { + version := GetOpenStackVersion(names.OpenStackVersionName) + version.Spec.TargetVersion = updatedVersion + // Update the CinderVolumeImages (this should be allowed) + version.Spec.CustomContainerImages = corev1.CustomContainerImages{ + CinderVolumeImages: map[string]*string{ + "backend1": &newCustomCinderVolumeImage, + }, + } + g.Expect(k8sClient.Update(ctx, version)).To(Succeed()) + }, timeout, interval).Should(Succeed()) + + // Verify the update was successful + Eventually(func(g Gomega) { + updatedVersionObj := GetOpenStackVersion(names.OpenStackVersionName) + g.Expect(updatedVersionObj.Spec.TargetVersion).Should(Equal(updatedVersion)) + g.Expect(*updatedVersionObj.Spec.CustomContainerImages.CinderVolumeImages["backend1"]).Should(Equal(newCustomCinderVolumeImage)) + }, timeout, interval).Should(Succeed()) + }) + }) + + When("ManilaShare custom images", func() { + var customManilaShareImage = "custom.registry/manila-share:custom-tag" + + BeforeEach(func() { + // Create OpenStackVersion with custom ManilaShareImages + spec := GetDefaultOpenStackVersionSpec() + spec["customContainerImages"] = map[string]interface{}{ + "manilaShareImages": map[string]string{ + "share-backend1": customManilaShareImage, + }, + } + + DeferCleanup( + th.DeleteInstance, + CreateOpenStackVersion(names.OpenStackVersionName, spec), + ) + + // Wait for initial version to be created and initialized + Eventually(func(g Gomega) { + th.ExpectCondition( + names.OpenStackVersionName, + ConditionGetterFunc(OpenStackVersionConditionGetter), + corev1.OpenStackVersionInitialized, + k8s_corev1.ConditionTrue, + ) + + version := GetOpenStackVersion(names.OpenStackVersionName) + g.Expect(version).Should(Not(BeNil())) + g.Expect(*version.Status.AvailableVersion).Should(ContainSubstring(initialVersion)) + g.Expect(version.Spec.TargetVersion).Should(ContainSubstring(initialVersion)) + }, timeout, interval).Should(Succeed()) + + // Inject a newer version for testing minor updates + Eventually(func(g Gomega) { + version := GetOpenStackVersion(names.OpenStackVersionName) + // Create container defaults for the updated version + if version.Status.ContainerImageVersionDefaults == nil { + version.Status.ContainerImageVersionDefaults = make(map[string]*corev1.ContainerDefaults) + } + version.Status.ContainerImageVersionDefaults[updatedVersion] = &corev1.ContainerDefaults{} + g.Expect(th.K8sClient.Status().Update(th.Ctx, version)).To(Succeed()) + }, timeout, interval).Should(Succeed()) + + // Simulate deployment by setting DeployedVersion + Eventually(func(g Gomega) { + version := GetOpenStackVersion(names.OpenStackVersionName) + version.Status.DeployedVersion = &initialVersion + + // Track the custom images for the initial version + if version.Status.TrackedCustomImages == nil { + version.Status.TrackedCustomImages = make(map[string]corev1.CustomContainerImages) + } + version.Status.TrackedCustomImages[initialVersion] = corev1.CustomContainerImages{ + ManilaShareImages: map[string]*string{ + "share-backend1": &customManilaShareImage, + }, + } + + g.Expect(th.K8sClient.Status().Update(th.Ctx, version)).To(Succeed()) + }, timeout, interval).Should(Succeed()) + + // Remove finalizer as needed for cleanup + DeferCleanup( + OpenStackVersionRemoveFinalizer, + ctx, + names.OpenStackVersionName, + ) + }) + + It("should prevent targetVersion modification when ManilaShareImages are unchanged", func() { + // Attempt to update targetVersion without changing ManilaShareImages + Eventually(func(g Gomega) { + version := GetOpenStackVersion(names.OpenStackVersionName) + version.Spec.TargetVersion = updatedVersion + // Keep the same ManilaShareImages (this should trigger validation error) + version.Spec.CustomContainerImages = corev1.CustomContainerImages{ + ManilaShareImages: map[string]*string{ + "share-backend1": &customManilaShareImage, + }, + } + + err := k8sClient.Update(ctx, version) + if err != nil && strings.Contains(err.Error(), "the object has been modified") { + // Retry on conflict errors + return + } + g.Expect(err).Should(HaveOccurred()) + g.Expect(err.Error()).Should(ContainSubstring("CustomContainerImages must be updated when changing targetVersion")) + g.Expect(err.Error()).Should(ContainSubstring("prevents proper version tracking and validation")) + }, timeout, interval).Should(Succeed()) + }) + + It("should allow targetVersion modification when ManilaShareImages are updated", func() { + newCustomManilaShareImage := "custom.registry/manila-share:new-custom-tag" + + // Update ManilaShareImages along with targetVersion + Eventually(func(g Gomega) { + version := GetOpenStackVersion(names.OpenStackVersionName) + version.Spec.TargetVersion = updatedVersion + // Update the ManilaShareImages (this should be allowed) + version.Spec.CustomContainerImages = corev1.CustomContainerImages{ + ManilaShareImages: map[string]*string{ + "share-backend1": &newCustomManilaShareImage, + }, + } + g.Expect(k8sClient.Update(ctx, version)).To(Succeed()) + }, timeout, interval).Should(Succeed()) + + // Verify the update was successful + Eventually(func(g Gomega) { + updatedVersionObj := GetOpenStackVersion(names.OpenStackVersionName) + g.Expect(updatedVersionObj.Spec.TargetVersion).Should(Equal(updatedVersion)) + g.Expect(*updatedVersionObj.Spec.CustomContainerImages.ManilaShareImages["share-backend1"]).Should(Equal(newCustomManilaShareImage)) + }, timeout, interval).Should(Succeed()) + }) + }) + + When("skip-custom-images-validation annotation", func() { + var customKeystoneImage = "custom.registry/keystone:custom-tag" + + BeforeEach(func() { + // Create OpenStackVersion with custom container images + spec := GetDefaultOpenStackVersionSpec() + spec["customContainerImages"] = map[string]interface{}{ + "keystoneAPIImage": customKeystoneImage, + } + + DeferCleanup( + th.DeleteInstance, + CreateOpenStackVersion(names.OpenStackVersionName, spec), + ) + + // Wait for initial version to be created and initialized + Eventually(func(g Gomega) { + th.ExpectCondition( + names.OpenStackVersionName, + ConditionGetterFunc(OpenStackVersionConditionGetter), + corev1.OpenStackVersionInitialized, + k8s_corev1.ConditionTrue, + ) + + version := GetOpenStackVersion(names.OpenStackVersionName) + g.Expect(version).Should(Not(BeNil())) + g.Expect(*version.Status.AvailableVersion).Should(ContainSubstring(initialVersion)) + g.Expect(version.Spec.TargetVersion).Should(ContainSubstring(initialVersion)) + }, timeout, interval).Should(Succeed()) + + // Inject a newer version for testing minor updates + Eventually(func(g Gomega) { + version := GetOpenStackVersion(names.OpenStackVersionName) + // Create container defaults for the updated version + if version.Status.ContainerImageVersionDefaults == nil { + version.Status.ContainerImageVersionDefaults = make(map[string]*corev1.ContainerDefaults) + } + version.Status.ContainerImageVersionDefaults[updatedVersion] = &corev1.ContainerDefaults{ + ContainerTemplate: corev1.ContainerTemplate{ + KeystoneAPIImage: &customKeystoneImage, + }, + } + g.Expect(th.K8sClient.Status().Update(th.Ctx, version)).To(Succeed()) + }, timeout, interval).Should(Succeed()) + + // Simulate deployment by setting DeployedVersion + Eventually(func(g Gomega) { + version := GetOpenStackVersion(names.OpenStackVersionName) + version.Status.DeployedVersion = &initialVersion + + // Track the custom images for the initial version + if version.Status.TrackedCustomImages == nil { + version.Status.TrackedCustomImages = make(map[string]corev1.CustomContainerImages) + } + version.Status.TrackedCustomImages[initialVersion] = corev1.CustomContainerImages{ + ContainerTemplate: corev1.ContainerTemplate{ + KeystoneAPIImage: &customKeystoneImage, + }, + } + + g.Expect(th.K8sClient.Status().Update(th.Ctx, version)).To(Succeed()) + }, timeout, interval).Should(Succeed()) + + // Remove finalizer as needed for cleanup + DeferCleanup( + OpenStackVersionRemoveFinalizer, + ctx, + names.OpenStackVersionName, + ) + }) + + It("should allow targetVersion modification with skip annotation when CustomContainerImages are unchanged", func() { + // Update targetVersion with skip annotation, keeping same custom images + Eventually(func(g Gomega) { + version := GetOpenStackVersion(names.OpenStackVersionName) + + // Add the skip annotation + if version.Annotations == nil { + version.Annotations = make(map[string]string) + } + version.Annotations["core.openstack.org/skip-custom-images-validation"] = "true" + + version.Spec.TargetVersion = updatedVersion + // Keep the same custom container images (should be allowed with annotation) + version.Spec.CustomContainerImages = corev1.CustomContainerImages{ + ContainerTemplate: corev1.ContainerTemplate{ + KeystoneAPIImage: &customKeystoneImage, + }, + } + + g.Expect(k8sClient.Update(ctx, version)).To(Succeed()) + }, timeout, interval).Should(Succeed()) + + // Verify the update was successful + Eventually(func(g Gomega) { + updatedVersionObj := GetOpenStackVersion(names.OpenStackVersionName) + g.Expect(updatedVersionObj.Spec.TargetVersion).Should(Equal(updatedVersion)) + g.Expect(*updatedVersionObj.Spec.CustomContainerImages.KeystoneAPIImage).Should(Equal(customKeystoneImage)) + g.Expect(updatedVersionObj.Annotations["core.openstack.org/skip-custom-images-validation"]).Should(Equal("true")) + }, timeout, interval).Should(Succeed()) + }) + + It("should allow targetVersion modification with skip annotation set to empty value", func() { + // Update targetVersion with skip annotation set to empty string + Eventually(func(g Gomega) { + version := GetOpenStackVersion(names.OpenStackVersionName) + + // Add the skip annotation with empty value + if version.Annotations == nil { + version.Annotations = make(map[string]string) + } + version.Annotations["core.openstack.org/skip-custom-images-validation"] = "" + + version.Spec.TargetVersion = updatedVersion + // Keep the same custom container images (should be allowed with annotation) + version.Spec.CustomContainerImages = corev1.CustomContainerImages{ + ContainerTemplate: corev1.ContainerTemplate{ + KeystoneAPIImage: &customKeystoneImage, + }, + } + + g.Expect(k8sClient.Update(ctx, version)).To(Succeed()) + }, timeout, interval).Should(Succeed()) + + // Verify the update was successful + Eventually(func(g Gomega) { + updatedVersionObj := GetOpenStackVersion(names.OpenStackVersionName) + g.Expect(updatedVersionObj.Spec.TargetVersion).Should(Equal(updatedVersion)) + g.Expect(*updatedVersionObj.Spec.CustomContainerImages.KeystoneAPIImage).Should(Equal(customKeystoneImage)) + g.Expect(updatedVersionObj.Annotations["core.openstack.org/skip-custom-images-validation"]).Should(Equal("")) + }, timeout, interval).Should(Succeed()) + }) + + It("should prevent targetVersion modification without skip annotation when CustomContainerImages are unchanged", func() { + // Attempt to update targetVersion without skip annotation and unchanged custom images + Eventually(func(g Gomega) { + version := GetOpenStackVersion(names.OpenStackVersionName) + + // Ensure no skip annotation + if version.Annotations != nil { + delete(version.Annotations, "core.openstack.org/skip-custom-images-validation") + } + + version.Spec.TargetVersion = updatedVersion + // Keep the same custom container images (should be rejected without annotation) + version.Spec.CustomContainerImages = corev1.CustomContainerImages{ + ContainerTemplate: corev1.ContainerTemplate{ + KeystoneAPIImage: &customKeystoneImage, + }, + } + + err := k8sClient.Update(ctx, version) + if err != nil && strings.Contains(err.Error(), "the object has been modified") { + // Retry on conflict errors + return + } + g.Expect(err).Should(HaveOccurred()) + g.Expect(err.Error()).Should(ContainSubstring("CustomContainerImages must be updated when changing targetVersion")) + g.Expect(err.Error()).Should(ContainSubstring("prevents proper version tracking and validation")) + }, timeout, interval).Should(Succeed()) + }) + }) + }) + })