Skip to content

xds: Add IsDynamic field to cdsbalancer LB and make it true for RLS#8793

Merged
eshitachandwani merged 3 commits into
grpc:masterfrom
eshitachandwani:rls_change
Jan 6, 2026
Merged

xds: Add IsDynamic field to cdsbalancer LB and make it true for RLS#8793
eshitachandwani merged 3 commits into
grpc:masterfrom
eshitachandwani:rls_change

Conversation

@eshitachandwani

@eshitachandwani eshitachandwani commented Dec 26, 2025

Copy link
Copy Markdown
Member

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

codecov Bot commented Dec 26, 2025

Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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.

@easwars easwars Jan 6, 2026

Copy link
Copy Markdown
Contributor

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

Comment thread internal/xds/balancer/cdsbalancer/cdsbalancer_test.go Outdated
Comment thread internal/xds/clusterspecifier/rls/rls_test.go Outdated
Comment thread internal/xds/clusterspecifier/rls/rls_test.go Outdated
@easwars easwars assigned eshitachandwani and unassigned easwars and arjan-bal Jan 6, 2026
Comment thread internal/xds/clusterspecifier/rls/rls_test.go Outdated
@easwars

easwars commented Jan 6, 2026

Copy link
Copy Markdown
Contributor

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
mbissa pushed a commit to mbissa/grpc-go that referenced this pull request Feb 16, 2026
…grpc#8793)

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
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