xds: Add self config source type. #8201
Conversation
Signed-off-by: Mark D. Roth <[email protected]>
| @@ -113,6 +121,18 @@ message ConfigSource { | |||
| // When set, ADS will be used to fetch resources. The ADS API configuration | |||
| // source in the bootstrap configuration is used. | |||
| AggregatedConfigSource ads = 3; | |||
| // [#not-implemented-hide:] | |||
| // When set, the client will access the resources from the same server it got the | |||
There was a problem hiding this comment.
I think we need to have some more discussion on this approach. Specifically, if the idea is really to make things like LRS sticky, it's arguably not the most practical approach, since it provides little guarantee. After speaking to Vishal about this, I think a better approach might be credential indirection, so let's discuss this next week.
There was a problem hiding this comment.
What kind of guarantee are you concerned about not having here?
I think that even if we decide on a different approach for LRS, this would still be useful for cases where a management server is not using ADS but wants to redirect clients to itself when linking from LDS to RDS or from CDS to EDS. But I don't have a concrete use-case that needs that right now, so that is a bit speculative.
In any case, I'm happy to discuss this further when you get back.
There was a problem hiding this comment.
Let's just discuss in the meeting we already have scheduled for next week? While I don't have any inherent objection to the feature I am a bit concerned about implementation complexity.
There was a problem hiding this comment.
Yeah, let's talk about this when we meet next week. I had originally scheduled the meeting about the LRS-in-CDS issue, but we now have enough open discussions that I figure we can just use that meeting to talk about all of them (or at least as many as we can fit into the available time).
|
Just to record the current state of this: I discussed this with @htuch and @mattklein123 yesterday, and Harvey said he'd look into how much work it would be to implement this in Envoy. |
There was a problem hiding this comment.
This is reasonable I think. I had a look at the Envoy implementation and have the following observations:
- The resources we need to think about that feature
ConfigSourceare the bootstrap, cluster, HTTP connection manager, SDS, TAP and VHDS/SRDS. - Bootstrap needs to treat this as effectively invalid as a setting.
- When the parent
ConfigSourceispath, this also should be invalid. - We probably need to thread around the parent
ConfigSourcein factory contexts, latch it in various places such as theListener. It's not going to be too hard, but it is some non-zero work that needs someone to own if Envoy is going to support this. @markdroth I'd ask that we schedule this internally as per previous conversations and not take for granted that this support will appear once it is in the API.
With these caveats, I'm supportive of the PR; @mattklein123 for final comment and merge.
Signed-off-by: Mark D. Roth <[email protected]>
Signed-off-by: Mark D. Roth <[email protected]>
Signed-off-by: Mark D. Roth <[email protected]>
Description: Add self config source type to xDS.
Risk Level: Low
Testing: None
Docs Changes: Inline with API protos
Release Notes: N/A
Based on discussion in #7953.
cc @mattklein123 @htuch @vishalpowar