server: add a post init lifecycle stage (envoyproxy#8217)

Signed-off-by: Jose Nino <jnino@lyft.com>
danzh2010 · Sep 24, 2019 · f887f5e · f887f5e
1 parent 0117a8a
commit f887f5e
Show file tree

Hide file tree

Showing 4 changed files with 35 additions and 13 deletions.
diff --git a/docs/root/intro/version_history.rst b/docs/root/intro/version_history.rst
@@ -53,6 +53,7 @@ Version history
 * router check tool: add comprehensive coverage reporting.
 * router check tool: add deprecated field check.
 * router check tool: add flag for only printing results of failed tests.
+* server: added a post initialization lifecycle event, in addition to the existing startup and shutdown events.
 * thrift_proxy: fix crashing bug on invalid transport/protocol framing
 * tls: added verification of IP address SAN fields in certificates against configured SANs in the
 * tracing: added support to the Zipkin reporter for sending list of spans as Zipkin JSON v2 and protobuf message over HTTP.

diff --git a/include/envoy/server/lifecycle_notifier.h b/include/envoy/server/lifecycle_notifier.h
@@ -21,6 +21,11 @@ class ServerLifecycleNotifier {
      */
     Startup,
 
+    /**
+     * The server instance init manager has finished initialization.
+     */
+    PostInit,
+
     /**
      * 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
@@ -464,10 +464,10 @@ void InstanceImpl::loadServerFlags(const absl::optional<std::string>& flags_path
 RunHelper::RunHelper(Instance& instance, const Options& options, Event::Dispatcher& dispatcher,
                      Upstream::ClusterManager& cm, AccessLog::AccessLogManager& access_log_manager,
                      Init::Manager& init_manager, OverloadManager& overload_manager,
-                     std::function<void()> workers_start_cb)
-    : init_watcher_("RunHelper", [&instance, workers_start_cb]() {
+                     std::function<void()> post_init_cb)
+    : init_watcher_("RunHelper", [&instance, post_init_cb]() {
         if (!instance.isShutdown()) {
-          workers_start_cb();
+          post_init_cb();
         }
       }) {
   // Setup signals.
@@ -522,9 +522,11 @@ RunHelper::RunHelper(Instance& instance, const Options& options, Event::Dispatch
 void InstanceImpl::run() {
   // RunHelper exists primarily to facilitate testing of how we respond to early shutdown during
   // startup (see RunHelperTest in server_test.cc).
-  const auto run_helper =
-      RunHelper(*this, options_, *dispatcher_, clusterManager(), access_log_manager_, init_manager_,
-                overloadManager(), [this] { startWorkers(); });
+  const auto run_helper = RunHelper(*this, options_, *dispatcher_, clusterManager(),
+                                    access_log_manager_, init_manager_, overloadManager(), [this] {
+                                      notifyCallbacksForStage(Stage::PostInit);
+                                      startWorkers();
+                                    });
 
   // Run the main dispatch loop waiting to exit.
   ENVOY_LOG(info, "starting main dispatch loop");

diff --git a/test/server/server_test.cc b/test/server/server_test.cc
@@ -219,21 +219,26 @@ class ServerInstanceImplTestBase {
   Thread::ThreadPtr startTestServer(const std::string& bootstrap_path,
                                     const bool use_intializing_instance) {
     absl::Notification started;
+    absl::Notification post_init;
 
     auto server_thread = Thread::threadFactoryForTest().createThread([&] {
       initialize(bootstrap_path, use_intializing_instance);
       auto startup_handle = server_->registerCallback(ServerLifecycleNotifier::Stage::Startup,
                                                       [&] { started.Notify(); });
+      auto post_init_handle = server_->registerCallback(ServerLifecycleNotifier::Stage::PostInit,
+                                                        [&] { post_init.Notify(); });
       auto shutdown_handle = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit,
                                                        [&](Event::PostCb) { FAIL(); });
       shutdown_handle = nullptr; // unregister callback
       server_->run();
       startup_handle = nullptr;
+      post_init_handle = nullptr;
       server_ = nullptr;
       thread_local_ = nullptr;
     });
 
     started.WaitForNotification();
+    post_init.WaitForNotification();
     return server_thread;
   }
 
@@ -321,8 +326,8 @@ TEST_P(ServerInstanceImplTest, EmptyShutdownLifecycleNotifications) {
 }
 
 TEST_P(ServerInstanceImplTest, LifecycleNotifications) {
-  bool startup = false, shutdown = false, shutdown_with_completion = false;
-  absl::Notification started, shutdown_begin, completion_block, completion_done;
+  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([&] {
@@ -331,28 +336,33 @@ TEST_P(ServerInstanceImplTest, LifecycleNotifications) {
       startup = true;
       started.Notify();
     });
-    auto handle2 = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit, [&] {
+    auto handle2 = server_->registerCallback(ServerLifecycleNotifier::Stage::PostInit, [&] {
+      post_init = true;
+      post_init_fired.Notify();
+    });
+    auto handle3 = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit, [&] {
       shutdown = true;
       shutdown_begin.Notify();
     });
-    auto handle3 = 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();
                                                shutdown_with_completion = true;
                                                server_->dispatcher().post(completion_cb);
                                                completion_done.Notify();
                                              });
-    auto handle4 =
+    auto handle5 =
         server_->registerCallback(ServerLifecycleNotifier::Stage::Startup, [&] { FAIL(); });
-    handle4 = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit,
+    handle5 = server_->registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit,
                                         [&](Event::PostCb) { FAIL(); });
-    handle4 = nullptr;
+    handle5 = nullptr;
 
     server_->run();
     handle1 = nullptr;
     handle2 = nullptr;
     handle3 = nullptr;
+    handle4 = nullptr;
     server_ = nullptr;
     thread_local_ = nullptr;
   });
@@ -361,6 +371,10 @@ TEST_P(ServerInstanceImplTest, LifecycleNotifications) {
   EXPECT_TRUE(startup);
   EXPECT_FALSE(shutdown);
 
+  post_init_fired.WaitForNotification();
+  EXPECT_TRUE(post_init);
+  EXPECT_FALSE(shutdown);
+
   server_->dispatcher().post([&] { server_->shutdown(); });
   shutdown_begin.WaitForNotification();
   EXPECT_TRUE(shutdown);