Skip to content

Conversation

@dfawley
Copy link
Member

@dfawley dfawley commented Mar 13, 2025

(To avoid merge conflicts, this also contains #8169 as the first commit.)

While fixing #8169, we noticed that LRS was also not being plumbed to LOGICAL_DNS clusters. This change adds support for it and an e2e test to ensure the LRS server is being used as intended.

RELEASE NOTES:

  • xds: fix support for load reporting in LOGICAL_DNS clusters
@dfawley dfawley added this to the 1.72 Release milestone Mar 13, 2025
@dfawley dfawley requested a review from purnesh42H March 13, 2025 21:15
@codecov
Copy link

codecov bot commented Mar 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.96%. Comparing base (ce2fded) to head (fea6ee2).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8170      +/-   ##
==========================================
- Coverage   82.16%   81.96%   -0.21%     
==========================================
  Files         410      410              
  Lines       40235    40237       +2     
==========================================
- Hits        33060    32979      -81     
- Misses       5824     5889      +65     
- Partials     1351     1369      +18     
Files with missing lines Coverage Δ
xds/internal/balancer/cdsbalancer/cdsbalancer.go 84.63% <100.00%> (+0.03%) ⬆️
...internal/balancer/clusterresolver/configbuilder.go 91.17% <100.00%> (+0.05%) ⬆️

... and 28 files with indirect coverage changes

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
Copy link
Contributor

@purnesh42H purnesh42H left a comment

Choose a reason for hiding this comment

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

LRS change LGTM. Have added comments in other PR

return host, uint32(port)
}

func (s) TestLRSLogicalDNS(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add docstring for the test

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@purnesh42H purnesh42H assigned dfawley and unassigned purnesh42H Mar 18, 2025
Copy link
Member Author

@dfawley dfawley left a comment

Choose a reason for hiding this comment

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

Rebased and added comment. Thanks!

return host, uint32(port)
}

func (s) TestLRSLogicalDNS(t *testing.T) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@dfawley dfawley merged commit d8924ac into grpc:master Mar 20, 2025
15 checks passed
@dfawley dfawley deleted the logical_dns_lrs branch March 20, 2025 19:55
janardhanvissa pushed a commit to janardhanvissa/grpc-go that referenced this pull request Mar 24, 2025
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

2 participants