Rolling update fixes clean node removal v2

controlplane/controllers/ck8scontrolplane_controller.go

@@ -572,6 +572,39 @@ func (r *CK8sControlPlaneReconciler) reconcile(ctx context.Context, cluster *clu
 		return r.scaleDownControlPlane(ctx, cluster, kcp, controlPlane, collections.Machines{})
 	}
 
+	annotationsKcp := kcp.GetAnnotations()
+	orphanNode := annotationsKcp["orphan-node"]
+	controlPlane.SetOrphanNode(orphanNode)
+	orphanNodeReadyToBeRemoved := controlPlane.GetOrphanNodeReadyToBeRemoved()
+	logger.Info("Orphan node ready to be removed  name", "Orphan", orphanNodeReadyToBeRemoved)
+	logger.Info("Orphan node name", "Orphan", orphanNode)
+	if orphanNodeReadyToBeRemoved == "" && orphanNode != "" {
+		return ctrl.Result{Requeue: true}, nil
+	}
+
+	if orphanNodeReadyToBeRemoved != "" {
+		microclusterPort := controlPlane.KCP.Spec.CK8sConfigSpec.ControlPlaneConfig.GetMicroclusterPort()
+		clusterObjectKey := util.ObjectKey(cluster)
+		workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, clusterObjectKey, microclusterPort)
+		if err != nil {
+			logger.Error(err, "failed to create client to workload cluster")
+			return ctrl.Result{}, fmt.Errorf("failed to create client to workload cluster: %w", err)
+		}
+
+		// TODO: If the node is not part of the microcluster, this may still return an error. We should catch that case,
+		// and proceed with the machine removal.
+		logger.Info("Removing machine from cluster gracefully")
+		if err := workloadCluster.RemoveMachineFromCluster(ctx, orphanNodeReadyToBeRemoved, false); err != nil {
+			logger.Error(err, "Failed to remove machine from cluster gracefully")
+			logger.Info("Removing machine from cluster forcefully")
+			if errForce := workloadCluster.RemoveMachineFromCluster(ctx, orphanNodeReadyToBeRemoved, true); errForce != nil {
+				logger.Error(err, "failed to remove machine from microcluster forcefully")
+			}
+		}
+		annotationsKcp["orphan-node"] = ""
+		kcp.SetAnnotations(annotationsKcp)
+	}
+
 	return reconcile.Result{}, nil
 }
 

controlplane/controllers/remediation.go

