bootstrap/runtime: remove support for v2 bootstrap runtime field. by htuch · Pull Request #16274 · envoyproxy/envoy

htuch · 2021-05-03T01:10:21Z

This is the first PR in a series to remove v2 API support from Envoy.

The process is as follows:

Identify uses of a hidden_envoy_deprecated_* field in source/.
Remove implementation support for occurences identified in (1).
Remove any tests in test/ covering (1). It is necessary to validate
manually that other tests exist for replacement functionality; in
some cases the deprecated test might be providing functional coverage
not applied to the deprecated feature's replacement.
Wash, rinse and repeat.

Risk level: Low
Testing: Added a unit test to cover otherwise missing behaviors in
runtime.

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

This is the first PR in a series to remove v2 API support from Envoy. The process is as follows: 1. Identify uses of a hidden_envoy_deprecated_* field in source/. 2. Remove implementation support for occurences identified in (1). 3. Remove any tests in test/ covering (1). It is necessary to validate manually that other tests exist for replacement functionality; in some cases the deprecated test might be providing functional coverage not applied to the deprecated feature's replacement. 4. Wash, rinse and repeat. Risk level: Low Testing: Added a unit test to cover otherwise missing behaviors in runtime. Signed-off-by: Harvey Tuch <htuch@google.com>

ankatare · 2021-05-03T06:05:57Z

Thanks a lot @htuch .. let me check this

ankatare · 2021-05-03T07:45:30Z

would like to work on this with you... :) . please suggest how to proceed , means from any particular tree which i can target ?

htuch · 2021-05-03T15:19:11Z

@ankatare follow the set of steps 1-4 described in the commit message above.

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

ggreenway · 2021-05-03T19:10:25Z

/retest

repokitteh-read-only · 2021-05-03T19:10:30Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #16274 (comment) was created by @ggreenway.

see: more, trace.

ankatare · 2021-05-04T15:03:01Z

@ankatare follow the set of steps 1-4 described in the commit message above.

@htuch Ok. Sure. will get back for any query

…voyproxy#16274) This is the first PR in a series to remove v2 API support from Envoy. The process is as follows: 1. Identify uses of a hidden_envoy_deprecated_* field in source/. 2. Remove implementation support for occurences identified in (1). 3. Remove any tests in test/ covering (1). It is necessary to validate manually that other tests exist for replacement functionality; in some cases the deprecated test might be providing functional coverage not applied to the deprecated feature's replacement. 4. Wash, rinse and repeat. Risk level: Low Testing: Added a unit test to cover otherwise missing behaviors in runtime. Signed-off-by: Harvey Tuch <htuch@google.com> Signed-off-by: Gokul Nair <gnair@twitter.com>

ankatare · 2021-07-20T16:06:41Z

@htuch Just thinking if we have any milestone set for this activity ?

htuch · 2021-07-21T10:55:51Z

@ankatare I'd hope we could have all v2 dependencies gone by next release.

ankatare · 2021-07-22T04:48:56Z

@htuch Ok. Gt it. Thanks

ankatare · 2021-09-03T07:43:06Z

@htuch @tyxia just thinking if below code also need to be removed ?

https://github.com/envoyproxy/envoy/blob/main/source/common/protobuf/utility.cc#L372-L396

please suggest.

htuch · 2021-09-05T19:53:22Z

I think so, but we might want to wait until generated_api_shadow/ is gone to be sure; I'll be doing the PR for this in the next week.

tyxia · 2021-09-05T22:56:22Z

@htuch @tyxia just thinking if below code also need to be removed ?

https://github.com/envoyproxy/envoy/blob/main/source/common/protobuf/utility.cc#L372-L396

please suggest.
@ankatare

Yea, this is being worked on in PR #17924, but as htuch suggested we will wait for the generated_api_shadow/ removal first.

ankatare · 2021-09-06T04:50:39Z

@tyxia @htuch cool. Thanks for Confirmation 👍

ankatare · 2021-09-23T06:18:57Z

