Skip to content

fix: preserve header routes during canary deployment. Fixes #4486 #4575

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
downfa11 wants to merge 4 commits into
base: master
Choose a base branch
from
Open

fix: preserve header routes during canary deployment. Fixes #4486 #4575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ceee8ed
fix: preserve header routes during canary deployment
downfa11 Jan 3, 2026
7f469da
fix: check all canary steps before removing managed routes
downfa11 Jan 4, 2026
26cb17c
fix: check only current step for managed routes during traffic routing
downfa11 Jan 4, 2026
487e202
fix: resolved test coverage issues
downfa11 Jan 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions rollout/trafficrouting.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
64 changes: 64 additions & 0 deletions rollout/trafficrouting_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Loading