Skip to content

Conversation

@eshitachandwani
Copy link
Member

@eshitachandwani eshitachandwani commented Dec 26, 2025

This is Part of A74 changes.

This PR add a new IsDynamic field to the CDS balancer LB Config. Also sets it to true for RLS cluster specifier plugin.

This PR also fixes the test in rls_test.go which earlier was returning if the test has wantError=true effectively not testing the cases after that.

This will be used to dynamically start watch for cluster specifier plugin clusters from cds balancer.

RELEASE NOTES: None

@eshitachandwani eshitachandwani added this to the 1.79 Release milestone Dec 26, 2025
@eshitachandwani eshitachandwani added Type: Internal Cleanup Refactors, etc Area: xDS Includes everything xDS related, including LB policies used with xDS. labels Dec 26, 2025
@eshitachandwani eshitachandwani changed the title xds: Add Is_Dynamic field to cdsbalancer LB and make it true for RLS config xds: Add Is_Dynamic field to cdsbalancer LB and make it true for RLS Dec 26, 2025
@codecov
Copy link

codecov bot commented Dec 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.35%. Comparing base (4046676) to head (9fa1d5c).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8793      +/-   ##
==========================================
- Coverage   83.42%   83.35%   -0.07%     
==========================================
  Files         418      418              
  Lines       32897    33004     +107     
==========================================
+ Hits        27443    27510      +67     
- Misses       4069     4092      +23     
- Partials     1385     1402      +17     
Files with missing lines Coverage Δ
internal/xds/balancer/cdsbalancer/cdsbalancer.go 85.32% <ø> (ø)
internal/xds/clusterspecifier/rls/rls.go 62.50% <100.00%> (ø)

... and 43 files with indirect coverage changes

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

type lbConfig struct {
serviceconfig.LoadBalancingConfig
ClusterName string `json:"Cluster"`
IsDynamic bool `json:"Is_Dynamic"`
Copy link
Contributor

Choose a reason for hiding this comment

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

We use camelCase (starting with a lower case) for the json annotations. See the outlier detection LB policy's config definition for an example:

Copy link
Member Author

Choose a reason for hiding this comment

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

Got it! Changed both. Since the Cluster field was capital , I got confused.

Copy link
Contributor

@easwars easwars Jan 6, 2026

Choose a reason for hiding this comment

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

Since the Cluster field was capital

Hmm ... that is weird.

Looks like the json package uses the tags in a case insensitive manner. See: https://pkg.go.dev/encoding/json

@easwars easwars assigned eshitachandwani and unassigned easwars and arjan-bal Jan 6, 2026
@easwars
Copy link
Contributor

easwars commented Jan 6, 2026

The PR title needs to be updated to not include the underscore.

@easwars easwars assigned eshitachandwani and unassigned easwars Jan 6, 2026
@eshitachandwani eshitachandwani changed the title xds: Add Is_Dynamic field to cdsbalancer LB and make it true for RLS xds: Add IsDynamic field to cdsbalancer LB and make it true for RLS Jan 6, 2026
@eshitachandwani eshitachandwani merged commit bccbb10 into grpc:master Jan 6, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: xDS Includes everything xDS related, including LB policies used with xDS. Type: Internal Cleanup Refactors, etc

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants