🐛 Make KCP pre-terminate hook more robust

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 not exist anymore")
+
  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
  }