Skip to content

config: implement incremental CDS in Envoy#6191

Merged
htuch merged 1 commit intoenvoyproxy:masterfrom
fredlas:INC_incremental_CDS
Mar 7, 2019
Merged

config: implement incremental CDS in Envoy#6191
htuch merged 1 commit intoenvoyproxy:masterfrom
fredlas:INC_incremental_CDS

Conversation

@fredlas
Copy link
Copy Markdown
Contributor

@fredlas fredlas commented Mar 6, 2019

Signed-off-by: Fred Douglas fredlas@google.com

Description: Code for Envoy to speak delta CDS with a management server. DELTA_GRPC added to config_source.proto's ApiTypes, to allow bootstrap configs to ask for incremental xDS.

Part of #4991. Was #5466; giving up on broken DCO craziness.

Risk Level: medium
Testing: new integration test

Signed-off-by: Fred Douglas <fredlas@google.com>
Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Fantastic, thanks @fredlas for taking this from WiP to production ready, excited to see this land!

@htuch htuch merged commit 8116b6d into envoyproxy:master Mar 7, 2019
@fredlas fredlas deleted the INC_incremental_CDS branch March 11, 2019 17:40

rpc IncrementalRoutes(stream IncrementalDiscoveryRequest)
returns (stream IncrementalDiscoveryResponse) {
rpc DeltaRoutes(stream DeltaDiscoveryRequest) returns (stream DeltaDiscoveryResponse) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This method name change is not wire compatible (it changes the URL), thus is against Breaking change policy.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'd have to hear from a more experienced Envoy person to be sure, but I think this one is an exception because we are sure that both sides of this gRPC service are completely unimplemented until now.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, incremental xDS is entirely experimental, so this is legit.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this indicated in the proto file somewhere, or is there some other way we should be able to know what is experimental and what is considered "frozen"?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In this specific case, it's well known in the Envoy/xDS community that this feature is highly experimental; there is no client or server in the wild that implement this as it's a moving target. You can track this work at #4991.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants