Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9896 +/- ##
==========================================
+ Coverage 81.33% 81.34% +0.01%
==========================================
Files 264 264
Lines 40967 41002 +35
==========================================
+ Hits 33320 33353 +33
- Misses 7646 7648 +2
Partials 1 1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@codex review |
|
/retest |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: acb606f410
ℹ️ 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".
acb606f to
1502c88
Compare
Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
1502c88 to
2454452
Compare
Signed-off-by: kkk777-7 <kota.kimura0725@gmail.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5381b1907f
ℹ️ 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".
| setIfNil(&route.LoadBalancer, tf.LoadBalancer), | ||
| setIfNil(&route.DNS, tf.DNS), | ||
| } | ||
| hasApplicableRoute = hasApplicableRoute || slices.Contains(offered, true) |
There was a problem hiding this comment.
Count merged backend-cluster settings as applicable
When mergeBackends is enabled, a whole-Gateway BackendTrafficPolicy containing a cluster-scoped setting such as circuitBreaker can be applied to a UDP route through x.BackendClusters: lines 1810-1817 copy tf.ClusterTrafficFeatures into every merged cluster, and the merged-cluster translator consumes those settings. However, the UDP applicability check only considers load balancing and DNS, so this scenario leaves hasApplicableRoute false and reports NoAttachedRoutes even though the policy is present in the generated cluster configuration. Include settings applied through the merged backend-cluster path when determining applicability.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This comment is right. but in my opinion, merge backend behavior is uncorrect.
Only features appropriate for merge backend of each protocol should be applied.
This is different issue, so I'd like to raise another issue and handle this in different PR.
What this PR does / why we need it:
Currently, all policy status report
Accepted=trueeven when translator doesn't generate IR config.This case happens when policy attaches some Gateway or ListenerSet and those listeners aren't attached with xRoutes.
Fixed: emit
NoAttachedRoutesReason withWarningcondition.Which issue(s) this PR fixes:
Fixes #9837
PR Checklist
git commit -s). See DCO: Sign your work./api), the API was discussed and agreed before the implementation. The API change can be in a separate PR, or in the same PR, but the API must be agreed before implementation. N/A if this PR does not contain API changes.make generate gen-check,make lint, and the unit-test/coverage build pass. (Flaky e2e failures are not considered breakages, butgen-check,lint, and coverage MUST pass.)release-notes/current/<section>/<pr-number>-<slug>.md(seerelease-notes/current/README.mdfor sections and naming). N/A if this PR does not contain non-trivial changes.make gen-checkand committed the result if API/helm charts/modules changed.release-notes/current/breaking_changes/.