cluster manager init takes minutes due to slow DNS and STRICT_DNS behaviour

I noticed recently that cluster manager init can take quite some time. For example here it takes 3+ minutes

2020-11-10T16:53:13.792Z info envoy[44013] [Originator@6876 sub=upstream] cds: add 109 cluster(s), remove 1 cluster(s)
...
2020-11-10T16:53:13.943Z info envoy[44013] [Originator@6876 sub=upstream] cds: add/update cluster '/'
2020-11-10T16:56:58.923Z info envoy[44013] [Originator@6876 sub=upstream] cm init: all clusters initialized

After debugging the issue it turned out that a STRICT_DNS cluster is considered initialised once the very first DNS resolver completes either with success or failure.

This is a problem for me as during these 3+ minutes envoy wasn't working at all - it was unable to get its listeners and routes because it was stuck in CDS. I only had a few STRICT_DNS clusters and most of my other clusters 100+ are STATIC but unusable due to lack of listeners+routes.

Note that this only happens when DNS is (very) slow. I tried reproducing the issue with iptables+DROP rules but was unable to. c-ares returns failure immediately. This is the only way I was able to slow down DNS in order to reproduce the issue (10.10.10.10 is my DNS server).

tc qdisc add dev eth0 root handle 1: prio
tc qdisc add dev eth0 parent 1:3 handle 30: tbf rate 20kbit buffer 1600 limit 3000
tc qdisc add dev eth0 parent 30:1 handle 31: netem delay 20000ms 10ms distribution normal
tc filter add dev eth0 protocol ip parent 1:0 prio 3 u32 match ip dst 10.10.10.10/32 flowid 1:3

After the initial initialisation all is good as the DNS responses are cached and deduplicated. My apps and services tend to restart from time to time and having a 3+ minute gap when DNS is slow is bad. And DNS happens to sometimes be slow when running a cloud service.

I ended up implementing the following solution:

--- a/source/common/upstream/strict_dns_cluster.cc
+++ b/source/common/upstream/strict_dns_cluster.cc
@@ -59,6 +59,8 @@ void StrictDnsClusterImpl::startPreInit() {
   // resolved in failure.
   if (resolve_targets_.empty()) {
     onPreInitComplete();
+  } else if (info_->addedViaApi()) {
+    onPreInitComplete(); // see StrictDnsWarmupFilter and PR 2673888
   }
 }

