diff --git a/rollout/trafficrouting.go b/rollout/trafficrouting.go index d59db51151..c5ad75155d 100644 --- a/rollout/trafficrouting.go +++ b/rollout/trafficrouting.go @@ -227,9 +227,21 @@ func (c *rolloutContext) reconcileTrafficRouting() error { // During the V2 rollout, managed routes could have been setup and would continue // to direct traffic to the canary service which is now in front of 0 available replicas. // We want to remove these managed routes alongside the safety here of never weighting to the canary. - err := reconciler.RemoveManagedRoutes() - if err != nil { - return err + + // Check if current step uses managed routes (setHeaderRoute or setMirrorRoute) + currentStepUsesManagedRoutes := false + if currentStep != nil { + if currentStep.SetHeaderRoute != nil || currentStep.SetMirrorRoute != nil { + currentStepUsesManagedRoutes = true + } + } + + // Only remove managed routes if current step doesn't use them + if !currentStepUsesManagedRoutes { + err := reconciler.RemoveManagedRoutes() + if err != nil { + return err + } } } else if c.rollout.Status.PromoteFull { // on a promote full, desired stable weight should be 0 (100% to canary), diff --git a/rollout/trafficrouting_test.go b/rollout/trafficrouting_test.go index dd4a744710..2ed0e1021b 100644 --- a/rollout/trafficrouting_test.go +++ b/rollout/trafficrouting_test.go @@ -1688,3 +1688,67 @@ func TestDynamicScalingAbortWithZeroReplicas(t *testing.T) { // This should not panic or cause division by zero f.run(getKey(r1, t)) } + +func TestPreserveHeaderRoute(t *testing.T) { + f := newFixture(t) + defer f.Close() + + steps := []v1alpha1.CanaryStep{ + {SetHeaderRoute: &v1alpha1.SetHeaderRoute{Name: "test"}}, + } + + r := newCanaryRollout("foo", 1, nil, steps, pointer.Int32Ptr(0), intstr.FromInt(1), intstr.FromInt(0)) + r.Spec.Strategy.Canary.TrafficRouting = &v1alpha1.RolloutTrafficRouting{ + ALB: &v1alpha1.ALBTrafficRouting{Ingress: "test-ingress"}, + } + + f.fakeTrafficRouting = newUnmockedFakeTrafficRoutingReconciler() + f.fakeTrafficRouting.On("UpdateHash", mock.Anything, mock.Anything, mock.Anything).Return(nil) + f.fakeTrafficRouting.On("SetHeaderRoute", mock.Anything).Return(nil) + f.fakeTrafficRouting.On("SetWeight", mock.Anything, mock.Anything).Return(nil) + f.fakeTrafficRouting.On("VerifyWeight", mock.Anything).Return(pointer.BoolPtr(true), nil) + f.fakeTrafficRouting.On("RemoveManagedRoutes").Return(nil).Maybe() + + f.run(getKey(r, t)) + f.fakeTrafficRouting.AssertNotCalled(t, "RemoveManagedRoutes") +} + +func TestRemoveManagedRoute(t *testing.T) { + f := newFixture(t) + defer f.Close() + + steps := []v1alpha1.CanaryStep{{SetWeight: pointer.Int32(10)}} + + r1 := newCanaryRollout("foo", 5, nil, steps, pointer.Int32Ptr(0), intstr.FromInt(1), intstr.FromInt(0)) + r1.Spec.Strategy.Canary.TrafficRouting = &v1alpha1.RolloutTrafficRouting{ + ALB: &v1alpha1.ALBTrafficRouting{Ingress: "test-ingress"}, + } + r1.Spec.Strategy.Canary.StableService = "stable-svc" + r1.Spec.Strategy.Canary.CanaryService = "canary-svc" + + r2 := bumpVersion(r1) + rs1 := newReplicaSetWithStatus(r1, 5, 5) + rs2 := newReplicaSetWithStatus(r2, 0, 0) + + stableSvc := newService("stable-svc", 80, nil, r2) + canarySvc := newService("canary-svc", 80, nil, r2) + ingress := newIngress("test-ingress", canarySvc, stableSvc) + ingress.Spec.Rules[0].HTTP.Paths[0].Backend.ServiceName = stableSvc.Name + + r2.Status.StableRS = rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] + r2.Status.CurrentPodHash = rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] + r2.Status.CurrentStepIndex = pointer.Int32(0) + + f.objects = append(f.objects, r2) + f.kubeobjects = append(f.kubeobjects, rs1, rs2, stableSvc, canarySvc, ingress) + f.replicaSetLister = append(f.replicaSetLister, rs1, rs2) + f.serviceLister = append(f.serviceLister, stableSvc, canarySvc) + f.ingressLister = append(f.ingressLister, ingressutil.NewLegacyIngress(ingress)) + + f.fakeTrafficRouting = newUnmockedFakeTrafficRoutingReconciler() + f.fakeTrafficRouting.On("RemoveManagedRoutes").Return(errors.New("remove managed routes error")) + + f.expectPatchServiceAction(stableSvc, rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey]) + f.runExpectError(getKey(r2, t), true) + f.fakeTrafficRouting.AssertCalled(t, "RemoveManagedRoutes") +}