-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Conversation
Codecov ReportAttention: Patch coverage is
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
|
internal/xds/bootstrap/bootstrap.go
Outdated
return nil | ||
} | ||
|
||
// UnSetFallbackBootstrapConfigForTesting unsets the fallback bootstrap |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Unset?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
internal/xds/bootstrap/bootstrap.go
Outdated
// getFallbackBootstrapConfig returns the fallback bootstrap configuration | ||
// that will be used by the xDS client when the bootstrap environment | ||
// variables are unset. | ||
func getFallbackBootstrapConfig() *Config { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/get//?
There was a problem hiding this comment.
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,
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
NewConfig
withGetConfiguration
. The latter retrieves bootstrap configuration from the ordered list of sources.NewConfigForTesting
to create bootstrap configuration for testing purposes.ConfigOptionsForTesting
struct, replaced theNodeID
field with aNode
field that supports node information specified as JSON. This fixes an earlier shortsightedness.xds/googledirectpath
map[string]json.RawMessage
followed by a call tojson.Marshal
instead of creating it as a[]byte
.xds/internal/xdsclient
xdsclient.New
andxdsclient.NewForTesting
#a71-xds-fallback
Fixes #7346
RELEASE NOTES: none