@@ -174,7 +174,7 @@ func (r *CK8sControlPlaneReconciler) reconcileUnhealthyMachines(ctx context.Cont
 	if machineToBeRemediated.Status.NodeRef != nil {
 		// TODO: If the node is not part of the microcluster, this may still return an error. We should catch that case,
 		// and proceed with the machine removal.
-		if err := workloadCluster.RemoveMachineFromCluster(ctx, machineToBeRemediated); err != nil {
+		if err := workloadCluster.RemoveMachineFromCluster(ctx, machineToBeRemediated.Status.NodeRef.Name, true); err != nil {
 			log.Error(err, "failed to remove machine from microcluster")
 			return ctrl.Result{}, fmt.Errorf("failed to remove machine from microcluster: %w", err)
 		}

controlplane/controllers/scale.go

@@ -102,49 +102,65 @@ func (r *CK8sControlPlaneReconciler) scaleDownControlPlane(
 ) (ctrl.Result, error) {
 	logger := ctrl.LoggerFrom(ctx)
 
+	annotationsKcp := kcp.GetAnnotations()
+	orphanNode := annotationsKcp["orphan-node"]
+	controlPlane.SetOrphanNode(orphanNode)
+	orphanNodeReadyToBeRemoved := controlPlane.GetOrphanNodeReadyToBeRemoved()
+	logger.Info("Orphan node ready to be removed  name", "Orphan", orphanNodeReadyToBeRemoved)
+	logger.Info("Orphan node name", "Orphan", orphanNode)
+	if orphanNodeReadyToBeRemoved == "" && orphanNode != "" {
+		return ctrl.Result{Requeue: true}, nil
+	}
+
+	microclusterPort := controlPlane.KCP.Spec.CK8sConfigSpec.ControlPlaneConfig.GetMicroclusterPort()
+	clusterObjectKey := util.ObjectKey(cluster)
+	workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, clusterObjectKey, microclusterPort)
+	if err != nil {
+		logger.Error(err, "failed to create client to workload cluster")
+		return ctrl.Result{}, fmt.Errorf("failed to create client to workload cluster: %w", err)
+	}
+	if orphanNodeReadyToBeRemoved != "" {
+		logger.Info("Removing machine from cluster gracefully")
+		if err := workloadCluster.RemoveMachineFromCluster(ctx, orphanNodeReadyToBeRemoved, false); err != nil {
+			logger.Error(err, "Failed to remove machine from cluster gracefully")
+			logger.Info("Removing machine from cluster forcefully")
+			if errForce := workloadCluster.RemoveMachineFromCluster(ctx, orphanNodeReadyToBeRemoved, true); errForce != nil {
+				logger.Error(err, "failed to remove machine from microcluster forcefully")
+			}
+		}
+		annotationsKcp["orphan-node"] = ""
+		kcp.SetAnnotations(annotationsKcp)
+	}
+
 	// Pick the Machine that we should scale down.
 	machineToDelete, err := selectMachineForScaleDown(ctx, controlPlane, outdatedMachines)
 	if err != nil {
 		return ctrl.Result{}, fmt.Errorf("failed to select machine for scale down: %w", err)
 	}
 
-	// Run preflight checks ensuring the control plane is stable before proceeding with a scale up/scale down operation; if not, wait.
-	// Given that we're scaling down, we can exclude the machineToDelete from the preflight checks.
-	if result, err := r.preflightChecks(ctx, controlPlane, machineToDelete); err != nil || !result.IsZero() {
-		return result, err
-	}
-
 	if machineToDelete == nil {
 		logger.Info("Failed to pick control plane Machine to delete")
 		return ctrl.Result{}, fmt.Errorf("failed to pick control plane Machine to delete: %w", err)
 	}
 
-	microclusterPort := controlPlane.KCP.Spec.CK8sConfigSpec.ControlPlaneConfig.GetMicroclusterPort()
-	clusterObjectKey := util.ObjectKey(cluster)
-	workloadCluster, err := r.managementCluster.GetWorkloadCluster(ctx, clusterObjectKey, microclusterPort)
-	if err != nil {
-		logger.Error(err, "failed to create client to workload cluster")
-		return ctrl.Result{}, fmt.Errorf("failed to create client to workload cluster: %w", err)
+	// Run preflight checks ensuring the control plane is stable before proceeding with a scale up/scale down operation; if not, wait.
+	// Given that we're scaling down, we can exclude the machineToDelete from the preflight checks.
+	if result, err := r.preflightChecks(ctx, controlPlane, machineToDelete); err != nil || !result.IsZero() {
+		return result, err
 	}
 
-	if machineToDelete.Status.NodeRef != nil {
-		// TODO: If the node is not part of the microcluster, this may still return an error. We should catch that case,
-		// and proceed with the machine removal.
-		if err := workloadCluster.RemoveMachineFromCluster(ctx, machineToDelete); err != nil {
-			logger.Error(err, "failed to remove machine from microcluster")
-			return ctrl.Result{}, fmt.Errorf("failed to remove machine from microcluster: %w", err)
-		}
-	}
+	annotations := map[string]string{}
+	annotations["orphan-node"] = machineToDelete.Status.NodeRef.Name
+	kcp.SetAnnotations(annotations)
 
-	logger = logger.WithValues("machine", machineToDelete)
+	logger.Info("Removing control plane machine")
 	if err := r.Delete(ctx, machineToDelete); err != nil && !apierrors.IsNotFound(err) {
 		logger.Error(err, "Failed to delete control plane machine")
 		r.recorder.Eventf(kcp, corev1.EventTypeWarning, "FailedScaleDown",
 			"Failed to delete control plane Machine %s for cluster %s/%s control plane: %v", machineToDelete.Name, cluster.Namespace, cluster.Name, err)
 		return ctrl.Result{}, err
 	}
 
-	// Requeue the control plane, in case there are additional operations to perform
 	return ctrl.Result{Requeue: true}, nil
 }