From 0f79a67b1de7e38d0ecc6665f9d5144eac0d8ad6 Mon Sep 17 00:00:00 2001
From: Tong Cai <caitong93@gmail.com>
Date: Mon, 7 Dec 2020 20:41:24 +0800
Subject: [PATCH 01/13] server: wait workers to start before draining parent.

Signed-off-by: Tong Cai <caitong93@gmail.com>
---
 include/envoy/server/listener_manager.h   |  3 +-
 source/server/listener_manager_impl.cc    | 14 +++---
 source/server/listener_manager_impl.h     |  2 +-
 source/server/server.cc                   | 22 +++++----
 test/mocks/server/listener_manager.h      |  2 +-
 test/server/listener_manager_impl_test.cc | 60 +++++++++++------------
 test/server/listener_manager_impl_test.h  |  3 +-
 7 files changed, 57 insertions(+), 49 deletions(-)

diff --git a/include/envoy/server/listener_manager.h b/include/envoy/server/listener_manager.h
index d76d73cf315ed..01fe285b1c867 100644
--- a/include/envoy/server/listener_manager.h
+++ b/include/envoy/server/listener_manager.h
@@ -202,8 +202,9 @@ class ListenerManager {
   /**
    * Start all workers accepting new connections on all added listeners.
    * @param guard_dog supplies the guard dog to use for thread watching.
+   * @param callback supplies the callback to complete server initialization.
    */
-  virtual void startWorkers(GuardDog& guard_dog) PURE;
+  virtual void startWorkers(GuardDog& guard_dog, std::function<void()> callback) PURE;
 
   /**
    * Stop all listeners from accepting new connections without actually removing any of them. This
diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc
index fb17e6810ed26..a358354539a12 100644
--- a/source/server/listener_manager_impl.cc
+++ b/source/server/listener_manager_impl.cc
@@ -884,7 +884,7 @@ bool ListenerManagerImpl::removeListenerInternal(const std::string& name,
   return true;
 }
 
-void ListenerManagerImpl::startWorkers(GuardDog& guard_dog) {
+void ListenerManagerImpl::startWorkers(GuardDog& guard_dog, std::function<void()> callback) {
   ENVOY_LOG(info, "all dependencies initialized. starting workers");
   ASSERT(!workers_started_);
   workers_started_ = true;
@@ -899,11 +899,13 @@ void ListenerManagerImpl::startWorkers(GuardDog& guard_dog) {
     ENVOY_LOG(debug, "starting worker {}", i);
     ASSERT(warming_listeners_.empty());
     for (const auto& listener : active_listeners_) {
-      addListenerToWorker(*worker, absl::nullopt, *listener, [this, listeners_pending_init]() {
-        if (--(*listeners_pending_init) == 0) {
-          stats_.workers_started_.set(1);
-        }
-      });
+      addListenerToWorker(*worker, absl::nullopt, *listener,
+                          [this, listeners_pending_init, callback]() {
+                            if (--(*listeners_pending_init) == 0) {
+                              stats_.workers_started_.set(1);
+                              callback();
+                            }
+                          });
     }
     worker->start(guard_dog);
     if (enable_dispatcher_stats_) {
diff --git a/source/server/listener_manager_impl.h b/source/server/listener_manager_impl.h
index c29a0f8478eaf..63bef43f0993e 100644
--- a/source/server/listener_manager_impl.h
+++ b/source/server/listener_manager_impl.h
@@ -193,7 +193,7 @@ class ListenerManagerImpl : public ListenerManager, Logger::Loggable<Logger::Id:
   listeners(ListenerState state = ListenerState::ACTIVE) override;
   uint64_t numConnections() const override;
   bool removeListener(const std::string& listener_name) override;
-  void startWorkers(GuardDog& guard_dog) override;
+  void startWorkers(GuardDog& guard_dog, std::function<void()> callback) override;
   void stopListeners(StopListenersType stop_listeners_type) override;
   void stopWorkers() override;
   void beginListenerUpdate() override { error_state_tracker_.clear(); }
diff --git a/source/server/server.cc b/source/server/server.cc
index 9b9606b3f4e14..8a42a5fff1e81 100644
--- a/source/server/server.cc
+++ b/source/server/server.cc
@@ -606,15 +606,19 @@ void InstanceImpl::onRuntimeReady() {
 }
 
 void InstanceImpl::startWorkers() {
-  listener_manager_->startWorkers(*worker_guard_dog_);
-  initialization_timer_->complete();
-  // Update server stats as soon as initialization is done.
-  updateServerStats();
-  workers_started_ = true;
-  // At this point we are ready to take traffic and all listening ports are up. Notify our parent
-  // if applicable that they can stop listening and drain.
-  restarter_.drainParentListeners();
-  drain_manager_->startParentShutdownSequence();
+  const auto workers_pending_init = std::make_shared<std::atomic<uint64_t>>(options_.concurrency());
+  listener_manager_->startWorkers(*worker_guard_dog_, [this, workers_pending_init]() {
+    if (--(*workers_pending_init) == 0) {
+      initialization_timer_->complete();
+      // Update server stats as soon as initialization is done.
+      updateServerStats();
+      workers_started_ = true;
+      // At this point we are ready to take traffic and all listening ports are up. Notify our
+      // parent if applicable that they can stop listening and drain.
+      restarter_.drainParentListeners();
+      drain_manager_->startParentShutdownSequence();
+    }
+  });
 }
 
 Runtime::LoaderPtr InstanceUtil::createRuntime(Instance& server,
diff --git a/test/mocks/server/listener_manager.h b/test/mocks/server/listener_manager.h
index 582be9ac9bac4..c7c855508f6da 100644
--- a/test/mocks/server/listener_manager.h
+++ b/test/mocks/server/listener_manager.h
@@ -21,7 +21,7 @@ class MockListenerManager : public ListenerManager {
               (ListenerState state));
   MOCK_METHOD(uint64_t, numConnections, (), (const));
   MOCK_METHOD(bool, removeListener, (const std::string& listener_name));
-  MOCK_METHOD(void, startWorkers, (GuardDog & guard_dog));
+  MOCK_METHOD(void, startWorkers, (GuardDog & guard_dog, std::function<void()> callback));
   MOCK_METHOD(void, stopListeners, (StopListenersType listeners_type));
   MOCK_METHOD(void, stopWorkers, ());
   MOCK_METHOD(void, beginListenerUpdate, ());
diff --git a/test/server/listener_manager_impl_test.cc b/test/server/listener_manager_impl_test.cc
index 18ab269953227..eb0215f91553f 100644
--- a/test/server/listener_manager_impl_test.cc
+++ b/test/server/listener_manager_impl_test.cc
@@ -335,7 +335,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, TransportSocketConnectTimeout) {
 TEST_F(ListenerManagerImplWithRealFiltersTest, UdpAddress) {
   EXPECT_CALL(*worker_, start(_));
   EXPECT_FALSE(manager_->isWorkerStarted());
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
   // Validate that there are no active listeners and workers are started.
   EXPECT_EQ(0, server_.stats_store_
                    .gauge("listener_manager.total_active_listeners",
@@ -873,7 +873,7 @@ version_info: version1
 )EOF");
 
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   // Now add new version listener foo after workers start, note it's fine that server_init_mgr is
   // initialized, as no target will be added to it.
@@ -960,7 +960,7 @@ filter_chains: {}
       .RetiresOnSaturation();
 
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   EXPECT_EQ(0, server_.stats_store_.counter("listener_manager.listener_create_success").value());
   checkStats(__LINE__, 1, 0, 0, 0, 1, 0, 0);
@@ -1102,7 +1102,7 @@ version_info: version2
   // Start workers.
   EXPECT_CALL(*worker_, addListener(_, _, _));
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
   // Validate that workers_started stat is still zero before workers set the status via
   // completion callback.
   EXPECT_EQ(0, server_.stats_store_
@@ -1307,7 +1307,7 @@ TEST_F(ListenerManagerImplTest, UpdateActiveToWarmAndBack) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   // Add and initialize foo listener.
   const std::string listener_foo_yaml = R"EOF(
@@ -1368,7 +1368,7 @@ TEST_F(ListenerManagerImplTest, AddReusableDrainingListener) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener directly into active.
   const std::string listener_foo_yaml = R"EOF(
@@ -1428,7 +1428,7 @@ TEST_F(ListenerManagerImplTest, AddClosedDrainingListener) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener directly into active.
   const std::string listener_foo_yaml = R"EOF(
@@ -1481,7 +1481,7 @@ TEST_F(ListenerManagerImplTest, BindToPortEqualToFalse) {
   InSequence s;
   ProdListenerComponentFactory real_listener_factory(server_);
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
   const std::string listener_foo_yaml = R"EOF(
 name: foo
 address:
@@ -1519,7 +1519,7 @@ TEST_F(ListenerManagerImplTest, ReusePortEqualToTrue) {
   InSequence s;
   ProdListenerComponentFactory real_listener_factory(server_);
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
   const std::string listener_foo_yaml = R"EOF(
 name: foo
 address:
@@ -1574,7 +1574,7 @@ TEST_F(ListenerManagerImplTest, CantBindSocket) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   const std::string listener_foo_yaml = R"EOF(
 name: foo
@@ -1627,7 +1627,7 @@ TEST_F(ListenerManagerImplTest, ConfigDumpWithExternalError) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   // Make sure the config dump is empty by default.
   ListenerManager::FailureStates empty_failure_state;
@@ -1663,7 +1663,7 @@ TEST_F(ListenerManagerImplTest, ListenerDraining) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   const std::string listener_foo_yaml = R"EOF(
 name: foo
@@ -1713,7 +1713,7 @@ TEST_F(ListenerManagerImplTest, RemoveListener) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   // Remove an unknown listener.
   EXPECT_FALSE(manager_->removeListener("unknown"));
@@ -1795,7 +1795,7 @@ TEST_F(ListenerManagerImplTest, StopListeners) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener in inbound direction.
   const std::string listener_foo_yaml = R"EOF(
@@ -1900,7 +1900,7 @@ TEST_F(ListenerManagerImplTest, StopAllListeners) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener into warming.
   const std::string listener_foo_yaml = R"EOF(
@@ -1948,7 +1948,7 @@ TEST_F(ListenerManagerImplTest, StopWarmingListener) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener into warming.
   const std::string listener_foo_yaml = R"EOF(
@@ -2005,7 +2005,7 @@ TEST_F(ListenerManagerImplTest, AddListenerFailure) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener into active.
   const std::string listener_foo_yaml = R"EOF(
@@ -2042,7 +2042,7 @@ TEST_F(ListenerManagerImplTest, StaticListenerAddFailure) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener into active.
   const std::string listener_foo_yaml = R"EOF(
@@ -2096,7 +2096,7 @@ TEST_F(ListenerManagerImplTest, DuplicateAddressDontBind) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener into warming.
   const std::string listener_foo_yaml = R"EOF(
@@ -4193,7 +4193,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, VerifyIgnoreExpirationWithCA) {
 TEST_F(ListenerManagerImplWithDispatcherStatsTest, DispatherStatsWithCorrectPrefix) {
   EXPECT_CALL(*worker_, start(_));
   EXPECT_CALL(*worker_, initializeStats(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 }
 
 TEST_F(ListenerManagerImplWithRealFiltersTest, ApiListener) {
@@ -4322,7 +4322,7 @@ TEST_F(ListenerManagerImplTest, StopInplaceWarmingListener) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener into warming.
   const std::string listener_foo_yaml = R"EOF(
@@ -4384,7 +4384,7 @@ TEST_F(ListenerManagerImplTest, RemoveInplaceUpdatingListener) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener into warming.
   const std::string listener_foo_yaml = R"EOF(
@@ -4453,7 +4453,7 @@ TEST_F(ListenerManagerImplTest, UpdateInplaceWarmingListener) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener into warming.
   const std::string listener_foo_yaml = R"EOF(
@@ -4516,7 +4516,7 @@ TEST_F(ListenerManagerImplTest, DrainageDuringInplaceUpdate) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener into warming.
   const std::string listener_foo_yaml = R"EOF(
@@ -4666,7 +4666,7 @@ TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest, TraditionalUpdateIfWo
 
 TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest, TraditionalUpdateIfAnyListenerIsNotTcp) {
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   auto listener_proto = createDefaultListener();
 
@@ -4691,7 +4691,7 @@ TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest,
        TraditionalUpdateIfImplicitTlsInspectorChanges) {
 
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   auto listener_proto = createDefaultListener();
 
@@ -4717,7 +4717,7 @@ TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest,
        TraditionalUpdateIfImplicitProxyProtocolChanges) {
 
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   auto listener_proto = createDefaultListener();
 
@@ -4737,7 +4737,7 @@ TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest,
 
 TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest, TraditionalUpdateOnZeroFilterChain) {
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   auto listener_proto = createDefaultListener();
 
@@ -4761,7 +4761,7 @@ TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest, TraditionalUpdateOnZe
 TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest,
        TraditionalUpdateIfListenerConfigHasUpdateOtherThanFilterChain) {
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   auto listener_proto = createDefaultListener();
 
@@ -4785,7 +4785,7 @@ TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest,
 TEST_F(ListenerManagerImplTest, RuntimeDisabledInPlaceUpdateFallbacksToTraditionalUpdate) {
   InSequence s;
   EXPECT_CALL(*worker_, start(_));
-  manager_->startWorkers(guard_dog_);
+  manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener.
   const std::string listener_foo_yaml = R"EOF(
diff --git a/test/server/listener_manager_impl_test.h b/test/server/listener_manager_impl_test.h
index 9b22f3109bf85..c739d97c17529 100644
--- a/test/server/listener_manager_impl_test.h
+++ b/test/server/listener_manager_impl_test.h
@@ -53,7 +53,7 @@ class ListenerHandle {
 
 class ListenerManagerImplTest : public testing::Test {
 protected:
-  ListenerManagerImplTest() : api_(Api::createApiForTest(server_.api_.random_)) {}
+  ListenerManagerImplTest() : api_(Api::createApiForTest(server_.api_.random_)), callback_([] {}) {}
 
   void SetUp() override {
     ON_CALL(server_, api()).WillByDefault(ReturnRef(*api_));
@@ -296,6 +296,7 @@ class ListenerManagerImplTest : public testing::Test {
   std::unique_ptr<Network::MockConnectionSocket> socket_;
   uint64_t listener_tag_{1};
   bool enable_dispatcher_stats_{false};
+  std::function<void()> callback_;
 };
 
 } // namespace Server

From 3f09b9d0669b2e51863521ad50733600ebdb3c3e Mon Sep 17 00:00:00 2001
From: Tong Cai <caitong93@gmail.com>
Date: Wed, 9 Dec 2020 21:14:42 +0800
Subject: [PATCH 02/13] server: fix integration test

Signed-off-by: Tong Cai <caitong93@gmail.com>
---
 source/server/server.cc | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/source/server/server.cc b/source/server/server.cc
index 8a42a5fff1e81..e3bd69be644b0 100644
--- a/source/server/server.cc
+++ b/source/server/server.cc
@@ -606,18 +606,16 @@ void InstanceImpl::onRuntimeReady() {
 }
 
 void InstanceImpl::startWorkers() {
-  const auto workers_pending_init = std::make_shared<std::atomic<uint64_t>>(options_.concurrency());
-  listener_manager_->startWorkers(*worker_guard_dog_, [this, workers_pending_init]() {
-    if (--(*workers_pending_init) == 0) {
-      initialization_timer_->complete();
-      // Update server stats as soon as initialization is done.
-      updateServerStats();
-      workers_started_ = true;
-      // At this point we are ready to take traffic and all listening ports are up. Notify our
-      // parent if applicable that they can stop listening and drain.
-      restarter_.drainParentListeners();
-      drain_manager_->startParentShutdownSequence();
-    }
+  // The callback will be called after workers are started.
+  listener_manager_->startWorkers(*worker_guard_dog_, [this]() {
+    initialization_timer_->complete();
+    // Update server stats as soon as initialization is done.
+    updateServerStats();
+    workers_started_ = true;
+    // At this point we are ready to take traffic and all listening ports are up. Notify our
+    // parent if applicable that they can stop listening and drain.
+    restarter_.drainParentListeners();
+    drain_manager_->startParentShutdownSequence();
   });
 }
 

From 7627986a61618f422c75faa4d63d8dfb91380c1d Mon Sep 17 00:00:00 2001
From: Tong Cai <caitong93@gmail.com>
Date: Thu, 10 Dec 2020 17:02:00 +0800
Subject: [PATCH 03/13] server: fix server_test

Signed-off-by: Tong Cai <caitong93@gmail.com>
---
 source/server/listener_manager_impl.cc | 1 +
 source/server/server.cc                | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/source/server/listener_manager_impl.cc b/source/server/listener_manager_impl.cc
index a358354539a12..eb588456a9b9f 100644
--- a/source/server/listener_manager_impl.cc
+++ b/source/server/listener_manager_impl.cc
@@ -915,6 +915,7 @@ void ListenerManagerImpl::startWorkers(GuardDog& guard_dog, std::function<void()
   }
   if (active_listeners_.empty()) {
     stats_.workers_started_.set(1);
+    callback();
   }
 }
 
diff --git a/source/server/server.cc b/source/server/server.cc
index e3bd69be644b0..fed69fd227b17 100644
--- a/source/server/server.cc
+++ b/source/server/server.cc
@@ -608,6 +608,10 @@ void InstanceImpl::onRuntimeReady() {
 void InstanceImpl::startWorkers() {
   // The callback will be called after workers are started.
   listener_manager_->startWorkers(*worker_guard_dog_, [this]() {
+    if (isShutdown()) {
+      return;
+    }
+
     initialization_timer_->complete();
     // Update server stats as soon as initialization is done.
     updateServerStats();

From e6e3c90345dc3ba2045949d766fa4a273171ca8d Mon Sep 17 00:00:00 2001
From: Tong Cai <caitong93@gmail.com>
Date: Thu, 10 Dec 2020 22:29:14 +0800
Subject: [PATCH 04/13] server: add WorkerStarted stage

Signed-off-by: Tong Cai <caitong93@gmail.com>
---
 include/envoy/server/lifecycle_notifier.h |  5 +++++
 source/server/server.cc                   |  1 +
 test/server/server_test.cc                | 25 ++++++++++++++++-------
 3 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/include/envoy/server/lifecycle_notifier.h b/include/envoy/server/lifecycle_notifier.h
index 94b53af7311ec..47ba31b9d1cd9 100644
--- a/include/envoy/server/lifecycle_notifier.h
+++ b/include/envoy/server/lifecycle_notifier.h
@@ -26,6 +26,11 @@ class ServerLifecycleNotifier {
      */
     PostInit,
 
+    /**
+     * All workers have started.
+     */
+    WorkerStarted,
+
     /**
      * The server instance is being shutdown and the dispatcher is about to exit.
      * This provides listeners a last chance to run a callback on the main dispatcher.
diff --git a/source/server/server.cc b/source/server/server.cc
index fed69fd227b17..d92f577c60fb9 100644
--- a/source/server/server.cc
+++ b/source/server/server.cc
@@ -616,6 +616,7 @@ void InstanceImpl::startWorkers() {
     // Update server stats as soon as initialization is done.
     updateServerStats();
     workers_started_ = true;
+    notifyCallbacksForStage(Stage::WorkerStarted);
     // At this point we are ready to take traffic and all listening ports are up. Notify our
     // parent if applicable that they can stop listening and drain.
     restarter_.drainParentListeners();
diff --git a/test/server/server_test.cc b/test/server/server_test.cc
index 1db932a7c8c4c..f0b9b7e616bfe 100644
--- a/test/server/server_test.cc
+++ b/test/server/server_test.cc
@@ -358,8 +358,10 @@ TEST_P(ServerInstanceImplTest, EmptyShutdownLifecycleNotifications) {
 }
 
 TEST_P(ServerInstanceImplTest, LifecycleNotifications) {
-  bool startup = false, post_init = false, shutdown = false, shutdown_with_completion = false;
-  absl::Notification started, post_init_fired, shutdown_begin, completion_block, completion_done;
+  bool startup = false, post_init = false, worker_started = false, shutdown = false,
+       shutdown_with_completion = false;
+  absl::Notification started, post_init_fired, worker_started_fired, shutdown_begin,
+      completion_block, completion_done;
 
   // Run the server in a separate thread so we can test different lifecycle stages.
   auto server_thread = Thread::threadFactoryForTest().createThread([&] {
@@ -372,11 +374,15 @@ TEST_P(ServerInstanceImplTest, LifecycleNotifications) {
       post_init = true;
       post_init_fired.Notify();
     });
-    auto handle3 = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit, [&] {
+    auto handle3 = server_->registerCallback(ServerLifecycleNotifier::Stage::WorkerStarted, [&] {
+      worker_started = true;
+      worker_started_fired.Notify();
+    });
+    auto handle4 = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit, [&] {
       shutdown = true;
       shutdown_begin.Notify();
     });
-    auto handle4 = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit,
+    auto handle5 = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit,
                                              [&](Event::PostCb completion_cb) {
                                                // Block till we're told to complete
                                                completion_block.WaitForNotification();
@@ -384,17 +390,18 @@ TEST_P(ServerInstanceImplTest, LifecycleNotifications) {
                                                server_->dispatcher().post(completion_cb);
                                                completion_done.Notify();
                                              });
-    auto handle5 =
+    auto handle6 =
         server_->registerCallback(ServerLifecycleNotifier::Stage::Startup, [&] { FAIL(); });
-    handle5 = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit,
+    handle6 = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit,
                                         [&](Event::PostCb) { FAIL(); });
-    handle5 = nullptr;
+    handle6 = nullptr;
 
     server_->run();
     handle1 = nullptr;
     handle2 = nullptr;
     handle3 = nullptr;
     handle4 = nullptr;
+    handle5 = nullptr;
     server_ = nullptr;
     thread_local_ = nullptr;
   });
@@ -409,6 +416,10 @@ TEST_P(ServerInstanceImplTest, LifecycleNotifications) {
   EXPECT_TRUE(post_init);
   EXPECT_FALSE(shutdown);
 
+  worker_started_fired.WaitForNotification();
+  EXPECT_TRUE(worker_started);
+  EXPECT_FALSE(shutdown);
+
   server_->dispatcher().post([&] { server_->shutdown(); });
   shutdown_begin.WaitForNotification();
   EXPECT_TRUE(shutdown);

From f84c3f86b3b2fbe0f82204c73a6cfa6c64134858 Mon Sep 17 00:00:00 2001
From: Tong Cai <caitong93@gmail.com>
Date: Fri, 11 Dec 2020 22:46:05 +0800
Subject: [PATCH 05/13] server: fix tests

Signed-off-by: Tong Cai <caitong93@gmail.com>
---
 source/server/server.cc    | 12 +++++++++++-
 source/server/server.h     |  3 +++
 test/server/server_test.cc | 16 ++++++++++++----
 3 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/source/server/server.cc b/source/server/server.cc
index d92f577c60fb9..0f299eeb74d05 100644
--- a/source/server/server.cc
+++ b/source/server/server.cc
@@ -717,6 +717,11 @@ void InstanceImpl::run() {
   // startup (see RunHelperTest in server_test.cc).
   const auto run_helper = RunHelper(*this, options_, *dispatcher_, clusterManager(),
                                     access_log_manager_, init_manager_, overloadManager(), [this] {
+                                      if (!startup_) {
+                                        notifyCallbacksForStage(Stage::Startup);
+                                        startup_ = true;
+                                      }
+
                                       notifyCallbacksForStage(Stage::PostInit);
                                       startWorkers();
                                     });
@@ -725,7 +730,12 @@ void InstanceImpl::run() {
   ENVOY_LOG(info, "starting main dispatch loop");
   auto watchdog = main_thread_guard_dog_->createWatchDog(api_->threadFactory().currentThreadId(),
                                                          "main_thread", *dispatcher_);
-  dispatcher_->post([this] { notifyCallbacksForStage(Stage::Startup); });
+  dispatcher_->post([this] {
+    if (!startup_) {
+      notifyCallbacksForStage(Stage::Startup);
+      startup_ = true;
+    }
+  });
   dispatcher_->run(Event::Dispatcher::RunType::Block);
   ENVOY_LOG(info, "main dispatch loop exited");
   main_thread_guard_dog_->stopWatching(watchdog);
diff --git a/source/server/server.h b/source/server/server.h
index 0409b9db529bf..ffc2ac057de2b 100644
--- a/source/server/server.h
+++ b/source/server/server.h
@@ -373,6 +373,9 @@ class InstanceImpl final : Logger::Loggable<Logger::Id::main>,
     LifecycleCallbackHandle(std::list<T>& callbacks, T& callback)
         : RaiiListElement<T>(callbacks, callback) {}
   };
+
+  // startup_ is true means Startup notifications have been called.
+  bool startup_{};
 };
 
 // Local implementation of Stats::MetricSnapshot used to flush metrics to sinks. We could
diff --git a/test/server/server_test.cc b/test/server/server_test.cc
index f0b9b7e616bfe..e09dc59aa7807 100644
--- a/test/server/server_test.cc
+++ b/test/server/server_test.cc
@@ -360,8 +360,10 @@ TEST_P(ServerInstanceImplTest, EmptyShutdownLifecycleNotifications) {
 TEST_P(ServerInstanceImplTest, LifecycleNotifications) {
   bool startup = false, post_init = false, worker_started = false, shutdown = false,
        shutdown_with_completion = false;
-  absl::Notification started, post_init_fired, worker_started_fired, shutdown_begin,
-      completion_block, completion_done;
+  absl::Notification started, post_init_fired, worker_started_fired, worker_started_block,
+      shutdown_begin, completion_block, completion_done;
+  // Expect drainParentListeners not to be called before workers start.
+  EXPECT_CALL(restart_, drainParentListeners).Times(0);
 
   // Run the server in a separate thread so we can test different lifecycle stages.
   auto server_thread = Thread::threadFactoryForTest().createThread([&] {
@@ -377,6 +379,7 @@ TEST_P(ServerInstanceImplTest, LifecycleNotifications) {
     auto handle3 = server_->registerCallback(ServerLifecycleNotifier::Stage::WorkerStarted, [&] {
       worker_started = true;
       worker_started_fired.Notify();
+      worker_started_block.WaitForNotification();
     });
     auto handle4 = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit, [&] {
       shutdown = true;
@@ -409,8 +412,8 @@ TEST_P(ServerInstanceImplTest, LifecycleNotifications) {
   started.WaitForNotification();
   EXPECT_TRUE(startup);
   EXPECT_FALSE(shutdown);
-  EXPECT_TRUE(TestUtility::findGauge(stats_store_, "server.state")->used());
-  EXPECT_EQ(0L, TestUtility::findGauge(stats_store_, "server.state")->value());
+  // The first flushing is after workers start.
+  EXPECT_FALSE(TestUtility::findGauge(stats_store_, "server.state")->used());
 
   post_init_fired.WaitForNotification();
   EXPECT_TRUE(post_init);
@@ -419,6 +422,11 @@ TEST_P(ServerInstanceImplTest, LifecycleNotifications) {
   worker_started_fired.WaitForNotification();
   EXPECT_TRUE(worker_started);
   EXPECT_FALSE(shutdown);
+  EXPECT_TRUE(TestUtility::findGauge(stats_store_, "server.state")->used());
+  EXPECT_EQ(0L, TestUtility::findGauge(stats_store_, "server.state")->value());
+
+  EXPECT_CALL(restart_, drainParentListeners);
+  worker_started_block.Notify();
 
   server_->dispatcher().post([&] { server_->shutdown(); });
   shutdown_begin.WaitForNotification();

From 871c415268f7a712fbb91fd9d7bccacafa3e0557 Mon Sep 17 00:00:00 2001
From: Tong Cai <caitong93@gmail.com>
Date: Sat, 12 Dec 2020 12:31:38 +0800
Subject: [PATCH 06/13] server: fix flakey test

Signed-off-by: Tong Cai <caitong93@gmail.com>
---
 test/server/server_test.cc | 2 --
 1 file changed, 2 deletions(-)

diff --git a/test/server/server_test.cc b/test/server/server_test.cc
index e09dc59aa7807..2e6ff03e33c41 100644
--- a/test/server/server_test.cc
+++ b/test/server/server_test.cc
@@ -412,8 +412,6 @@ TEST_P(ServerInstanceImplTest, LifecycleNotifications) {
   started.WaitForNotification();
   EXPECT_TRUE(startup);
   EXPECT_FALSE(shutdown);
-  // The first flushing is after workers start.
-  EXPECT_FALSE(TestUtility::findGauge(stats_store_, "server.state")->used());
 
   post_init_fired.WaitForNotification();
   EXPECT_TRUE(post_init);

From 1143ecc38378dab76f085b2dc5ce3c6e334e4c0f Mon Sep 17 00:00:00 2001
From: Tong Cai <caitong93@gmail.com>
Date: Mon, 14 Dec 2020 21:50:51 +0800
Subject: [PATCH 07/13] Resolve comments.

Signed-off-by: Tong Cai <caitong93@gmail.com>
---
 include/envoy/server/lifecycle_notifier.h |  2 +-
 source/server/server.cc                   | 16 ++++++++----
 source/server/server.h                    |  4 +--
 test/server/listener_manager_impl_test.cc | 30 +++++++++++++++++++++++
 test/server/listener_manager_impl_test.h  |  4 +--
 test/server/server_test.cc                | 18 +++++++-------
 6 files changed, 54 insertions(+), 20 deletions(-)

diff --git a/include/envoy/server/lifecycle_notifier.h b/include/envoy/server/lifecycle_notifier.h
index 47ba31b9d1cd9..e16ee9d2200be 100644
--- a/include/envoy/server/lifecycle_notifier.h
+++ b/include/envoy/server/lifecycle_notifier.h
@@ -29,7 +29,7 @@ class ServerLifecycleNotifier {
     /**
      * All workers have started.
      */
-    WorkerStarted,
+    WorkersStarted,
 
     /**
      * The server instance is being shutdown and the dispatcher is about to exit.
diff --git a/source/server/server.cc b/source/server/server.cc
index 0f299eeb74d05..a7a2101e10e6e 100644
--- a/source/server/server.cc
+++ b/source/server/server.cc
@@ -616,7 +616,7 @@ void InstanceImpl::startWorkers() {
     // Update server stats as soon as initialization is done.
     updateServerStats();
     workers_started_ = true;
-    notifyCallbacksForStage(Stage::WorkerStarted);
+    notifyCallbacksForStage(Stage::WorkersStarted);
     // At this point we are ready to take traffic and all listening ports are up. Notify our
     // parent if applicable that they can stop listening and drain.
     restarter_.drainParentListeners();
@@ -717,9 +717,13 @@ void InstanceImpl::run() {
   // startup (see RunHelperTest in server_test.cc).
   const auto run_helper = RunHelper(*this, options_, *dispatcher_, clusterManager(),
                                     access_log_manager_, init_manager_, overloadManager(), [this] {
-                                      if (!startup_) {
+                                      // This ensures Startup event to be sent first, required by
+                                      // tests. In static configuration, cluster manager will
+                                      // initialize init_manager immediately when setInitializedCb
+                                      // is called.
+                                      if (!startup_lifecycle_event_raised_) {
                                         notifyCallbacksForStage(Stage::Startup);
-                                        startup_ = true;
+                                        startup_lifecycle_event_raised_ = true;
                                       }
 
                                       notifyCallbacksForStage(Stage::PostInit);
@@ -731,9 +735,11 @@ void InstanceImpl::run() {
   auto watchdog = main_thread_guard_dog_->createWatchDog(api_->threadFactory().currentThreadId(),
                                                          "main_thread", *dispatcher_);
   dispatcher_->post([this] {
-    if (!startup_) {
+    // It's possible that Startup event is already raised during RunHelper
+    // construction.
+    if (!startup_lifecycle_event_raised_) {
       notifyCallbacksForStage(Stage::Startup);
-      startup_ = true;
+      startup_lifecycle_event_raised_ = true;
     }
   });
   dispatcher_->run(Event::Dispatcher::RunType::Block);
diff --git a/source/server/server.h b/source/server/server.h
index ffc2ac057de2b..508bfd75d07bc 100644
--- a/source/server/server.h
+++ b/source/server/server.h
@@ -364,6 +364,7 @@ class InstanceImpl final : Logger::Loggable<Logger::Id::main>,
   // initialization_time is a histogram for tracking the initialization time across hot restarts
   // whenever we have support for histogram merge across hot restarts.
   Stats::TimespanPtr initialization_timer_;
+  bool startup_lifecycle_event_raised_{};
 
   ServerFactoryContextImpl server_contexts_;
 
@@ -373,9 +374,6 @@ class InstanceImpl final : Logger::Loggable<Logger::Id::main>,
     LifecycleCallbackHandle(std::list<T>& callbacks, T& callback)
         : RaiiListElement<T>(callbacks, callback) {}
   };
-
-  // startup_ is true means Startup notifications have been called.
-  bool startup_{};
 };
 
 // Local implementation of Stats::MetricSnapshot used to flush metrics to sinks. We could
diff --git a/test/server/listener_manager_impl_test.cc b/test/server/listener_manager_impl_test.cc
index eb0215f91553f..adc5a5bd10117 100644
--- a/test/server/listener_manager_impl_test.cc
+++ b/test/server/listener_manager_impl_test.cc
@@ -335,6 +335,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, TransportSocketConnectTimeout) {
 TEST_F(ListenerManagerImplWithRealFiltersTest, UdpAddress) {
   EXPECT_CALL(*worker_, start(_));
   EXPECT_FALSE(manager_->isWorkerStarted());
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
   // Validate that there are no active listeners and workers are started.
   EXPECT_EQ(0, server_.stats_store_
@@ -873,6 +874,7 @@ version_info: version1
 )EOF");
 
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   // Now add new version listener foo after workers start, note it's fine that server_init_mgr is
@@ -960,6 +962,7 @@ filter_chains: {}
       .RetiresOnSaturation();
 
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   EXPECT_EQ(0, server_.stats_store_.counter("listener_manager.listener_create_success").value());
@@ -1102,6 +1105,7 @@ version_info: version2
   // Start workers.
   EXPECT_CALL(*worker_, addListener(_, _, _));
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
   // Validate that workers_started stat is still zero before workers set the status via
   // completion callback.
@@ -1307,6 +1311,7 @@ TEST_F(ListenerManagerImplTest, UpdateActiveToWarmAndBack) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   // Add and initialize foo listener.
@@ -1368,6 +1373,7 @@ TEST_F(ListenerManagerImplTest, AddReusableDrainingListener) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener directly into active.
@@ -1428,6 +1434,7 @@ TEST_F(ListenerManagerImplTest, AddClosedDrainingListener) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener directly into active.
@@ -1481,6 +1488,7 @@ TEST_F(ListenerManagerImplTest, BindToPortEqualToFalse) {
   InSequence s;
   ProdListenerComponentFactory real_listener_factory(server_);
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
   const std::string listener_foo_yaml = R"EOF(
 name: foo
@@ -1519,6 +1527,7 @@ TEST_F(ListenerManagerImplTest, ReusePortEqualToTrue) {
   InSequence s;
   ProdListenerComponentFactory real_listener_factory(server_);
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
   const std::string listener_foo_yaml = R"EOF(
 name: foo
@@ -1574,6 +1583,7 @@ TEST_F(ListenerManagerImplTest, CantBindSocket) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   const std::string listener_foo_yaml = R"EOF(
@@ -1627,6 +1637,7 @@ TEST_F(ListenerManagerImplTest, ConfigDumpWithExternalError) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   // Make sure the config dump is empty by default.
@@ -1663,6 +1674,7 @@ TEST_F(ListenerManagerImplTest, ListenerDraining) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   const std::string listener_foo_yaml = R"EOF(
@@ -1713,6 +1725,7 @@ TEST_F(ListenerManagerImplTest, RemoveListener) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   // Remove an unknown listener.
@@ -1795,6 +1808,7 @@ TEST_F(ListenerManagerImplTest, StopListeners) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener in inbound direction.
@@ -1900,6 +1914,7 @@ TEST_F(ListenerManagerImplTest, StopAllListeners) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener into warming.
@@ -1948,6 +1963,7 @@ TEST_F(ListenerManagerImplTest, StopWarmingListener) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener into warming.
@@ -2005,6 +2021,7 @@ TEST_F(ListenerManagerImplTest, AddListenerFailure) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener into active.
@@ -2042,6 +2059,7 @@ TEST_F(ListenerManagerImplTest, StaticListenerAddFailure) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener into active.
@@ -2096,6 +2114,7 @@ TEST_F(ListenerManagerImplTest, DuplicateAddressDontBind) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener into warming.
@@ -4193,6 +4212,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, VerifyIgnoreExpirationWithCA) {
 TEST_F(ListenerManagerImplWithDispatcherStatsTest, DispatherStatsWithCorrectPrefix) {
   EXPECT_CALL(*worker_, start(_));
   EXPECT_CALL(*worker_, initializeStats(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 }
 
@@ -4322,6 +4342,7 @@ TEST_F(ListenerManagerImplTest, StopInplaceWarmingListener) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener into warming.
@@ -4384,6 +4405,7 @@ TEST_F(ListenerManagerImplTest, RemoveInplaceUpdatingListener) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener into warming.
@@ -4453,6 +4475,7 @@ TEST_F(ListenerManagerImplTest, UpdateInplaceWarmingListener) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener into warming.
@@ -4516,6 +4539,7 @@ TEST_F(ListenerManagerImplTest, DrainageDuringInplaceUpdate) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener into warming.
@@ -4666,6 +4690,7 @@ TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest, TraditionalUpdateIfWo
 
 TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest, TraditionalUpdateIfAnyListenerIsNotTcp) {
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   auto listener_proto = createDefaultListener();
@@ -4691,6 +4716,7 @@ TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest,
        TraditionalUpdateIfImplicitTlsInspectorChanges) {
 
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   auto listener_proto = createDefaultListener();
@@ -4717,6 +4743,7 @@ TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest,
        TraditionalUpdateIfImplicitProxyProtocolChanges) {
 
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   auto listener_proto = createDefaultListener();
@@ -4737,6 +4764,7 @@ TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest,
 
 TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest, TraditionalUpdateOnZeroFilterChain) {
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   auto listener_proto = createDefaultListener();
@@ -4761,6 +4789,7 @@ TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest, TraditionalUpdateOnZe
 TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest,
        TraditionalUpdateIfListenerConfigHasUpdateOtherThanFilterChain) {
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   auto listener_proto = createDefaultListener();
@@ -4785,6 +4814,7 @@ TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest,
 TEST_F(ListenerManagerImplTest, RuntimeDisabledInPlaceUpdateFallbacksToTraditionalUpdate) {
   InSequence s;
   EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call);
   manager_->startWorkers(guard_dog_, callback_);
 
   // Add foo listener.
diff --git a/test/server/listener_manager_impl_test.h b/test/server/listener_manager_impl_test.h
index c739d97c17529..3dc638ea7ed97 100644
--- a/test/server/listener_manager_impl_test.h
+++ b/test/server/listener_manager_impl_test.h
@@ -53,7 +53,7 @@ class ListenerHandle {
 
 class ListenerManagerImplTest : public testing::Test {
 protected:
-  ListenerManagerImplTest() : api_(Api::createApiForTest(server_.api_.random_)), callback_([] {}) {}
+  ListenerManagerImplTest() : api_(Api::createApiForTest(server_.api_.random_)) {}
 
   void SetUp() override {
     ON_CALL(server_, api()).WillByDefault(ReturnRef(*api_));
@@ -296,7 +296,7 @@ class ListenerManagerImplTest : public testing::Test {
   std::unique_ptr<Network::MockConnectionSocket> socket_;
   uint64_t listener_tag_{1};
   bool enable_dispatcher_stats_{false};
-  std::function<void()> callback_;
+  MockFunction<void()> callback_;
 };
 
 } // namespace Server
diff --git a/test/server/server_test.cc b/test/server/server_test.cc
index 2e6ff03e33c41..e770cd589acce 100644
--- a/test/server/server_test.cc
+++ b/test/server/server_test.cc
@@ -358,9 +358,9 @@ TEST_P(ServerInstanceImplTest, EmptyShutdownLifecycleNotifications) {
 }
 
 TEST_P(ServerInstanceImplTest, LifecycleNotifications) {
-  bool startup = false, post_init = false, worker_started = false, shutdown = false,
+  bool startup = false, post_init = false, workers_started = false, shutdown = false,
        shutdown_with_completion = false;
-  absl::Notification started, post_init_fired, worker_started_fired, worker_started_block,
+  absl::Notification started, post_init_fired, workers_started_fired, workers_started_block,
       shutdown_begin, completion_block, completion_done;
   // Expect drainParentListeners not to be called before workers start.
   EXPECT_CALL(restart_, drainParentListeners).Times(0);
@@ -376,10 +376,10 @@ TEST_P(ServerInstanceImplTest, LifecycleNotifications) {
       post_init = true;
       post_init_fired.Notify();
     });
-    auto handle3 = server_->registerCallback(ServerLifecycleNotifier::Stage::WorkerStarted, [&] {
-      worker_started = true;
-      worker_started_fired.Notify();
-      worker_started_block.WaitForNotification();
+    auto handle3 = server_->registerCallback(ServerLifecycleNotifier::Stage::WorkersStarted, [&] {
+      workers_started = true;
+      workers_started_fired.Notify();
+      workers_started_block.WaitForNotification();
     });
     auto handle4 = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit, [&] {
       shutdown = true;
@@ -417,14 +417,14 @@ TEST_P(ServerInstanceImplTest, LifecycleNotifications) {
   EXPECT_TRUE(post_init);
   EXPECT_FALSE(shutdown);
 
-  worker_started_fired.WaitForNotification();
-  EXPECT_TRUE(worker_started);
+  workers_started_fired.WaitForNotification();
+  EXPECT_TRUE(workers_started);
   EXPECT_FALSE(shutdown);
   EXPECT_TRUE(TestUtility::findGauge(stats_store_, "server.state")->used());
   EXPECT_EQ(0L, TestUtility::findGauge(stats_store_, "server.state")->value());
 
   EXPECT_CALL(restart_, drainParentListeners);
-  worker_started_block.Notify();
+  workers_started_block.Notify();
 
   server_->dispatcher().post([&] { server_->shutdown(); });
   shutdown_begin.WaitForNotification();

From 50c394617c7aa74ce35d06ea235871b23db4535e Mon Sep 17 00:00:00 2001
From: Tong Cai <caitong93@gmail.com>
Date: Tue, 15 Dec 2020 00:43:54 +0800
Subject: [PATCH 08/13] Fix listener_manager_impl_test.

Signed-off-by: Tong Cai <caitong93@gmail.com>
---
 test/server/listener_manager_impl_test.cc | 98 +++++++++--------------
 test/server/listener_manager_impl_test.h  |  2 +-
 2 files changed, 39 insertions(+), 61 deletions(-)

diff --git a/test/server/listener_manager_impl_test.cc b/test/server/listener_manager_impl_test.cc
index adc5a5bd10117..1ec82fe01979f 100644
--- a/test/server/listener_manager_impl_test.cc
+++ b/test/server/listener_manager_impl_test.cc
@@ -335,8 +335,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, TransportSocketConnectTimeout) {
 TEST_F(ListenerManagerImplWithRealFiltersTest, UdpAddress) {
   EXPECT_CALL(*worker_, start(_));
   EXPECT_FALSE(manager_->isWorkerStarted());
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
   // Validate that there are no active listeners and workers are started.
   EXPECT_EQ(0, server_.stats_store_
                    .gauge("listener_manager.total_active_listeners",
@@ -874,8 +873,7 @@ version_info: version1
 )EOF");
 
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   // Now add new version listener foo after workers start, note it's fine that server_init_mgr is
   // initialized, as no target will be added to it.
@@ -962,8 +960,7 @@ filter_chains: {}
       .RetiresOnSaturation();
 
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   EXPECT_EQ(0, server_.stats_store_.counter("listener_manager.listener_create_success").value());
   checkStats(__LINE__, 1, 0, 0, 0, 1, 0, 0);
@@ -1105,8 +1102,7 @@ version_info: version2
   // Start workers.
   EXPECT_CALL(*worker_, addListener(_, _, _));
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
   // Validate that workers_started stat is still zero before workers set the status via
   // completion callback.
   EXPECT_EQ(0, server_.stats_store_
@@ -1311,8 +1307,7 @@ TEST_F(ListenerManagerImplTest, UpdateActiveToWarmAndBack) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   // Add and initialize foo listener.
   const std::string listener_foo_yaml = R"EOF(
@@ -1373,8 +1368,7 @@ TEST_F(ListenerManagerImplTest, AddReusableDrainingListener) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   // Add foo listener directly into active.
   const std::string listener_foo_yaml = R"EOF(
@@ -1434,8 +1428,7 @@ TEST_F(ListenerManagerImplTest, AddClosedDrainingListener) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   // Add foo listener directly into active.
   const std::string listener_foo_yaml = R"EOF(
@@ -1488,8 +1481,7 @@ TEST_F(ListenerManagerImplTest, BindToPortEqualToFalse) {
   InSequence s;
   ProdListenerComponentFactory real_listener_factory(server_);
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
   const std::string listener_foo_yaml = R"EOF(
 name: foo
 address:
@@ -1527,8 +1519,7 @@ TEST_F(ListenerManagerImplTest, ReusePortEqualToTrue) {
   InSequence s;
   ProdListenerComponentFactory real_listener_factory(server_);
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
   const std::string listener_foo_yaml = R"EOF(
 name: foo
 address:
@@ -1583,8 +1574,7 @@ TEST_F(ListenerManagerImplTest, CantBindSocket) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   const std::string listener_foo_yaml = R"EOF(
 name: foo
@@ -1637,8 +1627,7 @@ TEST_F(ListenerManagerImplTest, ConfigDumpWithExternalError) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   // Make sure the config dump is empty by default.
   ListenerManager::FailureStates empty_failure_state;
@@ -1674,8 +1663,7 @@ TEST_F(ListenerManagerImplTest, ListenerDraining) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   const std::string listener_foo_yaml = R"EOF(
 name: foo
@@ -1725,8 +1713,7 @@ TEST_F(ListenerManagerImplTest, RemoveListener) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   // Remove an unknown listener.
   EXPECT_FALSE(manager_->removeListener("unknown"));
@@ -1808,8 +1795,7 @@ TEST_F(ListenerManagerImplTest, StopListeners) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   // Add foo listener in inbound direction.
   const std::string listener_foo_yaml = R"EOF(
@@ -1914,8 +1900,7 @@ TEST_F(ListenerManagerImplTest, StopAllListeners) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   // Add foo listener into warming.
   const std::string listener_foo_yaml = R"EOF(
@@ -1963,8 +1948,7 @@ TEST_F(ListenerManagerImplTest, StopWarmingListener) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   // Add foo listener into warming.
   const std::string listener_foo_yaml = R"EOF(
@@ -2021,8 +2005,7 @@ TEST_F(ListenerManagerImplTest, AddListenerFailure) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   // Add foo listener into active.
   const std::string listener_foo_yaml = R"EOF(
@@ -2059,8 +2042,7 @@ TEST_F(ListenerManagerImplTest, StaticListenerAddFailure) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   // Add foo listener into active.
   const std::string listener_foo_yaml = R"EOF(
@@ -2114,8 +2096,7 @@ TEST_F(ListenerManagerImplTest, DuplicateAddressDontBind) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   // Add foo listener into warming.
   const std::string listener_foo_yaml = R"EOF(
@@ -4212,8 +4193,7 @@ TEST_F(ListenerManagerImplWithRealFiltersTest, VerifyIgnoreExpirationWithCA) {
 TEST_F(ListenerManagerImplWithDispatcherStatsTest, DispatherStatsWithCorrectPrefix) {
   EXPECT_CALL(*worker_, start(_));
   EXPECT_CALL(*worker_, initializeStats(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 }
 
 TEST_F(ListenerManagerImplWithRealFiltersTest, ApiListener) {
@@ -4342,8 +4322,7 @@ TEST_F(ListenerManagerImplTest, StopInplaceWarmingListener) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   // Add foo listener into warming.
   const std::string listener_foo_yaml = R"EOF(
@@ -4405,8 +4384,7 @@ TEST_F(ListenerManagerImplTest, RemoveInplaceUpdatingListener) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   // Add foo listener into warming.
   const std::string listener_foo_yaml = R"EOF(
@@ -4475,8 +4453,7 @@ TEST_F(ListenerManagerImplTest, UpdateInplaceWarmingListener) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   // Add foo listener into warming.
   const std::string listener_foo_yaml = R"EOF(
@@ -4539,8 +4516,7 @@ TEST_F(ListenerManagerImplTest, DrainageDuringInplaceUpdate) {
   InSequence s;
 
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   // Add foo listener into warming.
   const std::string listener_foo_yaml = R"EOF(
@@ -4690,8 +4666,7 @@ TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest, TraditionalUpdateIfWo
 
 TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest, TraditionalUpdateIfAnyListenerIsNotTcp) {
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   auto listener_proto = createDefaultListener();
 
@@ -4716,8 +4691,7 @@ TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest,
        TraditionalUpdateIfImplicitTlsInspectorChanges) {
 
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   auto listener_proto = createDefaultListener();
 
@@ -4743,8 +4717,7 @@ TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest,
        TraditionalUpdateIfImplicitProxyProtocolChanges) {
 
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   auto listener_proto = createDefaultListener();
 
@@ -4764,8 +4737,7 @@ TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest,
 
 TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest, TraditionalUpdateOnZeroFilterChain) {
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   auto listener_proto = createDefaultListener();
 
@@ -4789,8 +4761,7 @@ TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest, TraditionalUpdateOnZe
 TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest,
        TraditionalUpdateIfListenerConfigHasUpdateOtherThanFilterChain) {
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   auto listener_proto = createDefaultListener();
 
@@ -4814,8 +4785,7 @@ TEST_F(ListenerManagerImplForInPlaceFilterChainUpdateTest,
 TEST_F(ListenerManagerImplTest, RuntimeDisabledInPlaceUpdateFallbacksToTraditionalUpdate) {
   InSequence s;
   EXPECT_CALL(*worker_, start(_));
-  EXPECT_CALL(callback_, Call);
-  manager_->startWorkers(guard_dog_, callback_);
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
 
   // Add foo listener.
   const std::string listener_foo_yaml = R"EOF(
@@ -4950,6 +4920,14 @@ TEST_F(ListenerManagerImplTest, TcpBacklogCustomConfig) {
   EXPECT_EQ(100U, manager_->listeners().back().get().tcpBacklogSize());
 }
 
+TEST_F(ListenerManagerImplTest, WorkersStartedCallbackCalled) {
+  InSequence s;
+
+  EXPECT_CALL(*worker_, start(_));
+  EXPECT_CALL(callback_, Call());
+  manager_->startWorkers(guard_dog_, callback_.AsStdFunction());
+}
+
 } // namespace
 } // namespace Server
 } // namespace Envoy
diff --git a/test/server/listener_manager_impl_test.h b/test/server/listener_manager_impl_test.h
index 3dc638ea7ed97..f0a80a338d80d 100644
--- a/test/server/listener_manager_impl_test.h
+++ b/test/server/listener_manager_impl_test.h
@@ -296,7 +296,7 @@ class ListenerManagerImplTest : public testing::Test {
   std::unique_ptr<Network::MockConnectionSocket> socket_;
   uint64_t listener_tag_{1};
   bool enable_dispatcher_stats_{false};
-  MockFunction<void()> callback_;
+  NiceMock<testing::MockFunction<void()>> callback_;
 };
 
 } // namespace Server

From 9c942c20ff79b645421a8ff24a84e71a42806151 Mon Sep 17 00:00:00 2001
From: Tong Cai <caitong93@gmail.com>
Date: Wed, 16 Dec 2020 22:17:52 +0800
Subject: [PATCH 09/13] Revert some changes and use ListenerHooks for testing.

Signed-off-by: Tong Cai <caitong93@gmail.com>
---
 include/envoy/server/lifecycle_notifier.h |  5 --
 source/server/listener_hooks.h            |  6 ++
 source/server/server.cc                   | 22 +-----
 source/server/server.h                    |  2 +-
 test/integration/server.h                 |  1 +
 test/server/server_test.cc                | 85 ++++++++++++++++-------
 6 files changed, 69 insertions(+), 52 deletions(-)

diff --git a/include/envoy/server/lifecycle_notifier.h b/include/envoy/server/lifecycle_notifier.h
index e16ee9d2200be..94b53af7311ec 100644
--- a/include/envoy/server/lifecycle_notifier.h
+++ b/include/envoy/server/lifecycle_notifier.h
@@ -26,11 +26,6 @@ class ServerLifecycleNotifier {
      */
     PostInit,
 
-    /**
-     * All workers have started.
-     */
-    WorkersStarted,
-
     /**
      * The server instance is being shutdown and the dispatcher is about to exit.
      * This provides listeners a last chance to run a callback on the main dispatcher.
diff --git a/source/server/listener_hooks.h b/source/server/listener_hooks.h
index 1b3de394ab13b..1d88ab4760aff 100644
--- a/source/server/listener_hooks.h
+++ b/source/server/listener_hooks.h
@@ -22,6 +22,11 @@ class ListenerHooks {
    */
   virtual void onWorkerListenerRemoved() PURE;
 
+  /**
+   * Called when all workers have started.
+   */
+  virtual void onWorkersStarted() PURE;
+
   /**
    * Called when the Runtime::ScopedLoaderSingleton is created by the server.
    */
@@ -36,6 +41,7 @@ class DefaultListenerHooks : public ListenerHooks {
   // ListenerHooks
   void onWorkerListenerAdded() override {}
   void onWorkerListenerRemoved() override {}
+  void onWorkersStarted() override {}
   void onRuntimeCreated() override {}
 };
 
diff --git a/source/server/server.cc b/source/server/server.cc
index a7a2101e10e6e..3db1f355a8a19 100644
--- a/source/server/server.cc
+++ b/source/server/server.cc
@@ -87,7 +87,7 @@ InstanceImpl::InstanceImpl(
                                                   : nullptr),
       grpc_context_(store.symbolTable()), http_context_(store.symbolTable()),
       process_context_(std::move(process_context)), main_thread_id_(std::this_thread::get_id()),
-      server_contexts_(*this) {
+      hooks_(hooks), server_contexts_(*this) {
   try {
     if (!options.logPath().empty()) {
       try {
@@ -616,7 +616,7 @@ void InstanceImpl::startWorkers() {
     // Update server stats as soon as initialization is done.
     updateServerStats();
     workers_started_ = true;
-    notifyCallbacksForStage(Stage::WorkersStarted);
+    hooks_->onWorkersStarted();
     // At this point we are ready to take traffic and all listening ports are up. Notify our
     // parent if applicable that they can stop listening and drain.
     restarter_.drainParentListeners();
@@ -717,15 +717,6 @@ void InstanceImpl::run() {
   // startup (see RunHelperTest in server_test.cc).
   const auto run_helper = RunHelper(*this, options_, *dispatcher_, clusterManager(),
                                     access_log_manager_, init_manager_, overloadManager(), [this] {
-                                      // This ensures Startup event to be sent first, required by
-                                      // tests. In static configuration, cluster manager will
-                                      // initialize init_manager immediately when setInitializedCb
-                                      // is called.
-                                      if (!startup_lifecycle_event_raised_) {
-                                        notifyCallbacksForStage(Stage::Startup);
-                                        startup_lifecycle_event_raised_ = true;
-                                      }
-
                                       notifyCallbacksForStage(Stage::PostInit);
                                       startWorkers();
                                     });
@@ -734,14 +725,7 @@ void InstanceImpl::run() {
   ENVOY_LOG(info, "starting main dispatch loop");
   auto watchdog = main_thread_guard_dog_->createWatchDog(api_->threadFactory().currentThreadId(),
                                                          "main_thread", *dispatcher_);
-  dispatcher_->post([this] {
-    // It's possible that Startup event is already raised during RunHelper
-    // construction.
-    if (!startup_lifecycle_event_raised_) {
-      notifyCallbacksForStage(Stage::Startup);
-      startup_lifecycle_event_raised_ = true;
-    }
-  });
+  dispatcher_->post([this] { notifyCallbacksForStage(Stage::Startup); });
   dispatcher_->run(Event::Dispatcher::RunType::Block);
   ENVOY_LOG(info, "main dispatch loop exited");
   main_thread_guard_dog_->stopWatching(watchdog);
diff --git a/source/server/server.h b/source/server/server.h
index 508bfd75d07bc..3d16493d19851 100644
--- a/source/server/server.h
+++ b/source/server/server.h
@@ -364,7 +364,7 @@ class InstanceImpl final : Logger::Loggable<Logger::Id::main>,
   // initialization_time is a histogram for tracking the initialization time across hot restarts
   // whenever we have support for histogram merge across hot restarts.
   Stats::TimespanPtr initialization_timer_;
-  bool startup_lifecycle_event_raised_{};
+  ListenerHooks& hooks_;
 
   ServerFactoryContextImpl server_contexts_;
 
diff --git a/test/integration/server.h b/test/integration/server.h
index 9d70d735bbb40..87544c711815e 100644
--- a/test/integration/server.h
+++ b/test/integration/server.h
@@ -415,6 +415,7 @@ class IntegrationTestServer : public Logger::Loggable<Logger::Id::testing>,
     on_server_ready_cb_ = std::move(on_server_ready);
   }
   void onRuntimeCreated() override {}
+  void onWorkersStarted() override {}
 
   void start(const Network::Address::IpVersion version,
              std::function<void()> on_server_init_function, bool deterministic,
diff --git a/test/server/server_test.cc b/test/server/server_test.cc
index e770cd589acce..cb81fa9830c4a 100644
--- a/test/server/server_test.cc
+++ b/test/server/server_test.cc
@@ -181,6 +181,11 @@ class ServerInstanceImplTestBase {
   void initialize(const std::string& bootstrap_path) { initialize(bootstrap_path, false); }
 
   void initialize(const std::string& bootstrap_path, const bool use_intializing_instance) {
+    initialize(bootstrap_path, use_intializing_instance, hooks_);
+  }
+
+  void initialize(const std::string& bootstrap_path, const bool use_intializing_instance,
+                  ListenerHooks& hooks) {
     if (options_.config_path_.empty()) {
       options_.config_path_ = TestEnvironment::temporaryFileSubstitute(
           bootstrap_path, {{"upstream_0", 0}, {"upstream_1", 0}}, version_);
@@ -194,7 +199,7 @@ class ServerInstanceImplTestBase {
 
     server_ = std::make_unique<InstanceImpl>(
         *init_manager_, options_, time_system_,
-        std::make_shared<Network::Address::Ipv4Instance>("127.0.0.1"), hooks_, restart_,
+        std::make_shared<Network::Address::Ipv4Instance>("127.0.0.1"), hooks, restart_,
         stats_store_, fakelock_, component_factory_,
         std::make_unique<NiceMock<Random::MockRandomGenerator>>(), *thread_local_,
         Thread::threadFactoryForTest(), Filesystem::fileSystemForTest(),
@@ -313,6 +318,18 @@ class CustomStatsSinkFactory : public Server::Configuration::StatsSinkFactory {
   std::string name() const override { return "envoy.custom_stats_sink"; }
 };
 
+// CustomListenerHooks is used for syncrinization between test thread and server thread.
+class CustomListenerHooks : public DefaultListenerHooks {
+public:
+  CustomListenerHooks(std::function<void()> workers_started_cb)
+      : on_workers_started_cb_(workers_started_cb) {}
+
+  void onWorkersStarted() { on_workers_started_cb_(); }
+
+private:
+  std::function<void()> on_workers_started_cb_;
+};
+
 INSTANTIATE_TEST_SUITE_P(IpVersions, ServerInstanceImplTest,
                          testing::ValuesIn(TestEnvironment::getIpVersionsForTest()),
                          TestUtility::ipTestParamsToString);
@@ -358,12 +375,8 @@ TEST_P(ServerInstanceImplTest, EmptyShutdownLifecycleNotifications) {
 }
 
 TEST_P(ServerInstanceImplTest, LifecycleNotifications) {
-  bool startup = false, post_init = false, workers_started = false, shutdown = false,
-       shutdown_with_completion = false;
-  absl::Notification started, post_init_fired, workers_started_fired, workers_started_block,
-      shutdown_begin, completion_block, completion_done;
-  // Expect drainParentListeners not to be called before workers start.
-  EXPECT_CALL(restart_, drainParentListeners).Times(0);
+  bool startup = false, post_init = false, shutdown = false, shutdown_with_completion = false;
+  absl::Notification started, post_init_fired, shutdown_begin, completion_block, completion_done;
 
   // Run the server in a separate thread so we can test different lifecycle stages.
   auto server_thread = Thread::threadFactoryForTest().createThread([&] {
@@ -376,16 +389,11 @@ TEST_P(ServerInstanceImplTest, LifecycleNotifications) {
       post_init = true;
       post_init_fired.Notify();
     });
-    auto handle3 = server_->registerCallback(ServerLifecycleNotifier::Stage::WorkersStarted, [&] {
-      workers_started = true;
-      workers_started_fired.Notify();
-      workers_started_block.WaitForNotification();
-    });
-    auto handle4 = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit, [&] {
+    auto handle3 = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit, [&] {
       shutdown = true;
       shutdown_begin.Notify();
     });
-    auto handle5 = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit,
+    auto handle4 = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit,
                                              [&](Event::PostCb completion_cb) {
                                                // Block till we're told to complete
                                                completion_block.WaitForNotification();
@@ -393,18 +401,17 @@ TEST_P(ServerInstanceImplTest, LifecycleNotifications) {
                                                server_->dispatcher().post(completion_cb);
                                                completion_done.Notify();
                                              });
-    auto handle6 =
+    auto handle5 =
         server_->registerCallback(ServerLifecycleNotifier::Stage::Startup, [&] { FAIL(); });
-    handle6 = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit,
+    handle5 = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit,
                                         [&](Event::PostCb) { FAIL(); });
-    handle6 = nullptr;
+    handle5 = nullptr;
 
     server_->run();
     handle1 = nullptr;
     handle2 = nullptr;
     handle3 = nullptr;
     handle4 = nullptr;
-    handle5 = nullptr;
     server_ = nullptr;
     thread_local_ = nullptr;
   });
@@ -412,20 +419,13 @@ TEST_P(ServerInstanceImplTest, LifecycleNotifications) {
   started.WaitForNotification();
   EXPECT_TRUE(startup);
   EXPECT_FALSE(shutdown);
+  EXPECT_TRUE(TestUtility::findGauge(stats_store_, "server.state")->used());
+  EXPECT_EQ(0L, TestUtility::findGauge(stats_store_, "server.state")->value());
 
   post_init_fired.WaitForNotification();
   EXPECT_TRUE(post_init);
   EXPECT_FALSE(shutdown);
 
-  workers_started_fired.WaitForNotification();
-  EXPECT_TRUE(workers_started);
-  EXPECT_FALSE(shutdown);
-  EXPECT_TRUE(TestUtility::findGauge(stats_store_, "server.state")->used());
-  EXPECT_EQ(0L, TestUtility::findGauge(stats_store_, "server.state")->value());
-
-  EXPECT_CALL(restart_, drainParentListeners);
-  workers_started_block.Notify();
-
   server_->dispatcher().post([&] { server_->shutdown(); });
   shutdown_begin.WaitForNotification();
   EXPECT_TRUE(shutdown);
@@ -440,6 +440,37 @@ TEST_P(ServerInstanceImplTest, LifecycleNotifications) {
   server_thread->join();
 }
 
+TEST_P(ServerInstanceImplTest, DrainParentListenerAfterWorkersStarted) {
+  bool workers_started = false;
+  absl::Notification workers_started_fired, workers_started_block;
+  // Expect drainParentListeners not to be called before workers start.
+  EXPECT_CALL(restart_, drainParentListeners).Times(0);
+
+  // Run the server in a separate thread so we can test different lifecycle stages.
+  auto server_thread = Thread::threadFactoryForTest().createThread([&] {
+    auto hooks = CustomListenerHooks([&]() {
+      workers_started = true;
+      workers_started_fired.Notify();
+      workers_started_block.WaitForNotification();
+    });
+    initialize("test/server/test_data/server/node_bootstrap.yaml", false, hooks);
+    server_->run();
+    server_ = nullptr;
+    thread_local_ = nullptr;
+  });
+
+  workers_started_fired.WaitForNotification();
+  EXPECT_TRUE(workers_started);
+  EXPECT_TRUE(TestUtility::findGauge(stats_store_, "server.state")->used());
+  EXPECT_EQ(0L, TestUtility::findGauge(stats_store_, "server.state")->value());
+
+  EXPECT_CALL(restart_, drainParentListeners);
+  workers_started_block.Notify();
+
+  server_->dispatcher().post([&] { server_->shutdown(); });
+  server_thread->join();
+}
+
 // A test target which never signals that it is ready.
 class NeverReadyTarget : public Init::TargetImpl {
 public:

From c6b8a32fa7bf12bbaf6ae758758f209a94d53364 Mon Sep 17 00:00:00 2001
From: Tong Cai <caitong93@gmail.com>
Date: Thu, 17 Dec 2020 02:17:36 +0800
Subject: [PATCH 10/13] Fix compile.

Signed-off-by: Tong Cai <caitong93@gmail.com>
---
 source/server/server.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source/server/server.cc b/source/server/server.cc
index af550c75d626a..fc41edc2ed081 100644
--- a/source/server/server.cc
+++ b/source/server/server.cc
@@ -619,7 +619,7 @@ void InstanceImpl::startWorkers() {
     // Update server stats as soon as initialization is done.
     updateServerStats();
     workers_started_ = true;
-    hooks_->onWorkersStarted();
+    hooks_.onWorkersStarted();
     // At this point we are ready to take traffic and all listening ports are up. Notify our
     // parent if applicable that they can stop listening and drain.
     restarter_.drainParentListeners();

From afc4d8bd087c612a8739b0d1856660ca7431559f Mon Sep 17 00:00:00 2001
From: Tong Cai <caitong93@gmail.com>
Date: Thu, 17 Dec 2020 21:17:11 +0800
Subject: [PATCH 11/13] Fix typo.

Signed-off-by: Tong Cai <caitong93@gmail.com>
---
 test/server/server_test.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/server/server_test.cc b/test/server/server_test.cc
index d9edb63e905e0..f51f7ece0aa57 100644
--- a/test/server/server_test.cc
+++ b/test/server/server_test.cc
@@ -318,7 +318,7 @@ class CustomStatsSinkFactory : public Server::Configuration::StatsSinkFactory {
   std::string name() const override { return "envoy.custom_stats_sink"; }
 };
 
-// CustomListenerHooks is used for syncrinization between test thread and server thread.
+// CustomListenerHooks is used for synchronization between test thread and server thread.
 class CustomListenerHooks : public DefaultListenerHooks {
 public:
   CustomListenerHooks(std::function<void()> workers_started_cb)

From 22f482fb9cd1f7e1ad1aa7ad1c11ac8454719c8d Mon Sep 17 00:00:00 2001
From: Tong Cai <caitong93@gmail.com>
Date: Thu, 17 Dec 2020 23:13:03 +0800
Subject: [PATCH 12/13] Fix format.

Signed-off-by: Tong Cai <caitong93@gmail.com>
---
 test/server/server_test.cc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/server/server_test.cc b/test/server/server_test.cc
index f51f7ece0aa57..0eb43337daf50 100644
--- a/test/server/server_test.cc
+++ b/test/server/server_test.cc
@@ -324,7 +324,7 @@ class CustomListenerHooks : public DefaultListenerHooks {
   CustomListenerHooks(std::function<void()> workers_started_cb)
       : on_workers_started_cb_(workers_started_cb) {}
 
-  void onWorkersStarted() { on_workers_started_cb_(); }
+  void onWorkersStarted() override { on_workers_started_cb_(); }
 
 private:
   std::function<void()> on_workers_started_cb_;

From bad5177857deae1e7c8b36fd3392f275c79201d5 Mon Sep 17 00:00:00 2001
From: Tong Cai <caitong93@gmail.com>
Date: Sat, 19 Dec 2020 02:12:47 +0800
Subject: [PATCH 13/13] server: test the server shutdown before workers start.

Signed-off-by: Tong Cai <caitong93@gmail.com>
---
 test/server/server_test.cc | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/test/server/server_test.cc b/test/server/server_test.cc
index 0eb43337daf50..c09fb90ef732a 100644
--- a/test/server/server_test.cc
+++ b/test/server/server_test.cc
@@ -511,6 +511,31 @@ TEST_P(ServerInstanceImplTest, NoLifecycleNotificationOnEarlyShutdown) {
   server_thread->join();
 }
 
+TEST_P(ServerInstanceImplTest, ShutdownBeforeWorkersStarted) {
+  // Test that drainParentListeners() should never be called because we will shutdown
+  // early before the server starts worker threads.
+  EXPECT_CALL(restart_, drainParentListeners).Times(0);
+
+  auto server_thread = Thread::threadFactoryForTest().createThread([&] {
+    initialize("test/server/test_data/server/node_bootstrap.yaml");
+
+    auto post_init_handle = server_->registerCallback(ServerLifecycleNotifier::Stage::PostInit,
+                                                      [&] { server_->shutdown(); });
+
+    // This shutdown notification should never be called because we will shutdown early.
+    auto shutdown_handle = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit,
+                                                     [&](Event::PostCb) { FAIL(); });
+    server_->run();
+
+    post_init_handle = nullptr;
+    shutdown_handle = nullptr;
+    server_ = nullptr;
+    thread_local_ = nullptr;
+  });
+
+  server_thread->join();
+}
+
 TEST_P(ServerInstanceImplTest, V2ConfigOnly) {
   options_.service_cluster_name_ = "some_cluster_name";
   options_.service_node_name_ = "some_node_name";