Skip to content

Commit fd85fbe

Browse files
Add ExcludeNodeVolumeDetachingAnnotation annotation to be able to skip shouldWaitForNodeVolumes check entirely
1 parent 1a64aeb commit fd85fbe

File tree

6 files changed

+51
-27
lines changed

6 files changed

+51
-27
lines changed

api/v1alpha3/machine_types.go

+3
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ const (
3333
// ExcludeNodeDrainingAnnotation annotation explicitly skips node draining if set.
3434
ExcludeNodeDrainingAnnotation = "machine.cluster.x-k8s.io/exclude-node-draining"
3535

36+
// ExcludeNodeVolumeDetachingAnnotation annotation explicitly skips volume detaching if set.
37+
ExcludeNodeVolumeDetachingAnnotation = "machine.cluster.x-k8s.io/exclude-node-volume-detaching"
38+
3639
// MachineSetLabelName is the label set on machines if they're controlled by MachineSet.
3740
MachineSetLabelName = "cluster.x-k8s.io/set-name"
3841

api/v1alpha4/condition_consts.go

+9-4
Original file line numberDiff line numberDiff line change
@@ -124,12 +124,17 @@ const (
124124
// WaitingExternalHookReason (Severity=Info) provide evidence that we are waiting for an external hook to complete.
125125
WaitingExternalHookReason = "WaitingExternalHook"
126126

127-
// VolumeDetachFinishedCondition reports a machine node volume detachment finished either by waiting for volumes to be
128-
// detached or left alone due to volume detach timeout exceeded.
129-
VolumeDetachFinishedCondition ConditionType = "VolumeDetachFinished"
127+
// VolumeDetachSucceededCondition reports a machine waiting for volumes to be detached.
128+
VolumeDetachSucceededCondition ConditionType = "VolumeDetachSucceeded"
130129

131-
// WaitingForVolumeDetachReason (Severity=Info) provide evidence that a machine node waiting for volumes to be attached.
130+
// VolumeDetachTimedOutCondition reports a machine waiting for node volumes detachment timeout to be exceeded.
131+
VolumeDetachTimedOutCondition ConditionType = "VolumeDetachTimedOut"
132+
133+
// WaitingForVolumeDetachReason (Severity=Info) provide evidence that a machine node waiting for volumes to be detached.
132134
WaitingForVolumeDetachReason = "WaitingForVolumeDetach"
135+
136+
// WaitingForVolumeDetachTimeoutReason (Severity=Info) provide evidence that a machine node waiting for node volumes detachment timeout to be exceeded.
137+
WaitingForVolumeDetachTimeoutReason = "WaitingForVolumeDetachTimeout"
133138
)
134139

135140
const (

api/v1alpha4/machine_types.go

+3
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ const (
3333
// ExcludeNodeDrainingAnnotation annotation explicitly skips node draining if set.
3434
ExcludeNodeDrainingAnnotation = "machine.cluster.x-k8s.io/exclude-node-draining"
3535

36+
// ExcludeNodeVolumeDetachingAnnotation annotation explicitly skips volume detaching if set.
37+
ExcludeNodeVolumeDetachingAnnotation = "machine.cluster.x-k8s.io/exclude-node-volume-detaching"
38+
3639
// MachineSetLabelName is the label set on machines if they're controlled by MachineSet.
3740
MachineSetLabelName = "cluster.x-k8s.io/set-name"
3841

api/v1beta1/condition_consts.go

+8-3
Original file line numberDiff line numberDiff line change
@@ -124,12 +124,17 @@ const (
124124
// WaitingExternalHookReason (Severity=Info) provide evidence that we are waiting for an external hook to complete.
125125
WaitingExternalHookReason = "WaitingExternalHook"
126126

127-
// VolumeDetachFinishedCondition reports a machine node volume detachment finished either by waiting for volumes to be
128-
// detached or left alone due to volume detach timeout exceeded.
129-
VolumeDetachFinishedCondition ConditionType = "VolumeDetachFinished"
127+
// VolumeDetachSucceededCondition reports a machine waiting for volumes to be detached.
128+
VolumeDetachSucceededCondition ConditionType = "VolumeDetachSucceeded"
129+
130+
// VolumeDetachTimedOutCondition reports a machine waiting for node volumes detachment timeout to be exceeded.
131+
VolumeDetachTimedOutCondition ConditionType = "VolumeDetachTimedOut"
130132

131133
// WaitingForVolumeDetachReason (Severity=Info) provide evidence that a machine node waiting for volumes to be attached.
132134
WaitingForVolumeDetachReason = "WaitingForVolumeDetach"
135+
136+
// WaitingForVolumeDetachTimeoutReason (Severity=Info) provide evidence that a machine node waiting for node volumes detachment timeout to be exceeded.
137+
WaitingForVolumeDetachTimeoutReason = "WaitingForVolumeDetachTimeout"
133138
)
134139

135140
const (

api/v1beta1/machine_types.go

+3
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ const (
3333
// ExcludeNodeDrainingAnnotation annotation explicitly skips node draining if set.
3434
ExcludeNodeDrainingAnnotation = "machine.cluster.x-k8s.io/exclude-node-draining"
3535

36+
// ExcludeNodeVolumeDetachingAnnotation annotation explicitly skips volume detaching if set.
37+
ExcludeNodeVolumeDetachingAnnotation = "machine.cluster.x-k8s.io/exclude-node-volume-detaching"
38+
3639
// MachineSetLabelName is the label set on machines if they're controlled by MachineSet.
3740
MachineSetLabelName = "cluster.x-k8s.io/set-name"
3841

internal/controllers/machine/machine_controller.go

+25-20
Original file line numberDiff line numberDiff line change
@@ -346,24 +346,29 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, cluster *clusterv1.Clu
346346
conditions.MarkTrue(m, clusterv1.DrainingSucceededCondition)
347347
r.recorder.Eventf(m, corev1.EventTypeNormal, "SuccessfulDrainNode", "success draining Machine's node %q", m.Status.NodeRef.Name)
348348

349-
// After node draining, make sure either volumes are detached or left alone after volume detach timeout
350-
// exceeded before deleting the Node.
351-
if conditions.Get(m, clusterv1.VolumeDetachFinishedCondition) == nil {
352-
conditions.MarkFalse(m, clusterv1.VolumeDetachFinishedCondition, clusterv1.WaitingForVolumeDetachReason, clusterv1.ConditionSeverityInfo, "Waiting for node volumes to be detached")
353-
}
354-
if r.nodeDrainTimeoutExceeded(m) || r.volumeDetachTimeoutExceeded(m) {
355-
conditions.MarkTrue(m, clusterv1.VolumeDetachFinishedCondition)
356-
r.recorder.Eventf(m, corev1.EventTypeNormal, "NodeVolumeUndetached", "node volumes detach timeout exceeded for Machine's node %q, moving on with node deletion", m.Status.NodeRef.Name)
357-
} else if ok, err := r.shouldWaitForNodeVolumes(ctx, cluster, m.Status.NodeRef.Name); ok || err != nil {
358-
if err != nil {
359-
r.recorder.Eventf(m, corev1.EventTypeWarning, "FailedWaitForVolumeDetach", "error wait for volume detach, node %q: %v", m.Status.NodeRef.Name, err)
360-
return ctrl.Result{}, err
349+
if _, exists := m.ObjectMeta.Annotations[clusterv1.ExcludeNodeVolumeDetachingAnnotation]; !exists {
350+
// After node draining, if ExcludeNodeVolumeDetachingAnnotation does not exist, make sure either volumes are
351+
// detached or left alone after volume detach timeout exceeded before deleting the Node.
352+
if conditions.Get(m, clusterv1.VolumeDetachTimedOutCondition) == nil {
353+
conditions.MarkFalse(m, clusterv1.VolumeDetachTimedOutCondition, clusterv1.WaitingForVolumeDetachTimeoutReason, clusterv1.ConditionSeverityInfo, "Waiting for node volumes detachment timeout to be exceeded")
354+
}
355+
if !r.volumeDetachTimeoutExceeded(m) {
356+
if conditions.Get(m, clusterv1.VolumeDetachSucceededCondition) == nil {
357+
conditions.MarkFalse(m, clusterv1.VolumeDetachSucceededCondition, clusterv1.WaitingForVolumeDetachReason, clusterv1.ConditionSeverityInfo, "Waiting for node volumes to be detached")
358+
}
359+
if ok, err := r.shouldWaitForNodeVolumes(ctx, cluster, m.Status.NodeRef.Name); ok || err != nil {
360+
if err != nil {
361+
r.recorder.Eventf(m, corev1.EventTypeWarning, "FailedWaitForVolumeDetach", "error wait for volume detach, node %q: %v", m.Status.NodeRef.Name, err)
362+
return ctrl.Result{}, err
363+
}
364+
log.Info("Waiting for node volumes to be detached", "node", klog.KRef("", m.Status.NodeRef.Name))
365+
return ctrl.Result{}, nil
366+
}
367+
conditions.MarkTrue(m, clusterv1.VolumeDetachSucceededCondition)
368+
r.recorder.Eventf(m, corev1.EventTypeNormal, "NodeVolumesDetached", "success waiting for node volumes detach Machine's node %q", m.Status.NodeRef.Name)
361369
}
362-
log.Info("Waiting for node volumes to be detached", "node", klog.KRef("", m.Status.NodeRef.Name))
363-
return ctrl.Result{}, nil
364-
} else {
365-
conditions.MarkTrue(m, clusterv1.VolumeDetachFinishedCondition)
366-
r.recorder.Eventf(m, corev1.EventTypeNormal, "NodeVolumesDetached", "success waiting for node volumes detach Machine's node %q", m.Status.NodeRef.Name)
370+
conditions.MarkTrue(m, clusterv1.VolumeDetachTimedOutCondition)
371+
r.recorder.Eventf(m, corev1.EventTypeNormal, "NodeVolumesDetachTimedOut", "node volumes detach timeout exceeded for Machine's node %q, moving on with node deletion", m.Status.NodeRef.Name)
367372
}
368373
}
369374
}
@@ -461,13 +466,13 @@ func (r *Reconciler) volumeDetachTimeoutExceeded(machine *clusterv1.Machine) boo
461466
return false
462467
}
463468

464-
// if the volume detaching finished condition does not exist
465-
if conditions.Get(machine, clusterv1.VolumeDetachFinishedCondition) == nil {
469+
// if the volume detaching succeeded condition does not exist
470+
if conditions.Get(machine, clusterv1.VolumeDetachSucceededCondition) == nil {
466471
return false
467472
}
468473

469474
now := time.Now()
470-
firstTimeDetach := conditions.GetLastTransitionTime(machine, clusterv1.VolumeDetachFinishedCondition)
475+
firstTimeDetach := conditions.GetLastTransitionTime(machine, clusterv1.VolumeDetachSucceededCondition)
471476
diff := now.Sub(firstTimeDetach.Time)
472477
return diff.Seconds() >= machine.Spec.VolumeDetachTimeout.Seconds()
473478
}

0 commit comments

Comments
 (0)