xds: introduce generic xds clients xDS and LRS Client API signatures #8042

purnesh42H · 2025-01-27T18:48:06Z

This is the next part of generic xds clients to be usable outside of grpc which builds on top of #8024. This change is adding the user API signatures for xDS and LRS client (without implementation) to communicate with xDS management server.

POC
Internal Design

RELEASE NOTES: None

codecov · 2025-01-27T18:52:53Z

Codecov Report

Attention: Patch coverage is 54.09836% with 28 lines in your changes missing coverage. Please review.

Project coverage is 82.36%. Comparing base (7505bf2) to head (b6ac539).
Report is 7 commits behind head on master.

Files with missing lines	Patch %	Lines
xds/internal/clients/lrsclient/load_store.go	0.00%	12 Missing ⚠️
xds/internal/clients/xdsclient/xdsclient.go	0.00%	8 Missing ⚠️
xds/internal/clients/lrsclient/lrsclient.go	0.00%	4 Missing ⚠️
xds/internal/clients/internal/internal.go	88.88%	3 Missing ⚠️
...s/internal/clients/grpctransport/grpc_transport.go	90.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8042      +/-   ##
==========================================
+ Coverage   82.32%   82.36%   +0.03%     
==========================================
  Files         388      392       +4     
  Lines       39016    39132     +116     
==========================================
+ Hits        32121    32232     +111     
- Misses       5574     5585      +11     
+ Partials     1321     1315       -6

Files with missing lines	Coverage Δ
...s/internal/clients/grpctransport/grpc_transport.go	`84.90% <90.00%> (ø)`
xds/internal/clients/internal/internal.go	`88.88% <88.88%> (ø)`
xds/internal/clients/lrsclient/lrsclient.go	`0.00% <0.00%> (ø)`
xds/internal/clients/xdsclient/xdsclient.go	`0.00% <0.00%> (ø)`
xds/internal/clients/lrsclient/load_store.go	`0.00% <0.00%> (ø)`

... and 27 files with indirect coverage changes

🚀 New features to boost your workflow:

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

xds/internal/clients/xdsclient/client.go

xds/internal/clients/config.go

xds/internal/clients/lrsclient/client.go

purnesh42H · 2025-02-04T11:07:57Z

@dfawley i did the godoc review and made following changes (see the last commit)

removed the helper methods on struct that are not being used to avoid exporting them. we can add later during implementation.
removed square brackets from docstrings because actual param and struct field already has the link to respective object definition.

xds/internal/clients/xdsclient/config.go

xds/internal/clients/xdsclient/resource_watcher.go

dfawley · 2025-02-06T18:14:30Z

xds/internal/clients/xdsclient/config.go

+	// Servers specifies a list of xDS servers to connect to. This field should
+	// be used only for old-style names without an authority.


Let's leave "old" and "style" out of the docstrings and describe what it is or what it is used for in specific terms instead.

I have modified the documentation. One more thing i have added in both Authority and here about order of servers in the list for precedence. Let me know if thats okay. I remember there was a question recently about how the fallback servers are chosen. Should we mention the gRFC as well?

xds/internal/clients/lrsclient/config.go

xds/internal/clients/xdsclient/resource_type.go

purnesh42H · 2025-03-03T05:07:47Z

@dfawley I have moved the config helpers and test to internal package because 1) we don't want to expose them 2) we don't want user to have proto dependencies.

dfawley

This seems basically done, just some very minor things left.

dfawley · 2025-03-03T23:26:39Z

xds/internal/clients/internal/clientsutils/clientsutils.go

