Skip to content

Commit

Permalink
Merge pull request kubernetes-sigs#10431 from k8s-infra-cherrypick-ro…
Browse files Browse the repository at this point in the history
…bot/cherry-pick-10087-to-release-1.6

[release-1.6] 🐛 fix: deletion priority to avoid deleting too many machines
  • Loading branch information
k8s-ci-robot authored Apr 15, 2024
2 parents cfefa6a + e844fb8 commit 36c0e55
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 6 deletions.
13 changes: 7 additions & 6 deletions internal/controllers/machineset/machineset_delete_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ type (

const (
mustDelete deletePriority = 100.0
shouldDelete deletePriority = 75.0
betterDelete deletePriority = 50.0
couldDelete deletePriority = 20.0
mustNotDelete deletePriority = 0.0
Expand All @@ -48,10 +49,10 @@ func oldestDeletePriority(machine *clusterv1.Machine) deletePriority {
return mustDelete
}
if _, ok := machine.ObjectMeta.Annotations[clusterv1.DeleteMachineAnnotation]; ok {
return mustDelete
return shouldDelete
}
if !isMachineHealthy(machine) {
return mustDelete
return betterDelete
}
if machine.ObjectMeta.CreationTimestamp.Time.IsZero() {
return mustNotDelete
Expand All @@ -60,20 +61,20 @@ func oldestDeletePriority(machine *clusterv1.Machine) deletePriority {
if d.Seconds() < 0 {
return mustNotDelete
}
return deletePriority(float64(mustDelete) * (1.0 - math.Exp(-d.Seconds()/secondsPerTenDays)))
return deletePriority(float64(betterDelete) * (1.0 - math.Exp(-d.Seconds()/secondsPerTenDays)))
}

func newestDeletePriority(machine *clusterv1.Machine) deletePriority {
if !machine.DeletionTimestamp.IsZero() {
return mustDelete
}
if _, ok := machine.ObjectMeta.Annotations[clusterv1.DeleteMachineAnnotation]; ok {
return mustDelete
return shouldDelete
}
if !isMachineHealthy(machine) {
return mustDelete
return betterDelete
}
return mustDelete - oldestDeletePriority(machine)
return betterDelete - oldestDeletePriority(machine)
}

func randomDeletePolicy(machine *clusterv1.Machine) deletePriority {
Expand Down
29 changes: 29 additions & 0 deletions internal/controllers/machineset/machineset_delete_policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,18 @@ func TestMachineOldestDelete(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -10))},
Status: clusterv1.MachineStatus{FailureReason: &statusError, NodeRef: nodeRef},
}
mustDeleteMachine := &clusterv1.Machine{
ObjectMeta: metav1.ObjectMeta{Name: "b", DeletionTimestamp: &currentTime},
Status: clusterv1.MachineStatus{NodeRef: nodeRef},
}
unhealthyMachineA := &clusterv1.Machine{
ObjectMeta: metav1.ObjectMeta{Name: "a", CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -10))},
Status: clusterv1.MachineStatus{FailureReason: &statusError, NodeRef: nodeRef},
}
unhealthyMachineZ := &clusterv1.Machine{
ObjectMeta: metav1.ObjectMeta{Name: "z", CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -10))},
Status: clusterv1.MachineStatus{FailureReason: &statusError, NodeRef: nodeRef},
}
deleteMachineWithoutNodeRef := &clusterv1.Machine{
ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -10))},
}
Expand Down Expand Up @@ -513,6 +525,23 @@ func TestMachineOldestDelete(t *testing.T) {
},
expect: []*clusterv1.Machine{nodeHealthyConditionUnknownMachine},
},
// these two cases ensures the mustDeleteMachine is always picked regardless of the machine names.
{
desc: "func=oldestDeletePriority, diff=1 (unhealthyMachineA)",
diff: 1,
machines: []*clusterv1.Machine{
empty, secondNewest, oldest, secondOldest, newest, mustDeleteMachine, unhealthyMachineA,
},
expect: []*clusterv1.Machine{mustDeleteMachine},
},
{
desc: "func=oldestDeletePriority, diff=1 (unhealthyMachineZ)",
diff: 1,
machines: []*clusterv1.Machine{
empty, secondNewest, oldest, secondOldest, newest, mustDeleteMachine, unhealthyMachineZ,
},
expect: []*clusterv1.Machine{mustDeleteMachine},
},
}

for _, test := range tests {
Expand Down

0 comments on commit 36c0e55

Please sign in to comment.