Conversation
Motivation:
When a Canary using the Gateway API provider (gatewayapi:v1) is deleted
with spec.revertOnDeletion: true, Flagger reverts the target Deployment
and the Kubernetes Service, but GatewayAPIRouter.Finalize was a no-op
that returned nil without touching the HTTPRoute. The HTTPRoute was
left pointing at the Flagger-managed *-primary/*-canary backends and
carrying the session-affinity cookie rule, both of which reference
Services that are deleted during finalization. This leaves the route
with unresolved backends, causing request failures until an operator
manually restores it.
Approach:
Apply the same annotation-based revert pattern already used by
IstioRouter.Finalize (VirtualService) and KubernetesDefaultRouter.Finalize
(Service) to GatewayAPIRouter:
- In Reconcile, the first time Flagger overwrites a pre-existing
HTTPRoute's spec, the original spec is preserved: either the
kubectl.kubernetes.io/last-applied-configuration annotation already
carries it (when the route was applied with kubectl), or Flagger
stores it under its own flagger.kubernetes.io/original-configuration
annotation before the first update.
- Finalize now looks up either annotation and, if found, restores the
HTTPRoute's spec from it. If neither annotation is present (e.g. the
HTTPRoute was created by Flagger itself and there is nothing to
revert to), it logs a warning and leaves the route untouched, as
before.
This change is scoped to pkg/router/gateway_api.go (the gatewayapi:v1
provider named in the report). GatewayAPIV1Beta1Router
(gateway_api_v1beta1.go) is a separate, independently implemented type
with its own copy of Reconcile/Finalize and has the same gap, but is
left untouched here to keep this change minimal and surgical.
Validation:
- go build ./pkg/router/...
- go test ./pkg/router/... (all tests pass)
- go vet ./pkg/router/... (clean)
- gofmt -l on both changed files (clean)
- Added TestGatewayAPIRouter_Finalize with four subtests: HTTPRoute not
found returns an error; no stored annotation is a graceful no-op;
restoring from the flagger config annotation; restoring from the
kubectl annotation. Verified these are a genuine regression test by
stashing the gateway_api.go change and re-running: 3 of 4 subtests
fail against the previous no-op Finalize, and all 4 pass with the
fix applied.
- Interface compliance (router.Interface.Finalize) is verified by
go build ./pkg/router/..., since pkg/router/factory.go assigns
&GatewayAPIRouter{} to that interface within the same package.
- Could not run a full-repo `go build ./...` or the make-based CI
target in this sandbox due to available disk space being exhausted
by unrelated heavy dependencies (client-go informers, cloud SDKs)
pulled in by other packages; the change itself is fully contained
to pkg/router and was validated at that scope.
Report: fluxcd#1974
Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
Assisted-by: claude-sonnet-5 (via Claude Code)
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation:
When a Canary using the Gateway API provider (gatewayapi:v1) is deleted
with spec.revertOnDeletion: true, Flagger reverts the target Deployment
and the Kubernetes Service, but GatewayAPIRouter.Finalize was a no-op
that returned nil without touching the HTTPRoute. The HTTPRoute was
left pointing at the Flagger-managed -primary/-canary backends and
carrying the session-affinity cookie rule, both of which reference
Services that are deleted during finalization. This leaves the route
with unresolved backends, causing request failures until an operator
manually restores it.
Approach:
Apply the same annotation-based revert pattern already used by
IstioRouter.Finalize (VirtualService) and KubernetesDefaultRouter.Finalize
(Service) to GatewayAPIRouter:
HTTPRoute's spec, the original spec is preserved: either the
kubectl.kubernetes.io/last-applied-configuration annotation already
carries it (when the route was applied with kubectl), or Flagger
stores it under its own flagger.kubernetes.io/original-configuration
annotation before the first update.
HTTPRoute's spec from it. If neither annotation is present (e.g. the
HTTPRoute was created by Flagger itself and there is nothing to
revert to), it logs a warning and leaves the route untouched, as
before.
This change is scoped to pkg/router/gateway_api.go (the gatewayapi:v1
provider named in the report). GatewayAPIV1Beta1Router
(gateway_api_v1beta1.go) is a separate, independently implemented type
with its own copy of Reconcile/Finalize and has the same gap, but is
left untouched here to keep this change minimal and surgical.
Validation:
found returns an error; no stored annotation is a graceful no-op;
restoring from the flagger config annotation; restoring from the
kubectl annotation. Verified these are a genuine regression test by
stashing the gateway_api.go change and re-running: 3 of 4 subtests
fail against the previous no-op Finalize, and all 4 pass with the
fix applied.
go build ./pkg/router/..., since pkg/router/factory.go assigns
&GatewayAPIRouter{} to that interface within the same package.
go build ./...or the make-based CItarget in this sandbox due to available disk space being exhausted
by unrelated heavy dependencies (client-go informers, cloud SDKs)
pulled in by other packages; the change itself is fully contained
to pkg/router and was validated at that scope.
Report: #1974
Signed-off-by: Pujitha Paladugu 10557236+pujitha24@users.noreply.github.com
Assisted-by: claude-sonnet-5 (via Claude Code)
Fixes #1974