-
Notifications
You must be signed in to change notification settings - Fork 10
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
Make consul dataplane handle bootstrap param response for V2 resources #242
Conversation
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.
See the question about whether or not the tenancy information is required now.
This is probably a future concern, but I imagined in a world or migration where V1 and V2 run in parallel, dataplane would need some way to indicate it wants a V1 or V2 style response if it's using the same, backwards-compatible endpoint. Is that something we should plan for?
NodeName *string `json:"nodeName,omitempty"` | ||
NodeID *string `json:"nodeID,omitempty"` |
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.
Are these needed with v2. All workload ids must be unique within a tenant so it seems like we may not need these any longer.
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.
These are not needed with V2, however, these flags can be used for both v1 and v2. My intention is to deprecate old flags and to have one set of generically named flags that could be used against either API.
78108a8
to
8b3322b
Compare
* Handle bootstrap_config param * Use new request and response params * Make flags/config more generic so that they can accomodate services or workloads and deprecate old flags/config.
8b3322b
to
1e3c558
Compare
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.
🎉
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.
2 x 🎉
…t exist and causes build errors. This points at the current HEAD of ishustava/v2-dp-bootstrap-params
Part of NET-5221