-
Notifications
You must be signed in to change notification settings - Fork 5.1k
xds: apply node identifier optimization #7876
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
Signed-off-by: Kuat Yessenov <kuat@google.com>
/assign @htuch |
neither of and, API, shepherds can be assigned to this issue. |
Should this be an opt-in option to protect management servers that have incorrectly relied on node data being present in every request? |
Yes, of course. My current plan is to add a bool to config source in xDS. |
Signed-off-by: Kuat Yessenov <kuat@google.com>
Signed-off-by: Kuat Yessenov <kuat@google.com>
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.
Mostly LGTM, thanks.
/wait
Signed-off-by: Kuat Yessenov <kuat@google.com>
Any chance you can take a look at it @htuch? |
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.
LGTM modulo a few nits, thanks.
Signed-off-by: Kuat Yessenov <kuat@google.com>
This should be ready for review again. |
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.
Thanks!
Signed-off-by: Kuat Yessenov kuat@google.com
Omit the node identifier from subsequent discovery requests on the same stream.
Restricted to non-incremental xDS for tractability.
Risk Level: low, affects xDS protocol but guarded by an option
Testing: Unit/integration tests are updated
Docs Changes: xDS spec clarification
Release Notes: omit the node identifier from subsequent discovery requests
Fixes: #7860