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.

Sign up for GitHub

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

Jump to bottom

api: Adding runtime override to disable fatal-by-default #13607

Merged
merged 9 commits into from
Oct 21, 2020
Merged

api: Adding runtime override to disable fatal-by-default #13607

merged 9 commits into from
Oct 21, 2020

Conversation

adisuissa
Copy link
Contributor

Commit Message: api: Adding runtime override to disable fatal-by-default
Additional Description:
Adding a runtime override flag envoy.features.enable_all_deprecated_features that when applied disables the fatal-by-default behavior of Envoy, and allows the use of all deprecated features.
Part of the work described in #8416.

Risk Level: Low.
Testing: Added unit tests.
Docs Changes: None.
Release Notes: None.
Platform Specific Features: None.

Signed-off-by: Adi Suissa-Peleg adip@google.com

@adisuissa
api: Adding runtime override to disable fatal-by-default
67b0646 
Signed-off-by: Adi Suissa-Peleg <adip@google.com>
@adisuissa
Copy link
Contributor Author

/assign @htuch
No need to define a CLI parameter, as the runtime override can be added to the bootstrap config.
I can also add tests to: test/config_test/deprecated_configs_test.cc, but as v2 support will soon be disabled, I'm not sure if I should.
PTAL.

@adisuissa
Merge remote-tracking branch 'upstream/master' into
b74f977 
fatal_by_default_flag

Signed-off-by: Adi Suissa-Peleg <adip@google.com>
htuch
htuch reviewed Oct 19, 2020
View reviewed changes
Copy link
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.

Looks good, couple of questions, otherwise I think this is good to go.

test/common/protobuf/utility_test.cc Show resolved Hide resolved
source/common/runtime/runtime_impl.cc Outdated Show resolved Hide resolved
@adisuissa
Addressing comments
09aac03 
Signed-off-by: Adi Suissa-Peleg <adip@google.com>
@adisuissa
Merge remote-tracking branch 'upstream/master' into fatal_by_default_…
46823b2 
…flag

Signed-off-by: Adi Suissa-Peleg <adip@google.com>
@adisuissa
Copy link
Contributor Author

/retest

@repokitteh-read-only
Copy link

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

🐱

Caused by: a #13607 (comment) was created by @adisuissa.

see: more, trace.

htuch
htuch reviewed Oct 20, 2020
View reviewed changes
CONTRIBUTING.md Show resolved Hide resolved
source/common/runtime/runtime_impl.cc Outdated Show resolved Hide resolved
@adisuissa
Adding version history and simplifying code
3335599 
Signed-off-by: Adi Suissa-Peleg <adip@google.com>
@adisuissa
Merge remote-tracking branch 'upstream/master' into fatal_by_default_…
0f8a131 
…flag

Signed-off-by: Adi Suissa-Peleg <adip@google.com>
htuch
htuch previously approved these changes Oct 20, 2020
View reviewed changes
Copy link
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.

LGTM, thanks!

mattklein123
mattklein123 requested changes Oct 20, 2020
View reviewed changes
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.

Thanks a ton. Quick docs drive by.

/wait

docs/root/version_history/current.rst
@@ -29,6 +29,7 @@ Removed Config or Runtime

New Features
------------
* config: added new runtime feature `envoy.features.enable_all_deprecated_features` that allows the use of all deprecated features.
Copy link
Member

Choose a reason for hiding this comment

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

Can you please also discuss this new feature here? https://www.envoyproxy.io/docs/envoy/latest/faq/configuration/deprecation

cc @alyssawilk if there are any other locations we should document.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ended up updating this: https://www.envoyproxy.io/docs/envoy/latest/configuration/operations/runtime#using-runtime-overrides-for-deprecated-features instead of the FAQ.
Please let me know if there's anything else that I've missed.

@mattklein123 mattklein123 self-assigned this Oct 20, 2020
@adisuissa
Updating docs
570b1a2 
Signed-off-by: Adi Suissa-Peleg <adip@google.com>
@mattklein123
Copy link
Member

LGTM thanks. Can you check format?

/wait

@adisuissa
fixing typo
c5ff199 
Signed-off-by: Adi Suissa-Peleg <adip@google.com>
@adisuissa
Merge remote-tracking branch 'upstream/master' into fatal_by_default_…
fcf7baf 
…flag

Signed-off-by: Adi Suissa-Peleg <adip@google.com>
@mattklein123 mattklein123 merged commit ea49a2d into envoyproxy:master Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattklein123 mattklein123 mattklein123 approved these changes

@htuch htuch htuch left review comments

Assignees

@mattklein123 mattklein123

@htuch htuch

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@adisuissa @mattklein123 @htuch