Perform draining and volume detaching once until completion

Signed-off-by: Danil-Grigorev <[email protected]>
kubernetes-sigs · Dec 17, 2024 · df2f637 · df2f637
1 parent 8b08484
commit df2f637
Show file tree

Hide file tree

Showing 2 changed files with 74 additions and 42 deletions.
diff --git a/internal/controllers/machine/machine_controller.go b/internal/controllers/machine/machine_controller.go
@@ -634,17 +634,17 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, s *scope) (ctrl.Result
 	return ctrl.Result{}, nil
 }
 
-const (
-	// KubeadmControlPlaneAPIVersion inlined from KCP (we want to avoid importing the KCP API package).
-	KubeadmControlPlaneAPIVersion = "controlplane.cluster.x-k8s.io/v1beta1"
-
-	// KubeadmControlPlanePreTerminateHookCleanupAnnotation inlined from KCP (we want to avoid importing the KCP API package).
-	KubeadmControlPlanePreTerminateHookCleanupAnnotation = clusterv1.PreTerminateDeleteHookAnnotationPrefix + "/kcp-cleanup"
-)
-
 func (r *Reconciler) isNodeDrainAllowed(m *clusterv1.Machine) bool {
-	if util.IsControlPlaneMachine(m) && util.HasOwner(m.GetOwnerReferences(), KubeadmControlPlaneAPIVersion, []string{"KubeadmControlPlane"}) {
-		if _, exists := m.Annotations[KubeadmControlPlanePreTerminateHookCleanupAnnotation]; !exists {
+	if util.IsControlPlaneMachine(m) {
+		exists := false
+		for annotation := range m.Annotations {
+			if strings.HasPrefix(annotation, clusterv1.PreTerminateDeleteHookAnnotationPrefix) {
+				exists = true
+				break
+			}
+		}
+
+		if !exists {
 			return false
 		}
 	}
@@ -657,14 +657,26 @@ func (r *Reconciler) isNodeDrainAllowed(m *clusterv1.Machine) bool {
 		return false
 	}
 
+	if conditions.IsTrue(m, clusterv1.DrainingSucceededCondition) {
+		return false
+	}
+
 	return true
 }
 
 // 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 {
-	if util.IsControlPlaneMachine(m) && util.HasOwner(m.GetOwnerReferences(), KubeadmControlPlaneAPIVersion, []string{"KubeadmControlPlane"}) {
-		if _, exists := m.Annotations[KubeadmControlPlanePreTerminateHookCleanupAnnotation]; !exists {
+	if util.IsControlPlaneMachine(m) {
+		exists := false
+		for annotation := range m.Annotations {
+			if strings.HasPrefix(annotation, clusterv1.PreTerminateDeleteHookAnnotationPrefix) {
+				exists = true
+				break
+			}
+		}
+
+		if !exists {
 			return false
 		}
 	}
@@ -677,6 +689,10 @@ func (r *Reconciler) isNodeVolumeDetachingAllowed(m *clusterv1.Machine) bool {
 		return false
 	}
 
+	if conditions.IsTrue(m, clusterv1.VolumeDetachSucceededCondition) {
+		return false
+	}
+
 	return true
 }
 

diff --git a/internal/controllers/machine/machine_controller_test.go b/internal/controllers/machine/machine_controller_test.go
@@ -1363,14 +1363,7 @@ func TestIsNodeDrainedAllowed(t *testing.T) {
 					Name:        "test-machine",
 					Namespace:   metav1.NamespaceDefault,
 					Labels:      map[string]string{clusterv1.MachineControlPlaneLabel: ""},
-					Annotations: map[string]string{KubeadmControlPlanePreTerminateHookCleanupAnnotation: ""},
-					OwnerReferences: []metav1.OwnerReference{
-						{
-							APIVersion: KubeadmControlPlaneAPIVersion,
-							Kind:       "KubeadmControlPlane",
-							Name:       "Foo",
-						},
-					},
+					Annotations: map[string]string{clusterv1.PreTerminateDeleteHookAnnotationPrefix + "/kcp-cleanup": ""},
 				},
 				Spec: clusterv1.MachineSpec{
 					ClusterName:       "test-cluster",
@@ -1388,13 +1381,6 @@ func TestIsNodeDrainedAllowed(t *testing.T) {
 					Name:      "test-machine",
 					Namespace: metav1.NamespaceDefault,
 					Labels:    map[string]string{clusterv1.MachineControlPlaneLabel: ""},
-					OwnerReferences: []metav1.OwnerReference{
-						{
-							APIVersion: KubeadmControlPlaneAPIVersion,
-							Kind:       "KubeadmControlPlane",
-							Name:       "Foo",
-						},
-					},
 				},
 				Spec: clusterv1.MachineSpec{
 					ClusterName:       "test-cluster",
@@ -1428,6 +1414,28 @@ func TestIsNodeDrainedAllowed(t *testing.T) {
 			},
 			expected: false,
 		},
+		{
+			name: "Node draining succeeded",
+			machine: &clusterv1.Machine{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:       "test-machine",
+					Namespace:  metav1.NamespaceDefault,
+					Finalizers: []string{clusterv1.MachineFinalizer},
+				},
+				Spec: clusterv1.MachineSpec{
+					ClusterName:       "test-cluster",
+					InfrastructureRef: corev1.ObjectReference{},
+					Bootstrap:         clusterv1.Bootstrap{DataSecretName: ptr.To("data")},
+				},
+				Status: clusterv1.MachineStatus{
+					Conditions: clusterv1.Conditions{{
+						Type:   clusterv1.DrainingSucceededCondition,
+						Status: corev1.ConditionTrue,
+					}},
+				},
+			},
+			expected: false,
+		},
 		{
 			name: "Node draining timeout is not yet over",
 			machine: &clusterv1.Machine{
@@ -1924,14 +1932,7 @@ func TestIsNodeVolumeDetachingAllowed(t *testing.T) {
 					Name:        "test-machine",
 					Namespace:   metav1.NamespaceDefault,
 					Labels:      map[string]string{clusterv1.MachineControlPlaneLabel: ""},
-					Annotations: map[string]string{KubeadmControlPlanePreTerminateHookCleanupAnnotation: ""},
-					OwnerReferences: []metav1.OwnerReference{
-						{
-							APIVersion: KubeadmControlPlaneAPIVersion,
-							Kind:       "KubeadmControlPlane",
-							Name:       "Foo",
-						},
-					},
+					Annotations: map[string]string{clusterv1.PreTerminateDeleteHookAnnotationPrefix + "/kcp-cleanup": ""},
 				},
 				Spec: clusterv1.MachineSpec{
 					ClusterName:       "test-cluster",
@@ -1949,13 +1950,6 @@ func TestIsNodeVolumeDetachingAllowed(t *testing.T) {
 					Name:      "test-machine",
 					Namespace: metav1.NamespaceDefault,
 					Labels:    map[string]string{clusterv1.MachineControlPlaneLabel: ""},
-					OwnerReferences: []metav1.OwnerReference{
-						{
-							APIVersion: KubeadmControlPlaneAPIVersion,
-							Kind:       "KubeadmControlPlane",
-							Name:       "Foo",
-						},
-					},
 				},
 				Spec: clusterv1.MachineSpec{
 					ClusterName:       "test-cluster",
@@ -1989,6 +1983,28 @@ func TestIsNodeVolumeDetachingAllowed(t *testing.T) {
 			},
 			expected: false,
 		},
+		{
+			name: "Volume detach completed",
+			machine: &clusterv1.Machine{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:       "test-machine",
+					Namespace:  metav1.NamespaceDefault,
+					Finalizers: []string{clusterv1.MachineFinalizer},
+				},
+				Spec: clusterv1.MachineSpec{
+					ClusterName:       "test-cluster",
+					InfrastructureRef: corev1.ObjectReference{},
+					Bootstrap:         clusterv1.Bootstrap{DataSecretName: ptr.To("data")},
+				},
+				Status: clusterv1.MachineStatus{
+					Conditions: clusterv1.Conditions{{
+						Type:   clusterv1.VolumeDetachSucceededCondition,
+						Status: corev1.ConditionTrue,
+					}},
+				},
+			},
+			expected: false,
+		},
 		{
 			name: "Volume detach timeout is not yet over",
 			machine: &clusterv1.Machine{