internal/envoy: enable Envoy ADS support #1864
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stevesloka
reviewed
Nov 8, 2019
cmd/contour/bootstrap.go
Outdated
| @@ -37,6 +37,7 @@ func registerBootstrap(app *kingpin.Application) (*kingpin.CmdClause, *bootstrap | |||
| bootstrap.Flag("envoy-cert-file", "gRPC Client cert filename for Envoy to load").Envar("ENVOY_CERT_FILE").StringVar(&ctx.config.GrpcClientCert) | |||
| bootstrap.Flag("envoy-key-file", "gRPC Client key filename for Envoy to load").Envar("ENVOY_KEY_FILE").StringVar(&ctx.config.GrpcClientKey) | |||
| bootstrap.Flag("namespace", "The namespace the Envoy container will run in").Envar("CONTOUR_NAMESPACE").Default("projectcontour").StringVar(&ctx.config.Namespace) | |||
| bootstrap.Flag("enable-ads", "Enable the Envoy Aggregated Discovery Service").BoolVar(&ctx.config.EnableADS) | |||
There was a problem hiding this comment.
Is there a reason to not just turn this on and not allow for a flag?
There was a problem hiding this comment.
Not especially, just risk mitigation. I'd be OK with removing it, or flipping the default. I'd slightly prefer the latter, since users could flip ADS off if something horrible happens :)
There was a problem hiding this comment.
Flipped this so it is enabled by default.
jpeach
force-pushed
the
add-ads-support
branch
from
31803b3 to
64734eb
Compare
jpeach
changed the title
jpeach
force-pushed
the
add-ads-support
branch
from
64734eb to
52bd5c6
Compare
|
@davecheney Can you please take a look? |
|
Thanks for working on this. I’d prefer to look at this after 1.1 is out.
… On 19 Nov 2019, at 03:35, James Peach ***@***.***> wrote:
@davecheney Can you please take a look?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
jpeach
force-pushed
the
add-ads-support
branch
from
52bd5c6 to
ee6af43
Compare
jpeach
force-pushed
the
add-ads-support
branch
from
ee6af43 to
469f89e
Compare
Since the Contour GRPC server already multiplexes Envoy requests using the resource type URL, we can support ADS simply by enabling it in the bootstrap configuration. This fixes projectcontour#1178. This fixes projectcontour#1286. Signed-off-by: James Peach <[email protected]>
jpeach
force-pushed
the
add-ads-support
branch
from
469f89e to
71b224a
Compare
|
Updated with a nonce de-synchronization fix. |
|
This needs a lot more work, based on my improved understanding of ADS. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Since the Contour GRPC server already multiplexes Envoy requests
using the resource type URL, we can support ADS simply by enabling
it in the bootstrap configuration.
Signed-off-by: James Peach [email protected]