xds/googlec2p: support custom bootstrap config per channel. #8648

Pranjali-2501 · 2025-10-14T10:45:46Z

This PR refactors the xdsclient pool and googlec2p resolver logic to support channel-specific bootstrap configuration, enabling use cases where applications need to create xDS clients with different configs.

Prior to this change, c2p resolver used SetFallbackBootstrapConfig to inject the directpath-specific bootstrap config, which became the singleton config for all xdsclients. This prevented users from running multiple xDS clients with distinct configs.

Key Changes:

The xdsclient pool now accepts an optional bootstrap configuration when creating a client.
When a client is requested for a target URI:
- If an xdsclient instance for this URI already exists, it is reused.
- If not, a new xdsclient is created:
  - If a bootstrap config is provided, it is used.
  - Otherwise, default config loading (env vars, file path) is used.
The logic to check for and reuse existing clients for a given URI is performed before loading configuration.
The c2p resolver is updated to stop using the global SetFallbackBootstrapConfig. Instead, it creates and retains an xdsclient for its channel target, supplying the required bootstrap config directly.
The c2p resolver keeps its client until the c2p resolver is Closed.

RELEASE NOTES:

xds/googlec2p: Fix channel-specific xDS bootstrap configurations by allowing xdsclient creation with per-target config. Removes global fallback config usage, enabling multiple distinct xDS clients to coexist in the same process.

codecov · 2025-10-14T10:49:24Z

Codecov Report

❌ Patch coverage is 75.00000% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.91%. Comparing base (b4000c8) to head (6f4b8df).
⚠️ Report is 28 commits behind head on master.

Files with missing lines	Patch %	Lines
xds/googledirectpath/googlec2p.go	65.00%	5 Missing and 2 partials ⚠️
internal/xds/xdsclient/pool.go	85.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8648      +/-   ##
==========================================
- Coverage   82.17%   81.91%   -0.26%     
==========================================
  Files         415      417       +2     
  Lines       40697    40815     +118     
==========================================
- Hits        33442    33433       -9     
- Misses       5874     6012     +138     
+ Partials     1381     1370      -11

Files with missing lines	Coverage Δ
internal/xds/xdsclient/pool.go	`81.29% <85.00%> (+5.92%)`	⬆️
xds/googledirectpath/googlec2p.go	`85.96% <65.00%> (-4.76%)`	⬇️

... and 64 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

easwars

Don't we need a test to also verify that we can actually create multiple xDS clients with different configs from the same pool? That functionality seems to be the core of this PR.

internal/xds/xdsclient/pool.go

xds/googledirectpath/googlec2p.go

easwars · 2025-10-15T19:01:28Z

xds/googledirectpath/googlec2p.go


+type c2pResolverWrapper struct {
+	resolver.Resolver
+	cancel func()


Please add a trailing comment to this line saying this cancel func is used to release the ref to the xDS client that we created in Build

I intended for the comment to be here, where the field is defined and not where it is used in Close.

xds/googledirectpath/googlec2p_test.go

Pranjali-2501 · 2025-10-17T13:58:43Z

Don't we need a test to also verify that we can actually create multiple xDS clients with different configs from the same pool? That functionality seems to be the core of this PR.

Yes, we do. I have added a test.

easwars

LGTM, modulo minor comments.

easwars · 2025-10-17T16:50:02Z

internal/xds/xdsclient/pool.go

 	if cfg != nil {
 		return cfg
 	}
+	// TODO(i/8661)


Sorry if I wasn't clear in my previous comment. We don't need a TODO on every line where this field is referenced. Just one TODO where the field is defined should be good enough.

Made the changes.

easwars · 2025-10-17T16:51:25Z

xds/googledirectpath/googlec2p.go


+type c2pResolverWrapper struct {
+	resolver.Resolver
+	cancel func()


I intended for the comment to be here, where the field is defined and not where it is used in Close.

easwars · 2025-10-17T16:58:04Z

xds/googledirectpath/googlec2p_test.go

+	defer func() { xdsClientPool = oldXdsClientPool }()
+
+	// Define bootstrap config for c2p resolver
+	c2pConfig := bootstrapConfig(t, bootstrap.ConfigOptionsForTesting{


Please move this definition to be right above the place where it is first used.

easwars · 2025-10-17T16:59:06Z

xds/googledirectpath/googlec2p_test.go


+// verifyXDSClientBootstrapConfig checks that an xDS client with the given name
+// exists in the pool and that its bootstrap config matches the expected config.
+func verifyXDSClientBootstrapConfig(t *testing.T, pool *xdsclient.Pool, name string, wantConfig *bootstrap.Config) xdsclient.XDSClient {


Let's not return the xds client from this function. The only place the returned value is used in the last test. See comment there as well.

easwars · 2025-10-17T16:59:56Z

xds/googledirectpath/googlec2p_test.go

+	// Verify that the two clients are distinct instances.
+	if clientC2P == clientGeneric {
+		t.Fatal("Expected two distinct xDS clients, but got same")
+	}


I don't think this is a very useful check. As long as we have verified that the two clients have their expected configs, we should be good.

Removed this check.

add custom bootstrap config

4265433

Pranjali-2501 added this to the 1.77 Release milestone Oct 14, 2025

Pranjali-2501 added the Type: Feature New features or improvements in behavior label Oct 14, 2025

Pranjali-2501 self-assigned this Oct 14, 2025

Pranjali-2501 requested review from dfawley and easwars October 14, 2025 10:52

Pranjali-2501 assigned easwars and dfawley and unassigned Pranjali-2501 Oct 14, 2025

Pranjali-2501 requested a review from arjan-bal October 15, 2025 05:31

Pranjali-2501 assigned arjan-bal Oct 15, 2025

dfawley removed their assignment Oct 15, 2025

dfawley removed their request for review October 15, 2025 16:16

easwars reviewed Oct 15, 2025

View reviewed changes

easwars assigned Pranjali-2501 and unassigned easwars and arjan-bal Oct 15, 2025

easwars added the Area: xDS Includes everything xDS related, including LB policies used with xDS. label Oct 15, 2025

added new test

c1311ba

Pranjali-2501 mentioned this pull request Oct 17, 2025

xds/googlec2p: cleanup in googlec2p resolver. #8661

Open

added todo

85e62a8

Pranjali-2501 assigned easwars and unassigned Pranjali-2501 Oct 17, 2025

Pranjali-2501 requested a review from easwars October 17, 2025 13:59

easwars approved these changes Oct 17, 2025

View reviewed changes

easwars assigned Pranjali-2501 and unassigned easwars Oct 17, 2025

resolving comments

6f4b8df

xds/googlec2p: support custom bootstrap config per channel. #8648

Are you sure you want to change the base?

xds/googlec2p: support custom bootstrap config per channel. #8648

Uh oh!

Conversation

Pranjali-2501 commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Oct 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

easwars left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Pranjali-2501 commented Oct 17, 2025

Uh oh!

easwars left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Pranjali-2501 commented Oct 14, 2025 •

edited

Loading

codecov bot commented Oct 14, 2025 •

edited

Loading