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()) + }) + }) + }) + })