+
+// IsServerIdentifierEqual returns true if clients.ServerIdentifier si1 and si2
+// are considered equal.
+func IsServerIdentifierEqual(si1, si2 *clients.ServerIdentifier) bool {


Is xdsclient using this? If so, how? I was thinking it would just be the transport implementation that needs something like it?

So, it will be used to de-duplicate the transport channel. Yes, the transport will need it. Also, when de-duplicating xDS channel the xDS ServerConfig equality check will call it as well.

So then only our transport implementation can use it if it's in internal. Maybe let's delete for now and add when needed and we can talk about how it should work and where it should go.

Because what we really need is ServerIdentifiers as map keys. But that's not directly possible, unless the any is compatible, perhaps?

Think its the same discussion #8042 (comment). Basically, it will be needed in xDS client implementation as well for server config because ServerIdentifier is embedded in that.

dfawley · 2025-03-03T23:29:25Z

xds/internal/clients/grpctransport/grpc_transport.go

 // such as an xDS management or an LRS server.
-type ServerConfigExtension struct {
+type ServerIdentifierExtension struct {


We don't have an Equal method, but our own docs recommend one? Are we sure we actually need one? If only the transport would need it, then I don't think we do.

We will need it when de-duplicating the transport channel. That will come in final xDS client implementation PR. I was thinking of adding it at that time. Why do you think we won't need one?

Deduplicating will require it to be a map key, not to have an Equal method.

I don't think we can hash the complex fields like extensions right? So, we need a equal method to get the right key for the map and then lookup that key. Something like done here for xDS Server Config

grpc-go/xds/internal/xdsclient/authority.go

Line 531 in bffa4be

if a.activeXDSChannel != nil && a.activeXDSChannel.serverConfig.Equal(serverConfig) {

.

So, ServerConfig needs equal method and in turn we need to check equality for embedded ServerIdentifier. I don't think we can get away without equality unless every field has a String() method.

@easwars wdyt?

Discussed internally with @dfawley. For now, we can remove and see if we can find other ways to tackle comparing channels.

xds/internal/clients/internal/clientsutils/clientsutils.go

xds/internal/clients/lrsclient/load_store.go

xds/internal/clients/xdsclient/resource_type.go

…rpc#8042)

purnesh42H changed the title ~~Generic xds client 2 interface~~ xds: introduce xDS and LRS Client API signatures Jan 27, 2025

purnesh42H requested review from dfawley and easwars January 27, 2025 18:50

purnesh42H assigned dfawley and easwars Jan 27, 2025

purnesh42H changed the title ~~xds: introduce xDS and LRS Client API signatures~~ xds: introduce generic xds client xDS and LRS Client API signatures Jan 27, 2025

purnesh42H changed the title ~~xds: introduce generic xds client xDS and LRS Client API signatures~~ xds: introduce generic xds clients xDS and LRS Client API signatures Jan 27, 2025

purnesh42H added Area: xDS Includes everything xDS related, including LB policies used with xDS. Type: Feature New features or improvements in behavior labels Jan 27, 2025

purnesh42H added this to the 1.71 Release milestone Jan 27, 2025

purnesh42H force-pushed the generic-xds-client-2-interface branch 3 times, most recently from 042b236 to f8f7af5 Compare January 30, 2025 13:56

dfawley reviewed Jan 30, 2025

View reviewed changes

dfawley assigned purnesh42H and unassigned easwars and dfawley Jan 30, 2025

purnesh42H force-pushed the generic-xds-client-2-interface branch from f8f7af5 to 415b2ca Compare January 31, 2025 16:14

purnesh42H requested a review from dfawley February 3, 2025 07:56

purnesh42H assigned dfawley and unassigned purnesh42H Feb 3, 2025

easwars self-assigned this Feb 3, 2025

dfawley reviewed Feb 6, 2025

View reviewed changes

dfawley assigned purnesh42H and unassigned dfawley Feb 6, 2025

purnesh42H force-pushed the generic-xds-client-2-interface branch from 20e26f8 to c3e96f7 Compare February 7, 2025 10:14

purnesh42H assigned markdroth and dfawley Feb 7, 2025

purnesh42H force-pushed the generic-xds-client-2-interface branch from ff89334 to 9b003bc Compare February 28, 2025 11:09

rebase over grpctransport changes

0d234e0

purnesh42H force-pushed the generic-xds-client-2-interface branch from 9b003bc to 0d234e0 Compare March 1, 2025 21:24

Move clients config helpers to clients/internal

0608187

purnesh42H force-pushed the generic-xds-client-2-interface branch 4 times, most recently from a25d436 to 7cbeb43 Compare March 3, 2025 17:39

remove ReportLoadTimeout from lrs config

833a19b

purnesh42H force-pushed the generic-xds-client-2-interface branch from 7cbeb43 to 833a19b Compare March 3, 2025 18:53

dfawley reviewed Mar 3, 2025

View reviewed changes

dfawley assigned purnesh42H and unassigned dfawley Mar 3, 2025

doug final nits

60166b7

purnesh42H force-pushed the generic-xds-client-2-interface branch from 1f74716 to 60166b7 Compare March 4, 2025 11:51

purnesh42H requested a review from dfawley March 4, 2025 11:51

purnesh42H assigned dfawley and unassigned easwars Mar 4, 2025

dfawley removed their assignment Mar 4, 2025

purnesh42H assigned dfawley and unassigned purnesh42H Mar 5, 2025

remove ServerIdentifierEqual

b6ac539

purnesh42H force-pushed the generic-xds-client-2-interface branch from 4f55bfc to b6ac539 Compare March 5, 2025 17:51

dfawley approved these changes Mar 5, 2025

View reviewed changes

dfawley assigned purnesh42H and unassigned dfawley Mar 5, 2025

purnesh42H merged commit af07815 into grpc:master Mar 6, 2025
15 checks passed

purnesh42H mentioned this pull request Mar 6, 2025

xds: generic xDS client transport channel and ads stream implementation #8144

Merged

janardhanvissa pushed a commit to janardhanvissa/grpc-go that referenced this pull request Mar 24, 2025

xds: introduce generic xds clients xDS and LRS Client API signatures (g…

bcaf6d8

…rpc#8042)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

xds: introduce generic xds clients xDS and LRS Client API signatures #8042

xds: introduce generic xds clients xDS and LRS Client API signatures #8042

purnesh42H commented Jan 27, 2025 •

edited

Loading

codecov bot commented Jan 27, 2025 •

edited

Loading

purnesh42H commented Feb 4, 2025 •

edited

Loading

dfawley Feb 6, 2025

purnesh42H Feb 7, 2025

purnesh42H commented Mar 3, 2025

dfawley left a comment

dfawley Mar 3, 2025

purnesh42H Mar 4, 2025 •

edited

Loading

dfawley Mar 4, 2025

purnesh42H Mar 5, 2025

dfawley Mar 3, 2025

purnesh42H Mar 4, 2025 •

edited

Loading

dfawley Mar 4, 2025

purnesh42H Mar 5, 2025

purnesh42H Mar 5, 2025

purnesh42H Mar 5, 2025

		// Servers specifies a list of xDS servers to connect to. This field should
		// be used only for old-style names without an authority.

xds: introduce generic xds clients xDS and LRS Client API signatures #8042

xds: introduce generic xds clients xDS and LRS Client API signatures #8042

Conversation

purnesh42H commented Jan 27, 2025 • edited Loading

codecov bot commented Jan 27, 2025 • edited Loading

Codecov Report

purnesh42H commented Feb 4, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

purnesh42H commented Mar 3, 2025

dfawley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

purnesh42H Mar 4, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

purnesh42H Mar 4, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

purnesh42H commented Jan 27, 2025 •

edited

Loading

codecov bot commented Jan 27, 2025 •

edited

Loading

purnesh42H commented Feb 4, 2025 •

edited

Loading

purnesh42H Mar 4, 2025 •

edited

Loading

purnesh42H Mar 4, 2025 •

edited

Loading