Skip to content

feat: create single cluster per backend - #9459

Closed
zhaohuabing wants to merge 2 commits into
envoyproxy:mainfrom
zhaohuabing:merge-backends
Closed

feat: create single cluster per backend#9459
zhaohuabing wants to merge 2 commits into
envoyproxy:mainfrom
zhaohuabing:merge-backends

Conversation

@zhaohuabing

@zhaohuabing zhaohuabing commented Jul 9, 2026

Copy link
Copy Markdown
Member

What this PR does / why we need it:

By default, Envoy Gateway generates one Envoy cluster per route rule. When the same backend is referenced by many routes, this produces many identical clusters, which inflates the xDS config, multiplies active health-check traffic, fragments passive health-check status, de-optimizes upstream connection pooling, and increases stats cardinality.

This PR adds an experimental mergeBackends field to EnvoyProxy that enables cluster deduplication: routes referencing the same backend — identified by kind/namespace/name/port (plus resolved protocol and routing mode) — share a single Envoy cluster.

Two modes are supported:

  • BestEffort — merge a backend into a shared cluster only when it is safe. Routes carrying backend-cluster-scoped settings (e.g. a route-targeted BackendTrafficPolicy configuring healthCheck, circuitBreaker, loadBalancer, timeout, connection, etc.) or using features incompatible with weighted clusters (consistent-hash LB, session persistence) transparently fall back to a dedicated per-route cluster.
  • Force — always merge into a single shared cluster (currently behaves as BestEffort; not yet implemented).

When unset, the default one-cluster-per-route-rule behavior is preserved, so this is fully backward compatible.

Includes API + CRDs, translator/xDS changes, unit and golden-file tests, a user-facing task doc, and a release note.

Conformance verification: to guarantee that cluster deduplication does not change routing semantics, this PR adds a new merge-backends conformance test matrix entry to build_and_test.yaml. It runs the full Gateway API conformance suite (v1.36.1, experimental channel) with mergeBackends enabled, so the merge path is exercised end-to-end against the same spec assertions as the default configuration — proving the feature is transparent to conformance.

Which issue(s) this PR fixes:

Fixes #5307


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: If this PR contains API changes (changes under /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.
  • Required checks pass: make generate gen-check, make lint, and the unit-test/coverage build pass. (Flaky e2e failures are not considered breakages, but gen-check, lint, and coverage MUST pass.)
  • Tests added/updated: New/changed code is covered by appropriate tests. N/A if this PR does not contain code changes.
  • Docs: User-facing changes update the docs, either in this PR or a follow-up PR. N/A if this PR does not contain user-facing changes.
  • Release notes: For any non-trivial change, added a release-note fragment under release-notes/current/<section>/<pr-number>-<slug>.md (see release-notes/current/README.md for sections and naming). N/A if this PR does not contain non-trivial changes.
  • Generated files committed: Ran make gen-check and committed the result if API/helm charts/modules changed.
  • Scope & compatibility: The PR is reasonably scoped (no unrelated changes) and preserves backward compatibility, or any breaking change is called out above and documented in release-notes/current/breaking_changes/.
  • Codex review: Requested a Codex review and addressed all of its comments.
  • Copilot review: Requested a Copilot review and addressed all of its comments.

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
@zhaohuabing
zhaohuabing requested a review from a team as a code owner July 9, 2026 13:43
@netlify

netlify Bot commented Jul 9, 2026

Copy link
Copy Markdown

Deploy Preview for cerulean-figolla-1f9435 ready!

Name Link
🔨 Latest commit d1a9f8b
🔍 Latest deploy log https://app.netlify.com/projects/cerulean-figolla-1f9435/deploys/6a5052ac0666900008697131
😎 Deploy Preview https://deploy-preview-9459--cerulean-figolla-1f9435.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@zhaohuabing
zhaohuabing marked this pull request as draft July 9, 2026 13:43

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 59caf423d8

ℹ️ 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".

Comment thread internal/xds/translator/route.go
Comment thread internal/gatewayapi/merge_backend.go
Comment thread internal/gatewayapi/backendtrafficpolicy.go Outdated
Comment on lines +69 to +70
if d.RouteLevelClusterSettings {
return

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor Force when route policies add cluster settings

Even when the user sets mergeBackends: Force, the translator only receives a boolean and this path always returns for RouteLevelClusterSettings; that flag is set for route-targeted BackendTrafficPolicies with cluster-scoped settings. As a result Force behaves like BestEffort in the exact scenario the new API documents as forced merging, so users cannot enable the advertised mode.

Useful? React with 👍 / 👎.

@zhaohuabing zhaohuabing Jul 10, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To keep the scope of this PR limited, I plan to implement the force mode in a follow-up PR.

@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.26087% with 30 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.38%. Comparing base (688ade8) to head (d1a9f8b).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
internal/gatewayapi/backendtrafficpolicy.go 58.62% 6 Missing and 6 partials ⚠️
internal/gatewayapi/merge_backend.go 80.00% 6 Missing and 5 partials ⚠️
internal/xds/translator/cluster.go 50.00% 1 Missing and 1 partial ⚠️
internal/xds/translator/route.go 89.47% 1 Missing and 1 partial ⚠️
internal/xds/translator/translator.go 77.77% 1 Missing and 1 partial ⚠️
internal/gatewayapi/runner/runner.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9459      +/-   ##
==========================================
+ Coverage   75.35%   75.38%   +0.03%     
==========================================
  Files         252      253       +1     
  Lines       41492    41624     +132     
==========================================
+ Hits        31266    31379     +113     
- Misses       8113     8119       +6     
- Partials     2113     2126      +13     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com>
@zhaohuabing

Copy link
Copy Markdown
Member Author

/retest

muwaqar-cflt pushed a commit to muwaqar/gateway that referenced this pull request Jul 11, 2026
Runs the Gateway API conformance suite with EnvoyProxy.spec.mergeBackends
enabled by default, proving the cluster-deduplication optimization doesn't
alter conformance behavior. Ports the CI-wiring half of what PR envoyproxy#9459 (a
competing implementation of the same feature) built, adapted to our own
MergeBackendsConfig{Enabled, Mode} CRD shape rather than their bare
MergeBackendsMode pointer. No Go code changes: MergeBackends has no
user-observable behavior difference, and no existing e2e test inspects
Envoy's cluster count/names/config_dump, so no test-variant file is needed
the way xds-name-scheme-v2 required one.

Signed-off-by: Muhammad Waqar <waqar.hameed08@gmail.com>
muwaqar-cflt pushed a commit to muwaqar/gateway that referenced this pull request Jul 13, 2026
Runs the Gateway API conformance suite with EnvoyProxy.spec.mergeBackends
enabled by default, proving the cluster-deduplication optimization doesn't
alter conformance behavior. Ports the CI-wiring half of what PR envoyproxy#9459 (a
competing implementation of the same feature) built, adapted to our own
MergeBackendsConfig{Enabled, Mode} CRD shape rather than their bare
MergeBackendsMode pointer. No Go code changes: MergeBackends has no
user-observable behavior difference, and no existing e2e test inspects
Envoy's cluster count/names/config_dump, so no test-variant file is needed
the way xds-name-scheme-v2 required one.

