fix: envoy service cluster name for zone-aware routing#6763
Merged
zirain merged 3 commits intoenvoyproxy:mainfrom Aug 13, 2025
Merged
fix: envoy service cluster name for zone-aware routing#6763zirain merged 3 commits intoenvoyproxy:mainfrom
zirain merged 3 commits intoenvoyproxy:mainfrom
Conversation
Contributor
|
Apologies for the oversight. I'll review this! |
Contributor
|
/retest |
Contributor
|
Thanks! Could you add an internal test case or another zone aware e2e in https://github.com/envoyproxy/gateway/blob/c2404fedaad0fb2a3e19c8d8aaac23f9d3df5c4c/test/e2e/tests/zone_aware_routing.go? |
zirain
changed the title
zirain
reviewed
Aug 10, 2025
y-rabie
force-pushed
the
fix-zone-aware-cluster
branch
2 times, most recently
from
7e81db9 to
049ccc6
Compare
Contributor
Author
|
@jukie Adjusted tests. PTAL and re-run them |
zirain
reviewed
Aug 11, 2025
zirain
reviewed
Aug 11, 2025
y-rabie
force-pushed
the
fix-zone-aware-cluster
branch
from
049ccc6 to
342310d
Compare
zirain
reviewed
Aug 11, 2025
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6763 +/- ##
==========================================
+ Coverage 71.00% 71.10% +0.09%
==========================================
Files 224 225 +1
Lines 39621 39795 +174
==========================================
+ Hits 28132 28295 +163
- Misses 9828 9838 +10
- Partials 1661 1662 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
y-rabie
force-pushed
the
fix-zone-aware-cluster
branch
2 times, most recently
from
c35f989 to
302ddff
Compare
y-rabie
force-pushed
the
fix-zone-aware-cluster
branch
2 times, most recently
from
5985c85 to
bbb50f3
Compare
Contributor
Author
|
Can tests be re-run please |
jukie
reviewed
Aug 11, 2025
y-rabie
force-pushed
the
fix-zone-aware-cluster
branch
from
bbb50f3 to
b1ef2d2
Compare
Contributor
|
/retest |
zirain
reviewed
Aug 11, 2025
Contributor
|
/retest |
y-rabie
force-pushed
the
fix-zone-aware-cluster
branch
2 times, most recently
from
e74c8b6 to
efd79de
Compare
Signed-off-by: y-rabie <[email protected]>
y-rabie
force-pushed
the
fix-zone-aware-cluster
branch
from
4cea8e7 to
111c510
Compare
Contributor
|
/retest |
jukie
approved these changes
Aug 12, 2025
Contributor
Author
|
Do you have an idea why only the ipv6 tests keep failing @jukie ? It looks like it's timing out on the gateway I added getting ready/programmed, but I'm not sure why or what to do about it |
Contributor
|
Nothing looks misconfigured so it could be flakiness. Will try another |
Contributor
|
/retest |
1 similar comment
Contributor
|
/retest |
zirain
previously approved these changes
Aug 13, 2025
zirain
reviewed
Aug 13, 2025
Signed-off-by: y-rabie <[email protected]>
Signed-off-by: y-rabie <[email protected]>
y-rabie
force-pushed
the
fix-zone-aware-cluster
branch
from
111c510 to
7b2d4e0
Compare
jukie
approved these changes
Aug 13, 2025
Contributor
|
/retest |
zirain
approved these changes
Aug 13, 2025
zirain
pushed a commit
to zirain/gateway
that referenced
this pull request
Sep 16, 2025
* fix!: fix envoy service cluster name for zone-aware routing Signed-off-by: y-rabie <[email protected]> * extend e2e tests for zone aware routing Signed-off-by: y-rabie <[email protected]> * extend unit tests for zone aware routing Signed-off-by: y-rabie <[email protected]> --------- Signed-off-by: y-rabie <[email protected]> Signed-off-by: zirain <[email protected]>
zirain
pushed a commit
to zirain/gateway
that referenced
this pull request
Sep 16, 2025
* fix!: fix envoy service cluster name for zone-aware routing Signed-off-by: y-rabie <[email protected]> * extend e2e tests for zone aware routing Signed-off-by: y-rabie <[email protected]> * extend unit tests for zone aware routing Signed-off-by: y-rabie <[email protected]> --------- Signed-off-by: y-rabie <[email protected]> Signed-off-by: zirain <[email protected]>
zirain
added a commit
that referenced
this pull request
Sep 16, 2025
* fix: cluster stat name: lowercase Kind (#6780) cluster stat name: lowercase Kind Signed-off-by: Guy Daich <[email protected]> Signed-off-by: zirain <[email protected]> * fix: envoy service cluster name for zone-aware routing (#6763) * fix!: fix envoy service cluster name for zone-aware routing Signed-off-by: y-rabie <[email protected]> * extend e2e tests for zone aware routing Signed-off-by: y-rabie <[email protected]> * extend unit tests for zone aware routing Signed-off-by: y-rabie <[email protected]> --------- Signed-off-by: y-rabie <[email protected]> Signed-off-by: zirain <[email protected]> * conformance: update experimental test report (#6782) * conformance: update experimental test report Signed-off-by: zirain <[email protected]> * fix version Signed-off-by: zirain <[email protected]> * fix(api): image validation regex, support port in repository (#6819) fix: match repository in image with port Signed-off-by: Windfarer <[email protected]> Signed-off-by: zirain <[email protected]> * fix: Actually update xdsIR with maxAcceptPerSocketEvent (#6834) * Actually update xdsIR with maxAcceptPerSocketEvent Signed-off-by: jukie <[email protected]> * release note Signed-off-by: jukie <[email protected]> * newline lint Signed-off-by: jukie <[email protected]> --------- Signed-off-by: jukie <[email protected]> Signed-off-by: zirain <[email protected]> * bugfix: fix the topologyInjectorDisabled and the local cluster was not defined (#6847) * bugfix: fix the topologyInjectorDisabled and the local cluster was not defined. Signed-off-by: qicz <[email protected]> * fix ut Signed-off-by: qicz <[email protected]> * add topology-injector-enabled ut Signed-off-by: qicz <[email protected]> * add release note Signed-off-by: qi <[email protected]> --------- Signed-off-by: qicz <[email protected]> Signed-off-by: qi <[email protected]> Signed-off-by: zirain <[email protected]> * fix(logging): correct log formatting to avoid DPANIC in controller-runtime logger (#6846) * Update filters.go Signed-off-by: TomerJLevy <[email protected]> * add release notes Signed-off-by: TomerJLevy <[email protected]> --------- Signed-off-by: TomerJLevy <[email protected]> Signed-off-by: zirain <[email protected]> * fix: handle context errors as transient errors (#6850) * handle context errors as transient errors Signed-off-by: TomerJLevy <[email protected]> * add test cases Signed-off-by: TomerJLevy <[email protected]> * no need the new line Signed-off-by: TomerJLevy <[email protected]> * add release notes Signed-off-by: TomerJLevy <[email protected]> * Return the error as is Signed-off-by: TomerJLevy <[email protected]> * revert redundant changes Signed-off-by: TomerJLevy <[email protected]> * revert unrelated changes Signed-off-by: TomerJLevy <[email protected]> * revert more changes... Signed-off-by: TomerJLevy <[email protected]> --------- Signed-off-by: TomerJLevy <[email protected]> Co-authored-by: zirain <[email protected]> Signed-off-by: zirain <[email protected]> * bugfix: the controller cannot read the EnvoyProxy attached gatewayclass only. (#6838) Signed-off-by: zirain <[email protected]> * chore: fix CVE (#6903) Signed-off-by: Shahar Harari <[email protected]> Signed-off-by: zirain <[email protected]> * fix: nil pointer dereference in btp configmap indexer (#6921) Signed-off-by: Rudrakh Panigrahi <[email protected]> Signed-off-by: zirain <[email protected]> * improve targetRef selection for targetSelectors (#6917) * improve targetRef selection for targetSelectors * only select refs in the same namespace as the policy Signed-off-by: Arko Dasgupta <[email protected]> * fix lint Signed-off-by: Arko Dasgupta <[email protected]> --------- Signed-off-by: Arko Dasgupta <[email protected]> Signed-off-by: zirain <[email protected]> * fix: suppress lua validation logs (#6929) Signed-off-by: Rudrakh Panigrahi <[email protected]> Signed-off-by: zirain <[email protected]> * fix: rm incorrectly set exclusiveMaximum field in CRD (#6926) * fix: rm incorrectly set exclusiveMaximum field in CRD * Also fix maximum value to 599 which includes 599 as a valid num Fixes: #6925 Signed-off-by: Arko Dasgupta <[email protected]> Signed-off-by: zirain <[email protected]> * fix: rm Strict SameSite default (#6941) * by default it should be unset which implies `Lax` Relates to #6347 Signed-off-by: Arko Dasgupta <[email protected]> Signed-off-by: zirain <[email protected]> * Optimize pod cache (#6936) * Optimize pod cache Signed-off-by: jukie <[email protected]> * release note Signed-off-by: jukie <[email protected]> * Remove retry Signed-off-by: jukie <[email protected]> * cleanup Signed-off-by: jukie <[email protected]> --------- Signed-off-by: jukie <[email protected]> Signed-off-by: Isaac <[email protected]> Co-authored-by: zirain <[email protected]> Signed-off-by: zirain <[email protected]> * reduce DeepCopy in gateway-api layer (#6940) * reduce deep copy in gateway-api layer * also fixed the DeepCopy implementation for ControllerResources which was performing a Shallow Copy resulting it lack of isolation b/w provider and gateway-api layer Relates to #6919 Signed-off-by: Arko Dasgupta <[email protected]> * fix lint Signed-off-by: Arko Dasgupta <[email protected]> --------- Signed-off-by: Arko Dasgupta <[email protected]> Signed-off-by: zirain <[email protected]> * fix: validation for grpc routes with extension ref filters (#6949) Signed-off-by: Rudrakh Panigrahi <[email protected]> Signed-off-by: zirain <[email protected]> * fix: cleanup dangling route status conditions (#6812) Signed-off-by: y-rabie <[email protected]> Signed-off-by: zirain <[email protected]> * Fix: Add missing patch annotations to Compression struct for proper Merge (#6951) * fix: merge compression annotation Signed-off-by: sudipto baral <[email protected]> * test: add more compression merge test cases Signed-off-by: sudipto baral <[email protected]> --------- Signed-off-by: sudipto baral <[email protected]> Signed-off-by: zirain <[email protected]> * fix: update distroless image to resolve glibc CVEs (#6953) Signed-off-by: Shahar Harari <[email protected]> Signed-off-by: zirain <[email protected]> * chore: bump golang to 1.24.7 (#6959) chore: bump golang Signed-off-by: zirain <[email protected]> * fix: Make sure proxy protocol filter is the first listener filter (#6972) Fixes: #6873 Signed-off-by: Arko Dasgupta <[email protected]> Co-authored-by: Jacob Neil Taylor <[email protected]> Signed-off-by: zirain <[email protected]> * release notes Signed-off-by: zirain <[email protected]> * Removes reflection from RouteContext to reduce allocations (#6820) * bench: adds APIToXDS bench & small opt Signed-off-by: Takeshi Yoneda <[email protected]> * no refect goos: darwin goarch: arm64 pkg: github.com/envoyproxy/gateway/test/gobench cpu: Apple M1 Pro │ old.txt │ new.txt │ │ sec/op │ sec/op vs base │ GatewayAPItoXDS/small-10 881.2µ ± 7% 803.4µ ± 1% -8.82% (p=0.000 n=10) GatewayAPItoXDS/medium-10 4.130m ± 26% 2.959m ± 4% -28.36% (p=0.000 n=10) GatewayAPItoXDS/large-10 5.375 ± 2% 4.553 ± 1% -15.28% (p=0.000 n=10) geomean 26.94m 22.12m -17.90% │ old.txt │ new.txt │ │ B/op │ B/op vs base │ GatewayAPItoXDS/small-10 507.2Ki ± 0% 492.9Ki ± 0% -2.83% (p=0.000 n=10) GatewayAPItoXDS/medium-10 2.545Mi ± 7% 1.954Mi ± 2% -23.21% (p=0.000 n=10) GatewayAPItoXDS/large-10 2.832Gi ± 0% 2.831Gi ± 0% ~ (p=0.529 n=10) geomean 15.40Mi 13.97Mi -9.31% │ old.txt │ new.txt │ │ allocs/op │ allocs/op vs base │ GatewayAPItoXDS/small-10 8.328k ± 0% 8.017k ± 0% -3.73% (p=0.000 n=10) GatewayAPItoXDS/medium-10 39.45k ± 6% 29.74k ± 2% -24.60% (p=0.000 n=10) GatewayAPItoXDS/large-10 38.75M ± 0% 38.71M ± 0% -0.11% (p=0.000 n=10) geomean 233.5k 209.8k -10.16% Signed-off-by: Takeshi Yoneda <[email protected]> * removes garbage Signed-off-by: Takeshi Yoneda <[email protected]> * more tests Signed-off-by: Takeshi Yoneda <[email protected]> * more tests Signed-off-by: Takeshi Yoneda <[email protected]> --------- Signed-off-by: Takeshi Yoneda <[email protected]> --------- Signed-off-by: Guy Daich <[email protected]> Signed-off-by: zirain <[email protected]> Signed-off-by: y-rabie <[email protected]> Signed-off-by: Windfarer <[email protected]> Signed-off-by: jukie <[email protected]> Signed-off-by: qicz <[email protected]> Signed-off-by: qi <[email protected]> Signed-off-by: TomerJLevy <[email protected]> Signed-off-by: Shahar Harari <[email protected]> Signed-off-by: Rudrakh Panigrahi <[email protected]> Signed-off-by: Arko Dasgupta <[email protected]> Signed-off-by: Isaac <[email protected]> Signed-off-by: sudipto baral <[email protected]> Signed-off-by: Takeshi Yoneda <[email protected]> Co-authored-by: Guy Daich <[email protected]> Co-authored-by: Youssef Rabie <[email protected]> Co-authored-by: Windfarer <[email protected]> Co-authored-by: Isaac <[email protected]> Co-authored-by: qi <[email protected]> Co-authored-by: TomerJLevy <[email protected]> Co-authored-by: shahar-h <[email protected]> Co-authored-by: Rudrakh Panigrahi <[email protected]> Co-authored-by: Arko Dasgupta <[email protected]> Co-authored-by: Sudipto Baral <[email protected]> Co-authored-by: Jacob Neil Taylor <[email protected]> Co-authored-by: Takeshi Yoneda <[email protected]>
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
4 participants
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.
What type of PR is this?
fix!: fix envoy service cluster name for zone-aware routing
What this PR does / why we need it:
Currently, the new zone aware routing is not always functional when not using ForceLocalZone and using Envoy's normal distributional zone-aware routing.
This is the case when having a hard-coded name for the envoy's service in EnvoyProxy. The code always assumes the service has an autogenerated name, and so in other cases the discovery fails for envoy endpoints.
I've moved the call for
processServiceClusterin case of unmerged gateways down after we do theprocessGatewayParamsRef, so that we can fetch EnvoyProxy directly from theresourceTree. If this might cause any other unintended problems, I can move it to where it was, and fetch EnvoyProxy manually instead.Fixes #
Release Notes: Yes/No