controller: unlabel nodes when pool is deleted

api/v1alpha1/constants.go

@@ -2,6 +2,11 @@
 
 package v1alpha1
 
+// FinalizerPoolCleanup is added to every BootcNodePool so the controller
+// can remove the bootc.dev/managed label from member nodes before the
+// pool object is fully deleted.
+const FinalizerPoolCleanup = "bootc.dev/pool-cleanup"
+
 // Well-known labels and annotations applied to Nodes by the controller.
 const (
 	// LabelManaged is set on Nodes that are managed by a BootcNodePool.

internal/controller/bootcnodepool_controller.go

@@ -231,6 +231,22 @@ func (r *BootcNodePoolReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		return ctrl.Result{}, fmt.Errorf("fetching pool: %w", err)
 	}
 
+	// If the pool is being deleted, clean up managed nodes before allowing
+	// Kubernetes to remove the object.
+	if pool.DeletionTimestamp != nil {
+		return r.handlePoolDeletion(ctx, &pool)
+	}
+
+	// Ensure the cleanup finalizer is present so we can react to deletion.
+	if !controllerutil.ContainsFinalizer(&pool, bootcv1alpha1.FinalizerPoolCleanup) {
+		controllerutil.AddFinalizer(&pool, bootcv1alpha1.FinalizerPoolCleanup)
+		if err := r.Update(ctx, &pool); err != nil {
+			return ctrl.Result{}, fmt.Errorf("adding finalizer: %w", err)
+		}
+		// Re-fetch will be triggered by the update event; return early.
+		return ctrl.Result{}, nil
+	}
+
 	// Snapshot status so we can detect changes and write once at the end.
 	statusOrig := pool.Status.DeepCopy()
 
@@ -303,6 +319,48 @@ func (r *BootcNodePoolReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 	return complete(resolveResult)
 }
 
+// handlePoolDeletion is called when a pool's DeletionTimestamp is set.
+// It removes the bootc.dev/managed label from all member nodes and deletes
+// the owned BootcNode objects, then removes the cleanup finalizer so
+// Kubernetes can complete the deletion.
+func (r *BootcNodePoolReconciler) handlePoolDeletion(ctx context.Context, pool *bootcv1alpha1.BootcNodePool) (ctrl.Result, error) {
+	log := logf.FromContext(ctx).WithValues("pool", pool.Name)
+
+	if !controllerutil.ContainsFinalizer(pool, bootcv1alpha1.FinalizerPoolCleanup) {
+		// Finalizer already removed; nothing left to do.
+		return ctrl.Result{}, nil
+	}
+
+	log.Info("Pool is being deleted; cleaning up managed nodes")
+
+	// List all BootcNodes owned by this pool.
+	allBootcNodes, err := r.listAllBootcNodes(ctx)
+	if err != nil {
+		return ctrl.Result{}, fmt.Errorf("listing BootcNodes for deletion cleanup: %w", err)
+	}
+
+	for _, bn := range allBootcNodes {
+		if !metav1.IsControlledBy(bn, pool) {
+			continue
+		}
+		log.Info("Removing BootcNode for pool deletion", "node", bn.Name)
+		if err := r.removeBootcNode(ctx, bn); err != nil {
+			return ctrl.Result{}, fmt.Errorf("removing BootcNode %s during pool deletion: %w", bn.Name, err)
+		}
+	}
+
+	// All nodes cleaned up — remove the finalizer to unblock deletion.
+	controllerutil.RemoveFinalizer(pool, bootcv1alpha1.FinalizerPoolCleanup)
+	if err := r.Update(ctx, pool); err != nil {
+		if apierrors.IsNotFound(err) {
+			// Pool was already fully deleted (e.g. race with another reconcile).
+			return ctrl.Result{}, nil
+		}
+		return ctrl.Result{}, fmt.Errorf("removing finalizer: %w", err)
+	}
+	return ctrl.Result{}, nil
+}
+
 // resolveTargetDigest resolves the target digest from the pool's image
 // ref. Digest refs are extracted directly. Tag refs are resolved via
 // the registry, respecting the re-resolution interval.

internal/controller/membership_test.go

@@ -203,6 +203,99 @@ func TestMembershipSyncsDesiredImage(t *testing.T) {
 	))
 }
 
