kubernetes: use AWS physical zone IDs for locality - #9960
Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Signed-off-by: Martin Baillie <martin@baillie.id>
b2efe51 to
24671d8
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b2efe51915
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| When the `topology.k8s.aws/zone-id` node label is nonempty, Envoy Gateway uses it for proxy locality. It also uses the label for local endpoints managed by Kubernetes EndpointSlice controllers. If the label is missing or empty, Envoy Gateway retains the existing zone. It does not change node labels or Kubernetes EndpointSlices. | ||
|
|
||
| Imported and custom EndpointSlices retain their supplied zones. For routing across AWS accounts, their producers must supply physical AZ IDs in `endpoints[].zone`. Envoy Gateway cannot resolve nodes in remote clusters. Explicit `Backend.spec.endpoints[].zone` values and `weightedZones` entries must use the same IDs. Zone names and IDs do not match, even when they describe the same physical zone. |
There was a problem hiding this comment.
AWS users who consult the generated API reference are still told that WeightedZoneConfig.Zone must match topology.kubernetes.io/zone (api/v1alpha1/loadbalancer_types.go:387-389 and generated site/content/en/latest/api/extension_types.md:6927). With this change, local endpoint localities become physical zone IDs, so following that documented contract makes configured weightedZones silently miss and receive the default weight. Update the API field description and regenerate the reference documentation to describe the AWS zone-ID behavior.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,3 @@ | |||
| Use physical AWS zone IDs for proxy and local Kubernetes-managed endpoint locality when the node | |||
There was a problem hiding this comment.
This fragment still uses 0 instead of a real PR or issue number, despite release-notes/current/README.md:20-23 requiring that number to keep fragment names unique; the commit already identifies issue 9959. Leaving the placeholder makes this fragment collide with any other pending 0-*.md entry and bypasses the intended traceability, so rename it to use 9959 (or the eventual PR number) before merging.
Useful? React with 👍 / 👎.
|
hey is there a way to represent this in BackendTrafficPolicy in a vendor agnostic way ? |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9960 +/- ##
==========================================
+ Coverage 81.30% 81.34% +0.03%
==========================================
Files 264 265 +1
Lines 40967 41003 +36
==========================================
+ Hits 33310 33354 +44
+ Misses 7656 7648 -8
Partials 1 1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Hmm |
2 participants
What this PR does / why we need it:
AWS zone names are logical and can identify different physical zones across accounts. That is, Account A's
us-east-1ais not guaranteed to be Account B'sus-east-1a.AWS also has physical IDs that are consistent across accounts. Account A's
use1-az1is guaranteed to be Account B'suse1-az1. These physical IDs are part of AWS EKS nodes from EKS 1.30+ in the node label:topology.k8s.aws/zone-id.Envoy Gateway currently uses the AWS zone name populated in the standard Kubernetes node label:
topology.kubernetes.io/zonewhen comparing for locality routing, which can route cross-account traffic to the wrong physical zone.Related:
This change uses the
topology.k8s.aws/zone-idNode label for proxy locality and local endpoint locality when it is available. It supports EndpointSlices from the Kubernetes EndpointSlice controller and the EndpointSlice mirroring controller. It retains the current zone when the ID is missing or empty. Imported and custom EndpointSlices keep the zones supplied by their producers, though these producers should also considerg supplying the accurated physical IDs for cross-account AWS routing.Validation:
make format generate manifests helm-template protos go.testdata.completewith a stable second generation passmake generate-manifestsmake lintGOMAXPROCS=4 make go.test.coverageWhich issue(s) this PR fixes:
#9959
PR Checklist
Authorship & ownership: Coding agents / AI assistants are welcome, but I have reviewed every change, understand how and why it works, can explain and maintain it, and take full responsibility for this PR. I have not submitted generated output I do not understand.
DCO: All commits are signed off (
git commit -s). See DCO: Sign your work.API agreed first: N/A: this PR does not change files under
/api.Required checks pass:
make generate gen-check,make lint, and the unit-test/coverage build pass. (The local generation-equivalent check ran eachgen-checkprerequisite twice and compared the output because the worktree has uncommitted changes.)Tests added/updated: New/changed code is covered by appropriate tests.
Docs: User-facing changes update the docs.
Release notes: The fragment is ready. Rename it with the PR or issue number before merge.
Generated files committed: N/A: this PR does not change generated inputs.
Scope & compatibility: The PR is limited to AWS locality identity. The breaking default change is in the release note and migration guide.
Codex review: Three adversarial reviews found no open code defect.
Copilot review: Request a Copilot review and address all comments after the PR opens.