Add waitForPreDrainHookStartTime and waitForPreTerminateHookStartTime and omit draining/detachment on existence

Signed-off-by: Danil-Grigorev <danil.grigorev@suse.com>

Author: Danil-Grigorev

api/core/v1beta1/machine_types.go

@@ -622,6 +622,18 @@ type MachineDeletionStatus struct {
 	// Only present when the Machine has a deletionTimestamp and waiting for volume detachments had been started.
 	// +optional
 	WaitForNodeVolumeDetachStartTime *metav1.Time `json:"waitForNodeVolumeDetachStartTime,omitempty"`
+
+	// waitForPreDrainHookStartTime is the time when waiting for pre-drain hooks started
+	// and is used to determine if the pre-drain hooks are taking too long.
+	// Only present when the Machine has a deletionTimestamp and waiting for pre-drain hooks had been started.
+	// +optional
+	WaitForPreDrainHookStartTime *metav1.Time `json:"waitForPreDrainHookStartTime,omitempty"`
+
+	// waitForPreTerminateHookStartTime is the time when waiting for pre-terminate hooks started
+	// and is used to determine if the pre-terminate hooks are taking too long.
+	// Only present when the Machine has a deletionTimestamp and waiting for pre-terminate hooks had been started.
+	// +optional
+	WaitForPreTerminateHookStartTime *metav1.Time `json:"waitForPreTerminateHookStartTime,omitempty"`
 }
 
 // SetTypedPhase sets the Phase field to the string representation of MachinePhase.

api/core/v1beta1/zz_generated.conversion.go

@@ -671,6 +671,18 @@ type MachineDeletionStatus struct {
 	// Only present when the Machine has a deletionTimestamp and waiting for volume detachments had been started.
 	// +optional
 	WaitForNodeVolumeDetachStartTime *metav1.Time `json:"waitForNodeVolumeDetachStartTime,omitempty"`
+
+	// waitForPreDrainHookStartTime is the time when waiting for pre-drain hooks started
+	// and is used to determine if the pre-drain hooks are taking too long.
+	// Only present when the Machine has a deletionTimestamp and waiting for pre-drain hooks had been started.
+	// +optional
+	WaitForPreDrainHookStartTime *metav1.Time `json:"waitForPreDrainHookStartTime,omitempty"`
+
+	// waitForPreTerminateHookStartTime is the time when waiting for pre-terminate hooks started
+	// and is used to determine if the pre-terminate hooks are taking too long.
+	// Only present when the Machine has a deletionTimestamp and waiting for pre-terminate hooks had been started.
+	// +optional
+	WaitForPreTerminateHookStartTime *metav1.Time `json:"waitForPreTerminateHookStartTime,omitempty"`
 }
 
 // SetTypedPhase sets the Phase field to the string representation of MachinePhase.

api/core/v1beta1/zz_generated.deepcopy.go

@@ -466,6 +466,12 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, s *scope) (ctrl.Result
 			}
 			slices.Sort(hooks)
 			log.Info("Waiting for pre-drain hooks to succeed", "hooks", strings.Join(hooks, ","))
+			if m.Status.Deletion == nil {
+				m.Status.Deletion = &clusterv1.MachineDeletionStatus{}
+			}
+			if m.Status.Deletion.WaitForPreDrainHookStartTime == nil {
+				m.Status.Deletion.WaitForPreDrainHookStartTime = ptr.To(metav1.Now())
+			}
 			v1beta1conditions.MarkFalse(m, clusterv1.PreDrainDeleteHookSucceededV1Beta1Condition, clusterv1.WaitingExternalHookV1Beta1Reason, clusterv1.ConditionSeverityInfo, "")
 			s.deletingReason = clusterv1.MachineDeletingWaitingForPreDrainHookReason
 			s.deletingMessage = fmt.Sprintf("Waiting for pre-drain hooks to succeed (hooks: %s)", strings.Join(hooks, ","))
