Skip to content

[2.2] fix: revert per-client xDS readiness gates (#13868, #13958) in …#14386

Merged
davidjumani merged 2 commits into
kgateway-dev:v2.2.xfrom
chandler-solo:chandler/14184-22x-backport-sleeprever
Jul 10, 2026
Merged

[2.2] fix: revert per-client xDS readiness gates (#13868, #13958) in …#14386
davidjumani merged 2 commits into
kgateway-dev:v2.2.xfrom
chandler-solo:chandler/14184-22x-backport-sleeprever

Conversation

@chandler-solo

@chandler-solo chandler-solo commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

…favor of a first-connect grace period

Description

Backport of #14380 (main: 714190cf85dcd8c06849f819053249ab8e0efbe7) to the v2.2.x release branch.

See the main PR for the full motivation: the whole-snapshot readiness gates from #13868/#13958 can stay unsatisfiable indefinitely, stranding warm proxies on stale endpoints and starving fresh proxy pods into crash loops (#14184). They're replaced with a per-stream first-connect grace period.

Not a clean cherry-pick

git cherry-pick does not apply — the v2.2.x per-client xDS baseline differs from main, so the change was applied by hand. Deltas from the upstream commit:

  • IR type spelling: v2.2.x uses ir.UniqlyConnectedClient / NewUniqlyConnectedClient (main dropped the typo); all references normalized.
  • perclient.go: v2.2.x never took proxy-syncer bug fix, but for a bug that never actually happens #13958's empty-cluster substitution, so the guard was already the combined clustersForUcc == nil || clientEndpointResources == nil form; only the fix: remedies a reconnect-time xDS race #13868 reference-cluster gates and their proto-walk helpers are removed.
  • uniqueclients.go add(): v2.2.x has no logAndCheckEnvoyVersion call, so the 4-value signature change touched one fewer return path than upstream.
  • e2e suite: v2.2.x exposes assertions via the TestInstallation.Assertions.Gomega field rather than main's AssertionsT(t) method; the new xds_starvation suite was adapted accordingly. Suite registration / CI matrix placement also differ (no websocket suite; the cluster-seven entry is still named api-validation).

Net effect is behaviorally identical to #14380; line counts differ only because of the smaller baseline above.

/kind fix

Changelog

Fixed an issue where a gateway whose configuration referenced a never-ready backend (for example an ExternalName Service that produces no endpoints) could withhold its entire per-client xDS snapshot indefinitely, stranding already-connected proxies on stale endpoints and preventing fresh proxy pods from becoming ready. The whole-snapshot readiness gates are replaced with a brief first-connect grace period (configurable via KGW_XDS_FIRST_CONNECT_DELAY).

Additional Notes

…gateway-dev#13958) in favor of a first-connect grace period

# Description

Backport of kgateway-dev#14380 (main: `714190cf85dcd8c06849f819053249ab8e0efbe7`) to the v2.2.x release branch.

See the main PR for the full motivation: the whole-snapshot readiness gates from
kgateway-dev#13868/kgateway-dev#13958 can stay unsatisfiable indefinitely, stranding warm proxies on stale
endpoints and starving fresh proxy pods into crash loops (kgateway-dev#14184). They're replaced
with a per-stream first-connect grace period.

## Not a clean cherry-pick

`git cherry-pick` does not apply — the v2.2.x per-client xDS baseline differs from
main, so the change was applied by hand. Deltas from the upstream commit:

- **IR type spelling**: v2.2.x uses `ir.UniqlyConnectedClient` / `NewUniqlyConnectedClient`
  (main dropped the typo); all references normalized.
- **perclient.go**: v2.2.x never took kgateway-dev#13958's empty-cluster substitution, so the
  guard was already the combined `clustersForUcc == nil || clientEndpointResources == nil`
  form; only the kgateway-dev#13868 reference-cluster gates and their proto-walk helpers are removed.
- **uniqueclients.go `add()`**: v2.2.x has no `logAndCheckEnvoyVersion` call, so the
  4-value signature change touched one fewer return path than upstream.
- **e2e suite**: v2.2.x exposes assertions via the `TestInstallation.Assertions.Gomega`
  field rather than main's `AssertionsT(t)` method; the new `xds_starvation` suite was
  adapted accordingly. Suite registration / CI matrix placement also differ (no
  `websocket` suite; the `cluster-seven` entry is still named `api-validation`).

Net effect is behaviorally identical to kgateway-dev#14380; line counts differ only because of the
smaller baseline above.

/kind fix

# Changelog

```release-note
Fixed an issue where a gateway whose configuration referenced a never-ready backend
(for example an ExternalName Service that produces no endpoints) could withhold its
entire per-client xDS snapshot indefinitely, stranding already-connected proxies on
stale endpoints and preventing fresh proxy pods from becoming ready. The whole-snapshot
readiness gates are replaced with a brief first-connect grace period
(configurable via KGW_XDS_FIRST_CONNECT_DELAY).
```

# Additional Notes

Verified locally: `go build` of the affected packages, `go vet -tags e2e` on the new
suite + registration, unit tests for `proxy_syncer` / `krtcollections` / `setup`,
`make analyze` (0 issues), and `make lint-actions` (0 findings).

Signed-off-by: David L. Chandler <[email protected]>
Copilot AI review requested due to automatic review settings July 10, 2026 19:16
@gateway-bot gateway-bot added kind/fix Categorizes issue or PR as related to a bug. release-note labels Jul 10, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This backport removes the per-client xDS whole-snapshot readiness gates (which could remain unsatisfiable and starve clients) and replaces them with a per-stream “first-connect” grace period to give per-client translation a head start before the first xDS response, plus regression coverage (unit + e2e) to pin the anti-starvation behavior.

Changes:

  • Introduce a lazily env-configured, atomic-backed KGW_XDS_FIRST_CONNECT_DELAY and sleep once on the first request of each new xDS stream after client registration.
  • Remove the proto-walk referenced-cluster readiness gating logic from per-client snapshot publication, retaining only the “inputs not derived yet -> defer” behavior.
  • Add a focused unit contract test for the delay and a new e2e suite (XdsStarvation) to prevent regressions involving never-ready backend references.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/krtcollections/uniqueclients.go Adds per-stream first-connect delay (lazy env read + atomic storage) and applies sleep on first request of new streams.
pkg/krtcollections/export_test.go Exposes a test-only override helper for the first-connect delay with safe restore semantics.
pkg/krtcollections/uniqueclients_test.go Disables the first-connect delay for existing unique-clients tests to avoid unintended slowdowns.
pkg/krtcollections/uniqueclients_delay_test.go Adds a contract test ensuring the delay applies once per stream and does not block registration.
pkg/kgateway/proxy_syncer/perclient.go Removes the whole-snapshot readiness gates and documents the shift to the first-connect delay mitigation.
pkg/kgateway/proxy_syncer/perclient_test.go Drops gate-specific tests and keeps/updates tests that assert snapshots still publish for misconfig/errored backends.
pkg/kgateway/proxy_syncer/proxy_syncer.go Updates subscriber Delete-branch commentary consistent with the new “defer only when inputs nil” behavior.
pkg/kgateway/setup/main_test.go Forces delay to 0 by default for live ADS setup tests to reduce runtime and avoid masking correctness issues.
test/e2e/features/xds_starvation/suite.go New e2e suite validating that never-ready references don’t strand warm clients or crashloop new pods.
test/e2e/features/xds_starvation/testdata/setup.yaml Manifests for the starvation scenario (ExternalName backend + valid route + ReferenceGrant).
test/e2e/features/xds_starvation/testdata/postrestart-route.yaml Route applied post-controller-restart to verify warm clients still receive updates.
test/e2e/tests/kgateway_tests.go Registers the new XdsStarvation suite in the kube gateway e2e runner.
.github/workflows/e2e.yaml Adds XdsStarvation to the api-validation cluster’s go test selection regex.
Makefile Updates kind-create cluster-existence guard to use grep -x (anchored match).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Makefile
Signed-off-by: David L. Chandler <[email protected]>
@davidjumani
davidjumani added this pull request to the merge queue Jul 10, 2026
Merged via the queue into kgateway-dev:v2.2.x with commit 8193c04 Jul 10, 2026
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/fix Categorizes issue or PR as related to a bug. release-note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants