Skip to content

config: v2 fatal-by-default.#13950

Merged
htuch merged 5 commits intoenvoyproxy:masterfrom
htuch:v2-fatal-by-default
Nov 11, 2020
Merged

config: v2 fatal-by-default.#13950
htuch merged 5 commits intoenvoyproxy:masterfrom
htuch:v2-fatal-by-default

Conversation

@htuch
Copy link
Member

@htuch htuch commented Nov 9, 2020

This patch makes the v2 xDS API fatal-by-default. It's possible to still
use v2 by:

  • Providing --bootstrap-version 2 on the CLI for v2 bootstrap files.
  • Setting envoy.reloadable_features.enable_deprecated_v2_api in the
    runtime.

Many tests required fixups:

  • Unit tests were either upgraded to v3 (where there was no significant
    reason to remain v2) or a test runtime was introduced permitting v2
    for deprecated feature tests.
  • Integration tests were generally migrated to v3. This now means that
    our integration tests coverage focuses on v3 (it was previously
    focused on v2). We have some limited v2 coverage still provided by the
    v2 ads_integration_tests.

Coverage remains stable, it was previously:

Overall coverage rate:
lines......: 96.6% (112119 of 116035 lines)
functions..: 79.8% (22543 of 28259 functions)

and is now:

Overall coverage rate:
lines......: 96.6% (112162 of 116096 lines)
functions..: 79.8% (22542 of 28257 functions)

Risk level: High (this will break anyone who is still using v2 and has
not enabled CLI or runtime override)
Testing: Various tests updated as described above. New unit test added
for bootstrap to server_test and to ads_integration_test for dynamic
rejection behavior.
Release Notes: Added.

Fixes #10816

Signed-off-by: Harvey Tuch htuch@google.com

This patch makes the v2 xDS API fatal-by-default. It's possible to still
use v2 by:
* Providing --bootstrap-version 2 on the CLI for v2 bootstrap files.
* Setting envoy.reloadable_features.enable_deprecated_v2_api in the
  runtime.

Many tests required fixups:
* Unit tests were either upgraded to v3 (where there was no significant
  reason to remain v2) or a test runtime was introduced permitting v2
  for deprecated feature tests.
* Integration tests were generally migrated to v3. This now means that
  our integration tests coverage focuses on v3 (it was previously
  focused on v2). We have some limited v2 coverage still provided by the
  v2 ads_integration_tests.

Risk level: High (this will break anyone who is still using v2 and has
  not enabled CLI or runtime override)
Testing: Various tests updated as described above. New unit test added
  for bootstrap to server_test and to ads_integration_test for dynamic
  rejection behavior.
Release Notes: Added.

Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
@mattklein123
Copy link
Member

Looks like legit CI issues.

/wait

@htuch
Copy link
Member Author

htuch commented Nov 10, 2020

@mattklein123 this requires envoyproxy/envoy-filter-example#137 to merge, if you can approve that I'll update the SHA.

Signed-off-by: Harvey Tuch <htuch@google.com>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Epic slog. One question.

Signed-off-by: Harvey Tuch <htuch@google.com>
Signed-off-by: Harvey Tuch <htuch@google.com>
Copy link
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Epic slogging!

@htuch
Copy link
Member Author

htuch commented Nov 11, 2020

@mattklein123 @lizan is the clang-tidy timeout due to the size of this PR?

@htuch
Copy link
Member Author

htuch commented Nov 11, 2020

Force merging as discussed with maintainers.

@htuch htuch merged commit 0e42452 into envoyproxy:master Nov 11, 2020
@htuch htuch deleted the v2-fatal-by-default branch November 11, 2020 20:56
htuch pushed a commit that referenced this pull request Nov 25, 2020
Follow up to #13950

Commit Message: kafka: fix broker integration test
Additional Description: fixes manually executed kafka integration test; started to crash due to using now-fatal v2 config AFACT
Risk Level: Low
Testing: manual; bazel test //test/extensions/filters/network/kafka/broker/integration_test:kafka_broker_integration_test

Signed-off-by: Adam Kotwasinski <adam.kotwasinski@gmail.com>
htuch added a commit to htuch/envoy that referenced this pull request Dec 1, 2020
This is a followup to envoyproxy#13950 in which the transport API is also
fatal-by-default.

Risk level: High (this will break anyone who is still using v2 and has
  not enabled CLI or runtime override)
Testing: Various tests updated as described above. New unit test added
 for bootstrap to server_test and to ads_integration_test for
 dynamic rejection behavior. api_version_integration_test continues to
 provide the definitive cross-version transport API integration test.
Release Notes: Same as envoyproxy#13950.

Signed-off-by: Harvey Tuch <htuch@google.com>
htuch added a commit that referenced this pull request Dec 7, 2020
This is a followup to #13950 in which the transport API is also
fatal-by-default.

Risk level: High (this will break anyone who is still using v2 and has
not enabled CLI or runtime override)
Testing: Various tests updated as described above. New unit test added
for bootstrap to server_test and to ads_integration_test for
dynamic rejection behavior. api_version_integration_test continues to
provide the definitive cross-version transport API integration test.
Release Notes: Same as #13950.

Signed-off-by: Harvey Tuch <htuch@google.com>
htuch added a commit to htuch/envoy that referenced this pull request Dec 14, 2020
This is a followup to envoyproxy#14223, covering remaining uses of the
transport_api_version field.

Risk level: High (this will break anyone who is still using v2 and has
   not enabled CLI or runtime override)
Testing: Various tests updated, some exemplar tests added to
  server_test.
Release Notes: Same as envoyproxy#13950.

Signed-off-by: Harvey Tuch <htuch@google.com>
htuch added a commit that referenced this pull request Dec 15, 2020
This is a followup to #14223, covering remaining uses of the
transport_api_version field.

Risk level: High (this will break anyone who is still using v2 and has
not enabled CLI or runtime override)
Testing: Various tests updated, some exemplar tests added to
server_test.
Release Notes: Same as #13950.

Signed-off-by: Harvey Tuch <htuch@google.com>
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.

Reject v2 xDS configuration by default at EOQ3

2 participants