Missed 1.17->1.18 migration path for staggler v2 extension's PreviousPrioritiesConfig

[robinp-tw]

The change c04a75e made upgrade for staggler v2 extensions possible to v3, but missed the PreviousPrioritiesConfig. This blocks the clean upgrade path.

See commit transferwise@b8c209e which fixes the issue (against v1.18.4 tag).

Repro steps:
Have a RetryPriority config like

	import io.envoyproxy.envoy.config.retry.previous_priorities.PreviousPrioritiesConfig;

        RetryPolicy.Builder builder = RetryPolicy.newBuilder()
                // ...
		.setRetryPriority(
		    RetryPolicy.RetryPriority.newBuilder()
			.setName("envoy.retry_priorities.previous_priorities")
			.setTypedConfig(Any.pack(
			    // staggler v2 config
			    PreviousPrioritiesConfig.newBuilder()
				.setUpdateFrequency(1)
				.build()
			)).build()

Envoy v1.18 configured with this would be expected to accept the config, but see logs how it fails.

Logs:

[2021-11-25 10:42:43.988][19][warning][misc] [source/common/protobuf/utility.cc:312] Configuration does not parse cleanly as v3. v2 configuration is deprecated and wil
l be removed from Envoy at the start of Q1 2021: envoy.config.retry.previous_priorities.PreviousPrioritiesConfig
[2021-11-25 10:42:43.990][19][warning][config] [source/common/config/delta_subscription_state.cc:155] delta config for type.googleapis.com/envoy.config.route.v3.RouteC
onfiguration rejected: The v2 xDS major version is deprecated and disabled by default. Support for v2 will be removed from Envoy at the start of Q1 2021. You may make 
use of v2 in Q4 2020 by following the advice in https://www.envoyproxy.io/docs/envoy/latest/faq/api/transition. (envoy.config.retry.previous_priorities.PreviousPriorit
iesConfig)
[2021-11-25 10:42:43.990][19][warning][config] [source/common/config/grpc_subscription_impl.cc:127] gRPC config for type.googleapis.com/envoy.config.route.v3.RouteConf
iguration rejected: The v2 xDS major version is deprecated and disabled by default. Support for v2 will be removed from Envoy at the start of Q1 2021. You may make use
 of v2 in Q4 2020 by following the advice in https://www.envoyproxy.io/docs/envoy/latest/faq/api/transition. (envoy.config.retry.previous_priorities.PreviousPriorities
Config)

[ggreenway]

cc @htuch

[htuch]

Not sure if this is the correct diagnosis. There was already a v3 PreviousPrioritiesConfig in https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/retry/priority/previous_priorities/v3/previous_priorities_config.proto which should have been boostable from the unversioned PreviousPrioritiesConfig (linked via previous_mesage_type). The warning seems legitimate; you're using the v2 (untyped is same as v2) version in your example.

[robinp-tw]

Hello @htuch ! Not sure what you mean, indeed the v2 should have been boostable to v3 - but it missed a whitelisting, see transferwise@b8c209e.

As I get, why that whitelist is needed, is since these specific v3 configs were forgotten to be added timely when the v2 deprecation happened. So they were added later and got special exception to be accepted & boosted, even when v2 would already be unsupported (see c04a75e).

Without adding the exception in the commit, there's no direct upgrade path from Envoy 1.17 to 1.18: Version 1.17 misses the v3 version of this config, while 1.18 has it but forgot to whitelist. So can't upgrade without stop-the-world. (With the patch we could upgrade fine).

That's what I see, but I might miss something as well.

[htuch]

https://github.com/envoyproxy/envoy/commits/main/api/envoy/extensions/retry/priority/previous_priorities/v3/previous_priorities_config.proto has existed since early 2020. Why wouldn't you be able to upgrade to this before going from 1.17 to 1.18?

In this case, since a v3 version had existed, and the v2 version was marked for deprecation, it didn't need special case treatment I think.

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

[robinp-tw]

@htuch I owe an actual repro that if the config snippet above would use V3 proto, Envoy 1.17 would be unhappy about it.

For now I have to believe that based on a comment in our code, and after brief searching #14735 might also point to something similar?

You commented #14735 (comment) so probably have better picture about this.

If you think this should not be a problem, I might take some time to repro (might turn out was a false alert). Thank you!

[htuch]

I see, thanks for the reminder. As per #14907, the v3 API was there before 1.17, but there was as bug in how we were using it in the extension. #14907 landed right after the 1.17 release.

Given how old the releases we're talking about are, I suggest the right approach would be to go straight to 1.18. We could probably accept a backport of #14907 to 1.17 as well, as per https://github.com/envoyproxy/envoy/blob/832bfe87ace5a02de702e89bee4d3c653493e321/RELEASES.md#backports, if there is enough demand.

Tagging this for #10943

[robinp-tw]

Thanks! Not sure what going straight to 1.18 means, but I think we were doing that. We patched 1.18 (see transferwise@b8c209e) to allow this missing upgrade path. Gradually releasing that version works for us.

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

[github-actions]

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.