Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

xds: make fallback bootstrap configuration per-process #7401

Merged
merged 3 commits into from
Jul 10, 2024

Conversation

easwars
Copy link
Contributor

@easwars easwars commented Jul 10, 2024

This PR makes fallback bootstrap configuration to be a per-process configuration. This means that it applies to all xDS clients created by the process when the bootstrap environment variables are unset. This switches us to the model used in C-core.

Summary of changes:

  • internal/xds/bootstrap
    • Added a package global variable to store the fallback bootstrap configuration.
    • Added methods to set and get fallback bootstrap configuration.
    • Replaced NewConfig with GetConfiguration. The latter retrieves bootstrap configuration from the ordered list of sources.
    • Added NewConfigForTesting to create bootstrap configuration for testing purposes.
    • In the ConfigOptionsForTesting struct, replaced the NodeID field with a Node field that supports node information specified as JSON. This fixes an earlier shortsightedness.
  • xds/googledirectpath
    • Switched to creating the bootstrap JSON as a map[string]json.RawMessage followed by a call to json.Marshal instead of creating it as a []byte.
    • Sets the global fallback bootstrap configuration.
    • No longer creates an xDS client.
  • xds/internal/xdsclient
    • The only APIs to create an xDS client now are xdsclient.New and xdsclient.NewForTesting

#a71-xds-fallback

Fixes #7346

RELEASE NOTES: none

@easwars easwars added the Type: Feature New features or improvements in behavior label Jul 10, 2024
@easwars easwars added this to the 1.66 Release milestone Jul 10, 2024
Copy link

codecov bot commented Jul 10, 2024

Codecov Report

Attention: Patch coverage is 85.50725% with 10 lines in your changes missing coverage. Please review.

Project coverage is 81.31%. Comparing base (45d44a7) to head (560eab3).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7401      +/-   ##
==========================================
- Coverage   81.47%   81.31%   -0.16%     
==========================================
  Files         348      348              
  Lines       26741    26752      +11     
==========================================
- Hits        21786    21753      -33     
- Misses       3772     3795      +23     
- Partials     1183     1204      +21     
Files Coverage Δ
internal/testutils/xds/e2e/bootstrap.go 64.00% <100.00%> (ø)
xds/internal/xdsclient/client_new.go 85.36% <100.00%> (-0.69%) ⬇️
xds/internal/xdsclient/client_refcounted.go 86.11% <100.00%> (+2.39%) ⬆️
xds/googledirectpath/googlec2p.go 92.45% <87.50%> (+4.95%) ⬆️
internal/xds/bootstrap/bootstrap.go 67.67% <80.55%> (+0.52%) ⬆️

... and 28 files with indirect coverage changes

return nil
}

// UnSetFallbackBootstrapConfigForTesting unsets the fallback bootstrap
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Unset?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

// getFallbackBootstrapConfig returns the fallback bootstrap configuration
// that will be used by the xDS client when the bootstrap environment
// variables are unset.
func getFallbackBootstrapConfig() *Config {
Copy link
Member

Choose a reason for hiding this comment

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

s/get//?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Had to also rename the var to make this possible,

@easwars easwars merged commit e54f441 into grpc:master Jul 10, 2024
12 of 13 checks passed
printchard pushed a commit to printchard/grpc-go that referenced this pull request Jul 11, 2024
printchard pushed a commit to printchard/grpc-go that referenced this pull request Jul 30, 2024
printchard pushed a commit to printchard/grpc-go that referenced this pull request Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New features or improvements in behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

xds: make the bootstrap configuration a singleton
2 participants