-
Notifications
You must be signed in to change notification settings - Fork 4.6k
xds: make fallback bootstrap configuration per-process #7401
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
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/bootstrapNewConfigwithGetConfiguration. The latter retrieves bootstrap configuration from the ordered list of sources.NewConfigForTestingto create bootstrap configuration for testing purposes.ConfigOptionsForTestingstruct, replaced theNodeIDfield with aNodefield that supports node information specified as JSON. This fixes an earlier shortsightedness.xds/googledirectpathmap[string]json.RawMessagefollowed by a call tojson.Marshalinstead of creating it as a[]byte.xds/internal/xdsclientxdsclient.Newandxdsclient.NewForTesting#a71-xds-fallback
Fixes #7346
RELEASE NOTES: none