Dynamic filter config provider provides empty config when listener updates

[bianpengyuan]

I am using ECDS to provide dynamic extension configuration. When listener updates, dynamic filter config provider will return empty config for the new listener:

envoy/source/common/filter/http/filter_config_discovery_impl.cc

Line 40 in 4e1146b

return tls_->config_;

I think what happens is that when listener updates, old listener takes time to drain, ECDS subscription is not destroyed when new listener is constructed, so it is reused by the new listener:

envoy/source/common/filter/http/filter_config_discovery_impl.cc

Line 184 in 4e1146b

auto it = subscriptions_.find(subscription_id);

But a new dynamic filter config provider is created with the new listener and will be warming and wait for any subscription update:

envoy/source/extensions/filters/network/http_connection_manager/config.cc

Line 552 in 4e1146b

auto filter_config_provider = filter_config_provider_manager_.createDynamicFilterConfigProvider(

However, when subscription receives new update, if config does not have any change from last update, it will skip updating provider. Listener will be considered as warmed, and provider will stuck with empty config until next ECDS update with diff.

envoy/source/common/filter/http/filter_config_discovery_impl.cc

Line 118 in 4e1146b

if (new_hash == last_config_hash_) {

[antoniovicente]

cc @htuch @mattklein123

[htuch]

@kyessenov thoughts here? This seems to be ECDS warming behavior.

[bianpengyuan]

@kyessenov is on leave this month. Not sure if there is someone else who is familiar with ecds code path.

[htuch]

I can take a shot at this. I think I need to understand in your example which listener and ECDS subscriptions are involved. It's a bit confusing, e.g. are the listeners sharing a filter chain with ECDS subscription to the same resource? Are the listeners pointing at different ECDS resources?

I think in the PR fix this is not quite the right thing, as it's a work-around to maybe a more fundamental issue around how listeners and ECDS warming is happening.

[lambdai]

However, when subscription receives new update, if config does not have any change from last update, it will skip updating provider. Listener will be considered as warmed, and provider will stuck with empty config until next ECDS update with diff.

@htuch @bianpengyuan
Based on my perception there are two "benign" dedups prevent filling the providers created along with a new listener. Correct me if I am wrong

1. New providers wait for a config push from ECDS server.
   NOT fixed by this PR.
2. Upon new ECDS push, if the dedup on hash may skip to push to the newly created providers.
   Fixed by this PR.

[bianpengyuan]

I think I need to understand in your example which listener and ECDS subscriptions are involved.

@htuch The old and new listeners share a ECDS subscription.

This is how I understand the object lifecycle: it starts with listener1 which owns a dynamic filter config provider provider1. provider1 was warmed properly and holds a filter configuration from subscription.

Then a new listener listener2 comes in, which has the same dynamic filter and ECDS config. listener2 creates a new dynamic filter config provider provider2 and put it into warming state. Since the ECDS subscription is not yet destructed, provider2 is registered with the same subscription.

Then a ECDS update come in, which is the same as the last one received by the subscription based on hash comparison, thus the subscription thinks that all known providers should be up-to-dated and skip updating any providers. However, provider2 does not hold any filter config yet. The ECDS update also makes init manager think that provider2 is warmed. listener2 starts to work with a not actually warmed provider2, and causes request failure.

New providers wait for a config push from ECDS server.
NOT fixed by this PR.

@lambdai I am not sure what is the expected behavior, should the new provider skip waiting and just use available configuration that subscription has, or wait for an ECDS update regardless.

[lambdai]

Then a ECDS update come in...

This is exactly what I am referring to. Why does the listener2 need to wait? Ideally ECDS server doesn't have to push the same config. Envoy may not even have the chance to compare the old ECDS config hashe and the new hash.

[htuch]

Agree with @lambdai here. If we're talking about an ECDS subscription that has already delivered configuration previously, the new listener shouldn't have to warm on this. I'm not super familiar with the code here, but I would expect it to be the responsibility of the dynamic filter config provider system to handle this.

The analogy is with route config providers. When two listeners share a route config provider, listener2 doesn't need to wait for an RDS update to continue.

[kyessenov]

Agree with @htuch. I think we can possibly solve this by immediately applying warmed subscription in the provider. The interesting case is mis-matched config provider: what if filter config provider type_urls disallows the one from the subscription? I think in this case, we should warm because there is a possibility that the new config (different by necessity) may unblock the listener.

[lambdai]

Is it possible to store the shared latest config in the subscription?
Thus providers with different type_urls can choose to use the latest config independently.

When a new config arrives, always update the latest config. Meanwhile, iterate over the providers to give each provider a chance to adopt.

[kyessenov]

@lambdai Yeah, although I think it's better to avoid storing protobuf and just carry the factory callback.