e2e: add test for watched namespace#8786
e2e: add test for watched namespace#8786zhaohuabing wants to merge 2 commits intoenvoyproxy:mainfrom
Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 canceled.
|
![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: d00e0984ca
ℹ️ 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".
| namespaces: | ||
| - gateway-conformance-infra | ||
| - gateway-preserve-case-backend | ||
| - gateway-upgrade-infra | ||
| - listenerset-tls-termination-secret |
There was a problem hiding this comment.
This watch.namespaces list omits envoy-gateway-system, but in Namespaces watch mode Envoy Gateway constrains its cache to exactly the listed namespaces (internal/provider/kubernetes/kubernetes.go:197-201). The infra manager still reads/manages control-plane resources in the controller namespace (internal/infrastructure/kubernetes/infra.go:71-73), and ratelimit reconciliation specifically does GetUID for envoy-gateway Service/Deployment/ServiceAccount there (internal/infrastructure/kubernetes/ratelimit_infra.go:26-47), so the new watch-namespaces profile can fail reconciliation unless envoy-gateway-system is included; the same omission appears in test/config/helm/watch-namespaces.yaml.
Useful? React with 👍 / 👎.
zhaohuabing
force-pushed
the
watch-namespace-e2e
branch
from
d00e098 to
8da04fe
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8786 +/- ##
=======================================
Coverage 74.35% 74.35%
=======================================
Files 245 245
Lines 38847 38847
=======================================
Hits 28883 28883
Misses 7963 7963
Partials 2001 2001 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
zhaohuabing
force-pushed
the
watch-namespace-e2e
branch
from
8da04fe to
f33a03f
Compare
Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8dbbebff2f
ℹ️ 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".
| @@ -0,0 +1 @@ | |||
| # the watch-namespaces configuration is applied via test/config/envoy-gateaway-config/watch-namespaces.yaml | |||
There was a problem hiding this comment.
watch-namespaces is added to conformance matrices, but this profile's Helm values file is effectively empty, so those jobs still deploy Envoy Gateway with default settings instead of provider.kubernetes.watch.type=Namespaces. In our make targets, conformance paths (conformance/experimental-conformance) only consume test/config/helm/<profile>.yaml during kube-deploy and do not apply test/config/envoy-gateaway-config/<profile>.yaml, so these new matrix entries do not exercise watched-namespace mode and can miss regressions they are intended to catch.
Useful? React with 👍 / 👎.
1 participant
This PR adds an e2e test for "watched namespaces" are specified.
The test verifies that EG correctly reconciles resources in theenvoy-gateway-systemnamespace even when that namespace is not explicitly included in the watched namespaces.It turns out
envoy-gateway-systemnamespace needs to be explicitly configured in the watched namespaces in the current controller logic. I'm not sure if this is a bug or intended behavior, we can address this later if we want to implicitly includeenvoy-gateway-systemnamespace.This test is required to ensure that the #8764 doesn't break the "watched namespaces" mode.