+// TestPoolDeletionRemovesManagedLabel verifies that when a BootcNodePool is
+// deleted, the controller removes the bootc.dev/managed label from all member
+// nodes and deletes all owned BootcNode objects. It also verifies that
+// control-plane nodes belonging to a separate pool are unaffected.
+func TestPoolDeletionRemovesManagedLabel(t *testing.T) {
+	g := NewWithT(t)
+	g.SetDefaultEventuallyTimeout(pollTimeout)
+	g.SetDefaultEventuallyPollingInterval(pollInterval)
+	ctx := context.Background()
+
+	workerNodes := []*corev1.Node{
+		testutil.NewK8sNode("del-worker-1", testutil.WorkerLabels()),
+		testutil.NewK8sNode("del-worker-2", testutil.WorkerLabels()),
+	}
+
+	controlPlaneNodes := []*corev1.Node{
+		testutil.NewK8sNode("del-control-plane-1", testutil.ControlPlaneLabels()),
+		testutil.NewK8sNode("del-control-plane-2", testutil.ControlPlaneLabels()),
+	}
+
+	allNodes := append(workerNodes, controlPlaneNodes...)
+
+	// Create nodes.
+	for _, node := range allNodes {
+		g.Expect(k8sClient.Create(ctx, node)).To(Succeed())
+		t.Cleanup(func() {
+			_ = k8sClient.Delete(ctx, node)
+		})
+	}
+
+	// Create a worker pool and a separate control-plane pool.
+	workerPool := testutil.NewPool("del-workers", testImageDigestRefA, testutil.WithWorkerSelector())
+	g.Expect(k8sClient.Create(ctx, workerPool)).To(Succeed())
+
+	cpPool := testutil.NewPool("del-control-plane", testImageDigestRefA,
+		testutil.WithNodeSelector(testutil.ControlPlaneLabels()))
+	g.Expect(k8sClient.Create(ctx, cpPool)).To(Succeed())
+	t.Cleanup(func() {
+		_ = k8sClient.Delete(ctx, cpPool)
+	})
+
+	// Wait for BootcNodes to appear and managed labels to be applied on all nodes.
+	for _, node := range allNodes {
+		g.Eventually(func() error {
+			return k8sClient.Get(ctx, client.ObjectKey{Name: node.Name}, &bootcv1alpha1.BootcNode{})
+		}).Should(Succeed(), "BootcNode %s should exist", node.Name)
+
+		g.Eventually(func() (map[string]string, error) {
+			var n corev1.Node
+			err := k8sClient.Get(ctx, client.ObjectKey{Name: node.Name}, &n)
+			return n.Labels, err
+		}).Should(HaveKey(bootcv1alpha1.LabelManaged), "node %s should have managed label", node.Name)
+	}
+
+	// Delete only the worker pool.
+	g.Expect(k8sClient.Delete(ctx, workerPool)).To(Succeed())
+
+	// The worker pool's finalizer must remove the managed label from worker nodes.
+	for _, node := range workerNodes {
+		g.Eventually(func() (map[string]string, error) {
+			var n corev1.Node
+			err := k8sClient.Get(ctx, client.ObjectKey{Name: node.Name}, &n)
+			return n.Labels, err
+		}).ShouldNot(HaveKey(bootcv1alpha1.LabelManaged), "worker node %s should not have managed label after pool deletion", node.Name)
+	}
+
+	// Worker BootcNodes should be deleted.
+	for _, node := range workerNodes {
+		g.Eventually(func() error {
+			return k8sClient.Get(ctx, client.ObjectKey{Name: node.Name}, &bootcv1alpha1.BootcNode{})
+		}).Should(MatchError(apierrors.IsNotFound, "IsNotFound"), "BootcNode %s should be deleted", node.Name)
+	}
+
+	// The worker pool itself should be fully deleted (finalizer removed).
+	g.Eventually(func() error {
+		return k8sClient.Get(ctx, client.ObjectKeyFromObject(workerPool), &bootcv1alpha1.BootcNodePool{})
+	}).Should(MatchError(apierrors.IsNotFound, "IsNotFound"), "worker pool should be fully deleted")
+
+	// Control-plane nodes must still carry the managed label — their pool was not deleted.
+	for _, node := range controlPlaneNodes {
+		var n corev1.Node
+		g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: node.Name}, &n)).To(Succeed())
+		g.Expect(n.Labels).To(HaveKey(bootcv1alpha1.LabelManaged),
+			"control-plane node %s should still have managed label", node.Name)
+	}
+
+	// Control-plane BootcNodes must still exist.
+	for _, node := range controlPlaneNodes {
+		g.Expect(k8sClient.Get(ctx, client.ObjectKey{Name: node.Name}, &bootcv1alpha1.BootcNode{})).To(Succeed(),
+			"BootcNode %s should still exist", node.Name)
+	}
+}
+
 // TestMembershipConflictDetection verifies that when a node matches two
 // pools, the conflicting pool is marked Degraded with reason
 // NodeConflict for the contested node, but non-contested nodes in

test/util/builders.go

@@ -37,6 +37,11 @@ func WorkerLabels() map[string]string {
 	return map[string]string{"node-role.kubernetes.io/worker": ""}
 }
 
+// ControlPlaneLabels returns the conventional control-plane node label map.
+func ControlPlaneLabels() map[string]string {
+	return map[string]string{"node-role.kubernetes.io/control-plane": ""}
+}
+
 // NewPool creates a BootcNodePool with the given name and image ref.
 // A nodeSelector must be provided via WithNodeSelector or
 // WithWorkerSelector. Override fields via functional options.