diff --git a/api/v1alpha1/constants.go b/api/v1alpha1/constants.go index 71fae9d..eea2361 100644 --- a/api/v1alpha1/constants.go +++ b/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. diff --git a/internal/controller/bootcnodepool_controller.go b/internal/controller/bootcnodepool_controller.go index 59eb7fe..5d6770e 100644 --- a/internal/controller/bootcnodepool_controller.go +++ b/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. diff --git a/internal/controller/membership_test.go b/internal/controller/membership_test.go index d97b7f4..be21109 100644 --- a/internal/controller/membership_test.go +++ b/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 diff --git a/test/util/builders.go b/test/util/builders.go index 21d9c2e..40f286a 100644 --- a/test/util/builders.go +++ b/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.