[2.2] fix: revert per-client xDS readiness gates (#13868, #13958) in …#14386
Merged
davidjumani merged 2 commits intoJul 10, 2026
Merged
Conversation
…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]>
Contributor
There was a problem hiding this comment.
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_DELAYand 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.
Signed-off-by: David L. Chandler <[email protected]>
davidjumani
approved these changes
Jul 10, 2026
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.
…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-pickdoes 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.UniqlyConnectedClient/NewUniqlyConnectedClient(main dropped the typo); all references normalized.clustersForUcc == nil || clientEndpointResources == nilform; only the fix: remedies a reconnect-time xDS race #13868 reference-cluster gates and their proto-walk helpers are removed.add(): v2.2.x has nologAndCheckEnvoyVersioncall, so the 4-value signature change touched one fewer return path than upstream.TestInstallation.Assertions.Gomegafield rather than main'sAssertionsT(t)method; the newxds_starvationsuite was adapted accordingly. Suite registration / CI matrix placement also differ (nowebsocketsuite; thecluster-sevenentry is still namedapi-validation).Net effect is behaviorally identical to #14380; line counts differ only because of the smaller baseline above.
/kind fix
Changelog
Additional Notes