From c3904f77b9b585c223b67b877bb84630dca3734f Mon Sep 17 00:00:00 2001 From: fabriziopandini Date: Wed, 20 Nov 2024 21:09:44 +0100 Subject: [PATCH] Improve Drain for control plane machines --- .../controllers/machine/machine_controller.go | 15 +++ .../machine/machine_controller_test.go | 98 +++++++++++++++++++ 2 files changed, 113 insertions(+) diff --git a/internal/controllers/machine/machine_controller.go b/internal/controllers/machine/machine_controller.go index 48ac07adf65d..268ce67f6e40 100644 --- a/internal/controllers/machine/machine_controller.go +++ b/internal/controllers/machine/machine_controller.go @@ -633,7 +633,16 @@ func (r *Reconciler) reconcileDelete(ctx context.Context, s *scope) (ctrl.Result return ctrl.Result{}, nil } +// KubeadmControlPlanePreTerminateHookCleanupAnnotation inlined from KCP (we want to avoid importing the KCP API package). +const KubeadmControlPlanePreTerminateHookCleanupAnnotation = clusterv1.PreTerminateDeleteHookAnnotationPrefix + "/kcp-cleanup" + func (r *Reconciler) isNodeDrainAllowed(m *clusterv1.Machine) bool { + if util.IsControlPlaneMachine(m) && util.HasOwner(m.GetOwnerReferences(), clusterv1.GroupVersion.String(), []string{"KubeadmControlPlane"}) { + if _, exists := m.Annotations[KubeadmControlPlanePreTerminateHookCleanupAnnotation]; !exists { + return false + } + } + if _, exists := m.ObjectMeta.Annotations[clusterv1.ExcludeNodeDrainingAnnotation]; exists { return false } @@ -648,6 +657,12 @@ 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 { + if util.IsControlPlaneMachine(m) && util.HasOwner(m.GetOwnerReferences(), clusterv1.GroupVersion.String(), []string{"KubeadmControlPlane"}) { + if _, exists := m.Annotations[KubeadmControlPlanePreTerminateHookCleanupAnnotation]; !exists { + return false + } + } + if _, exists := m.ObjectMeta.Annotations[clusterv1.ExcludeWaitForNodeVolumeDetachAnnotation]; exists { return false } diff --git a/internal/controllers/machine/machine_controller_test.go b/internal/controllers/machine/machine_controller_test.go index 0a0d37572030..1c7d12e7484b 100644 --- a/internal/controllers/machine/machine_controller_test.go +++ b/internal/controllers/machine/machine_controller_test.go @@ -1356,6 +1356,55 @@ func TestIsNodeDrainedAllowed(t *testing.T) { }, expected: false, }, + { + name: "KCP machine with the pre terminate hook should drain", + machine: &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + Namespace: metav1.NamespaceDefault, + Labels: map[string]string{clusterv1.MachineControlPlaneLabel: ""}, + Annotations: map[string]string{KubeadmControlPlanePreTerminateHookCleanupAnnotation: ""}, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: clusterv1.GroupVersion.String(), + Kind: "KubeadmControlPlane", + Name: "Foo", + }, + }, + }, + Spec: clusterv1.MachineSpec{ + ClusterName: "test-cluster", + InfrastructureRef: corev1.ObjectReference{}, + Bootstrap: clusterv1.Bootstrap{DataSecretName: ptr.To("data")}, + }, + Status: clusterv1.MachineStatus{}, + }, + expected: true, + }, + { + name: "KCP machine without the pre terminate hook should stop draining", + machine: &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + Namespace: metav1.NamespaceDefault, + Labels: map[string]string{clusterv1.MachineControlPlaneLabel: ""}, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: clusterv1.GroupVersion.String(), + Kind: "KubeadmControlPlane", + Name: "Foo", + }, + }, + }, + Spec: clusterv1.MachineSpec{ + ClusterName: "test-cluster", + InfrastructureRef: corev1.ObjectReference{}, + Bootstrap: clusterv1.Bootstrap{DataSecretName: ptr.To("data")}, + }, + Status: clusterv1.MachineStatus{}, + }, + expected: false, + }, { name: "Node draining timeout is over", machine: &clusterv1.Machine{ @@ -1868,6 +1917,55 @@ func TestIsNodeVolumeDetachingAllowed(t *testing.T) { }, expected: false, }, + { + name: "KCP machine with the pre terminate hook should wait", + machine: &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + Namespace: metav1.NamespaceDefault, + Labels: map[string]string{clusterv1.MachineControlPlaneLabel: ""}, + Annotations: map[string]string{KubeadmControlPlanePreTerminateHookCleanupAnnotation: ""}, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: clusterv1.GroupVersion.String(), + Kind: "KubeadmControlPlane", + Name: "Foo", + }, + }, + }, + Spec: clusterv1.MachineSpec{ + ClusterName: "test-cluster", + InfrastructureRef: corev1.ObjectReference{}, + Bootstrap: clusterv1.Bootstrap{DataSecretName: ptr.To("data")}, + }, + Status: clusterv1.MachineStatus{}, + }, + expected: true, + }, + { + name: "KCP machine without the pre terminate hook should stop waiting", + machine: &clusterv1.Machine{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-machine", + Namespace: metav1.NamespaceDefault, + Labels: map[string]string{clusterv1.MachineControlPlaneLabel: ""}, + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: clusterv1.GroupVersion.String(), + Kind: "KubeadmControlPlane", + Name: "Foo", + }, + }, + }, + Spec: clusterv1.MachineSpec{ + ClusterName: "test-cluster", + InfrastructureRef: corev1.ObjectReference{}, + Bootstrap: clusterv1.Bootstrap{DataSecretName: ptr.To("data")}, + }, + Status: clusterv1.MachineStatus{}, + }, + expected: false, + }, { name: "Volume detach timeout is over", machine: &clusterv1.Machine{