Skip to content

Restore original HTTPRoute spec on Gateway API canary finalize - #1976

Open
pujitha24 wants to merge 1 commit into
fluxcd:mainfrom
pujitha24:auto/issue-1974
Open

pujitha24 wants to merge 1 commit into
fluxcd:mainfrom
pujitha24:auto/issue-1974

Conversation

@pujitha24

Copy link
Copy Markdown
Contributor

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: #1974
Signed-off-by: Pujitha Paladugu 10557236+pujitha24@users.noreply.github.com
Assisted-by: claude-sonnet-5 (via Claude Code)

Fixes #1974

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

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flagger Gateway API Canary deletion does not restore HTTPRoute

1 participant