Skip to content

Commit

Permalink
Use RevisionVersion field instead of CreationTimestamp to decide whic…
Browse files Browse the repository at this point in the history
…h 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.
  • Loading branch information
ingvagabund committed Jan 30, 2019
1 parent a3d39b1 commit 6867c29
Showing 1 changed file with 11 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package machinedeployment

import (
"strconv"
"testing"
"time"

Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 6867c29

Please sign in to comment.