Skip to content

Commit

Permalink
machine-controller: fix phase tests race condition in tests on lastUp…
Browse files Browse the repository at this point in the history
…dated field

The test patches the status of the Machine, while the reconciliation may be already happening.
This leads to inconsistent state which is not deterministic and into the Machine's status to
never reach the targeted state.
  • Loading branch information
chrischdi committed Apr 5, 2023
1 parent 94f5468 commit 8e52cea
Showing 1 changed file with 6 additions and 13 deletions.
19 changes: 6 additions & 13 deletions internal/controllers/machine/machine_controller_phases_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,14 +367,13 @@ func TestReconcileMachinePhases(t *testing.T) {

g.Expect(env.Create(ctx, bootstrapConfig)).To(Succeed())
g.Expect(env.Create(ctx, infraMachine)).To(Succeed())
// We have to subtract 2 seconds, because .status.lastUpdated does not contain miliseconds.
preUpdate := time.Now().Add(-2 * time.Second)
g.Expect(env.Create(ctx, machine)).To(Succeed())

modifiedMachine := machine.DeepCopy()
// Set NodeRef.
machine.Status.NodeRef = &corev1.ObjectReference{Kind: "Node", Name: node.Name}
// Set the LastUpdated to be able to verify it is updated when the phase changes
lastUpdated := metav1.NewTime(time.Now().Add(-10 * time.Second))
machine.Status.LastUpdated = &lastUpdated
g.Expect(env.Status().Patch(ctx, modifiedMachine, client.MergeFrom(machine))).To(Succeed())

// Set bootstrap ready.
Expand All @@ -397,7 +396,7 @@ func TestReconcileMachinePhases(t *testing.T) {
g.Expect(machine.Status.Addresses).To(BeEmpty())
// Verify that the LastUpdated timestamp was updated
g.Expect(machine.Status.LastUpdated).NotTo(BeNil())
g.Expect(machine.Status.LastUpdated.After(lastUpdated.Time)).To(BeTrue())
g.Expect(machine.Status.LastUpdated.After(preUpdate)).To(BeTrue())
return true
}, 10*time.Second).Should(BeTrue())
})
Expand Down Expand Up @@ -506,16 +505,10 @@ func TestReconcileMachinePhases(t *testing.T) {

g.Expect(env.Create(ctx, bootstrapConfig)).To(Succeed())
g.Expect(env.Create(ctx, infraMachine)).To(Succeed())
// We have to subtract 2 seconds, because .status.lastUpdated does not contain miliseconds.
preUpdate := time.Now().Add(-2 * time.Second)
g.Expect(env.Create(ctx, machine)).To(Succeed())

modifiedMachine := machine.DeepCopy()
// Set NodeRef to nil.
machine.Status.NodeRef = nil
// Set the LastUpdated to be able to verify it is updated when the phase changes
lastUpdated := metav1.NewTime(time.Now().Add(-10 * time.Second))
machine.Status.LastUpdated = &lastUpdated
g.Expect(env.Status().Patch(ctx, modifiedMachine, client.MergeFrom(machine))).To(Succeed())

// Set bootstrap ready.
modifiedBootstrapConfig := bootstrapConfig.DeepCopy()
g.Expect(unstructured.SetNestedField(modifiedBootstrapConfig.Object, true, "status", "ready")).To(Succeed())
Expand All @@ -535,7 +528,7 @@ func TestReconcileMachinePhases(t *testing.T) {
g.Expect(machine.Status.GetTypedPhase()).To(Equal(clusterv1.MachinePhaseProvisioned))
// Verify that the LastUpdated timestamp was updated
g.Expect(machine.Status.LastUpdated).NotTo(BeNil())
g.Expect(machine.Status.LastUpdated.After(lastUpdated.Time)).To(BeTrue())
g.Expect(machine.Status.LastUpdated.After(preUpdate)).To(BeTrue())
return true
}, 10*time.Second).Should(BeTrue())
})
Expand Down

0 comments on commit 8e52cea

Please sign in to comment.