Merge pull request #4242 from fabriziopandini/fix-kcp-remediation-whe…

…n-nodeName!=machineName-0.3b 🐛 Fix KCP remediation when node Name & etcd member Name != machine Name
kubernetes-sigs · Mar 2, 2021 · 681073d · 681073d
2 parents a2ead11 + e25ec2c
commit 681073d
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 14 deletions.
diff --git a/controlplane/kubeadm/controllers/remediation.go b/controlplane/kubeadm/controllers/remediation.go
@@ -207,7 +207,7 @@ func (r *KubeadmControlPlaneReconciler) canSafelyRemoveEtcdMember(ctx context.Co
 	unhealthyMembers := []string{}
 	for _, etcdMember := range etcdMembers {
 		// Skip the machine to be deleted because it won't be part of the target etcd cluster.
-		if etcdMember == machineToBeRemediated.Name {
+		if machineToBeRemediated.Status.NodeRef != nil && machineToBeRemediated.Status.NodeRef.Name == etcdMember {
 			continue
 		}
 

diff --git a/controlplane/kubeadm/controllers/remediation_test.go b/controlplane/kubeadm/controllers/remediation_test.go
@@ -18,6 +18,7 @@ package controllers
 
 import (
 	"context"
+	"fmt"
 	"testing"
 
 	. "github.com/onsi/gomega"
@@ -155,7 +156,7 @@ func TestReconcileUnhealthyMachines(t *testing.T) {
 			recorder: record.NewFakeRecorder(32),
 			managementCluster: &fakeManagementCluster{
 				Workload: fakeWorkloadCluster{
-					EtcdMembersResult: controlPlane.Machines.Names(),
+					EtcdMembersResult: nodes(controlPlane.Machines),
 				},
 			},
 		}
@@ -191,7 +192,7 @@ func TestReconcileUnhealthyMachines(t *testing.T) {
 			recorder: record.NewFakeRecorder(32),
 			managementCluster: &fakeManagementCluster{
 				Workload: fakeWorkloadCluster{
-					EtcdMembersResult: controlPlane.Machines.Names(),
+					EtcdMembersResult: nodes(controlPlane.Machines),
 				},
 			},
 		}
@@ -230,7 +231,7 @@ func TestReconcileUnhealthyMachines(t *testing.T) {
 			recorder: record.NewFakeRecorder(32),
 			managementCluster: &fakeManagementCluster{
 				Workload: fakeWorkloadCluster{
-					EtcdMembersResult: controlPlane.Machines.Names(),
+					EtcdMembersResult: nodes(controlPlane.Machines),
 				},
 			},
 		}
@@ -283,7 +284,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) {
 			recorder: record.NewFakeRecorder(32),
 			managementCluster: &fakeManagementCluster{
 				Workload: fakeWorkloadCluster{
-					EtcdMembersResult: controlPlane.Machines.Names(),
+					EtcdMembersResult: nodes(controlPlane.Machines),
 				},
 			},
 		}
@@ -314,7 +315,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) {
 			recorder: record.NewFakeRecorder(32),
 			managementCluster: &fakeManagementCluster{
 				Workload: fakeWorkloadCluster{
-					EtcdMembersResult: controlPlane.Machines.Names(),
+					EtcdMembersResult: nodes(controlPlane.Machines),
 				},
 			},
 		}
@@ -346,7 +347,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) {
 			recorder: record.NewFakeRecorder(32),
 			managementCluster: &fakeManagementCluster{
 				Workload: fakeWorkloadCluster{
-					EtcdMembersResult: controlPlane.Machines.Names(),
+					EtcdMembersResult: nodes(controlPlane.Machines),
 				},
 			},
 		}
@@ -378,7 +379,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) {
 			recorder: record.NewFakeRecorder(32),
 			managementCluster: &fakeManagementCluster{
 				Workload: fakeWorkloadCluster{
-					EtcdMembersResult: controlPlane.Machines.Names(),
+					EtcdMembersResult: nodes(controlPlane.Machines),
 				},
 			},
 		}
@@ -412,7 +413,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) {
 			recorder: record.NewFakeRecorder(32),
 			managementCluster: &fakeManagementCluster{
 				Workload: fakeWorkloadCluster{
-					EtcdMembersResult: controlPlane.Machines.Names(),
+					EtcdMembersResult: nodes(controlPlane.Machines),
 				},
 			},
 		}
@@ -446,7 +447,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) {
 			recorder: record.NewFakeRecorder(32),
 			managementCluster: &fakeManagementCluster{
 				Workload: fakeWorkloadCluster{
-					EtcdMembersResult: controlPlane.Machines.Names(),
+					EtcdMembersResult: nodes(controlPlane.Machines),
 				},
 			},
 		}
@@ -482,7 +483,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) {
 			recorder: record.NewFakeRecorder(32),
 			managementCluster: &fakeManagementCluster{
 				Workload: fakeWorkloadCluster{
-					EtcdMembersResult: controlPlane.Machines.Names(),
+					EtcdMembersResult: nodes(controlPlane.Machines),
 				},
 			},
 		}
@@ -518,7 +519,7 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) {
 			recorder: record.NewFakeRecorder(32),
 			managementCluster: &fakeManagementCluster{
 				Workload: fakeWorkloadCluster{
-					EtcdMembersResult: controlPlane.Machines.Names(),
+					EtcdMembersResult: nodes(controlPlane.Machines),
 				},
 			},
 		}
@@ -532,6 +533,16 @@ func TestCanSafelyRemoveEtcdMember(t *testing.T) {
 	g.Expect(testEnv.Cleanup(ctx, ns)).To(Succeed())
 }
 
+func nodes(machines internal.FilterableMachineCollection) []string {
+	nodes := make([]string, 0, machines.Len())
+	for _, m := range machines {
+		if m.Status.NodeRef != nil {
+			nodes = append(nodes, m.Status.NodeRef.Name)
+		}
+	}
+	return nodes
+}
+
 type machineOption func(*clusterv1.Machine)
 
 func withMachineHealthCheckFailed() machineOption {
@@ -580,7 +591,7 @@ func createMachine(ctx context.Context, g *WithT, namespace, name string, option
 	patchHelper, err := patch.NewHelper(m, testEnv.GetClient())
 	g.Expect(err).ToNot(HaveOccurred())
 
-	for _, opt := range append(options, withNodeRef(m.Name)) {
+	for _, opt := range append(options, withNodeRef(fmt.Sprintf("node-%s", m.Name))) {
 		opt(m)
 	}
 
@@ -604,7 +615,7 @@ func getDeletingMachine(namespace, name string, options ...machineOption) *clust
 		},
 	}
 
-	for _, opt := range append(options, withNodeRef(m.Name)) {
+	for _, opt := range append(options, withNodeRef(fmt.Sprintf("node-%s", m.Name))) {
 		opt(m)
 	}
 	return m