From 6867c2926292820664ab91c6c1d03ecabaee5118 Mon Sep 17 00:00:00 2001 From: Jan Chaloupka Date: Wed, 30 Jan 2019 13:37:03 +0100 Subject: [PATCH] Use RevisionVersion field instead of CreationTimestamp to decide which MS is new/old The CreationTimestamp is not always the referential value for comparision. The time.Time.Before method does not always return true when it should. Leading to new and old MSs to be selected in an oposite way. Using the RevisionVersion for comparision is more precise and it solves the case where CreationTimestamp fields of both MSs has the same value. From observation the following machineset CreationTimestamps led to incorrect ordering of MSs: item[0]: ResourceVersion:"78", CreationTimestamp:v1.Time{Time:time.Time{wall:0x0, ext:63684460660, loc:(*time.Location)(0x2cb5040)}} item[1]: ResourceVersion:"82", CreationTimestamp:v1.Time{Time:time.Time{wall:0x0, ext:63684460661, loc:(*time.Location)(0x2cb5040)}} Even though item[1]'s ext field has higher value than item[0]'s ext field, item[1] was selected by time.Time.Before as old MS instead of new one. --- .../machinedeployment_controller_test.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/pkg/controller/machinedeployment/machinedeployment_controller_test.go b/pkg/controller/machinedeployment/machinedeployment_controller_test.go index f4288da6a248..4825d5aa2482 100644 --- a/pkg/controller/machinedeployment/machinedeployment_controller_test.go +++ b/pkg/controller/machinedeployment/machinedeployment_controller_test.go @@ -17,6 +17,7 @@ limitations under the License. package machinedeployment import ( + "strconv" "testing" "time" @@ -153,7 +154,16 @@ func TestReconcile(t *testing.T) { // Wait for the new MachineSet to get scaled up and set .Status.Replicas and .Status.AvailableReplicas // at each step var newMachineSet, oldMachineSet *machinev1beta1.MachineSet - if machineSets.Items[0].CreationTimestamp.Before(&machineSets.Items[1].CreationTimestamp) { + resourceVersion0, err0 := strconv.Atoi(machineSets.Items[0].ResourceVersion) + resourceVersion1, err1 := strconv.Atoi(machineSets.Items[1].ResourceVersion) + if err0 != nil { + t.Fatalf("Unable to convert MS %q ResourceVersion to a number: %v", machineSets.Items[0].Name, err0) + } + if err1 != nil { + t.Fatalf("Unable to convert MS %q ResourceVersion to a number: %v", machineSets.Items[1].Name, err1) + } + + if resourceVersion0 > resourceVersion1 { newMachineSet = &machineSets.Items[0] oldMachineSet = &machineSets.Items[1] } else {