Hi @tyxia cc @htuch i Could see some pending work in my local main. Please let me know if you are working on those OR i can take anyone if not.
/envoy$ grep -rnw ./source/ -e 'hidden_envoy_deprecated_.*'
./source/extensions/tracers/zipkin/span_buffer.cc:53: "hidden_envoy_deprecated_HTTP_JSON_V1 has been deprecated. Please use a non-default "
./source/common/protobuf/utility.cc:372: if (absl::StartsWith(field.name(), "hidden_envoy_deprecated_")) {
./source/common/protobuf/utility.cc:380: "Illegal use of hidden_envoy_deprecated_ V2 field '", field.full_name(),

ankatare · 2021-09-23T06:19:47Z

also under test/ Directory as below
envoy$ grep -rnw ./test/ -e 'hidden_envoy_deprecated_.*'
./test/extensions/tracers/zipkin/span_buffer_test.cc:469: "hidden_envoy_deprecated_HTTP_JSON_V1 has been deprecated. Please use a non-default "
./test/proto/deprecated.proto:41: string hidden_envoy_deprecated_is_deprecated = 2 [deprecated = true];
./test/proto/deprecated.proto:42: string hidden_envoy_deprecated_is_deprecated_fatal = 3 [deprecated = true];
./test/proto/deprecated.proto:45: string hidden_envoy_deprecated_inner_deprecated = 2 [deprecated = true];
./test/proto/deprecated.proto:46: string hidden_envoy_deprecated_inner_deprecated_fatal = 3 [deprecated = true];
./test/proto/deprecated.proto:48: InnerMessage hidden_envoy_deprecated_deprecated_message = 4 [deprecated = true];
./test/proto/deprecated.proto:51: repeated InnerMessage hidden_envoy_deprecated_deprecated_repeated_message = 7 [deprecated = true];
./test/proto/deprecated.proto:56: hidden_envoy_deprecated_DEPRECATED_DEFAULT = 0 [deprecated = true];
./test/proto/deprecated.proto:58: hidden_envoy_deprecated_DEPRECATED_NOT_DEFAULT = 2 [deprecated = true];
./test/server/server_corpus/clusterfuzz-testcase-minimized-config_fuzz_test-5702999713513472:196: hidden_envoy_deprecated_config {
./test/server/server_corpus/crash-ac725507195d840cdb90bed3079b877e6e9419e3:6:hidden_envoy_deprecated_runtime {
./test/server/server_corpus/clusterfuzz-testcase-minimized-config_fuzz_test-5186283155750912:40: hidden_envoy_deprecated_disable_overprovisioning: true
./test/server/server_corpus/clusterfuzz-testcase-minimized-config_fuzz_test-5186283155750912:51: hidden_envoy_deprecated_config {
./test/common/protobuf/utility_test.cc:1889: hidden_envoy_deprecated_is_deprecated: hidden_field1
./test/common/protobuf/utility_test.cc:1900: "Illegal use of hidden_envoy_deprecated_ V2 field "
./test/common/protobuf/utility_test.cc:1901: "'envoy.test.deprecation_test.UpgradedBase.hidden_envoy_deprecated_is_deprecated'");

ankatare · 2021-09-29T11:05:31Z

@tyxia would you please look on my above comments.

ankatare · 2021-09-29T11:07:08Z

i am seeing below WI is pending.. please suggest if i can take this to mark "remove v2 support "as finished

envoy$ grep -rnw ./source/ -e 'hidden_envoy_deprecated_.*'
./source/extensions/tracers/zipkin/span_buffer.cc:53: "hidden_envoy_deprecated_HTTP_JSON_V1 has been deprecated. Please use a non-default "

ankatare · 2021-09-30T04:53:52Z

@htuch Need your inputs on this remaining WI.

ankatare · 2021-09-30T05:00:00Z

@htuch @tyxia Sorry, It's My Bad :( .
i can see, now there is no pending WI for V2 support removal. Above cases are just exceptions which is required to handle the cases.

tyxia · 2021-09-30T14:02:45Z

@htuch @tyxia Sorry, It's My Bad :( . i can see, now there is no pending WI for V2 support removal. Above cases are just exceptions which is required to handle the cases.

@ankatare Yea, the remaining hidden_envoy_deprecated*s are either used for exceptions to catch invalid configs or used to skip unused cases . So, in short I think there is no pending work for removing hidden_envoy_deprecated*. There are a few cleanup work though and I have a draft CL.

Thank you for your help on this task!

ankatare · 2021-09-30T16:18:31Z

@tyxia Great, Thanks for update.

Fix compile_time_options.

6db23db

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

ggreenway approved these changes May 3, 2021

View reviewed changes

ggreenway self-assigned this May 3, 2021

htuch merged commit 15cd9a6 into envoyproxy:main May 4, 2021

htuch deleted the remove-v2-runtime branch May 4, 2021 00:26

htuch mentioned this pull request May 13, 2021

Remove --bootstrap-version command line option #16456

Closed

phlax mentioned this pull request Jun 11, 2021

Resource Type in Dynamic Configuration Control Plane pointing to v2 #16937

Closed

This was referenced Jun 14, 2021

remove support for v2 UNSUPPORTED_REST_LEGACY #16968

Merged

revove v2 suport for hosts #17022

Closed

remove v2 support for hosts from logical_dns_cluster.cc #17023

Closed

ankatare mentioned this pull request Jun 28, 2021

remove support of hidden_deprecated_use_alpha #17169

Merged

This was referenced Sep 2, 2021

Removing support of hidden_envoy_deprecated_extension_protocol_options #17962

Closed

remove support of hidden_envoy_deprecated_remove_accept_encoding_header #17963

Closed

Remove Support of hidden_envoy_deprecated_UNSUPPORTED_REST_LEGACY #17972

Closed

Conversation

htuch commented May 3, 2021

Uh oh!

ankatare commented May 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ankatare commented May 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

htuch commented May 3, 2021

Uh oh!

ggreenway commented May 3, 2021

Uh oh!

repokitteh-read-only bot commented May 3, 2021

Uh oh!

ankatare commented May 4, 2021

Uh oh!

ankatare commented Jul 20, 2021

Uh oh!

htuch commented Jul 21, 2021

Uh oh!

ankatare commented Jul 22, 2021

Uh oh!

ankatare commented Sep 3, 2021

Uh oh!

htuch commented Sep 5, 2021

Uh oh!

tyxia commented Sep 5, 2021

Uh oh!

ankatare commented Sep 6, 2021

Uh oh!

ankatare commented Sep 23, 2021

Uh oh!

ankatare commented Sep 23, 2021

Uh oh!

ankatare commented Sep 29, 2021

Uh oh!

ankatare commented Sep 29, 2021

Uh oh!

ankatare commented Sep 30, 2021

Uh oh!

ankatare commented Sep 30, 2021

Uh oh!

tyxia commented Sep 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ankatare commented Sep 30, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ankatare commented May 3, 2021 •

edited

Loading

ankatare commented May 3, 2021 •

edited

Loading

tyxia commented Sep 30, 2021 •

edited

Loading