diff --git a/api/v1alpha1/vmwarecloudfoundationmigration_types.go b/api/v1alpha1/vmwarecloudfoundationmigration_types.go index 1fb48476b..aa47dec33 100644 --- a/api/v1alpha1/vmwarecloudfoundationmigration_types.go +++ b/api/v1alpha1/vmwarecloudfoundationmigration_types.go @@ -39,6 +39,17 @@ const ( // infrastructures.config.openshift.io/cluster). const SingletonName = "cluster" +// Finalizer is set on the singleton VmwareCloudFoundationMigration while a +// migration is in progress, so the object is not removed out from under an +// interrupted migration (e.g. a test timeout or accidental delete), leaving +// the cluster split across vCenters with nothing left to track or finish it. +const Finalizer = "migration.openshift.io/vcfm-protection" + +// ForceDeleteAnnotation, when set to "true" on the singleton +// VmwareCloudFoundationMigration, allows the object to be deleted even while +// a migration is in progress, abandoning it deliberately. +const ForceDeleteAnnotation = "migration.openshift.io/force-delete" + // SecretReference references a secret by name and namespace. type SecretReference struct { // Name is the secret name. diff --git a/internal/controller/vmwarecloudfoundationmigration_controller.go b/internal/controller/vmwarecloudfoundationmigration_controller.go index 8ea81aca3..5d1e99200 100644 --- a/internal/controller/vmwarecloudfoundationmigration_controller.go +++ b/internal/controller/vmwarecloudfoundationmigration_controller.go @@ -19,6 +19,7 @@ package controller import ( "context" "fmt" + "strconv" "strings" "time" @@ -38,6 +39,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/reconcile" migrationv1alpha1 "github.com/openshift/vcf-migration-operator/api/v1alpha1" @@ -71,6 +73,7 @@ var conditionOrder = []string{ } const reasonWaitingForVSpherePods = "WaitingForVSpherePods" +const reasonDeletionBlocked = "DeletionBlocked" // +kubebuilder:rbac:groups=migration.openshift.io,resources=vmwarecloudfoundationmigrations,verbs=get;list;watch;update;patch // +kubebuilder:rbac:groups=migration.openshift.io,resources=vmwarecloudfoundationmigrations/status,verbs=get;update;patch @@ -121,9 +124,17 @@ func (r *VmwareCloudFoundationMigrationReconciler) Reconcile(ctx context.Context return ctrl.Result{}, nil } + result, done, err := r.handleFinalizer(ctx, migration) + if err != nil { + return ctrl.Result{}, err + } + if done { + return result, nil + } + if migration.Spec.State != migrationv1alpha1.MigrationStateRunning { log.V(1).Info("migration not in Running state, skipping", "state", migration.Spec.State) - return ctrl.Result{}, nil + return result, nil } // Set start time on first reconcile in Running state. @@ -181,6 +192,59 @@ func (r *VmwareCloudFoundationMigrationReconciler) Reconcile(ctx context.Context return ctrl.Result{}, nil } +// handleFinalizer ensures the singleton migration carries the protection +// finalizer while not being deleted, and guards deletion while a migration is +// in progress. It returns done=true when the caller should return immediately +// with the given result/error; done=false means the caller should continue +// with normal reconciliation (e.g. a Running migration keeps progressing +// toward Ready even while the object is Terminating, so it can finish and +// finalize its own deletion). +func (r *VmwareCloudFoundationMigrationReconciler) handleFinalizer(ctx context.Context, migration *migrationv1alpha1.VmwareCloudFoundationMigration) (ctrl.Result, bool, error) { + if migration.DeletionTimestamp.IsZero() { + if controllerutil.AddFinalizer(migration, migrationv1alpha1.Finalizer) { + if err := r.Update(ctx, migration); err != nil { + return ctrl.Result{}, true, fmt.Errorf("adding finalizer: %w", err) + } + return ctrl.Result{}, true, nil + } + return ctrl.Result{RequeueAfter: 10 * time.Second}, false, nil + } + + if !controllerutil.ContainsFinalizer(migration, migrationv1alpha1.Finalizer) { + return ctrl.Result{}, true, nil + } + + blocked := migration.Status.StartTime != nil && !r.isConditionTrue(migration, migrationv1alpha1.ConditionReady) + forced := false + if val, ok := migration.Annotations[migrationv1alpha1.ForceDeleteAnnotation]; ok { + forced, _ = strconv.ParseBool(strings.TrimSpace(val)) + } + if !blocked || forced { + controllerutil.RemoveFinalizer(migration, migrationv1alpha1.Finalizer) + if err := r.Update(ctx, migration); err != nil { + return ctrl.Result{}, true, fmt.Errorf("removing finalizer: %w", err) + } + return ctrl.Result{}, true, nil + } + + cond := apimeta.FindStatusCondition(migration.Status.Conditions, migrationv1alpha1.ConditionReady) + alreadyRecorded := cond != nil && + cond.Status == metav1.ConditionFalse && + cond.Reason == reasonDeletionBlocked + if !alreadyRecorded { + r.setCondition(migration, migrationv1alpha1.ConditionReady, metav1.ConditionFalse, reasonDeletionBlocked, + fmt.Sprintf("migration is in progress; deletion is deferred until it reaches %s, or add the %q annotation to force it", + migrationv1alpha1.ConditionReady, migrationv1alpha1.ForceDeleteAnnotation)) + if err := r.updateStatus(ctx, migration); err != nil { + return ctrl.Result{}, true, fmt.Errorf("updating deletion-blocked status: %w", err) + } + r.Recorder.Eventf(migration, "Warning", reasonDeletionBlocked, + "migration is in progress; deletion is deferred until it reaches %s, or add the %q annotation to force it", + migrationv1alpha1.ConditionReady, migrationv1alpha1.ForceDeleteAnnotation) + } + return ctrl.Result{RequeueAfter: 30 * time.Second}, false, nil +} + // ensureInfrastructurePrepared validates preflight checks and selects the // migration path without performing disruptive cluster changes. func (r *VmwareCloudFoundationMigrationReconciler) ensureInfrastructurePrepared(ctx context.Context, migration *migrationv1alpha1.VmwareCloudFoundationMigration) (ctrl.Result, error) { diff --git a/internal/controller/vmwarecloudfoundationmigration_controller_test.go b/internal/controller/vmwarecloudfoundationmigration_controller_test.go index 885db5d65..6789aa92a 100644 --- a/internal/controller/vmwarecloudfoundationmigration_controller_test.go +++ b/internal/controller/vmwarecloudfoundationmigration_controller_test.go @@ -19,6 +19,7 @@ package controller import ( "context" "testing" + "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -27,6 +28,7 @@ import ( apimeta "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/reconcile" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -157,6 +159,20 @@ var _ = Describe("VmwareCloudFoundationMigration Controller", func() { By("Cleanup the specific resource instance VmwareCloudFoundationMigration") Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) + + // The migration never started (State stayed Pending), so + // deletion is not blocked; one more reconcile removes the + // protection finalizer and lets the object actually go away. + controllerReconciler := &VmwareCloudFoundationMigrationReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + Recorder: record.NewFakeRecorder(10), + } + _, err = controllerReconciler.Reconcile(ctx, reconcile.Request{ + NamespacedName: typeNamespacedName, + }) + Expect(err).NotTo(HaveOccurred()) + Expect(errors.IsNotFound(k8sClient.Get(ctx, typeNamespacedName, &migrationv1alpha1.VmwareCloudFoundationMigration{}))).To(BeTrue()) }) It("should successfully reconcile the resource", func() { By("Reconciling the created resource") @@ -255,4 +271,230 @@ var _ = Describe("VmwareCloudFoundationMigration Controller", func() { Expect(resource.Status.StartTime).To(BeNil()) }) }) + + Context("When the singleton migration is deleted", func() { + const resourceName = migrationv1alpha1.SingletonName + + ctx := context.Background() + + typeNamespacedName := types.NamespacedName{ + Name: resourceName, + Namespace: "default", + } + + newReconciler := func() *VmwareCloudFoundationMigrationReconciler { + return &VmwareCloudFoundationMigrationReconciler{ + Client: k8sClient, + Scheme: k8sClient.Scheme(), + Recorder: record.NewFakeRecorder(10), + } + } + + newResource := func() *migrationv1alpha1.VmwareCloudFoundationMigration { + return &migrationv1alpha1.VmwareCloudFoundationMigration{ + ObjectMeta: metav1.ObjectMeta{ + Name: resourceName, + Namespace: "default", + }, + Spec: migrationv1alpha1.VmwareCloudFoundationMigrationSpec{ + State: migrationv1alpha1.MigrationStatePending, + TargetVCenterCredentialsSecret: migrationv1alpha1.SecretReference{ + Name: "target-vcenter-creds", + Namespace: "default", + }, + FailureDomains: []configv1.VSpherePlatformFailureDomainSpec{ + { + Name: "target-fd-1", + Region: "target-region", + Zone: "target-zone-1", + Server: "vcenter-target.example.com", + Topology: configv1.VSpherePlatformTopology{ + Datacenter: "TargetDC", + ComputeCluster: "/TargetDC/host/TargetCluster", + Datastore: "/TargetDC/datastore/TargetDatastore", + Networks: []string{"VM Network"}, + ResourcePool: "/TargetDC/host/TargetCluster/Resources", + Template: "/TargetDC/vm/rhcos-template", + Folder: "/TargetDC/vm/my-cluster-infra-id", + }, + }, + }, + }, + } + } + + AfterEach(func() { + // Force any still-blocked deletion through so a failed assertion + // earlier in the test doesn't hang the suite. Re-fetches on every + // attempt so a resourceVersion conflict just causes a retry. + reconciler := newReconciler() + Eventually(func() bool { + resource := &migrationv1alpha1.VmwareCloudFoundationMigration{} + if err := k8sClient.Get(ctx, typeNamespacedName, resource); err != nil { + return errors.IsNotFound(err) + } + + if resource.Annotations[migrationv1alpha1.ForceDeleteAnnotation] != "true" { + if resource.Annotations == nil { + resource.Annotations = map[string]string{} + } + resource.Annotations[migrationv1alpha1.ForceDeleteAnnotation] = "true" + if err := k8sClient.Update(ctx, resource); err != nil { + return false + } + } + + if resource.DeletionTimestamp.IsZero() { + if err := k8sClient.Delete(ctx, resource); err != nil { + return false + } + if err := k8sClient.Get(ctx, typeNamespacedName, resource); err != nil { + return errors.IsNotFound(err) + } + } + + if _, _, err := reconciler.handleFinalizer(ctx, resource); err != nil { + return false + } + return errors.IsNotFound(k8sClient.Get(ctx, typeNamespacedName, &migrationv1alpha1.VmwareCloudFoundationMigration{})) + }).WithTimeout(10 * time.Second).WithPolling(500 * time.Millisecond).Should(BeTrue()) + }) + + It("adds the protection finalizer on first reconcile", func() { + Expect(k8sClient.Create(ctx, newResource())).To(Succeed()) + _, err := newReconciler().Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName}) + Expect(err).NotTo(HaveOccurred()) + + resource := &migrationv1alpha1.VmwareCloudFoundationMigration{} + Expect(k8sClient.Get(ctx, typeNamespacedName, resource)).To(Succeed()) + Expect(controllerutil.ContainsFinalizer(resource, migrationv1alpha1.Finalizer)).To(BeTrue()) + }) + + It("deletes immediately when the migration never started", func() { + Expect(k8sClient.Create(ctx, newResource())).To(Succeed()) + reconciler := newReconciler() + _, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName}) + Expect(err).NotTo(HaveOccurred()) + + resource := &migrationv1alpha1.VmwareCloudFoundationMigration{} + Expect(k8sClient.Get(ctx, typeNamespacedName, resource)).To(Succeed()) + Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) + + _, err = reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName}) + Expect(err).NotTo(HaveOccurred()) + Expect(errors.IsNotFound(k8sClient.Get(ctx, typeNamespacedName, &migrationv1alpha1.VmwareCloudFoundationMigration{}))).To(BeTrue()) + }) + + Context("deletion blocking", func() { + var resource *migrationv1alpha1.VmwareCloudFoundationMigration + var reconciler *VmwareCloudFoundationMigrationReconciler + + BeforeEach(func() { + // Exercised directly against handleFinalizer (rather than the + // full Reconcile) for the deletion steps: once deletion is + // blocked, Reconcile deliberately falls through to keep driving + // the migration's state machine, which needs the real vSphere/ + // Kubernetes clients this test doesn't wire up. + resource = newResource() + Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + reconciler = newReconciler() + _, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName}) + Expect(err).NotTo(HaveOccurred()) + + Expect(k8sClient.Get(ctx, typeNamespacedName, resource)).To(Succeed()) + now := metav1.Now() + resource.Status.StartTime = &now + Expect(k8sClient.Status().Update(ctx, resource)).To(Succeed()) + + Expect(k8sClient.Get(ctx, typeNamespacedName, resource)).To(Succeed()) + Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) + + Expect(k8sClient.Get(ctx, typeNamespacedName, resource)).To(Succeed()) + }) + + It("blocks deletion while running and not yet Ready", func() { + fakeRecorder := record.NewFakeRecorder(10) + reconciler.Recorder = fakeRecorder + result, done, err := reconciler.handleFinalizer(ctx, resource) + Expect(err).NotTo(HaveOccurred()) + Expect(done).To(BeFalse()) + Expect(result.RequeueAfter).To(BeNumerically(">", 0)) + + Expect(fakeRecorder.Events).To(Receive(SatisfyAll( + ContainSubstring("Warning"), + ContainSubstring(reasonDeletionBlocked), + ))) + + blocked := &migrationv1alpha1.VmwareCloudFoundationMigration{} + Expect(k8sClient.Get(ctx, typeNamespacedName, blocked)).To(Succeed()) + Expect(controllerutil.ContainsFinalizer(blocked, migrationv1alpha1.Finalizer)).To(BeTrue()) + Expect(blocked.DeletionTimestamp.IsZero()).To(BeFalse()) + + cond := apimeta.FindStatusCondition(blocked.Status.Conditions, migrationv1alpha1.ConditionReady) + Expect(cond).NotTo(BeNil()) + Expect(cond.Status).To(Equal(metav1.ConditionFalse)) + Expect(cond.Reason).To(Equal(reasonDeletionBlocked)) + }) + + It("does not emit a duplicate warning event on a second pass", func() { + // First call to establish the blocked condition. + fakeRecorder1 := record.NewFakeRecorder(10) + reconciler.Recorder = fakeRecorder1 + _, _, err := reconciler.handleFinalizer(ctx, resource) + Expect(err).NotTo(HaveOccurred()) + + // A second call while still blocked must NOT emit another event. + fakeRecorder2 := record.NewFakeRecorder(10) + reconciler.Recorder = fakeRecorder2 + Expect(k8sClient.Get(ctx, typeNamespacedName, resource)).To(Succeed()) + _, done, err := reconciler.handleFinalizer(ctx, resource) + Expect(err).NotTo(HaveOccurred()) + Expect(done).To(BeFalse()) + Expect(fakeRecorder2.Events).ToNot(Receive()) + }) + + It("unblocks on force-delete annotation", func() { + // First call to establish the blocked condition. + fakeRecorder := record.NewFakeRecorder(10) + reconciler.Recorder = fakeRecorder + _, _, err := reconciler.handleFinalizer(ctx, resource) + Expect(err).NotTo(HaveOccurred()) + + // Force it through. + Expect(k8sClient.Get(ctx, typeNamespacedName, resource)).To(Succeed()) + resource.Annotations = map[string]string{migrationv1alpha1.ForceDeleteAnnotation: "true"} + Expect(k8sClient.Update(ctx, resource)).To(Succeed()) + + Expect(k8sClient.Get(ctx, typeNamespacedName, resource)).To(Succeed()) + _, done, err := reconciler.handleFinalizer(ctx, resource) + Expect(err).NotTo(HaveOccurred()) + Expect(done).To(BeTrue()) + Expect(errors.IsNotFound(k8sClient.Get(ctx, typeNamespacedName, &migrationv1alpha1.VmwareCloudFoundationMigration{}))).To(BeTrue()) + }) + }) + + It("deletes immediately once the migration has reached Ready", func() { + resource := newResource() + Expect(k8sClient.Create(ctx, resource)).To(Succeed()) + reconciler := newReconciler() + _, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName}) + Expect(err).NotTo(HaveOccurred()) + + Expect(k8sClient.Get(ctx, typeNamespacedName, resource)).To(Succeed()) + now := metav1.Now() + resource.Status.StartTime = &now + apimeta.SetStatusCondition(&resource.Status.Conditions, metav1.Condition{ + Type: migrationv1alpha1.ConditionReady, + Status: metav1.ConditionTrue, + Reason: migrationv1alpha1.ReasonCompleted, + Message: "migration complete", + }) + Expect(k8sClient.Status().Update(ctx, resource)).To(Succeed()) + + Expect(k8sClient.Delete(ctx, resource)).To(Succeed()) + _, err = reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: typeNamespacedName}) + Expect(err).NotTo(HaveOccurred()) + Expect(errors.IsNotFound(k8sClient.Get(ctx, typeNamespacedName, &migrationv1alpha1.VmwareCloudFoundationMigration{}))).To(BeTrue()) + }) + }) })