From 9645a740c3ad35d76c3a12a7450e6518192af1ff Mon Sep 17 00:00:00 2001 From: Warren Fernandes Date: Thu, 16 Apr 2020 16:43:35 -0600 Subject: [PATCH] Add DeleteMachineAnnotation for MachineSet Delete Policy --- controllers/machineset_delete_policy.go | 9 ++- controllers/machineset_delete_policy_test.go | 84 ++++++++++++++------ 2 files changed, 66 insertions(+), 27 deletions(-) diff --git a/controllers/machineset_delete_policy.go b/controllers/machineset_delete_policy.go index 50fbc11d818d..ef67dff88437 100644 --- a/controllers/machineset_delete_policy.go +++ b/controllers/machineset_delete_policy.go @@ -34,6 +34,9 @@ const ( // DeleteNodeAnnotation marks nodes that will be given priority for deletion // when a machineset scales down. This annotation is given top priority on all delete policies. 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. + DeleteMachineAnnotation = "cluster.x-k8s.io/delete-machine" mustDelete deletePriority = 100.0 betterDelete deletePriority = 50.0 @@ -48,7 +51,7 @@ func oldestDeletePriority(machine *clusterv1.Machine) deletePriority { if !machine.DeletionTimestamp.IsZero() { return mustDelete } - if machine.ObjectMeta.Annotations != nil && machine.ObjectMeta.Annotations[DeleteNodeAnnotation] != "" { + if machine.ObjectMeta.Annotations != nil && (machine.ObjectMeta.Annotations[DeleteNodeAnnotation] != "" || machine.ObjectMeta.Annotations[DeleteMachineAnnotation] != "") { return mustDelete } if machine.Status.FailureReason != nil || machine.Status.FailureMessage != nil { @@ -68,7 +71,7 @@ func newestDeletePriority(machine *clusterv1.Machine) deletePriority { if !machine.DeletionTimestamp.IsZero() { return mustDelete } - if machine.ObjectMeta.Annotations != nil && machine.ObjectMeta.Annotations[DeleteNodeAnnotation] != "" { + if machine.ObjectMeta.Annotations != nil && (machine.ObjectMeta.Annotations[DeleteNodeAnnotation] != "" || machine.ObjectMeta.Annotations[DeleteMachineAnnotation] != "") { return mustDelete } if machine.Status.FailureReason != nil || machine.Status.FailureMessage != nil { @@ -81,7 +84,7 @@ func randomDeletePolicy(machine *clusterv1.Machine) deletePriority { if !machine.DeletionTimestamp.IsZero() { return mustDelete } - if machine.ObjectMeta.Annotations != nil && machine.ObjectMeta.Annotations[DeleteNodeAnnotation] != "" { + if machine.ObjectMeta.Annotations != nil && (machine.ObjectMeta.Annotations[DeleteNodeAnnotation] != "" || machine.ObjectMeta.Annotations[DeleteMachineAnnotation] != "") { return betterDelete } if machine.Status.FailureReason != nil || machine.Status.FailureMessage != nil { diff --git a/controllers/machineset_delete_policy_test.go b/controllers/machineset_delete_policy_test.go index ecdd9cd9dfd5..7efcf8926e4d 100644 --- a/controllers/machineset_delete_policy_test.go +++ b/controllers/machineset_delete_policy_test.go @@ -31,7 +31,8 @@ func TestMachineToDelete(t *testing.T) { now := metav1.Now() mustDeleteMachine := &clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: &now}} betterDeleteMachine := &clusterv1.Machine{Status: clusterv1.MachineStatus{FailureMessage: &msg}} - deleteMeMachine := &clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{DeleteNodeAnnotation: "yes"}}} + deleteMachineWithNodeAnnotation := &clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{DeleteNodeAnnotation: "yes"}}} + deleteMachineWithMachineAnnotation := &clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{DeleteMachineAnnotation: "yes"}}} tests := []struct { desc string @@ -139,28 +140,42 @@ func TestMachineToDelete(t *testing.T) { }, }, { - desc: "func=randomDeletePolicy, annotated, diff=1", + desc: "func=randomDeletePolicy, DeleteNodeAnnotation, diff=1", diff: 1, machines: []*clusterv1.Machine{ {}, - deleteMeMachine, + deleteMachineWithNodeAnnotation, {}, }, expect: []*clusterv1.Machine{ - deleteMeMachine, + deleteMachineWithNodeAnnotation, }, - }} + }, + { + desc: "func=randomDeletePolicy, DeleteMachineAnnotation, diff=1", + diff: 1, + machines: []*clusterv1.Machine{ + {}, + deleteMachineWithMachineAnnotation, + {}, + }, + expect: []*clusterv1.Machine{ + deleteMachineWithMachineAnnotation, + }, + }, + } for _, test := range tests { - g := NewWithT(t) + t.Run(test.desc, func(t *testing.T) { + g := NewWithT(t) - result := getMachinesToDeletePrioritized(test.machines, test.diff, randomDeletePolicy) - g.Expect(result).To(Equal(test.expect)) + result := getMachinesToDeletePrioritized(test.machines, test.diff, randomDeletePolicy) + g.Expect(result).To(Equal(test.expect)) + }) } } func TestMachineNewestDelete(t *testing.T) { - currentTime := metav1.Now() statusError := capierrors.MachineStatusError("I'm unhealthy!") mustDeleteMachine := &clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{DeletionTimestamp: ¤tTime}} @@ -168,7 +183,8 @@ func TestMachineNewestDelete(t *testing.T) { new := &clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -5))}} old := &clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -10))}} oldest := &clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -10))}} - annotatedMachine := &clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{DeleteNodeAnnotation: "yes"}, CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -10))}} + deleteMachineWithNodeAnnotation := &clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{DeleteNodeAnnotation: "yes"}, CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -10))}} + deleteMachineWithMachineAnnotation := &clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{DeleteMachineAnnotation: "yes"}, CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -10))}} unhealthyMachine := &clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -10))}, Status: clusterv1.MachineStatus{FailureReason: &statusError}} tests := []struct { @@ -202,12 +218,20 @@ func TestMachineNewestDelete(t *testing.T) { expect: []*clusterv1.Machine{mustDeleteMachine, newest, new}, }, { - desc: "func=newestDeletePriority, diff=1 (annotated)", + desc: "func=newestDeletePriority, diff=1 (DeleteNodeAnnotation)", diff: 1, machines: []*clusterv1.Machine{ - new, oldest, old, newest, annotatedMachine, + new, oldest, old, newest, deleteMachineWithNodeAnnotation, }, - expect: []*clusterv1.Machine{annotatedMachine}, + expect: []*clusterv1.Machine{deleteMachineWithNodeAnnotation}, + }, + { + desc: "func=newestDeletePriority, diff=1 (DeleteMachineAnnotation)", + diff: 1, + machines: []*clusterv1.Machine{ + new, oldest, old, newest, deleteMachineWithMachineAnnotation, + }, + expect: []*clusterv1.Machine{deleteMachineWithMachineAnnotation}, }, { desc: "func=newestDeletePriority, diff=1 (unhealthy)", @@ -220,15 +244,16 @@ func TestMachineNewestDelete(t *testing.T) { } for _, test := range tests { - g := NewWithT(t) + t.Run(test.desc, func(t *testing.T) { + g := NewWithT(t) - result := getMachinesToDeletePrioritized(test.machines, test.diff, newestDeletePriority) - g.Expect(result).To(Equal(test.expect)) + result := getMachinesToDeletePrioritized(test.machines, test.diff, newestDeletePriority) + g.Expect(result).To(Equal(test.expect)) + }) } } func TestMachineOldestDelete(t *testing.T) { - currentTime := metav1.Now() statusError := capierrors.MachineStatusError("I'm unhealthy!") empty := &clusterv1.Machine{} @@ -236,7 +261,8 @@ func TestMachineOldestDelete(t *testing.T) { new := &clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -5))}} old := &clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -10))}} oldest := &clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -10))}} - annotatedMachine := &clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{DeleteNodeAnnotation: "yes"}, CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -10))}} + deleteMachineWithNodeAnnotation := &clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{DeleteNodeAnnotation: "yes"}, CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -10))}} + deleteMachineWithMachineAnnotation := &clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{DeleteMachineAnnotation: "yes"}, CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -10))}} unhealthyMachine := &clusterv1.Machine{ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -10))}, Status: clusterv1.MachineStatus{FailureReason: &statusError}} tests := []struct { @@ -278,12 +304,20 @@ func TestMachineOldestDelete(t *testing.T) { expect: []*clusterv1.Machine{oldest, old, new, newest}, }, { - desc: "func=oldestDeletePriority, diff=1 (annotated)", + desc: "func=oldestDeletePriority, diff=1 (DeleteNodeAnnotation)", + diff: 1, + machines: []*clusterv1.Machine{ + empty, new, oldest, old, newest, deleteMachineWithNodeAnnotation, + }, + expect: []*clusterv1.Machine{deleteMachineWithNodeAnnotation}, + }, + { + desc: "func=oldestDeletePriority, diff=1 (DeleteMachineAnnotation)", diff: 1, machines: []*clusterv1.Machine{ - empty, new, oldest, old, newest, annotatedMachine, + empty, new, oldest, old, newest, deleteMachineWithMachineAnnotation, }, - expect: []*clusterv1.Machine{annotatedMachine}, + expect: []*clusterv1.Machine{deleteMachineWithMachineAnnotation}, }, { desc: "func=oldestDeletePriority, diff=1 (unhealthy)", @@ -296,9 +330,11 @@ func TestMachineOldestDelete(t *testing.T) { } for _, test := range tests { - g := NewWithT(t) + t.Run(test.desc, func(t *testing.T) { + g := NewWithT(t) - result := getMachinesToDeletePrioritized(test.machines, test.diff, oldestDeletePriority) - g.Expect(result).To(Equal(test.expect)) + result := getMachinesToDeletePrioritized(test.machines, test.diff, oldestDeletePriority) + g.Expect(result).To(Equal(test.expect)) + }) } }