-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
extensions: update straggler v2 extensions to v3. #14907
Conversation
* Add v3 equivalents of v2 configs that were included in v3 due to no transitive deprecation. This increases consistency and reduces user confusion. We will continue to support these straggler v2 configs beyond the v2 turndown due to the late addition of v3 counterparts, special case code is added to utility.cc to handle this. * There were two extensions, //envoy/config/cluster/redis and //envoy/config/retry/previous_priorities, that for some reason were not upgraded to use v3 config. This is now fixed and I've grepped for other v2 in //source, none remain. Risk level: Medium (changes to extension config types and deprecated config handling). Testing: Additional unit test added for utility.cc handling, upgraded configs to v3 for other tests. Fixes envoyproxy#14735 Fixes envoyproxy#12841 Signed-off-by: Harvey Tuch <htuch@google.com> Co-authored-by: Abhay Narayan Katare <abhay.katare@india.nec.com>
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
@snowp if you can look at the retry related extension bits it would be a helpful sanity check. |
@htuch Thanks :) |
This is great, can you fix CI? |
@lizan this should be ready for review; the CI fails that remain are unrelated flakes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/wait
@@ -1,10 +1,11 @@ | |||
syntax = "proto3"; | |||
|
|||
package envoy.config.retry.omit_canary_hosts.v3; | |||
package envoy.extensions.retry.host.omit_canary_hosts.v3; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
won't this break wire compatibility?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I merged this one only a short while back and it's never been used by Envoy, so we can do this.
@lizan other thoughts? Would be great to merge this soon. |
Signed-off-by: Harvey Tuch <htuch@google.com>
@lizan can I get another rubber stamp? Thanks. |
Add v3 equivalents of v2 configs that were included in v3 due to no
transitive deprecation. This increases consistency and reduces user
confusion. We will continue to support these straggler v2 configs
beyond the v2 turndown due to the late addition of v3 counterparts,
special case code is added to utility.cc to handle this.
There were two extensions, //envoy/config/cluster/redis and
//envoy/config/retry/previous_priorities, that for some reason were
not upgraded to use v3 config. This is now fixed and I've grepped for
other v2 in //source, none remain.
Risk level: Medium (changes to extension config types and deprecated
config handling).
Testing: Additional unit test added for utility.cc handling, upgraded
configs to v3 for other tests.
Fixes #14735
Fixes #12841
Signed-off-by: Harvey Tuch htuch@google.com
Co-authored-by: Abhay Narayan Katare abhay.katare@india.nec.com