Merge pull request #4019 from Nordix/backporting-3948-furkat

✨ Mark specific KCP machines with delete annotation for scaling down

api/v1alpha3/common_types.go

@@ -38,6 +38,10 @@ const (
 	// on the reconciled object.
 	PausedAnnotation = "cluster.x-k8s.io/paused"
 
+	// DeleteMachineAnnotation marks control plane and worker nodes that will be given priority for deletion
+	// when KCP or a machineset scales down. This annotation is given top priority on all delete policies.
+	DeleteMachineAnnotation = "cluster.x-k8s.io/delete-machine"
+
 	// TemplateClonedFromNameAnnotation is the infrastructure machine annotation that stores the name of the infrastructure template resource
 	// that was cloned for the machine. This annotation is set only during cloning a template. Older/adopted machines will not have this annotation.
 	TemplateClonedFromNameAnnotation = "cluster.x-k8s.io/cloned-from-name"

controllers/machineset_delete_policy.go

@@ -37,6 +37,7 @@ const (
 	DeleteNodeAnnotation = "cluster.k8s.io/delete-machine"
 	// DeleteMachineAnnotation marks nodes that will be given priority for deletion
 	// when a machineset scales down. This annotation is given top priority on all delete policies.
+	// Deprecated: Please use DeleteMachineAnnotation under api/v1alpha3 instead.
 	DeleteMachineAnnotation = "cluster.x-k8s.io/delete-machine"
 
 	mustDelete    deletePriority = 100.0
@@ -55,7 +56,7 @@ func oldestDeletePriority(machine *clusterv1.Machine) deletePriority {
 	if machine.ObjectMeta.Annotations != nil && machine.ObjectMeta.Annotations[DeleteNodeAnnotation] != "" {
 		return mustDelete
 	}
-	if _, ok := machine.ObjectMeta.Annotations[DeleteMachineAnnotation]; ok {
+	if _, ok := machine.ObjectMeta.Annotations[clusterv1.DeleteMachineAnnotation]; ok {
 		return mustDelete
 	}
 	if machine.Status.NodeRef == nil {
@@ -81,7 +82,7 @@ func newestDeletePriority(machine *clusterv1.Machine) deletePriority {
 	if machine.ObjectMeta.Annotations != nil && machine.ObjectMeta.Annotations[DeleteNodeAnnotation] != "" {
 		return mustDelete
 	}
-	if _, ok := machine.ObjectMeta.Annotations[DeleteMachineAnnotation]; ok {
+	if _, ok := machine.ObjectMeta.Annotations[clusterv1.DeleteMachineAnnotation]; ok {
 		return mustDelete
 	}
 	if machine.Status.NodeRef == nil {
@@ -100,7 +101,7 @@ func randomDeletePolicy(machine *clusterv1.Machine) deletePriority {
 	if machine.ObjectMeta.Annotations != nil && machine.ObjectMeta.Annotations[DeleteNodeAnnotation] != "" {
 		return betterDelete
 	}
-	if _, ok := machine.ObjectMeta.Annotations[DeleteMachineAnnotation]; ok {
+	if _, ok := machine.ObjectMeta.Annotations[clusterv1.DeleteMachineAnnotation]; ok {
 		return betterDelete
 	}
 	if machine.Status.NodeRef == nil {

controllers/machineset_delete_policy_test.go

@@ -44,7 +44,7 @@ func TestMachineToDelete(t *testing.T) {
 		Status:     clusterv1.MachineStatus{NodeRef: nodeRef},
 	}
 	deleteMachineWithMachineAnnotation := &clusterv1.Machine{
-		ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{DeleteMachineAnnotation: ""}},
+		ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{clusterv1.DeleteMachineAnnotation: ""}},
 		Status:     clusterv1.MachineStatus{NodeRef: nodeRef},
 	}
 	deleteMachineWithoutNodeRef := &clusterv1.Machine{}
@@ -231,7 +231,7 @@ func TestMachineNewestDelete(t *testing.T) {
 		Status:     clusterv1.MachineStatus{NodeRef: nodeRef},
 	}
 	deleteMachineWithMachineAnnotation := &clusterv1.Machine{
-		ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{DeleteMachineAnnotation: ""}, CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -10))},
+		ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{clusterv1.DeleteMachineAnnotation: ""}, CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -10))},
 		Status:     clusterv1.MachineStatus{NodeRef: nodeRef},
 	}
 	unhealthyMachine := &clusterv1.Machine{
@@ -344,7 +344,7 @@ func TestMachineOldestDelete(t *testing.T) {
 		Status:     clusterv1.MachineStatus{NodeRef: nodeRef},
 	}
 	deleteMachineWithMachineAnnotation := &clusterv1.Machine{
-		ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{DeleteMachineAnnotation: ""}, CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -10))},
+		ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{clusterv1.DeleteMachineAnnotation: ""}, CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -10))},
 		Status:     clusterv1.MachineStatus{NodeRef: nodeRef},
 	}
 	unhealthyMachine := &clusterv1.Machine{

controlplane/kubeadm/controllers/scale.go

@@ -227,7 +227,12 @@ func preflightCheckCondition(kind string, obj conditions.Getter, condition clust
 
 func selectMachineForScaleDown(controlPlane *internal.ControlPlane, outdatedMachines internal.FilterableMachineCollection) (*clusterv1.Machine, error) {
 	machines := controlPlane.Machines
-	if outdatedMachines.Len() > 0 {
+	switch {
+	case controlPlane.MachineWithDeleteAnnotation(outdatedMachines).Len() > 0:
+		machines = controlPlane.MachineWithDeleteAnnotation(outdatedMachines)
+	case controlPlane.MachineWithDeleteAnnotation(machines).Len() > 0:
+		machines = controlPlane.MachineWithDeleteAnnotation(machines)
+	case outdatedMachines.Len() > 0:
 		machines = outdatedMachines
 	}
 	return controlPlane.MachineInFailureDomainWithMostMachines(machines)

controlplane/kubeadm/controllers/scale_test.go

@@ -294,8 +294,12 @@ func TestSelectMachineForScaleDown(t *testing.T) {
 	m3 := machine("machine-3", withFailureDomain("one"), withTimestamp(startDate.Add(-4*time.Hour)))
 	m4 := machine("machine-4", withFailureDomain("two"), withTimestamp(startDate.Add(-time.Hour)))
 	m5 := machine("machine-5", withFailureDomain("two"), withTimestamp(startDate.Add(-2*time.Hour)))
+	m6 := machine("machine-6", withFailureDomain("two"), withTimestamp(startDate.Add(-7*time.Hour)))
+	m7 := machine("machine-7", withFailureDomain("two"), withTimestamp(startDate.Add(-5*time.Hour)), withAnnotation("cluster.x-k8s.io/delete-machine"))
+	m8 := machine("machine-8", withFailureDomain("two"), withTimestamp(startDate.Add(-6*time.Hour)), withAnnotation("cluster.x-k8s.io/delete-machine"))
 
 	mc3 := internal.NewFilterableMachineCollection(m1, m2, m3, m4, m5)
+	mc6 := internal.NewFilterableMachineCollection(m6, m7, m8)
 	fd := clusterv1.FailureDomains{
 		"one": failureDomain(true),
 		"two": failureDomain(true),
@@ -313,6 +317,11 @@ func TestSelectMachineForScaleDown(t *testing.T) {
 			return m.Name != "machine-5"
 		}),
 	}
+	annotatedControlPlane := &internal.ControlPlane{
+		KCP:      &kcp,
+		Cluster:  &clusterv1.Cluster{Status: clusterv1.ClusterStatus{FailureDomains: fd}},
+		Machines: mc6,
+	}
 
 	testCases := []struct {
 		name             string
@@ -322,7 +331,7 @@ func TestSelectMachineForScaleDown(t *testing.T) {
 		expectedMachine  clusterv1.Machine
 	}{
 		{
-			name:             "when there are are machines needing upgrade, it returns the oldest machine in the failure domain with the most machines needing upgrade",
+			name:             "when there are machines needing upgrade, it returns the oldest machine in the failure domain with the most machines needing upgrade",
 			cp:               needsUpgradeControlPlane,
 			outDatedMachines: internal.NewFilterableMachineCollection(m5),
 			expectErr:        false,
@@ -335,6 +344,41 @@ func TestSelectMachineForScaleDown(t *testing.T) {
 			expectErr:        false,
 			expectedMachine:  clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Name: "machine-3"}},
 		},
+		{
+			name:             "when there is a single machine marked with delete annotation key in machine collection, it returns only that marked machine",
+			cp:               annotatedControlPlane,
+			outDatedMachines: internal.NewFilterableMachineCollection(m6, m7),
+			expectErr:        false,
+			expectedMachine:  clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Name: "machine-7"}},
+		},
+		{
+			name:             "when there are machines marked with delete annotation key in machine collection, it returns the oldest marked machine first",
+			cp:               annotatedControlPlane,
+			outDatedMachines: internal.NewFilterableMachineCollection(m7, m8),
+			expectErr:        false,
+			expectedMachine:  clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Name: "machine-8"}},
+		},
+		{
+			name:             "when there are annotated machines which are part of the annotatedControlPlane but not in outdatedMachines, it returns the oldest marked machine first",
+			cp:               annotatedControlPlane,
+			outDatedMachines: internal.NewFilterableMachineCollection(),
+			expectErr:        false,
+			expectedMachine:  clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Name: "machine-8"}},
+		},
+		{
+			name:             "when there are machines needing upgrade, it returns the oldest machine in the failure domain with the most machines needing upgrade",
+			cp:               needsUpgradeControlPlane,
+			outDatedMachines: internal.NewFilterableMachineCollection(m7, m3),
+			expectErr:        false,
+			expectedMachine:  clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Name: "machine-7"}},
+		},
+		{
+			name:             "when there is an up to date machine with delete annotation, while there are any outdated machines without annotatio that still exist, it returns oldest marked machine first",
+			cp:               upToDateControlPlane,
+			outDatedMachines: internal.NewFilterableMachineCollection(m5, m3, m8, m7, m6, m1, m2),
+			expectErr:        false,
+			expectedMachine:  clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Name: "machine-8"}},
+		},
 	}
 
 	for _, tc := range testCases {
@@ -519,6 +563,12 @@ func withFailureDomain(fd string) machineOpt {
 	}
 }
 
