Fix race condition on metrics plugin E2E test

Signed-off-by: Jonathan West <[email protected]>

tests/e2e/fixture/k8s/fixture.go

@@ -8,6 +8,8 @@ import (
 	matcher "github.com/onsi/gomega/types"
 	"k8s.io/client-go/util/retry"
 
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -27,14 +29,26 @@ func ExistByName(k8sClient client.Client) matcher.GomegaMatcher {
 	}, BeTrue())
 }
 
+// DoesNotExistByName checks if the given resource does not exist, when retrieving it by name/namespace.
+// Does NOT check if the resource content matches.
+func NotExistByName(k8sClient client.Client) matcher.GomegaMatcher {
+
+	return WithTransform(func(k8sObject client.Object) bool {
+
+		err := k8sClient.Get(context.Background(), client.ObjectKeyFromObject(k8sObject), k8sObject)
+		return apierrors.IsNotFound(err)
+	}, BeTrue())
+}
+
 // UpdateWithoutConflict will keep trying to update object until it succeeds, or times out.
 func UpdateWithoutConflict(ctx context.Context, obj client.Object, k8sClient client.Client, modify func(client.Object)) error {
 	err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
 		// Retrieve the latest version of the object
-		err := k8sClient.Get(context.Background(), client.ObjectKeyFromObject(obj), obj)
+		err := k8sClient.Get(ctx, client.ObjectKeyFromObject(obj), obj)
 		if err != nil {
 			return err
 		}
+
 		modify(obj)
 
 		// Attempt to update the object

tests/e2e/rollout_tests_all.go

@@ -646,6 +646,7 @@ func RunRolloutsTests(namespaceScopedParam bool) {
 			By("Get existing Rollouts Pod(s) before update")
 			var oldPods corev1.PodList
 			Expect(k8sClient.List(ctx, &oldPods, client.InNamespace(rolloutsManager.Namespace), client.MatchingLabels{"app.kubernetes.io/name": "argo-rollouts"})).To(Succeed())
+			Expect(oldPods.Items).To(HaveLen(1))
 
 			Expect(k8sClient.Update(ctx, &rolloutsManager)).To(Succeed())
 			Eventually(rolloutsManager, "1m", "1s").Should(rolloutManagerFixture.HavePhase(rolloutsmanagerv1alpha1.PhaseAvailable))
@@ -661,14 +662,19 @@ func RunRolloutsTests(namespaceScopedParam bool) {
 					strings.Contains(configMap.Data[controllers.MetricPluginConfigMapKey], rolloutsManager.Spec.Plugins.Metric[0].Location)
 			}, "1m", "1s").Should(BeTrue())
 
-			By("Remove traffic and metric plugins")
 			Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(&rolloutsManager), &rolloutsManager)).To(Succeed())
 
 			By("Remove plugins from RolloutManager CR")
-			rolloutsManager.Spec.Plugins.TrafficManagement = []rolloutsmanagerv1alpha1.Plugin{}
-			rolloutsManager.Spec.Plugins.Metric = []rolloutsmanagerv1alpha1.Plugin{}
-			Expect(k8sClient.Update(ctx, &rolloutsManager)).To(Succeed())
-			Eventually(rolloutsManager, "1m", "1s").Should(rolloutManagerFixture.HavePhase(rolloutsmanagerv1alpha1.PhaseAvailable))
+
+			Expect(k8s.UpdateWithoutConflict(ctx, &rolloutsManager, k8sClient, func(obj client.Object) {
+
+				rmo, ok := obj.(*rolloutsmanagerv1alpha1.RolloutManager)
+				Expect(ok).To(BeTrue())
+
+				rmo.Spec.Plugins.TrafficManagement = []rolloutsmanagerv1alpha1.Plugin{}
+				rmo.Spec.Plugins.Metric = []rolloutsmanagerv1alpha1.Plugin{}
+
+			})).To(Succeed())
 
 			By("Verify that traffic and metric plugins are removed from ConfigMap")
 			Eventually(func() bool {
@@ -679,13 +685,22 @@ func RunRolloutsTests(namespaceScopedParam bool) {
 					!strings.Contains(configMap.Data[controllers.MetricPluginConfigMapKey], "prometheus")
 			}, "1m", "1s").Should(BeTrue())
 
-			By("Get existing Rollouts Pod(s) after update")
+			By("Verifying the old pod is deleted")
+			Eventually(&oldPods.Items[0], "60s", "1s").Should(k8s.NotExistByName(k8sClient))
+
 			var newPods corev1.PodList
-			Expect(k8sClient.List(ctx, &newPods, client.InNamespace(rolloutsManager.Namespace), client.MatchingLabels{"app.kubernetes.io/name": "argo-rollouts"})).To(Succeed())
 
 			By("Verify Rollouts Pod is restarted")
-			Expect(newPods.Items).To(HaveLen(1))                              // Ensure a new Pod is created
-			Expect(oldPods.Items).To(HaveLen(1))                              // Ensure there was an old Pod
+			Eventually(func() bool {
+
+				if err := k8sClient.List(ctx, &newPods, client.InNamespace(rolloutsManager.Namespace), client.MatchingLabels{"app.kubernetes.io/name": "argo-rollouts"}); err != nil {
+					return false
+				}
+
+				return len(newPods.Items) == 1
+
+			}, "60s", "1s").Should(BeTrue())
+
 			Expect(newPods.Items[0].Name).NotTo(Equal(oldPods.Items[0].Name)) // Ensure the Pod names are different
 		})