From e229cf931e7ebb15b87d3094dfa2c6fb880061d3 Mon Sep 17 00:00:00 2001
From: Matt Klein <mklein@lyft.com>
Date: Sun, 14 Apr 2019 13:36:42 -0700
Subject: [PATCH] cluster manager: destroy warming clusters during shutdown()

Fixes https://github.com/envoyproxy/envoy/issues/6513.

This issue has existed for quite some time, so I'm unclear why we
just started seeing this. It's possible it's chance and it's also
possible it's in some way related to the init changes, but either
way, this is the correct fix.

Signed-off-by: Matt Klein <mklein@lyft.com>
---
 source/common/upstream/cluster_manager_impl.h |  3 ++
 .../upstream/cluster_manager_impl_test.cc     | 28 +++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/source/common/upstream/cluster_manager_impl.h b/source/common/upstream/cluster_manager_impl.h
index 9586357bc4aa1..4cb0914ee20c4 100644
--- a/source/common/upstream/cluster_manager_impl.h
+++ b/source/common/upstream/cluster_manager_impl.h
@@ -203,9 +203,12 @@ class ClusterManagerImpl : public ClusterManager, Logger::Loggable<Logger::Id::u
   Http::AsyncClient& httpAsyncClientForCluster(const std::string& cluster) override;
   bool removeCluster(const std::string& cluster) override;
   void shutdown() override {
+    // Make sure we destroy all potential outgoing connections before this returns.
     cds_api_.reset();
     ads_mux_.reset();
     active_clusters_.clear();
+    warming_clusters_.clear();
+    updateGauges();
   }
 
   const envoy::api::v2::core::BindConfig& bindConfig() const override { return bind_config_; }
diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc
index a15b364c8b2d6..c826b50cea322 100644
--- a/test/common/upstream/cluster_manager_impl_test.cc
+++ b/test/common/upstream/cluster_manager_impl_test.cc
@@ -1107,6 +1107,34 @@ TEST_F(ClusterManagerImplTest, RemoveWarmingCluster) {
   EXPECT_TRUE(Mock::VerifyAndClearExpectations(cluster1.get()));
 }
 
+// Verify that shutting down the cluster manager destroys warming clusters.
+TEST_F(ClusterManagerImplTest, ShutdownWithWarming) {
+  const std::string json = R"EOF(
+  {
+    "clusters": []
+  }
+  )EOF";
+
+  create(parseBootstrapFromJson(json));
+
+  InSequence s;
+  ReadyWatcher initialized;
+  EXPECT_CALL(initialized, ready());
+  cluster_manager_->setInitializedCb([&]() -> void { initialized.ready(); });
+
+  std::shared_ptr<MockClusterMockPrioritySet> cluster1(new NiceMock<MockClusterMockPrioritySet>());
+  EXPECT_CALL(factory_, clusterFromProto_(_, _, _, _)).WillOnce(Return(cluster1));
+  EXPECT_CALL(*cluster1, initializePhase()).Times(0);
+  EXPECT_CALL(*cluster1, initialize(_));
+  EXPECT_TRUE(cluster_manager_->addOrUpdateCluster(defaultStaticCluster("fake_cluster"), "version1",
+                                                   dummyWarmingCb));
+  checkStats(1 /*added*/, 0 /*modified*/, 0 /*removed*/, 0 /*active*/, 1 /*warming*/);
+  cluster_manager_->shutdown();
+  checkStats(1 /*added*/, 0 /*modified*/, 0 /*removed*/, 0 /*active*/, 0 /*warming*/);
+
+  EXPECT_TRUE(Mock::VerifyAndClearExpectations(cluster1.get()));
+}
+
 TEST_F(ClusterManagerImplTest, DynamicAddRemove) {
   const std::string json = R"EOF(
   {