+func withAnnotation(annotation string) machineOpt {
+	return func(m *clusterv1.Machine) {
+		m.ObjectMeta.Annotations = (map[string]string{annotation: ""})
+	}
+}
+
 func withTimestamp(t time.Time) machineOpt {
 	return func(m *clusterv1.Machine) {
 		m.CreationTimestamp = metav1.NewTime(t)

controlplane/kubeadm/internal/control_plane.go

@@ -137,6 +137,14 @@ func (c *ControlPlane) MachineInFailureDomainWithMostMachines(machines Filterabl
 	return machineToMark, nil
 }
 
+// MachineWithDeleteAnnotation returns a machine that has been annotated with DeleteMachineAnnotation key.
+func (c *ControlPlane) MachineWithDeleteAnnotation(machines FilterableMachineCollection) FilterableMachineCollection {
+	// See if there are any machines with DeleteMachineAnnotation key.
+	annotatedMachines := machines.Filter(machinefilters.HasAnnotationKey(clusterv1.DeleteMachineAnnotation))
+	// If there are, return list of annotated machines.
+	return annotatedMachines
+}
+
 // FailureDomainWithMostMachines returns a fd which exists both in machines and control-plane machines and has the most
 // control-plane machines on it.
 func (c *ControlPlane) FailureDomainWithMostMachines(machines FilterableMachineCollection) *string {
@@ -150,7 +158,6 @@ func (c *ControlPlane) FailureDomainWithMostMachines(machines FilterableMachineC
 		// in the cluster status.
 		return notInFailureDomains.Oldest().Spec.FailureDomain
 	}
-
 	return PickMost(c, machines)
 }
 
@@ -275,7 +282,7 @@ func getInfraResources(ctx context.Context, cl client.Client, machines Filterabl
 	return result, nil
 }
 
-// getInfraResources fetches the kubeadm config for each machine in the collection and returns a map of machine.Name -> KubeadmConfig.
+// getKubeadmConfigs fetches the kubeadm config for each machine in the collection and returns a map of machine.Name -> KubeadmConfig.
 func getKubeadmConfigs(ctx context.Context, cl client.Client, machines FilterableMachineCollection) (map[string]*bootstrapv1.KubeadmConfig, error) {
 	result := map[string]*bootstrapv1.KubeadmConfig{}
 	for _, m := range machines {

controlplane/kubeadm/internal/machine_collection.go

@@ -97,6 +97,7 @@ func (s FilterableMachineCollection) Len() int {
 	return len(s)
 }
 
+// newFilteredMachineCollection creates a FilterableMachineCollection from a filtered list of values.
 func newFilteredMachineCollection(filter machinefilters.Func, machines ...*clusterv1.Machine) FilterableMachineCollection {
 	ss := make(FilterableMachineCollection, len(machines))
 	for i := range machines {