From 4a3a62ee2aaa075cdb1a7fe441b1e214a32a0b3e Mon Sep 17 00:00:00 2001 From: Jonathan West Date: Mon, 14 Oct 2024 19:45:53 -0400 Subject: [PATCH] Fix race condition on metrics plugin E2E test Signed-off-by: Jonathan West --- tests/e2e/fixture/k8s/fixture.go | 16 +++++++++++++++- tests/e2e/rollout_tests_all.go | 33 +++++++++++++++++++++++--------- 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/tests/e2e/fixture/k8s/fixture.go b/tests/e2e/fixture/k8s/fixture.go index aee2077..70555f6 100644 --- a/tests/e2e/fixture/k8s/fixture.go +++ b/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 diff --git a/tests/e2e/rollout_tests_all.go b/tests/e2e/rollout_tests_all.go index 00c5c84..75c11b4 100644 --- a/tests/e2e/rollout_tests_all.go +++ b/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 })