From 8cd72b313a3792c57e39b94b7bcd3a35c2a01bff Mon Sep 17 00:00:00 2001
From: Sunjay Bhatia <sunjayb@vmware.com>
Date: Thu, 11 Jan 2024 10:48:08 -0500
Subject: [PATCH] Improve e2e helper updateAndWaitFor to avoid conflicts

Will retry updates on conflict rather than failing immediately

To fix failures like: https://github.com/projectcontour/contour/actions/runs/7460847887/job/20299759302

Signed-off-by: Sunjay Bhatia <sunjayb@vmware.com>
---
 test/e2e/fixtures.go  | 32 ++++++++++++++------------------
 test/e2e/framework.go | 18 +++++++++++-------
 2 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/test/e2e/fixtures.go b/test/e2e/fixtures.go
index e23cec58d5d..675d0f76a07 100644
--- a/test/e2e/fixtures.go
+++ b/test/e2e/fixtures.go
@@ -30,7 +30,6 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
-	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/clientcmd"
@@ -185,26 +184,23 @@ func (e *Echo) DeployN(ns, name string, replicas int32) (func(), *appsv1.Deploym
 }
 
 func (e *Echo) ScaleAndWaitDeployment(name, ns string, replicas int32) {
-	deployment := &appsv1.Deployment{}
-	key := types.NamespacedName{
-		Namespace: ns,
-		Name:      name,
+	deployment := &appsv1.Deployment{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      name,
+			Namespace: ns,
+		},
 	}
 
-	require.NoError(e.t, e.client.Get(context.TODO(), key, deployment))
-
-	deployment.Spec.Replicas = &replicas
-
-	updateAndWaitFor(e.t, e.client, deployment, func(d *appsv1.Deployment) bool {
-		err := e.client.Get(context.Background(), key, deployment)
-		if err != nil {
+	updateAndWaitFor(e.t, e.client, deployment,
+		func(d *appsv1.Deployment) {
+			d.Spec.Replicas = ref.To(replicas)
+		},
+		func(d *appsv1.Deployment) bool {
+			if d.Status.Replicas == replicas && d.Status.ReadyReplicas == replicas {
+				return true
+			}
 			return false
-		}
-		if deployment.Status.Replicas == replicas && deployment.Status.ReadyReplicas == replicas {
-			return true
-		}
-		return false
-	}, time.Second, time.Second*10)
+		}, time.Second, time.Second*10)
 }
 
 func (e *Echo) ListPodIPs(ns, name string) ([]string, error) {
diff --git a/test/e2e/framework.go b/test/e2e/framework.go
index 999f439b848..2fd3252f591 100644
--- a/test/e2e/framework.go
+++ b/test/e2e/framework.go
@@ -43,6 +43,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/wait"
 	kubescheme "k8s.io/client-go/kubernetes/scheme"
+	"k8s.io/client-go/util/retry"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/config"
 	"sigs.k8s.io/controller-runtime/pkg/log"
@@ -334,16 +335,19 @@ func createAndWaitFor[T client.Object](t require.TestingT, client client.Client,
 	return obj, true
 }
 
-func updateAndWaitFor[T client.Object](t require.TestingT, client client.Client, obj T, condition func(T) bool, interval, timeout time.Duration) (T, bool) {
-	require.NoError(t, client.Update(context.Background(), obj))
+func updateAndWaitFor[T client.Object](t require.TestingT, cli client.Client, obj T, mutate func(T), condition func(T) bool, interval, timeout time.Duration) (T, bool) {
+	key := client.ObjectKeyFromObject(obj)
 
-	key := types.NamespacedName{
-		Namespace: obj.GetNamespace(),
-		Name:      obj.GetName(),
-	}
+	require.NoError(t, retry.RetryOnConflict(retry.DefaultBackoff, func() error {
+		if err := cli.Get(context.TODO(), key, obj); err != nil {
+			return err
+		}
+		mutate(obj)
+		return cli.Update(context.Background(), obj)
+	}))
 
 	if err := wait.PollUntilContextTimeout(context.Background(), interval, timeout, true, func(ctx context.Context) (bool, error) {
-		if err := client.Get(ctx, key, obj); err != nil {
+		if err := cli.Get(ctx, key, obj); err != nil {
 			// if there was an error, we want to keep
 			// retrying, so just return false, not an
 			// error.