From a840781a5b29e3044ffa9b7b88e7ed328d86b1e9 Mon Sep 17 00:00:00 2001 From: Zach Aller Date: Tue, 10 Dec 2024 09:06:55 -0600 Subject: [PATCH] Revert "fix: switch service selector back to stable on canary service when aborted (#2540)" This reverts commit 81b015d15c942458308c8c7a266e47403ed1d9f5. Signed-off-by: Zach Aller --- rollout/trafficrouting.go | 12 ------------ rollout/trafficrouting_test.go | 3 +-- test/e2e/canary_test.go | 9 +++++++++ 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/rollout/trafficrouting.go b/rollout/trafficrouting.go index eef25e1c1c..23ac2308b2 100644 --- a/rollout/trafficrouting.go +++ b/rollout/trafficrouting.go @@ -179,22 +179,10 @@ func (c *rolloutContext) reconcileTrafficRouting() error { } } else if c.pauseContext.IsAborted() { desiredWeight = c.calculateDesiredWeightOnAbortOrStableRollback() - if (c.rollout.Spec.Strategy.Canary.DynamicStableScale && desiredWeight == 0) || !c.rollout.Spec.Strategy.Canary.DynamicStableScale { - // If we are using dynamic stable scale we need to also make sure that desiredWeight=0 aka we are completely - // done with aborting before resetting the canary service selectors back to stable. For non-dynamic scale we do not check for availability because we are - // fully aborted and stable pods will be there, if we check for availability it causes issues with ALB readiness gates if all stable pods - // have the desired readiness gate on them during an abort we get stuck in a loop because all the stable go unready and rollouts won't be able - // to switch the desired services because there is no ready pods which causes pods to get stuck progressing forever waiting for readiness. - err = c.ensureSVCTargets(c.rollout.Spec.Strategy.Canary.CanaryService, c.stableRS, false) - if err != nil { - return err - } - } err := reconciler.RemoveManagedRoutes() if err != nil { return err } - } else if c.newRS == nil || c.newRS.Status.AvailableReplicas == 0 { // when newRS is not available or replicas num is 0. never weight to canary weightDestinations = append(weightDestinations, c.calculateWeightDestinationsFromExperiment()...) diff --git a/rollout/trafficrouting_test.go b/rollout/trafficrouting_test.go index 288430f7a6..0eb3066dbd 100644 --- a/rollout/trafficrouting_test.go +++ b/rollout/trafficrouting_test.go @@ -952,7 +952,6 @@ func TestDynamicScalingDontIncreaseWeightWhenAborted(t *testing.T) { f.objects = append(f.objects, r2) f.expectPatchRolloutAction(r2) - f.expectPatchServiceAction(canarySvc, rs1PodHash) f.fakeTrafficRouting = newUnmockedFakeTrafficRoutingReconciler() f.fakeTrafficRouting.On("UpdateHash", mock.Anything, mock.Anything, mock.Anything).Return(nil) @@ -1096,7 +1095,7 @@ func TestDynamicScalingDecreaseWeightAccordingToStableAvailabilityWhenAbortedAnd f.objects = append(f.objects, r2) f.expectPatchRolloutAction(r2) - f.expectPatchServiceAction(canarySvc, rs1PodHash) + //f.expectPatchServiceAction(canarySvc, rs1PodHash) f.fakeTrafficRouting = newUnmockedFakeTrafficRoutingReconciler() f.fakeTrafficRouting.On("UpdateHash", mock.Anything, mock.Anything, mock.Anything).Return(nil) diff --git a/test/e2e/canary_test.go b/test/e2e/canary_test.go index 7c39f6e3bd..b0de658e9e 100644 --- a/test/e2e/canary_test.go +++ b/test/e2e/canary_test.go @@ -567,10 +567,13 @@ func (s *CanarySuite) TestCanaryScaleDownOnAbort() { WaitForRolloutStatus("Paused"). AbortRollout(). WaitForRolloutStatus("Degraded"). +<<<<<<< HEAD Then(). // Expect that the canary service selector has been moved back to stable ExpectServiceSelector("canary-scaledowndelay-canary", map[string]string{"app": "canary-scaledowndelay", "rollouts-pod-template-hash": "66597877b7"}, false). When(). +======= +>>>>>>> parent of 81b015d15 (fix: switch service selector back to stable on canary service when aborted (#2540)) Sleep(3*time.Second). Then(). ExpectRevisionPodCount("2", 0) @@ -654,18 +657,24 @@ func (s *CanarySuite) TestCanaryDynamicStableScale() { WaitForRevisionPodCount("2", 1). Then(). ExpectRevisionPodCount("1", 4). +<<<<<<< HEAD // Assert that the canary service selector is still not set to stable rs because of dynamic stable scale still in progress Assert(func(t *fixtures.Then) { canarySvc, stableSvc := t.GetServices() assert.NotEqual(s.T(), canarySvc.Spec.Selector["rollouts-pod-template-hash"], stableSvc.Spec.Selector["rollouts-pod-template-hash"]) }). +======= +>>>>>>> parent of 81b015d15 (fix: switch service selector back to stable on canary service when aborted (#2540)) When(). MarkPodsReady("1", 1). // mark last remaining stable pod as ready (4/4 stable are ready) WaitForRevisionPodCount("2", 0). Sleep(2*time.Second). //WaitForRevisionPodCount does not wait for terminating pods and so ExpectServiceSelector fails sleep a bit for the terminating pods to be deleted Then(). +<<<<<<< HEAD // Expect that the canary service selector is now set to stable because of dynamic stable scale is over and we have all pods up on stable rs ExpectServiceSelector("dynamic-stable-scale-canary", map[string]string{"app": "dynamic-stable-scale", "rollouts-pod-template-hash": "868d98995b"}, false). +======= +>>>>>>> parent of 81b015d15 (fix: switch service selector back to stable on canary service when aborted (#2540)) ExpectRevisionPodCount("1", 4) }