Skip to content
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.

Sign up for GitHub

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: Add self config source type. #8201

Merged
merged 1 commit into from
Sep 20, 2019
Merged

xds: Add self config source type. #8201

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions api/envoy/api/v2/core/config_source.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,13 @@ message ApiConfigSource {
message AggregatedConfigSource {
}

// [#not-implemented-hide:]
// Self-referencing config source options. This is currently empty, but when
// set in :ref:`ConfigSource <envoy_api_msg_core.ConfigSource>` can be used to
// specify that other data can be obtained from the same server.
message SelfConfigSource {
}

// Rate Limit settings to be applied for discovery requests made by Envoy.
message RateLimitSettings {
// Maximum number of tokens to be used for rate limiting discovery request calls. If not set, a
Expand All @@ -93,6 +100,7 @@ message RateLimitSettings {
// <arch_overview_service_discovery>` etc. may either be sourced from the
// filesystem or from an xDS API source. Filesystem configs are watched with
// inotify for updates.
// [#comment:next free field: 6]
message ConfigSource {
oneof config_source_specifier {
option (validate.required) = true;
Expand All @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

// ConfigSource from, although not necessarily from the same stream. This is similar to the
// :ref:`ads<envoy_api_field.ConfigSource.ads>` field, except that the client may use a
// different stream to the same server. As a result, this field can be used for things
// like LRS that cannot be sent on an ADS stream. It can also be used to link from (e.g.)
// LDS to RDS on the same server without requiring the management server to know its name
// or required credentials.
// [#next-major-version: In xDS v3, consider replacing the ads field with this one, since
// this field can implicitly mean to use the same stream in the case where the ConfigSource
// is provided via ADS and the specified data can also be obtained via ADS.]
SelfConfigSource self = 5;
}

// When this timeout is specified, Envoy will wait no longer than the specified time for first
Expand Down
1 change: 1 addition & 0 deletions tools/spelling_dictionary.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ LDS
LEV
LHS
LF
LRS
MB
MD
MGET
Expand Down