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.

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

Support Incremental xDS mode #9855

Merged
merged 26 commits into from
Apr 29, 2021
Merged

Support Incremental xDS mode #9855

merged 26 commits into from
Apr 29, 2021

Conversation

rboyer
Copy link
Member

@rboyer rboyer commented Mar 9, 2021

This adds support for the Incremental xDS protocol when using xDS v3. This is best reviewed commit-by-commit and will not be squashed when merged.

Union of all commit messages follows to give an overarching summary:

  • xds: exclusively support incremental xDS when using xDS v3

    • Attempts to use SoTW via v3 will fail, much like attempts to use incremental via v2 will fail.
    • Work around a strange older envoy behavior involving empty CDS responses over incremental xDS.
  • xds: various cleanups and refactors that don't strictly concern the addition of incremental xDS support

    • Dissolve the connectionInfo struct in favor of per-connection ResourceGenerators instead.
    • Do a better job of ensuring the xds code uses a well configured logger that accurately describes the connected client.
  • xds: pull out checkStreamACLs method in advance of a later commit

  • xds: rewrite SoTW xDS protocol tests to use protobufs rather than hand-rolled json strings

    • In the test we very lightly reuse some of the more boring protobuf construction helper code that is also technically under test. The important thing of the protocol tests is testing the protocol. The actual inputs and outputs are largely already handled by the xds golden output tests now so these protocol tests don't have to do double-duty.

    • This also updates the SoTW protocol test to exclusively use xDS v2 which is the only variant of SoTW that will be supported in Consul 1.10.

  • xds: default xds.Server.AuthCheckFrequency at use-time instead of construction-time

TODO

@github-actions github-actions bot added theme/cli Flags and documentation for the CLI interface theme/envoy/xds Related to Envoy support labels Mar 9, 2021
@vercel vercel bot temporarily deployed to Preview – consul March 9, 2021 20:38 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 9, 2021 20:38 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 9, 2021 20:38 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 9, 2021 20:38 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 9, 2021 20:50 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 9, 2021 20:50 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 9, 2021 21:03 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 9, 2021 21:03 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 15, 2021 21:00 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 15, 2021 21:00 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 16, 2021 20:30 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 16, 2021 20:30 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 16, 2021 20:34 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 16, 2021 20:34 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 16, 2021 20:55 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 16, 2021 20:55 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 16, 2021 21:03 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 16, 2021 21:03 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 16, 2021 21:13 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 16, 2021 21:13 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 16, 2021 21:28 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 16, 2021 21:28 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 17, 2021 14:50 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 17, 2021 14:50 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging March 24, 2021 15:39 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 24, 2021 15:39 Inactive
@vercel vercel bot temporarily deployed to Preview – consul March 24, 2021 19:55 Inactive
@vercel vercel bot temporarily deployed to Preview – consul April 26, 2021 21:00 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging April 26, 2021 21:00 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging April 28, 2021 22:18 Inactive
@vercel vercel bot temporarily deployed to Preview – consul April 28, 2021 22:18 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging April 29, 2021 15:44 Inactive
@vercel vercel bot temporarily deployed to Preview – consul April 29, 2021 15:44 Inactive
@rboyer rboyer requested a review from freddygv April 29, 2021 16:27
@@ -28,6 +29,8 @@ type supportedProxyFeatures struct {
// add version dependent feature flags here

GatewaysNeedStubClusterWhenEmptyWithIncrementalXDS bool
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be useful to also add comments here for why these are used

@vercel vercel bot temporarily deployed to Preview – consul-ui-staging April 29, 2021 18:49 Inactive
@vercel vercel bot temporarily deployed to Preview – consul April 29, 2021 18:49 Inactive
@rboyer rboyer merged commit 71d45a3 into master Apr 29, 2021
@rboyer rboyer deleted the xds-incr branch April 29, 2021 18:54
@hc-github-team-consul-core
Copy link
Contributor

🍒 If backport labels were added before merging, cherry-picking will start automatically.

To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/359719.

@hc-github-team-consul-core
Copy link
Contributor

🍒✅ Cherry pick of commit 71d45a3 onto release/1.10.x succeeded!

hc-github-team-consul-core pushed a commit that referenced this pull request Apr 29, 2021
This adds support for the Incremental xDS protocol when using xDS v3. This is best reviewed commit-by-commit and will not be squashed when merged.

Union of all commit messages follows to give an overarching summary:

xds: exclusively support incremental xDS when using xDS v3

Attempts to use SoTW via v3 will fail, much like attempts to use incremental via v2 will fail.
Work around a strange older envoy behavior involving empty CDS responses over incremental xDS.
xds: various cleanups and refactors that don't strictly concern the addition of incremental xDS support

Dissolve the connectionInfo struct in favor of per-connection ResourceGenerators instead.
Do a better job of ensuring the xds code uses a well configured logger that accurately describes the connected client.
xds: pull out checkStreamACLs method in advance of a later commit

xds: rewrite SoTW xDS protocol tests to use protobufs rather than hand-rolled json strings

In the test we very lightly reuse some of the more boring protobuf construction helper code that is also technically under test. The important thing of the protocol tests is testing the protocol. The actual inputs and outputs are largely already handled by the xds golden output tests now so these protocol tests don't have to do double-duty.

This also updates the SoTW protocol test to exclusively use xDS v2 which is the only variant of SoTW that will be supported in Consul 1.10.

xds: default xds.Server.AuthCheckFrequency at use-time instead of construction-time
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/cli Flags and documentation for the CLI interface theme/envoy/xds Related to Envoy support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants