config: making v2-config-only a boolean flag #3847
Conversation
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
|
The alternate would be to simply add a switch --allow-deprecated-v1 config and make them exclusive. I'm up for either. once this lands I'll update the bug and reply on the envoy-announce email with updated syntax. |
test/server/options_impl_test.cc
Outdated
| "envoy --mode validate --concurrency 2 -c hello --admin-address-path path --restart-epoch 1 " | ||
| "--local-address-ip-version v6 -l info --service-cluster cluster --service-node node " | ||
| "--service-zone zone --file-flush-interval-msec 9000 --drain-time-s 60 --log-format [%v] " | ||
| "--parent-shutdown-time-s 90 --log-path /foo/bar --v2-config-only 1 --disable-hot-restart"); |
There was a problem hiding this comment.
tclap only supports 0/1 rather than false/true? We might want to use an independent flag to avoid breaking folks production equivalent of Borg config.
There was a problem hiding this comment.
Yeah, I tested and true/false chokes. I was tempted to do a string literal true/false but I figured folks use to TCLAP rather than google flags would find that more confusing
There was a problem hiding this comment.
Also I'm open to just adding a second flag but if the concern is folks who want v2 only will have it specified, we're going to break them eventually when we remove v1 config and the flag. Is it worth adding the complexity of a second flag to avoid a problem they're going to face in a few months anyway?
There was a problem hiding this comment.
IMO we should add a second flag and not change this one (basically make it do nothing since the default is already what it implies). I think this is less likely to break people?
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
docs/root/intro/version_history.rst
Outdated
| @@ -7,7 +7,7 @@ Version history | |||
| to filter based on the presence of Envoy response flags. | |||
| * admin: added :http:get:`/hystrix_event_stream` as an endpoint for monitoring envoy's statistics | |||
| through `Hystrix dashboard <https://github.com/Netflix-Skunkworks/hystrix-dashboard/wiki>`_. | |||
| * config: v1 disabled by default. v1 support remains available until October via flipping --v2-config-only=false. | |||
| * config: v1 disabled by default. v1 support remains available until October via setting --allow-deprecated-v1-api. | |||
There was a problem hiding this comment.
Can you deep link this using :option: ?
docs/root/operations/cli.rst
Outdated
| config parse fails, a second attempt to parse the config as a :ref:`v1 JSON | ||
| configuration file <config_overview_v1>` will be made. | ||
|
|
|
Yep, as said I'll do it once this lands (just in case there's last minute changes) |
* origin/master: config: making v2-config-only a boolean flag (envoyproxy#3847) lc trie: add exclusive flag. (envoyproxy#3825) upstream: introduce PriorityStateManager, refactor EDS (envoyproxy#3783) test: deflaking header_integration_test (envoyproxy#3849) http: new style WebSockets, where headers and data are processed by the filter chain. (envoyproxy#3776) common: minor doc updates (envoyproxy#3845) fix master build (envoyproxy#3844) logging: Requiring details for RELEASE_ASSERT (envoyproxy#3842) test: add test for consistency of RawStatData internal memory representation (envoyproxy#3843) common: jittered backoff implementation (envoyproxy#3791) format: run buildifier on .bzl files. (envoyproxy#3824) Support mutable metadata for endpoints (envoyproxy#3814) test: deflaking a test, improving debugability (envoyproxy#3829) Update ApiConfigSource docs with grpc_services only for GRPC configs (envoyproxy#3834) Add hard-coded /hot_restart_version test (envoyproxy#3832) healthchecks: Add interval_jitter_percent healthcheck option (envoyproxy#3816) Signed-off-by: Snow Pettersen <snowp@squareup.com>
As v2-config-only is the default, adding a command line switch --allow-deprecated-v1-api for opting into legacy config.
As noted in #3846 flipping the default on a command line switch is confusing at best. Adding an new flag to restore v1 use, and deprecating the old switch.
Risk Level: Low
Testing: Added unit tests
Docs Changes: updated docs
Release Notes: n/a
Fixes #3846