diff --git a/api/migrationcontroller/v1alpha1/multinamespacevmstoragemigrationplan_types.go b/api/migrationcontroller/v1alpha1/multinamespacevmstoragemigrationplan_types.go index c829f198..b12ef052 100644 --- a/api/migrationcontroller/v1alpha1/multinamespacevmstoragemigrationplan_types.go +++ b/api/migrationcontroller/v1alpha1/multinamespacevmstoragemigrationplan_types.go @@ -28,6 +28,7 @@ const ( ) // MultiNamespaceVirtualMachineStorageMigrationPlanSpec defines the desired state of MultiNamespaceVirtualMachineStorageMigrationPlan +// +kubebuilder:validation:XValidation:rule="!has(oldSelf.retentionPolicy) || has(self.retentionPolicy)", message="retentionPolicy cannot be removed once set" type MultiNamespaceVirtualMachineStorageMigrationPlanSpec struct { // +patchStrategy=merge // +patchMergeKey=name @@ -37,7 +38,7 @@ type MultiNamespaceVirtualMachineStorageMigrationPlanSpec struct { Namespaces []VirtualMachineStorageMigrationPlanNamespaceSpec `json:"namespaces"` // +kubebuilder:validation:Optional // +kubebuilder:default=keepSource - // +kubebuilder:validation:XValidation:rule="self == oldSelf", message="retentionPolicy is immutable" + // +kubebuilder:validation:XValidation:rule="self == oldSelf", message="retentionPolicy value cannot be changed once set" // RetentionPolicy indicates whether to keep or delete the source DataVolume/PVC after each VM migration completes // in each created namespace plan. When set to "deleteSource", every created VirtualMachineStorageMigrationPlan // will have retentionPolicy set to deleteSource. When "keepSource" or unset, child plans keep their per-namespace diff --git a/api/migrationcontroller/v1alpha1/virtualmachinestoragemigrationplan_types.go b/api/migrationcontroller/v1alpha1/virtualmachinestoragemigrationplan_types.go index 7f3b69ff..ef1f0049 100644 --- a/api/migrationcontroller/v1alpha1/virtualmachinestoragemigrationplan_types.go +++ b/api/migrationcontroller/v1alpha1/virtualmachinestoragemigrationplan_types.go @@ -37,12 +37,13 @@ const ( ) // VirtualMachineStorageMigrationPlanSpec defines the desired state of VirtualMachineStorageMigrationPlan +// +kubebuilder:validation:XValidation:rule="!has(oldSelf.retentionPolicy) || has(self.retentionPolicy)", message="retentionPolicy cannot be removed once set" type VirtualMachineStorageMigrationPlanSpec struct { // The virtual machines to migrate. VirtualMachines []VirtualMachineStorageMigrationPlanVirtualMachine `json:"virtualMachines"` // +kubebuilder:validation:Optional // +kubebuilder:default=keepSource - // +kubebuilder:validation:XValidation:rule="self == oldSelf", message="retentionPolicy is immutable" + // +kubebuilder:validation:XValidation:rule="self == oldSelf", message="retentionPolicy value cannot be changed once set" // RetentionPolicy indicates whether to keep or delete the source DataVolume/PVC after each VM migration completes. // When "keepSource" (default), the source is preserved. When "deleteSource", the source DataVolume is deleted // if it exists, otherwise the source PVC is deleted. diff --git a/config/crd/bases/migrations.kubevirt.io_multinamespacevirtualmachinestoragemigrationplans.yaml b/config/crd/bases/migrations.kubevirt.io_multinamespacevirtualmachinestoragemigrationplans.yaml index e18e752c..be4143a6 100644 --- a/config/crd/bases/migrations.kubevirt.io_multinamespacevirtualmachinestoragemigrationplans.yaml +++ b/config/crd/bases/migrations.kubevirt.io_multinamespacevirtualmachinestoragemigrationplans.yaml @@ -63,7 +63,7 @@ spec: - deleteSource type: string x-kubernetes-validations: - - message: retentionPolicy is immutable + - message: retentionPolicy value cannot be changed once set rule: self == oldSelf virtualMachines: description: The virtual machines to migrate. @@ -146,6 +146,9 @@ spec: - name - virtualMachines type: object + x-kubernetes-validations: + - message: retentionPolicy cannot be removed once set + rule: '!has(oldSelf.retentionPolicy) || has(self.retentionPolicy)' type: array x-kubernetes-list-map-keys: - name @@ -162,11 +165,14 @@ spec: - deleteSource type: string x-kubernetes-validations: - - message: retentionPolicy is immutable + - message: retentionPolicy value cannot be changed once set rule: self == oldSelf required: - namespaces type: object + x-kubernetes-validations: + - message: retentionPolicy cannot be removed once set + rule: '!has(oldSelf.retentionPolicy) || has(self.retentionPolicy)' status: description: MultiNamespaceVirtualMachineStorageMigrationPlanStatus defines the observed state of MultiNamespaceVirtualMachineStorageMigrationPlan diff --git a/config/crd/bases/migrations.kubevirt.io_virtualmachinestoragemigrationplans.yaml b/config/crd/bases/migrations.kubevirt.io_virtualmachinestoragemigrationplans.yaml index dcb61b9d..a02e5e6a 100644 --- a/config/crd/bases/migrations.kubevirt.io_virtualmachinestoragemigrationplans.yaml +++ b/config/crd/bases/migrations.kubevirt.io_virtualmachinestoragemigrationplans.yaml @@ -65,7 +65,7 @@ spec: - deleteSource type: string x-kubernetes-validations: - - message: retentionPolicy is immutable + - message: retentionPolicy value cannot be changed once set rule: self == oldSelf virtualMachines: description: The virtual machines to migrate. @@ -145,6 +145,9 @@ spec: required: - virtualMachines type: object + x-kubernetes-validations: + - message: retentionPolicy cannot be removed once set + rule: '!has(oldSelf.retentionPolicy) || has(self.retentionPolicy)' status: description: VirtualMachineStorageMigrationPlanStatus defines the observed state of VirtualMachineStorageMigrationPlan diff --git a/internal/controller/multinamespacestoragemigplan/controller.go b/internal/controller/multinamespacestoragemigplan/controller.go index d890f76d..b714907d 100644 --- a/internal/controller/multinamespacestoragemigplan/controller.go +++ b/internal/controller/multinamespacestoragemigplan/controller.go @@ -187,10 +187,23 @@ func (r *MultiNamespaceStorageMigPlanReconciler) validateNamespace(ctx context.C func (r *MultiNamespaceStorageMigPlanReconciler) createNamespacePlan(ctx context.Context, plan *migrations.MultiNamespaceVirtualMachineStorageMigrationPlan, namespace *migrations.VirtualMachineStorageMigrationPlanNamespaceSpec) error { spec := namespace.VirtualMachineStorageMigrationPlanSpec.DeepCopy() - // When the multinamespace plan has RetentionPolicy set (e.g. deleteSource), apply it to the child plan - if plan.Spec.RetentionPolicy != nil { - spec.RetentionPolicy = plan.Spec.RetentionPolicy + + // Retention policy priority (with CRD defaults in place): + // 1. Namespace-specific retentionPolicy (if explicitly set to non-default value) + // 2. Multi-namespace plan retentionPolicy (if set to non-default value) + // 3. CRD default (keepSource) + // + // Due to CRD defaulting, both spec.RetentionPolicy and plan.Spec.RetentionPolicy will be non-nil + // (defaulted to keepSource). We treat keepSource as "not explicitly set" when determining priority. + if plan.Spec.RetentionPolicy != nil && *plan.Spec.RetentionPolicy != migrations.RetentionPolicyKeepSource { + // Parent has non-default value + if spec.RetentionPolicy == nil || *spec.RetentionPolicy == migrations.RetentionPolicyKeepSource { + // Namespace has default or nil - inherit from parent + spec.RetentionPolicy = plan.Spec.RetentionPolicy + } + // else: namespace has explicit non-default value, use it (priority 1) } + // else: parent has default (keepSource), namespace keeps its value (default or explicit) namespacePlan := &migrations.VirtualMachineStorageMigrationPlan{ ObjectMeta: metav1.ObjectMeta{ Name: migrations.GetNamespacedPlanName(plan.Name, namespace.Name), diff --git a/internal/controller/multinamespacestoragemigplan/controller_test.go b/internal/controller/multinamespacestoragemigplan/controller_test.go index 42a36daf..09a3a824 100644 --- a/internal/controller/multinamespacestoragemigplan/controller_test.go +++ b/internal/controller/multinamespacestoragemigplan/controller_test.go @@ -87,27 +87,86 @@ var _ = Describe("MultiNamespaceStorageMigPlan controller", func() { }) }) -func createMultiNamespaceStorageMigrationPlan(name string) *migrations.MultiNamespaceVirtualMachineStorageMigrationPlan { - return &migrations.MultiNamespaceVirtualMachineStorageMigrationPlan{ +type multiNamespacePlanOptions struct { + name string + namespace string + uid types.UID + retentionPolicy *migrations.RetentionPolicy + namespaceSpecs []migrations.VirtualMachineStorageMigrationPlanNamespaceSpec +} + +func newMultiNamespacePlanOptions(name string) *multiNamespacePlanOptions { + return &multiNamespacePlanOptions{ + name: name, + namespace: testutils.TestNamespace, + } +} + +func (o *multiNamespacePlanOptions) withUID(uid string) *multiNamespacePlanOptions { + o.uid = types.UID(uid) + return o +} + +func (o *multiNamespacePlanOptions) withRetentionPolicy(policy migrations.RetentionPolicy) *multiNamespacePlanOptions { + o.retentionPolicy = &policy + return o +} + +func (o *multiNamespacePlanOptions) withNamespaceSpec(spec *migrations.VirtualMachineStorageMigrationPlanSpec) *multiNamespacePlanOptions { + o.namespaceSpecs = append(o.namespaceSpecs, migrations.VirtualMachineStorageMigrationPlanNamespaceSpec{ + Name: testutils.TestNamespace, + VirtualMachineStorageMigrationPlanSpec: spec, + }) + return o +} + +func (o *multiNamespacePlanOptions) build() *migrations.MultiNamespaceVirtualMachineStorageMigrationPlan { + plan := &migrations.MultiNamespaceVirtualMachineStorageMigrationPlan{ ObjectMeta: metav1.ObjectMeta{ - Name: name, - Namespace: testutils.TestNamespace, + Name: o.name, + Namespace: o.namespace, }, Spec: migrations.MultiNamespaceVirtualMachineStorageMigrationPlanSpec{ - Namespaces: []migrations.VirtualMachineStorageMigrationPlanNamespaceSpec{ + RetentionPolicy: o.retentionPolicy, + Namespaces: o.namespaceSpecs, + }, + } + if o.uid != "" { + plan.UID = o.uid + } + return plan +} + +// createMultiNamespaceStorageMigrationPlan creates a basic multi-namespace plan with default values +func createMultiNamespaceStorageMigrationPlan(name string) *migrations.MultiNamespaceVirtualMachineStorageMigrationPlan { + return newMultiNamespacePlanOptions(name). + withNamespaceSpec(&migrations.VirtualMachineStorageMigrationPlanSpec{ + VirtualMachines: []migrations.VirtualMachineStorageMigrationPlanVirtualMachine{ { - Name: testutils.TestNamespace, - VirtualMachineStorageMigrationPlanSpec: &migrations.VirtualMachineStorageMigrationPlanSpec{ - VirtualMachines: []migrations.VirtualMachineStorageMigrationPlanVirtualMachine{ - { - Name: "simple-vm", - TargetMigrationPVCs: []migrations.VirtualMachineStorageMigrationPlanTargetMigrationPVC{ - { - VolumeName: "dv-disk", - DestinationPVC: migrations.VirtualMachineStorageMigrationPlanDestinationPVC{}, - }, - }, - }, + Name: "simple-vm", + TargetMigrationPVCs: []migrations.VirtualMachineStorageMigrationPlanTargetMigrationPVC{ + { + VolumeName: "dv-disk", + DestinationPVC: migrations.VirtualMachineStorageMigrationPlanDestinationPVC{}, + }, + }, + }, + }, + }).build() +} + +// newTestVMSpec creates a basic VM spec for testing +func newTestVMSpec(vmName string, retentionPolicy *migrations.RetentionPolicy) *migrations.VirtualMachineStorageMigrationPlanSpec { + return &migrations.VirtualMachineStorageMigrationPlanSpec{ + RetentionPolicy: retentionPolicy, + VirtualMachines: []migrations.VirtualMachineStorageMigrationPlanVirtualMachine{ + { + Name: vmName, + TargetMigrationPVCs: []migrations.VirtualMachineStorageMigrationPlanTargetMigrationPVC{ + { + VolumeName: "test-volume", + DestinationPVC: migrations.VirtualMachineStorageMigrationPlanDestinationPVC{ + Name: ptr.To("target-pvc"), }, }, }, @@ -184,7 +243,7 @@ var _ = Describe("MultiNamespaceStorageMigPlan Controller", func() { Expect(*childPlan.Spec.RetentionPolicy).To(Equal(migrations.RetentionPolicyDeleteSource)) }) - It("Should allow setting the field for the first time, but not update/delete it", func() { + It("should enforce retentionPolicy immutability with type-level and field-level validation", func() { key := types.NamespacedName{Name: "test-resource", Namespace: "default"} created := &migrations.MultiNamespaceVirtualMachineStorageMigrationPlan{ ObjectMeta: metav1.ObjectMeta{Name: key.Name, Namespace: key.Namespace}, @@ -204,22 +263,181 @@ var _ = Describe("MultiNamespaceStorageMigPlan Controller", func() { existing := &migrations.MultiNamespaceVirtualMachineStorageMigrationPlan{} Expect(k8sClient.Get(ctx, key, existing)).To(Succeed()) - // Attempt to change the value + By("Attempting to change value from deleteSource to keepSource - field-level validation should fail") existing.Spec.RetentionPolicy = ptr.To(migrations.RetentionPolicyKeepSource) err := k8sClient.Update(ctx, existing) Expect(err).To(HaveOccurred()) - // Verify the specific CEL error message from your marker - Expect(err.Error()).To(ContainSubstring("retentionPolicy is immutable")) + Expect(err.Error()).To(ContainSubstring("retentionPolicy value cannot be changed once set")) existing = &migrations.MultiNamespaceVirtualMachineStorageMigrationPlan{} Expect(k8sClient.Get(ctx, key, existing)).To(Succeed()) + Expect(*existing.Spec.RetentionPolicy).To(Equal(migrations.RetentionPolicyDeleteSource), + "Value should not have changed") - // Attempt to delete (set to nil) + By("Attempting to remove retentionPolicy (set to nil)") existing.Spec.RetentionPolicy = nil err = k8sClient.Update(ctx, existing) + // With CRD defaulting, nil becomes keepSource, so this triggers field-level validation + // The error will be about value change (deleteSource -> keepSource) not removal Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("retentionPolicy is immutable")) + Expect(err.Error()).To(Or( + ContainSubstring("retentionPolicy value cannot be changed"), + ContainSubstring("retentionPolicy cannot be removed"), + ), "Either field-level or type-level validation should prevent the change") + existing = &migrations.MultiNamespaceVirtualMachineStorageMigrationPlan{} + Expect(k8sClient.Get(ctx, key, existing)).To(Succeed()) + Expect(*existing.Spec.RetentionPolicy).To(Equal(migrations.RetentionPolicyDeleteSource), + "Value should still be deleteSource") + }) + }) + + Context("createNamespacePlan", func() { + var ( + reconciler *MultiNamespaceStorageMigPlanReconciler + ctx context.Context + ) + + BeforeEach(func() { + ctx = context.Background() + reconciler = &MultiNamespaceStorageMigPlanReconciler{ + Client: k8sClient, + Scheme: scheme.Scheme, + EventRecorder: record.NewFakeRecorder(10), + Log: logf.Log.WithName("createNamespacePlan-test"), + } + }) + + AfterEach(func() { + if reconciler != nil { + close(reconciler.EventRecorder.(*record.FakeRecorder).Events) + testutils.CleanupResources(ctx, reconciler.Client) + reconciler = nil + } + }) + + It("should create namespace plan with basic spec", func() { + multiPlan := newMultiNamespacePlanOptions("test-multi-plan"). + withUID("test-uid-123"). + withNamespaceSpec(newTestVMSpec("test-vm", nil)). + build() + + err := reconciler.createNamespacePlan(ctx, multiPlan, &multiPlan.Spec.Namespaces[0]) + Expect(err).NotTo(HaveOccurred()) + + createdPlan := &migrations.VirtualMachineStorageMigrationPlan{} + planName := migrations.GetNamespacedPlanName(multiPlan.Name, testutils.TestNamespace) + err = k8sClient.Get(ctx, types.NamespacedName{Name: planName, Namespace: testutils.TestNamespace}, createdPlan) + Expect(err).NotTo(HaveOccurred()) + + By("Verifying the spec was copied correctly") + Expect(createdPlan.Spec.VirtualMachines).To(HaveLen(1)) + Expect(createdPlan.Spec.VirtualMachines[0].Name).To(Equal("test-vm")) + Expect(createdPlan.Spec.VirtualMachines[0].TargetMigrationPVCs).To(HaveLen(1)) + Expect(createdPlan.Spec.VirtualMachines[0].TargetMigrationPVCs[0].VolumeName).To(Equal("test-volume")) + + By("Verifying labels are set correctly") + Expect(createdPlan.Labels).To(HaveKeyWithValue(multiNamespaceStorageMigPlanUIDLabel, "test-uid-123")) + + By("Verifying annotations are set correctly") + Expect(createdPlan.Annotations).To(HaveKeyWithValue(multiNamespaceStorageMigPlanNameAnnotation, "test-multi-plan")) + Expect(createdPlan.Annotations).To(HaveKeyWithValue(multiNamespaceStorageMigPlanNamespaceAnnotation, testutils.TestNamespace)) + }) + + It("should handle already existing plan gracefully", func() { + multiPlan := newMultiNamespacePlanOptions("test-multi-plan"). + withUID("test-uid-456"). + withNamespaceSpec(newTestVMSpec("test-vm", nil)). + build() + + By("Creating the plan the first time") + err := reconciler.createNamespacePlan(ctx, multiPlan, &multiPlan.Spec.Namespaces[0]) + Expect(err).NotTo(HaveOccurred()) + + By("Creating the plan again should not error") + err = reconciler.createNamespacePlan(ctx, multiPlan, &multiPlan.Spec.Namespaces[0]) + Expect(err).NotTo(HaveOccurred()) + }) + + DescribeTable("should handle retention policy correctly", + func(multiPlanPolicy migrations.RetentionPolicy, namespacePolicy *migrations.RetentionPolicy, expectedPolicy migrations.RetentionPolicy) { + multiPlan := newMultiNamespacePlanOptions("test-multi-plan"). + withUID("test-uid-retention"). + withRetentionPolicy(multiPlanPolicy). + withNamespaceSpec(newTestVMSpec("test-vm", namespacePolicy)). + build() + + err := reconciler.createNamespacePlan(ctx, multiPlan, &multiPlan.Spec.Namespaces[0]) + Expect(err).NotTo(HaveOccurred()) + + createdPlan := &migrations.VirtualMachineStorageMigrationPlan{} + planName := migrations.GetNamespacedPlanName(multiPlan.Name, testutils.TestNamespace) + err = k8sClient.Get(ctx, types.NamespacedName{Name: planName, Namespace: testutils.TestNamespace}, createdPlan) + Expect(err).NotTo(HaveOccurred()) + + Expect(createdPlan.Spec.RetentionPolicy).NotTo(BeNil()) + Expect(*createdPlan.Spec.RetentionPolicy).To(Equal(expectedPolicy)) + }, + Entry("inherit from multi-namespace plan when namespace has default", + migrations.RetentionPolicyDeleteSource, + ptr.To(migrations.RetentionPolicyKeepSource), + migrations.RetentionPolicyDeleteSource, + ), + Entry("preserve namespace retention policy when it has explicit non-default value", + migrations.RetentionPolicyKeepSource, + ptr.To(migrations.RetentionPolicyDeleteSource), + migrations.RetentionPolicyDeleteSource, + ), + Entry("use default retention policy when both use default", + migrations.RetentionPolicyKeepSource, + ptr.To(migrations.RetentionPolicyKeepSource), + migrations.RetentionPolicyKeepSource, + ), + Entry("inherit from multi-namespace plan when namespace retention policy is nil", + migrations.RetentionPolicyDeleteSource, + nil, + migrations.RetentionPolicyDeleteSource, + ), + ) + + It("should create plan with correct naming", func() { + multiPlan := newMultiNamespacePlanOptions("my-migration-plan"). + withUID("test-uid-jkl"). + withNamespaceSpec(newTestVMSpec("test-vm", nil)). + build() + + err := reconciler.createNamespacePlan(ctx, multiPlan, &multiPlan.Spec.Namespaces[0]) + Expect(err).NotTo(HaveOccurred()) + + createdPlan := &migrations.VirtualMachineStorageMigrationPlan{} + expectedPlanName := migrations.GetNamespacedPlanName("my-migration-plan", testutils.TestNamespace) + err = k8sClient.Get(ctx, types.NamespacedName{Name: expectedPlanName, Namespace: testutils.TestNamespace}, createdPlan) + Expect(err).NotTo(HaveOccurred()) + + By("Verifying the plan name follows the naming convention") + Expect(createdPlan.Name).To(Equal(expectedPlanName)) + Expect(createdPlan.Namespace).To(Equal(testutils.TestNamespace)) + }) + + It("should deep copy the spec to avoid mutations", func() { + originalVMName := "original-vm" + multiPlan := newMultiNamespacePlanOptions("test-multi-plan"). + withUID("test-uid-mno"). + withNamespaceSpec(newTestVMSpec(originalVMName, nil)). + build() + + err := reconciler.createNamespacePlan(ctx, multiPlan, &multiPlan.Spec.Namespaces[0]) + Expect(err).NotTo(HaveOccurred()) + + By("Modifying the original spec") + multiPlan.Spec.Namespaces[0].VirtualMachineStorageMigrationPlanSpec.VirtualMachines[0].Name = "modified-vm" + + By("Verifying the created plan was not affected by the mutation") + createdPlan := &migrations.VirtualMachineStorageMigrationPlan{} + planName := migrations.GetNamespacedPlanName(multiPlan.Name, testutils.TestNamespace) + err = k8sClient.Get(ctx, types.NamespacedName{Name: planName, Namespace: testutils.TestNamespace}, createdPlan) + Expect(err).NotTo(HaveOccurred()) + Expect(createdPlan.Spec.VirtualMachines[0].Name).To(Equal(originalVMName)) }) }) }) diff --git a/internal/controller/storagemig/controller_test.go b/internal/controller/storagemig/controller_test.go index 5f80bec4..a5ba9aa7 100644 --- a/internal/controller/storagemig/controller_test.go +++ b/internal/controller/storagemig/controller_test.go @@ -104,7 +104,7 @@ var _ = Describe("StorageMigration Controller", func() { }) Context("When reconciling a migration that is completed", func() { - It("Should allow setting the field for the first time, but not update/delete it", func() { + It("should enforce retentionPolicy immutability with type-level and field-level validation", func() { key := types.NamespacedName{Name: "test-resource", Namespace: "default"} created := &migrations.VirtualMachineStorageMigrationPlan{ ObjectMeta: metav1.ObjectMeta{Name: key.Name, Namespace: key.Namespace}, @@ -126,22 +126,32 @@ var _ = Describe("StorageMigration Controller", func() { existing := &migrations.VirtualMachineStorageMigrationPlan{} Expect(k8sClient.Get(ctx, key, existing)).To(Succeed()) - // Attempt to change the value + By("Attempting to change value from deleteSource to keepSource - field-level validation should fail") existing.Spec.RetentionPolicy = ptr.To(migrations.RetentionPolicyKeepSource) err := k8sClient.Update(ctx, existing) Expect(err).To(HaveOccurred()) - // Verify the specific CEL error message from your marker - Expect(err.Error()).To(ContainSubstring("retentionPolicy is immutable")) + Expect(err.Error()).To(ContainSubstring("retentionPolicy")) existing = &migrations.VirtualMachineStorageMigrationPlan{} Expect(k8sClient.Get(ctx, key, existing)).To(Succeed()) + Expect(*existing.Spec.RetentionPolicy).To(Equal(migrations.RetentionPolicyDeleteSource), + "Value should not have changed") - // Attempt to delete (set to nil) + By("Attempting to remove retentionPolicy (set to nil)") existing.Spec.RetentionPolicy = nil err = k8sClient.Update(ctx, existing) + // With CRD defaulting, nil becomes keepSource, so this triggers field-level validation + // The error will be about value change (deleteSource -> keepSource) not removal Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("retentionPolicy is immutable")) + Expect(err.Error()).To(Or( + ContainSubstring("retentionPolicy value cannot be changed"), + ContainSubstring("retentionPolicy cannot be removed"), + ), "Either field-level or type-level validation should prevent the change") + existing = &migrations.VirtualMachineStorageMigrationPlan{} + Expect(k8sClient.Get(ctx, key, existing)).To(Succeed()) + Expect(*existing.Spec.RetentionPolicy).To(Equal(migrations.RetentionPolicyDeleteSource), + "Value should still be deleteSource") }) }) }) diff --git a/internal/controller/testutils/test-setup.go b/internal/controller/testutils/test-setup.go index 058ce540..ee259d26 100644 --- a/internal/controller/testutils/test-setup.go +++ b/internal/controller/testutils/test-setup.go @@ -47,10 +47,16 @@ func CreateMigPlanNamespace(ctx context.Context, client client.Client, namespace } func DeleteKubeVirtNamespace(ctx context.Context, client client.Client, namespace string) { + if client == nil { + return + } Expect(client.Delete(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}})).To(Succeed()) } func DeleteMigPlanNamespace(ctx context.Context, client client.Client, namespace string) { + if client == nil { + return + } Expect(client.Delete(ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: namespace}})).To(Succeed()) }