Merge pull request #1313 from openshift-cherrypick-robot/cherry-pick-1293-to-release-4.16

[release-4.16] OCPBUGS-45998: Ensure deletion annotation takes priority and oldestPolicy can distinguish longer ages

Author: openshift-merge-bot[bot]

pkg/controller/machineset/delete_policy.go

@@ -44,7 +44,7 @@ const (
 	couldDelete   deletePriority = 20.0
 	mustNotDelete deletePriority = 0.0
 
-	secondsPerTenDays float64 = 864000
+	secondsPerHundredDays float64 = 8640000
 )
 
 type deletePriorityFunc func(machine *machinev1.Machine) deletePriority
@@ -67,7 +67,12 @@ func oldestDeletePriority(machine *machinev1.Machine) deletePriority {
 	if d.Seconds() < 0 {
 		return mustNotDelete
 	}
-	return deletePriority(float64(mustDelete) * (1.0 - math.Exp(-d.Seconds()/secondsPerTenDays)))
+
+	// The oldest machine, not already marked for deletion, should be deleted first.
+	// The priority is calculated as a function of the time since the machine was created.
+	// As the machine gets older the exponential term will approach 1, and the priority will approach the value of betterDelete.
+	// With the current division by secondsPerHundredDays, the priority will tend to betterDelete after approx ~3750 days (~10.3 years).
+	return deletePriority(float64(betterDelete) * (1.0 - math.Exp(-d.Seconds()/secondsPerHundredDays)))
 }
 
 func newestDeletePriority(machine *machinev1.Machine) deletePriority {

pkg/controller/machineset/delete_policy_test.go

@@ -281,6 +281,9 @@ func TestMachineOldestDelete(t *testing.T) {
 	new := &machinev1.Machine{ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -5))}}
 	old := &machinev1.Machine{ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -10))}}
 	oldest := &machinev1.Machine{ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -10))}}
+	old500Day := &machinev1.Machine{ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -500))}}
+	old1000Day := &machinev1.Machine{ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -1000))}}
+	old3750Day := &machinev1.Machine{ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -3750))}}
 	annotatedMachine := &machinev1.Machine{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{DeleteNodeAnnotation: "yes"}, CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -1))}}
 	oldAnnotatedMachine := &machinev1.Machine{ObjectMeta: metav1.ObjectMeta{Annotations: map[string]string{oldDeleteNodeAnnotation: "yes"}, CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -1))}}
 	unhealthyMachine := &machinev1.Machine{ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.NewTime(currentTime.Time.AddDate(0, 0, -10))}, Status: machinev1.MachineStatus{ErrorReason: &statusError}}
@@ -347,6 +350,22 @@ func TestMachineOldestDelete(t *testing.T) {
 			},
 			expect: []*machinev1.Machine{unhealthyMachine},
 		},
+		{
+			desc: "func=oldestDeletePriority, diff=1 (old but with annotate priority)",
+			diff: 1,
+			machines: []*machinev1.Machine{
+				new, annotatedMachine, old500Day, old1000Day, old3750Day,
+			},
+			expect: []*machinev1.Machine{annotatedMachine},
+		},
+		{
+			desc: "func=oldestDeletePriority, diff=3 (old machines in order)",
+			diff: 3,
+			machines: []*machinev1.Machine{
+				new, old500Day, old1000Day, old3750Day,
+			},
+			expect: []*machinev1.Machine{old3750Day, old1000Day, old500Day},
+		},
 	}
 
 	for _, test := range tests {