From 381c289007241a2fc8ef6854ab6aacdcdca67977 Mon Sep 17 00:00:00 2001 From: Christian Schlotter Date: Wed, 5 Apr 2023 18:28:18 +0200 Subject: [PATCH] machine-controller: fix phase tests race condition in tests on lastUpdated 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. --- .../machine/machine_controller_phases_test.go | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/internal/controllers/machine/machine_controller_phases_test.go b/internal/controllers/machine/machine_controller_phases_test.go index 5ae4ee07c319..3366dd70d82d 100644 --- a/internal/controllers/machine/machine_controller_phases_test.go +++ b/internal/controllers/machine/machine_controller_phases_test.go @@ -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. @@ -397,7 +396,7 @@ func TestReconcileMachinePhases(t *testing.T) { g.Expect(machine.Status.Addresses).To(HaveLen(0)) // 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()) }) @@ -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()) @@ -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()) })