--- a/source/extensions/filters/http/common/BUILD
+++ b/source/extensions/filters/http/common/BUILD
@@ -42,6 +42,17 @@ envoy_cc_library(
 )

 envoy_cc_library(
+    name = "strict_dns_warmup_filter_lib",
+    srcs = ["strict_dns_warmup_filter.cc"],
+    hdrs = ["strict_dns_warmup_filter.h"],
+    visibility = ["//visibility:public"],
+    deps = [
+        "//include/envoy/upstream:cluster_manager_interface",
+        "//source/extensions/filters/http/common:pass_through_filter_lib",
+    ],
+)
+
+envoy_cc_library(
     name = "utility_lib",
     hdrs = ["utility.h"],
     # Used by the router filter.  TODO(#9953) clean up.

--- /dev/null
+++ b/source/extensions/filters/http/common/strict_dns_warmup_filter.cc
@@ -0,0 +1,65 @@
+#include "extensions/filters/http/common/strict_dns_warmup_filter.h"
+
+namespace Envoy {
+namespace Http {
+
+Http::FilterHeadersStatus StrictDnsWarmupFilter::decodeHeaders(
+    Http::RequestHeaderMap& /* headers */, bool /* end_stream */) {
+  auto route = decoder_callbacks_->route();
+
+  if (route == nullptr) {
+    return Http::FilterHeadersStatus::Continue;
+  }
+
+  const Router::RouteEntry* entry = route->routeEntry();
+
+  if (entry == nullptr) {
+    return Http::FilterHeadersStatus::Continue;
+  }
+
+  Upstream::ThreadLocalCluster* cluster = cm_.get(entry->clusterName());
+
+  if (cluster == nullptr) {
+    return Http::FilterHeadersStatus::Continue;
+  }
+
+  if (cluster->info()->type() != envoy::config::cluster::v3::Cluster::STRICT_DNS) {
+    return Http::FilterHeadersStatus::Continue;
+  }
+
+  const auto& host_sets = cluster->prioritySet().hostSetsPerPriority();
+
+  if (std::any_of(host_sets.begin(), host_sets.end(),
+                  [](const Upstream::HostSetPtr& host_set) {
+                    const auto& hosts = host_set->hosts();
+                    return std::any_of(hosts.begin(), hosts.end(),
+                                       [](const Upstream::HostSharedPtr& host) {
+                                         return host != nullptr;
+                                       });
+                  })) {
+    return Http::FilterHeadersStatus::Continue;
+  }
+
+  timer_ = decoder_callbacks_->dispatcher().createTimer([this] {
+    host_set_member_update_cb_handle_->remove();
+    decoder_callbacks_->continueDecoding();
+  });
+
+  host_set_member_update_cb_handle_ = cluster->prioritySet().addMemberUpdateCb(
+      [this](const Upstream::HostVector&, const Upstream::HostVector&) {
+    if (timer_ != nullptr) {
+      timer_.reset();
+      decoder_callbacks_->dispatcher().post([this] {
+        host_set_member_update_cb_handle_->remove();
+        decoder_callbacks_->continueDecoding();
+      });
+    }
+  });
+
+  timer_->enableTimer(std::chrono::milliseconds(30000));
+
+  return Http::FilterHeadersStatus::StopAllIterationAndWatermark;
+}
+
+} // namespace Http
+} // namespace Envoy

--- /dev/null
+++ b/source/extensions/filters/http/common/strict_dns_warmup_filter.h
@@ -0,0 +1,32 @@
+#pragma once
+
+#include "envoy/upstream/cluster_manager.h"
+
+#include "extensions/filters/http/common/pass_through_filter.h"
+
+namespace Envoy {
+namespace Http {
+
+
+/**
+ * @class StrictDnsWarmupFilter
+ *
+ *      StrictDns warmup filter.
+ */
+class StrictDnsWarmupFilter : public PassThroughDecoderFilter {
+public:
+  StrictDnsWarmupFilter(Upstream::ClusterManager& cm) : cm_(cm) {}
+
+  // Http::StreamDecoderFilter
+  Http::FilterHeadersStatus decodeHeaders(Http::RequestHeaderMap& headers,
+                                          bool end_stream) override;
+
+private:
+  Upstream::ClusterManager& cm_;
+  Event::TimerPtr timer_;
+  Common::CallbackHandle* host_set_member_update_cb_handle_{};
+};
+
+
+} // namespace Http
+} // namespace Envoy

--- a/source/extensions/filters/http/router/BUILD
+++ b/source/extensions/filters/http/router/BUILD
@@ -24,6 +24,7 @@ envoy_cc_extension(
         "//source/common/router:shadow_writer_lib",
         "//source/extensions/filters/http:well_known_names",
         "//source/extensions/filters/http/common:factory_base_lib",
+        "//source/extensions/filters/http/common:strict_dns_warmup_filter_lib",
         "//source/extensions/filters/http/on_demand:on_demand_update_lib",
         "@envoy_api//envoy/extensions/filters/http/router/v3:pkg_cc_proto",
     ],

--- a/source/extensions/filters/http/router/config.cc
+++ b/source/extensions/filters/http/router/config.cc
@@ -1,5 +1,7 @@
 #include "extensions/filters/http/router/config.h"

+#include "extensions/filters/http/common/strict_dns_warmup_filter.h"
+
 #include "envoy/extensions/filters/http/router/v3/router.pb.h"
 #include "envoy/extensions/filters/http/router/v3/router.pb.validate.h"

@@ -19,6 +21,8 @@ Http::FilterFactoryCb RouterFilterConfig::createFilterFactoryFromProtoTyped(
       proto_config));

   return [filter_config](Http::FilterChainFactoryCallbacks& callbacks) -> void {
+    callbacks.addStreamDecoderFilter(
+        std::make_shared<Http::StrictDnsWarmupFilter>(filter_config->cm_));
     callbacks.addStreamDecoderFilter(std::make_shared<Router::ProdFilter>(*filter_config));
   };
 }

The solution does the following:

1. All STRICT_DNS clusters received via ADS/CDS are considered initialised immediately. We do not wait for the first DNS resolve to complete. STRICT_DNS clusters in bootstrap are not affected as it is assumed that one may use a STRICT_DNS for ADS/CDS.
2. Whenever a L7 router filter is requested we install the new StrictDnsWarmupFilter before the router filter.
3. The StrictDnsWarmupFilter checks if the selected cluster is STRICT_DNS and if it has no hosts waits for at most 30 seconds (currently hardcoded) for any hosts to appear. If no hosts appear then either DNS is completely dead or very slow. In either case we continue to the router filter which either returns no_healthy_upstream or proxies the request.

At the moment I am not using L4 filters so StrictDnsWarmupFilter is specifically for L7 router.

I would like to ask if it would be possible to have the above use case incorporated into envoy for broader use (the patch is only for reference) - that is - initialise STRICT_DNS clusters immediately so that some routing may happen and have a grace period when DNS is slow or down.