@@ -474,7 +480,8 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, s *scope) (ctrl.Result
 		v1beta1conditions.MarkTrue(m, clusterv1.PreDrainDeleteHookSucceededV1Beta1Condition)
 
 		// Drain node before deletion and issue a patch in order to make this operation visible to the users.
-		if r.isNodeDrainAllowed(m) {
+		// In case the preTerminateHook is started or the infra machine is not found the detachment is skipped.
+		if r.isNodeDrainAllowed(m, s.infraMachine) {
 			patchHelper, err := patch.NewHelper(m, r.Client)
 			if err != nil {
 				s.deletingReason = clusterv1.MachineDeletingInternalErrorReason
@@ -521,8 +528,8 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, s *scope) (ctrl.Result
 
 		// After node draining is completed, and if isNodeVolumeDetachingAllowed returns True, make sure all
 		// volumes are detached before proceeding to delete the Node.
-		// In case the node is unreachable, the detachment is skipped.
-		if r.isNodeVolumeDetachingAllowed(m) {
+		// In case the node is unreachable, preTerminateHook is started or the infra machine is not found the detachment is skipped.
+		if r.isNodeVolumeDetachingAllowed(m, s.infraMachine) {
 			if m.Status.Deletion == nil {
 				m.Status.Deletion = &clusterv1.MachineDeletionStatus{}
 			}
@@ -563,6 +570,12 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, s *scope) (ctrl.Result
 		}
 		slices.Sort(hooks)
 		log.Info("Waiting for pre-terminate hooks to succeed", "hooks", strings.Join(hooks, ","))
+		if m.Status.Deletion == nil {
+			m.Status.Deletion = &clusterv1.MachineDeletionStatus{}
+		}
+		if m.Status.Deletion.WaitForPreTerminateHookStartTime == nil {
+			m.Status.Deletion.WaitForPreTerminateHookStartTime = ptr.To(metav1.Now())
+		}
 		v1beta1conditions.MarkFalse(m, clusterv1.PreTerminateDeleteHookSucceededV1Beta1Condition, clusterv1.WaitingExternalHookV1Beta1Reason, clusterv1.ConditionSeverityInfo, "")
 		s.deletingReason = clusterv1.MachineDeletingWaitingForPreTerminateHookReason
 		s.deletingMessage = fmt.Sprintf("Waiting for pre-terminate hooks to succeed (hooks: %s)", strings.Join(hooks, ","))
@@ -637,7 +650,7 @@ const (
 	KubeadmControlPlanePreTerminateHookCleanupAnnotation = clusterv1.PreTerminateDeleteHookAnnotationPrefix + "/kcp-cleanup"
 )
 
-func (r *Reconciler) isNodeDrainAllowed(m *clusterv1.Machine) bool {
+func (r *Reconciler) isNodeDrainAllowed(m *clusterv1.Machine, infraMachine client.Object) bool {
 	if util.IsControlPlaneMachine(m) && util.HasOwner(m.GetOwnerReferences(), clusterv1.GroupVersionControlPlane.String(), []string{"KubeadmControlPlane"}) {
 		if _, exists := m.Annotations[KubeadmControlPlanePreTerminateHookCleanupAnnotation]; !exists {
 			return false
@@ -648,6 +661,14 @@ func (r *Reconciler) isNodeDrainAllowed(m *clusterv1.Machine) bool {
 		return false
 	}
 
+	if m.Status.Deletion != nil && m.Status.Deletion.WaitForPreTerminateHookStartTime != nil {
+		return false
+	}
+
+	if infraMachine == nil || !infraMachine.GetDeletionTimestamp().IsZero() {
+		return false
+	}
+
 	if r.nodeDrainTimeoutExceeded(m) {
 		return false
 	}
@@ -657,7 +678,7 @@ func (r *Reconciler) isNodeDrainAllowed(m *clusterv1.Machine) bool {
 
 // isNodeVolumeDetachingAllowed returns False if either ExcludeWaitForNodeVolumeDetachAnnotation annotation is set OR
 // nodeVolumeDetachTimeoutExceeded timeout is exceeded, otherwise returns True.
-func (r *Reconciler) isNodeVolumeDetachingAllowed(m *clusterv1.Machine) bool {
+func (r *Reconciler) isNodeVolumeDetachingAllowed(m *clusterv1.Machine, infraMachine client.Object) bool {
 	if util.IsControlPlaneMachine(m) && util.HasOwner(m.GetOwnerReferences(), clusterv1.GroupVersionControlPlane.String(), []string{"KubeadmControlPlane"}) {
 		if _, exists := m.Annotations[KubeadmControlPlanePreTerminateHookCleanupAnnotation]; !exists {
 			return false
@@ -668,6 +689,14 @@ func (r *Reconciler) isNodeVolumeDetachingAllowed(m *clusterv1.Machine) bool {
 		return false
 	}
 
+	if m.Status.Deletion != nil && m.Status.Deletion.WaitForPreTerminateHookStartTime != nil {
+		return false
+	}
+
+	if infraMachine == nil || !infraMachine.GetDeletionTimestamp().IsZero() {
+		return false
+	}
+
 	if r.nodeVolumeDetachTimeoutExceeded(m) {
 		return false
 	}