fix(controller): drop ChainInstance finalizer to unblock deletion

The teardown finalizer scaled the StatefulSet to zero and waited for
ReadyReplicas to drain before removing itself, which made
`kubectl delete chaininstance` hang for tens of seconds (often longer
on bitcoin pods) while doing what the kube-apiserver does for free —
ownerReferences cascade already SIGTERMs the pod with its grace period.

New CRs no longer get the finalizer. Existing CRs created by older
operator versions are auto-cleaned on first reconcile so the upgrade
doesn't leave them undeletable. The teardown function is gone; tests
were rewritten accordingly.

Context: reproduced the hang on a bitcoin-test ChainInstance, traced
the wait dance in teardown(), confirmed via grep that FinalizerName
guards no external resource — so removing it is safe. Spent ~30min
verifying with envtest suite (44 → 44 passing).

Author: tazhate

internal/controller/blockchainnode_controller.go

@@ -102,20 +102,20 @@ func (r *ChainInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		return r.patchPhase(ctx, node, chainsv1alpha2.NodePhaseFailed)
 	}
 
-	// Handle deletion before anything else.
-	if !node.DeletionTimestamp.IsZero() {
-		return r.teardown(ctx, node)
-	}
-
-	// Ensure the finalizer is present.
-	if !controllerutil.ContainsFinalizer(node, chainsv1alpha2.FinalizerName) {
-		controllerutil.AddFinalizer(node, chainsv1alpha2.FinalizerName)
+	// Strip legacy finalizer left on CRs created by older operator versions;
+	// owner-reference cascade now handles graceful pod shutdown.
+	if controllerutil.ContainsFinalizer(node, chainsv1alpha2.FinalizerName) {
+		controllerutil.RemoveFinalizer(node, chainsv1alpha2.FinalizerName)
 		if err := r.Update(ctx, node); err != nil {
-			return ctrl.Result{}, fmt.Errorf("adding finalizer to %s/%s: %w", node.Namespace, node.Name, err)
+			return ctrl.Result{}, fmt.Errorf("removing legacy finalizer from %s/%s: %w", node.Namespace, node.Name, err)
 		}
 		return ctrl.Result{Requeue: true}, nil
 	}
 
+	if !node.DeletionTimestamp.IsZero() {
+		return ctrl.Result{}, nil
+	}
+
 	// Reconcile owned resources in dependency order.
 	cfgHash, err := r.ensureConfigMap(ctx, node, adapter)
 	if err != nil {
@@ -150,42 +150,6 @@ func (r *ChainInstanceReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	return ctrl.Result{RequeueAfter: reconcileInterval}, nil
 }
 
-// ---------------------------------------------------------------------------
-// Deletion
-// ---------------------------------------------------------------------------
-
-// teardown performs a graceful scale-down and removes the finalizer once pods
-// have terminated.
-func (r *ChainInstanceReconciler) teardown(ctx context.Context, node *chainsv1alpha2.ChainInstance) (ctrl.Result, error) {
-	logger := log.FromContext(ctx)
-
-	sts := &appsv1.StatefulSet{}
-	key := client.ObjectKeyFromObject(node)
-	if err := r.Get(ctx, key, sts); err == nil {
-		if sts.Spec.Replicas == nil || *sts.Spec.Replicas > 0 {
-			zero := int32(0)
-			sts.Spec.Replicas = &zero
-			if err := r.Update(ctx, sts); err != nil {
-				return ctrl.Result{}, fmt.Errorf("scaling StatefulSet %s/%s to zero: %w", node.Namespace, node.Name, err)
-			}
-			return ctrl.Result{RequeueAfter: 5 * time.Second}, nil
-		}
-		if sts.Status.ReadyReplicas > 0 {
-			return ctrl.Result{RequeueAfter: 5 * time.Second}, nil
-		}
-	}
-
-	if controllerutil.ContainsFinalizer(node, chainsv1alpha2.FinalizerName) {
-		controllerutil.RemoveFinalizer(node, chainsv1alpha2.FinalizerName)
-		if err := r.Update(ctx, node); err != nil {
-			return ctrl.Result{}, fmt.Errorf("removing finalizer from %s/%s: %w", node.Namespace, node.Name, err)
-		}
-		logger.Info("finalizer removed, node deleted gracefully")
-	}
-
-	return ctrl.Result{}, nil
-}
-
 // ---------------------------------------------------------------------------
 // Phase patch
 // ---------------------------------------------------------------------------

internal/controller/blockchainnode_controller_test.go

@@ -96,13 +96,14 @@ func reconcileOnce(ctx context.Context, name types.NamespacedName) (reconcile.Re
 var _ = Describe("ChainInstance Controller", func() {
 	const testNS = "default"
 
-	Context("Finalizer management", func() {
-		It("should add finalizer on first reconcile", func() {
+	Context("Legacy finalizer cleanup", func() {
+		It("should strip legacy finalizer if present on existing CR", func() {
 			ctx := context.Background()
-			name := "test-finalizer"
+			name := "test-legacy-finalizer"
 			nn := types.NamespacedName{Name: name, Namespace: testNS}
 
 			node := newTestNode(name, testNS)
+			controllerutil.AddFinalizer(node, chainsv1alpha2.FinalizerName)
 			Expect(k8sClient.Create(ctx, node)).To(Succeed())
 			DeferCleanup(func() {
 				n := &chainsv1alpha2.ChainInstance{}
@@ -113,14 +114,13 @@ var _ = Describe("ChainInstance Controller", func() {
 				}
 			})
 
-			// First reconcile adds the finalizer and requeues.
 			result, err := reconcileOnce(ctx, nn)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(result.Requeue).To(BeTrue())
 
 			updated := &chainsv1alpha2.ChainInstance{}
 			Expect(k8sClient.Get(ctx, nn, updated)).To(Succeed())
-			Expect(controllerutil.ContainsFinalizer(updated, chainsv1alpha2.FinalizerName)).To(BeTrue())
+			Expect(controllerutil.ContainsFinalizer(updated, chainsv1alpha2.FinalizerName)).To(BeFalse())
 		})
 	})
 
@@ -384,40 +384,25 @@ var _ = Describe("ChainInstance Controller", func() {
 	})
 
 	Context("Deletion handling", func() {
-		It("should remove finalizer and allow deletion", func() {
+		It("should not block deletion (no finalizer set on new CRs)", func() {
 			ctx := context.Background()
 			name := fmt.Sprintf("test-delete-%d", time.Now().UnixNano())
 			nn := types.NamespacedName{Name: name, Namespace: testNS}
 
 			node := newTestNode(name, testNS)
 			Expect(k8sClient.Create(ctx, node)).To(Succeed())
 
-			// Reconcile to add finalizer and create resources.
 			_, _ = reconcileOnce(ctx, nn)
 			_, _ = reconcileOnce(ctx, nn)
 
-			// Verify finalizer exists.
 			fetched := &chainsv1alpha2.ChainInstance{}
 			Expect(k8sClient.Get(ctx, nn, fetched)).To(Succeed())
-			Expect(controllerutil.ContainsFinalizer(fetched, chainsv1alpha2.FinalizerName)).To(BeTrue())
+			Expect(controllerutil.ContainsFinalizer(fetched, chainsv1alpha2.FinalizerName)).To(BeFalse())
 
-			// Delete the resource.
 			Expect(k8sClient.Delete(ctx, fetched)).To(Succeed())
 
-			// Reconcile handles deletion: scales down and removes finalizer.
-			// May take multiple reconciles for the full flow.
-			for i := 0; i < 5; i++ {
-				_, _ = reconcileOnce(ctx, nn)
-			}
-
-			// Object should be gone (or have no finalizer, allowing GC).
 			err := k8sClient.Get(ctx, nn, fetched)
-			if err == nil {
-				// If still exists, finalizer should be gone.
-				Expect(controllerutil.ContainsFinalizer(fetched, chainsv1alpha2.FinalizerName)).To(BeFalse())
-			} else {
-				Expect(errors.IsNotFound(err)).To(BeTrue())
-			}
+			Expect(errors.IsNotFound(err)).To(BeTrue())
 		})
 	})