Signed-off-by: Muhammad Waqar <waqar.hameed08@gmail.com>
muwaqar-cflt pushed a commit to muwaqar/gateway that referenced this pull request Jul 13, 2026
Found while comparing test coverage against the competing PR envoyproxy#9459, which
has its own TestIsMergeBackendsEnabled - we had none, only indirect
coverage via translator_test.go's fixture harness (which exercises the
function but doesn't specifically pin its precedence behavior). Add
direct coverage for GatewayClass-level vs. default-spec precedence,
including two cases the competing PR's version didn't cover: GatewayClass
EnvoyProxy explicitly disabling MergeBackends still wins over a
default spec that enables it, and a GatewayClass EnvoyProxy with no
MergeBackends field at all correctly falls back to the default spec.

Signed-off-by: Muhammad Waqar <waqar.hameed08@gmail.com>
@zhaohuabing

Copy link
Copy Markdown
Member Author

Closing this in favor of #9477

muwaqar-cflt pushed a commit to muwaqar/gateway that referenced this pull request Jul 19, 2026
Runs the Gateway API conformance suite with EnvoyProxy.spec.mergeBackends
enabled by default, proving the cluster-deduplication optimization doesn't
alter conformance behavior. Ports the CI-wiring half of what PR envoyproxy#9459 (a
competing implementation of the same feature) built, adapted to our own
MergeBackendsConfig{Enabled, Mode} CRD shape rather than their bare
MergeBackendsMode pointer. No Go code changes: MergeBackends has no
user-observable behavior difference, and no existing e2e test inspects
Envoy's cluster count/names/config_dump, so no test-variant file is needed
the way xds-name-scheme-v2 required one.

Signed-off-by: Muhammad Waqar <waqar.hameed08@gmail.com>
muwaqar-cflt pushed a commit to muwaqar/gateway that referenced this pull request Jul 19, 2026
Found while comparing test coverage against the competing PR envoyproxy#9459, which
has its own TestIsMergeBackendsEnabled - we had none, only indirect
coverage via translator_test.go's fixture harness (which exercises the
function but doesn't specifically pin its precedence behavior). Add
direct coverage for GatewayClass-level vs. default-spec precedence,
including two cases the competing PR's version didn't cover: GatewayClass
EnvoyProxy explicitly disabling MergeBackends still wins over a
default spec that enables it, and a GatewayClass EnvoyProxy with no
MergeBackends field at all correctly falls back to the default spec.

Signed-off-by: Muhammad Waqar <waqar.hameed08@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Revisit Kubernetes Services mapping to Envoy v3.Cluster

1 participant