Skip to content

Conversation

@ejona86
Copy link
Member

@ejona86 ejona86 commented Sep 15, 2025

No longer need to hard-code pick_first because of gRFC A61.
grpc/proposal#477


This is still a WIP but I wanted to start the review as the remaining work is mostly tests at this point. (Yes, I there is one small TODO still in the main code.)

This leaves ClusterResolverLb in-place. It'd be a follow-up to combine it with CdsLb. The tests include xdsDepManager instead of faking the XdsConfig object, which should make that PR easier, as well as many refactorings.

I'm almost to the logical dns tests, which will be the point XdsDependencyManager.enableLogicalDns will flip to true, as I can properly test it (and it is required).

@kannanjgithub
Copy link
Contributor

A61 says there will be a single Endpoint comprised of all the logical DNS addresses but we are still creating one endpoint for each address.

@ejona86 ejona86 force-pushed the clusterresolver-to-xdsdepman branch from 1233254 to fc652ec Compare September 17, 2025 16:42
@ejona86
Copy link
Member Author

ejona86 commented Sep 17, 2025

Rebased past #12202, just fixing XdsAttributes.ATTR_LOCALITY_NAME -> EquivalentAddressGroup.ATTR_LOCALITY_NAME

Copy link
Contributor

@kannanjgithub kannanjgithub left a comment

Choose a reason for hiding this comment

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

Merge conflict is there.

No longer need to hard-code pick_first because of gRFC A61.
grpc/proposal#477
Yes, that was a bug discovered in OutlierDetection
(EnvoyServerProtoData). And yes, now XdsDepManager combines EAGs
into a single endpoint.
@ejona86 ejona86 force-pushed the clusterresolver-to-xdsdepman branch from fc652ec to 029d825 Compare September 18, 2025 17:13
@ejona86 ejona86 merged commit 0c179e3 into grpc:master Sep 18, 2025
17 checks passed
@ejona86 ejona86 deleted the clusterresolver-to-xdsdepman branch September 18, 2025 18:08
@ejona86
Copy link
Member Author

ejona86 commented Sep 19, 2025

@shivaspeaks, you should be unblocked for the XdsClient changes.

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.

2 participants