Merge pull request #12818 from ivelichkovich/flakecreateandwait

🌱 Fix TestReconcileMachinePhases flake

Author: k8s-ci-robot

internal/controllers/machine/machine_controller_status_test.go

@@ -2071,7 +2071,8 @@ func TestReconcileMachinePhases(t *testing.T) {
 
 		g.Expect(env.Create(ctx, bootstrapConfig)).To(Succeed())
 		g.Expect(env.Create(ctx, infraMachine)).To(Succeed())
-		g.Expect(env.Create(ctx, machine)).To(Succeed())
+		// Create and wait on machine to make sure caches sync and reconciliation triggers.
+		g.Expect(env.CreateAndWait(ctx, machine)).To(Succeed())
 
 		// Wait until BootstrapConfig has the ownerReference.
 		g.Eventually(func(g Gomega) bool {
@@ -2123,7 +2124,8 @@ func TestReconcileMachinePhases(t *testing.T) {
 
 		g.Expect(env.Create(ctx, bootstrapConfig)).To(Succeed())
 		g.Expect(env.Create(ctx, infraMachine)).To(Succeed())
-		g.Expect(env.Create(ctx, machine)).To(Succeed())
+		// Create and wait on machine to make sure caches sync and reconciliation triggers.
+		g.Expect(env.CreateAndWait(ctx, machine)).To(Succeed())
 
 		// Wait until Machine was reconciled.
 		g.Eventually(func(g Gomega) bool {
@@ -2168,7 +2170,7 @@ func TestReconcileMachinePhases(t *testing.T) {
 		g.Expect(env.Create(ctx, infraMachine)).To(Succeed())
 		// We have to subtract 2 seconds, because .status.lastUpdated does not contain milliseconds.
 		preUpdate := time.Now().Add(-2 * time.Second)
-		g.Expect(env.Create(ctx, machine)).To(Succeed())
+		g.Expect(env.CreateAndWait(ctx, machine)).To(Succeed())
 
 		// Set the LastUpdated to be able to verify it is updated when the phase changes
 		modifiedMachine := machine.DeepCopy()
@@ -2240,7 +2242,9 @@ func TestReconcileMachinePhases(t *testing.T) {
 		g.Expect(env.Create(ctx, infraMachine)).To(Succeed())
 		// We have to subtract 2 seconds, because .status.lastUpdated does not contain milliseconds.
 		preUpdate := time.Now().Add(-2 * time.Second)
-		g.Expect(env.Create(ctx, machine)).To(Succeed())
+
+		// Create and wait on machine to make sure caches sync and reconciliation triggers.
+		g.Expect(env.CreateAndWait(ctx, machine)).To(Succeed())
 
 		modifiedMachine := machine.DeepCopy()
 		// Set NodeRef.
@@ -2329,7 +2333,8 @@ func TestReconcileMachinePhases(t *testing.T) {
 		g.Expect(env.Create(ctx, infraMachine)).To(Succeed())
 		// We have to subtract 2 seconds, because .status.lastUpdated does not contain milliseconds.
 		preUpdate := time.Now().Add(-2 * time.Second)
-		g.Expect(env.Create(ctx, machine)).To(Succeed())
+		// Create and wait on machine to make sure caches sync and reconciliation triggers.
+		g.Expect(env.CreateAndWait(ctx, machine)).To(Succeed())
 
 		modifiedMachine := machine.DeepCopy()
 		// Set NodeRef.
@@ -2407,7 +2412,8 @@ func TestReconcileMachinePhases(t *testing.T) {
 		g.Expect(env.Create(ctx, infraMachine)).To(Succeed())
 		// We have to subtract 2 seconds, because .status.lastUpdated does not contain milliseconds.
 		preUpdate := time.Now().Add(-2 * time.Second)
-		g.Expect(env.Create(ctx, machine)).To(Succeed())
+		// Create and wait on machine to make sure caches sync and reconciliation triggers.
+		g.Expect(env.CreateAndWait(ctx, machine)).To(Succeed())
 
 		modifiedMachine := machine.DeepCopy()
 		// Set NodeRef.
@@ -2474,7 +2480,8 @@ func TestReconcileMachinePhases(t *testing.T) {
 		g.Expect(env.Create(ctx, infraMachine)).To(Succeed())
 		// We have to subtract 2 seconds, because .status.lastUpdated does not contain milliseconds.
 		preUpdate := time.Now().Add(-2 * time.Second)
-		g.Expect(env.Create(ctx, machine)).To(Succeed())
+		// Create and wait on machine to make sure caches sync and reconciliation triggers.
+		g.Expect(env.CreateAndWait(ctx, machine)).To(Succeed())
 
 		// Set bootstrap ready.
 		modifiedBootstrapConfig := bootstrapConfig.DeepCopy()
@@ -2546,7 +2553,8 @@ func TestReconcileMachinePhases(t *testing.T) {
 		g.Expect(env.Create(ctx, infraMachine)).To(Succeed())
 		// We have to subtract 2 seconds, because .status.lastUpdated does not contain milliseconds.
 		preUpdate := time.Now().Add(-2 * time.Second)
-		g.Expect(env.Create(ctx, machine)).To(Succeed())
+		// Create and wait on machine to make sure caches sync and reconciliation triggers.
+		g.Expect(env.CreateAndWait(ctx, machine)).To(Succeed())
 
 		// Set bootstrap ready.
 		modifiedBootstrapConfig := bootstrapConfig.DeepCopy()
@@ -2578,7 +2586,7 @@ func TestReconcileMachinePhases(t *testing.T) {
 		g.Expect(env.Patch(ctx, modifiedMachine, client.MergeFrom(machine))).To(Succeed())
 
 		// Delete Machine
-		g.Expect(env.Delete(ctx, machine)).To(Succeed())
+		g.Expect(env.DeleteAndWait(ctx, machine)).To(Succeed())
 
 		// Wait until Machine was reconciled.
 		g.Eventually(func(g Gomega) bool {

internal/test/envtest/environment.go

@@ -527,6 +527,37 @@ func (e *Environment) CreateAndWait(ctx context.Context, obj client.Object, opts
 	return nil
 }
 
+// DeleteAndWait deletes the given object and waits for the cache to be updated accordingly.
+//
+// NOTE: Waiting for the cache to be updated helps in preventing test flakes due to the cache sync delays.
+func (e *Environment) DeleteAndWait(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error {
+	if err := e.Delete(ctx, obj, opts...); err != nil {
+		return err
+	}
+
+	// Makes sure the cache is updated with the new object
+	objCopy := obj.DeepCopyObject().(client.Object)
+	key := client.ObjectKeyFromObject(obj)
+	if err := wait.ExponentialBackoff(
+		cacheSyncBackoff,
+		func() (done bool, err error) {
+			if err := e.Get(ctx, key, objCopy); err != nil {
+				if apierrors.IsNotFound(err) {
+					// if not found possible no finalizer and delete just removed the object.
+					return true, nil
+				}
+				return false, err
+			}
+			if objCopy.GetDeletionTimestamp() != nil {
+				return true, nil
+			}
+			return false, nil
+		}); err != nil {
+		return errors.Wrapf(err, "object %s, %s is not being added to the testenv client cache", obj.GetObjectKind().GroupVersionKind().String(), key)
+	}
+	return nil
+}
+
 // PatchAndWait creates or updates the given object using server-side apply and waits for the cache to be updated accordingly.
 //
 // NOTE: Waiting for the cache to be updated helps in preventing test flakes due to the cache sync delays.