Make KCP pre-terminate hook more robust

Signed-off-by: Stefan Büringer [email protected]

controlplane/kubeadm/internal/controllers/controller.go

@@ -567,7 +567,7 @@ func (r *KubeadmControlPlaneReconciler) reconcileDelete(ctx context.Context, con
 	}
 
 	// Delete control plane machines in parallel
-	machinesToDelete := controlPlane.Machines.Filter(collections.Not(collections.HasDeletionTimestamp))
+	machinesToDelete := controlPlane.Machines
 	var errs []error
 	for _, machineToDelete := range machinesToDelete {
 		log := log.WithValues("Machine", klog.KObj(machineToDelete))
@@ -584,6 +584,11 @@ func (r *KubeadmControlPlaneReconciler) reconcileDelete(ctx context.Context, con
 			continue
 		}
 
+		if !machineToDelete.DeletionTimestamp.IsZero() {
+			// Nothing to do, Machine already has deletionTimestamp set.
+			continue
+		}
+
 		log.Info("Deleting control plane Machine")
 		if err := r.Client.Delete(ctx, machineToDelete); err != nil && !apierrors.IsNotFound(err) {
 			errs = append(errs, errors.Wrapf(err, "failed to delete control plane Machine %s", klog.KObj(machineToDelete)))
@@ -595,11 +600,19 @@ func (r *KubeadmControlPlaneReconciler) reconcileDelete(ctx context.Context, con
 			"Failed to delete control plane Machines for cluster %s control plane: %v", klog.KObj(controlPlane.Cluster), err)
 		return ctrl.Result{}, err
 	}
+
+	log.Info("Waiting for control plane Machines to go away")
+
 	conditions.MarkFalse(controlPlane.KCP, controlplanev1.ResizedCondition, clusterv1.DeletingReason, clusterv1.ConditionSeverityInfo, "")
 	return ctrl.Result{RequeueAfter: deleteRequeueAfter}, nil
 }
 
 func (r *KubeadmControlPlaneReconciler) removePreTerminateHookAnnotationFromMachine(ctx context.Context, machine *clusterv1.Machine) error {
+	if _, exists := machine.Annotations[controlplanev1.PreTerminateHookCleanupAnnotation]; !exists {
+		// Nothing to do, the annotation is not set (anymore) on the Machine
+		return nil
+	}
+
 	log := ctrl.LoggerFrom(ctx)
 	log.Info("Removing pre-terminate hook from control plane Machine")
 

controlplane/kubeadm/internal/controllers/controller_test.go

@@ -2478,6 +2478,8 @@ func TestKubeadmControlPlaneReconciler_reconcileDelete(t *testing.T) {
 			initObjs = append(initObjs, m)
 			machines.Insert(m)
 		}
+		// One Machine was already deleted before KCP, validate the pre-terminate hook is still removed.
+		machines.UnsortedList()[2].DeletionTimestamp = &metav1.Time{Time: time.Now()}
 
 		fakeClient := newFakeClient(initObjs...)
 

internal/controllers/machine/machine_controller.go

@@ -19,6 +19,7 @@ package machine
 import (
 	"context"
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/pkg/errors"
@@ -366,6 +367,13 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, cluster *clusterv1.Clu
 		// pre-drain.delete lifecycle hook
 		// Return early without error, will requeue if/when the hook owner removes the annotation.
 		if annotations.HasWithPrefix(clusterv1.PreDrainDeleteHookAnnotationPrefix, m.ObjectMeta.Annotations) {
+			var hooks []string
+			for key := range m.ObjectMeta.Annotations {
+				if strings.HasPrefix(key, clusterv1.PreDrainDeleteHookAnnotationPrefix) {
+					hooks = append(hooks, key)
+				}
+			}
+			log.Info("Waiting for pre-drain hooks to succeed", "hooks", strings.Join(hooks, ","))
 			conditions.MarkFalse(m, clusterv1.PreDrainDeleteHookSucceededCondition, clusterv1.WaitingExternalHookReason, clusterv1.ConditionSeverityInfo, "")
 			return ctrl.Result{}, nil
 		}
@@ -430,6 +438,13 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, cluster *clusterv1.Clu
 	// pre-term.delete lifecycle hook
 	// Return early without error, will requeue if/when the hook owner removes the annotation.
 	if annotations.HasWithPrefix(clusterv1.PreTerminateDeleteHookAnnotationPrefix, m.ObjectMeta.Annotations) {
+		var hooks []string
+		for key := range m.ObjectMeta.Annotations {
+			if strings.HasPrefix(key, clusterv1.PreTerminateDeleteHookAnnotationPrefix) {
+				hooks = append(hooks, key)
+			}
+		}
+		log.Info("Waiting for pre-terminate hooks to succeed", "hooks", strings.Join(hooks, ","))
 		conditions.MarkFalse(m, clusterv1.PreTerminateDeleteHookSucceededCondition, clusterv1.WaitingExternalHookReason, clusterv1.ConditionSeverityInfo, "")
 		return ctrl.Result{}, nil
 	}