fix: register ListenerSet and route indexes in offline controller#9320
Open
Aias00 wants to merge 8 commits into
Open
fix: register ListenerSet and route indexes in offline controller#9320Aias00 wants to merge 8 commits into
Aias00 wants to merge 8 commits into
Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes standalone (Host) mode + File provider reconciliation failures by bringing the offline controller’s fake client field index registrations in sync with what the Kubernetes provider reconciliation logic expects (notably for ListenerSet and ListenerSet-owned routes, plus ClusterTrustBundle refs in EnvoyExtensionPolicy).
Changes:
- Register missing field indexes in
newOfflineGatewayAPIClientforListenerSet, ListenerSet-owned Routes, and ClusterTrustBundle refs inEnvoyExtensionPolicy. - Extend offline controller unit tests to assert the new indexes are registered and usable via
MatchingFields.
Findings
Required fixes
internal/provider/kubernetes/controller_offline.go:158-162: Add a release note entry torelease-notes/current.yamlunder bug fixes describing the standalone/offline controller index-registration fix (ListenerSet + related indexes), per repository release note practice.
Optional follow-ups
- None.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/provider/kubernetes/controller_offline.go | Registers the missing ListenerSet/ListenerSet-route/ClusterTrustBundle EEP field indexes in the offline fake client. |
| internal/provider/kubernetes/controller_offline_test.go | Adds/extends unit tests to verify all newly added indexes are registered. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
158
to
+162
| return fake.NewClientBuilder(). | ||
| WithScheme(scheme). | ||
| WithIndex(&gwapiv1.Gateway{}, classGatewayIndex, gatewayIndexFunc). | ||
| WithIndex(&gwapiv1.Gateway{}, secretGatewayIndex, secretGatewayIndexFunc). | ||
| WithIndex(&gwapiv1.ListenerSet{}, gatewayListenerSetIndex, gatewayListenerSetIndexFunc). |
![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: f5ed9b34be
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| WithScheme(scheme). | ||
| WithIndex(&gwapiv1.Gateway{}, classGatewayIndex, gatewayIndexFunc). | ||
| WithIndex(&gwapiv1.Gateway{}, secretGatewayIndex, secretGatewayIndexFunc). | ||
| WithIndex(&gwapiv1.ListenerSet{}, gatewayListenerSetIndex, gatewayListenerSetIndexFunc). |
There was a problem hiding this comment.
This registers the fake-client index, but in standalone/File mode the objects still never reach that client: user YAML is loaded via loadKubernetesYAMLToResources and persisted via resourcesStore.storeResources, and neither path handles resource.Resources.ListenerSets (I checked internal/provider/file for ListenerSets). With a file containing a ListenerSet, processListenerSets will therefore still list an empty ListenerSetList, so routes/policies attached through the ListenerSet remain non-functional even though the index exists.
Useful? React with 👍 / 👎.
Contributor
Author
|
close: #9319 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9320 +/- ##
=======================================
Coverage 75.17% 75.18%
=======================================
Files 252 252
Lines 41049 41056 +7
=======================================
+ Hits 30857 30866 +9
Misses 8095 8095
+ Partials 2097 2095 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Member
|
can you fix DCO? |
Aias00
added a commit
to Aias00/gateway
that referenced
this pull request
Jun 24, 2026
PR envoyproxy#9320 already restores the missing offline controller indexes, but the follow-up review correctly called out that the user-visible fix still needs a release note fragment under the repository's new per-change changelog layout. Constraint: The repository no longer uses release-notes/current.yaml; release notes must be added as current/<section>/<pr-number>-<slug>.md fragments. Rejected: Editing release-notes/current.yaml | the file has been replaced on the PR branch by fragment-based release notes. Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep follow-up-only PR fixes constrained to the missing release-note fragment unless a new reviewer finding requires code changes. Tested: bash tools/hack/check-release-notes-filenames.sh; go test ./internal/provider/kubernetes -run TestNewOfflineGatewayAPIControllerIndexRegistration -count=1 Not-tested: Full repository test suite
Contributor
Author
yes, I am handling this right now |
Aias00
force-pushed
the
worktree-fix-listenerset-offline-index
branch
from
b6740ec to
acee7a6
Compare
Aias00
added a commit
to Aias00/gateway
that referenced
this pull request
Jun 24, 2026
PR envoyproxy#9320 already restores the missing offline controller indexes, but the follow-up review correctly called out that the user-visible fix still needs a release note fragment under the repository's new per-change changelog layout. Constraint: The repository no longer uses release-notes/current.yaml; release notes must be added as current/<section>/<pr-number>-<slug>.md fragments. Rejected: Editing release-notes/current.yaml | the file has been replaced on the PR branch by fragment-based release notes. Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep follow-up-only PR fixes constrained to the missing release-note fragment unless a new reviewer finding requires code changes. Tested: bash tools/hack/check-release-notes-filenames.sh; go test ./internal/provider/kubernetes -run TestNewOfflineGatewayAPIControllerIndexRegistration -count=1 Not-tested: Full repository test suite Signed-off-by: liuhy <liuhongyu@apache.org>
The offline controller used by the File provider was missing several field indexes that the Gateway reconciliation logic depends on. This caused repeated errors during reconciliation: 'failed to list ListenerSets: no index with name gatewayListenerSetIndex has been registered' The following indexes were missing: - gatewayListenerSetIndex on ListenerSet (root cause of the error) - listenerSetHTTPRouteIndex on HTTPRoute - listenerSetGRPCRouteIndex on GRPCRoute - listenerSetTCPRouteIndex on TCPRoute - listenerSetUDPRouteIndex on UDPRoute - listenerSetTLSRouteIndex on TLSRoute - clusterTrustBundleEepIndex on EnvoyExtensionPolicy These indexes are registered by the Kubernetes provider but were not carried over when the offline controller was created for standalone mode. Register them in newOfflineGatewayAPIClient to match the K8s provider setup, and add corresponding test coverage. Fixes: Standalone mode ListenerSet reconciliation errors Signed-off-by: liuhy <liuhongyu@apache.org>
PR envoyproxy#9320 already restores the missing offline controller indexes, but the follow-up review correctly called out that the user-visible fix still needs a release note fragment under the repository's new per-change changelog layout. Constraint: The repository no longer uses release-notes/current.yaml; release notes must be added as current/<section>/<pr-number>-<slug>.md fragments. Rejected: Editing release-notes/current.yaml | the file has been replaced on the PR branch by fragment-based release notes. Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep follow-up-only PR fixes constrained to the missing release-note fragment unless a new reviewer finding requires code changes. Tested: bash tools/hack/check-release-notes-filenames.sh; go test ./internal/provider/kubernetes -run TestNewOfflineGatewayAPIControllerIndexRegistration -count=1 Not-tested: Full repository test suite Signed-off-by: liuhy <liuhongyu@apache.org>
Aias00
force-pushed
the
worktree-fix-listenerset-offline-index
branch
from
3a88d33 to
5a1e31a
Compare
The dual-stack UDPRouteBackendFQDNTest reuses the same coredns service hostname that the preceding UDPRoute test deletes and recreates. In CI this can leave Envoy resolving a stale service address from its DNS cache, causing the UDP FQDN backend check to time out for the full poll window even though the route and backend are accepted. Use a dedicated service hostname for the FQDN-specific UDPRoute manifest so the test exercises the intended backend path without inheriting cached DNS state from the previous test. Constraint: The fix must stay within PR envoyproxy#9320 scope and avoid unrelated product-code changes. Rejected: Add sleeps or longer polling to the test | hides the stale-hostname issue without removing the cache collision. Confidence: medium Scope-risk: narrow Reversibility: clean Directive: Keep UDP FQDN backend test hostnames isolated from other UDP backend fixtures when tests create and recreate services in the same namespace. Tested: go test ./e2e/... -run '^$' -tags=e2e (from /test) Not-tested: Full e2e matrix, including v1.35.0 dual-stack runtime execution Signed-off-by: liuhy <liuhongyu@apache.org>
This reverts commit 1426f4f. Signed-off-by: liuhy <liuhongyu@apache.org>
Aias00
force-pushed
the
worktree-fix-listenerset-offline-index
branch
from
8edbb36 to
a8065ef
Compare
Contributor
Author
|
/retest |
Contributor
Author
|
/retest |
2 similar comments
Contributor
Author
|
/retest |
Contributor
Author
|
/retest |
Contributor
Author
|
@zirain hi, ci passed, pls take a look |
zirain
approved these changes
Jun 25, 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.
Problem
When running Envoy Gateway in standalone (Host) mode with the File provider, the offline controller does not register the
gatewayListenerSetIndexfield index. This causes repeated errors during Gateway reconciliation:The ListenerSet functionality is completely non-functional in standalone mode.
Root Cause
The offline controller (
internal/provider/kubernetes/controller_offline.go) used by the File provider was missing several field indexes that the Gateway reconciliation logic depends on. These indexes are registered by the Kubernetes provider inaddListenerSetIndexers,addHTTPRouteIndexers, etc., but were not carried over when the offline controller was created for standalone mode.Changes
internal/provider/kubernetes/controller_offline.goAdded 7 missing field indexes to
newOfflineGatewayAPIClient:gatewayListenerSetIndexListenerSetlistenerSetHTTPRouteIndexHTTPRoutelistenerSetGRPCRouteIndexGRPCRoutelistenerSetTCPRouteIndexTCPRoutelistenerSetUDPRouteIndexUDPRoutelistenerSetTLSRouteIndexTLSRouteclusterTrustBundleEepIndexEnvoyExtensionPolicyinternal/provider/kubernetes/controller_offline_test.goAdded test coverage for all newly registered indexes in
TestNewOfflineGatewayAPIControllerIndexRegistration:ListenerSet_indexsub-testHTTPRoute_indices,GRPCRoute_indices,TCPRoute_indices,UDPRoute_indices,TLSRoute_indices, andEnvoyExtensionPolicy_indicessub-tests with listenerSet/clusterTrustBundle index checksVerification
ListenerSet_indextest)gatewayListenerSetIndexerror count drops from multiple-per-reconcile to 0Impact