server: allowing for custom impls (envoyproxy#30121)

Adding run() to the server API (so that stripped main only tracks having a Server::Instance) and splitting server creation into a sub-function. This will allow Envoy mobile to run a stripped down server. Risk Level: low - minor refactor Testing: n/a Docs Changes: n/a Release Notes: n/a Signed-off-by: Alyssa Wilk <[email protected]> Signed-off-by: Alyssa Wilk <[email protected]>
fredyw · Oct 19, 2023 · f254e9f · f254e9f
1 parent f0e4949
commit f254e9f
Show file tree

Hide file tree

Showing 9 changed files with 75 additions and 12 deletions.
diff --git a/envoy/server/instance.h b/envoy/server/instance.h
@@ -46,6 +46,11 @@ class Instance {
 public:
   virtual ~Instance() = default;
 
+  /**
+   * Runs the server.
+   */
+  virtual void run() PURE;
+
   /**
    * @return OptRef<Admin> the global HTTP admin endpoint for the server.
    */

diff --git a/mobile/library/common/engine_common.cc b/mobile/library/common/engine_common.cc
@@ -61,9 +61,25 @@ EngineCommon::EngineCommon(std::unique_ptr<Envoy::OptionsImpl>&& options)
   registerMobileProtoDescriptors();
 #endif
 
+  StrippedMainBase::CreateInstanceFunction create_instance =
+      [](Init::Manager& init_manager, const Server::Options& options,
+         Event::TimeSystem& time_system, ListenerHooks& hooks, Server::HotRestart& restarter,
+         Stats::StoreRoot& store, Thread::BasicLockable& access_log_lock,
+         Server::ComponentFactory& component_factory, Random::RandomGeneratorPtr&& random_generator,
+         ThreadLocal::Instance& tls, Thread::ThreadFactory& thread_factory,
+         Filesystem::Instance& file_system, std::unique_ptr<ProcessContext> process_context,
+         Buffer::WatermarkFactorySharedPtr watermark_factory) {
+        // TODO(alyssawilk) use InstanceLite not InstanceImpl.
+        auto local_address = Network::Utility::getLocalAddress(options.localAddressIpVersion());
+        return std::make_unique<Server::InstanceImpl>(
+            init_manager, options, time_system, local_address, hooks, restarter, store,
+            access_log_lock, component_factory, std::move(random_generator), tls, thread_factory,
+            file_system, std::move(process_context), watermark_factory);
+      };
   base_ = std::make_unique<StrippedMainBase>(
       *options_, real_time_system_, default_listener_hooks_, prod_component_factory_,
-      std::make_unique<PlatformImpl>(), std::make_unique<Random::RandomGeneratorImpl>(), nullptr);
+      std::make_unique<PlatformImpl>(), std::make_unique<Random::RandomGeneratorImpl>(), nullptr,
+      create_instance);
 
   // Disabling signal handling in the options makes it so that the server's event dispatcher _does
   // not_ listen for termination signals such as SIGTERM, SIGINT, etc

diff --git a/source/exe/main_common.cc b/source/exe/main_common.cc
@@ -29,6 +29,33 @@
 
 namespace Envoy {
 
+StrippedMainBase::CreateInstanceFunction createFunction() {
+  return
+      [](Init::Manager& init_manager, const Server::Options& options,
+         Event::TimeSystem& time_system, ListenerHooks& hooks, Server::HotRestart& restarter,
+         Stats::StoreRoot& store, Thread::BasicLockable& access_log_lock,
+         Server::ComponentFactory& component_factory, Random::RandomGeneratorPtr&& random_generator,
+         ThreadLocal::Instance& tls, Thread::ThreadFactory& thread_factory,
+         Filesystem::Instance& file_system, std::unique_ptr<ProcessContext> process_context,
+         Buffer::WatermarkFactorySharedPtr watermark_factory) {
+        auto local_address = Network::Utility::getLocalAddress(options.localAddressIpVersion());
+        return std::make_unique<Server::InstanceImpl>(
+            init_manager, options, time_system, local_address, hooks, restarter, store,
+            access_log_lock, component_factory, std::move(random_generator), tls, thread_factory,
+            file_system, std::move(process_context), watermark_factory);
+      };
+}
+
+MainCommonBase::MainCommonBase(const Server::Options& options, Event::TimeSystem& time_system,
+                               ListenerHooks& listener_hooks,
+                               Server::ComponentFactory& component_factory,
+                               std::unique_ptr<Server::Platform> platform_impl,
+                               std::unique_ptr<Random::RandomGenerator>&& random_generator,
+                               std::unique_ptr<ProcessContext> process_context)
+    : StrippedMainBase(options, time_system, listener_hooks, component_factory,
+                       std::move(platform_impl), std::move(random_generator),
+                       std::move(process_context), createFunction()) {}
+
 bool MainCommonBase::run() {
   switch (options_.mode()) {
   case Server::Mode::Serve:

diff --git a/source/exe/main_common.h b/source/exe/main_common.h
@@ -25,7 +25,11 @@ namespace Envoy {
 
 class MainCommonBase : public StrippedMainBase {
 public:
-  using StrippedMainBase::StrippedMainBase;
+  MainCommonBase(const Server::Options& options, Event::TimeSystem& time_system,
+                 ListenerHooks& listener_hooks, Server::ComponentFactory& component_factory,
+                 std::unique_ptr<Server::Platform> platform_impl,
+                 std::unique_ptr<Random::RandomGenerator>&& random_generator,
+                 std::unique_ptr<ProcessContext> process_context);
 
   bool run();
 

diff --git a/source/exe/stripped_main_base.cc b/source/exe/stripped_main_base.cc
@@ -46,7 +46,8 @@ StrippedMainBase::StrippedMainBase(const Server::Options& options, Event::TimeSy
                                    Server::ComponentFactory& component_factory,
                                    std::unique_ptr<Server::Platform> platform_impl,
                                    std::unique_ptr<Random::RandomGenerator>&& random_generator,
-                                   std::unique_ptr<ProcessContext> process_context)
+                                   std::unique_ptr<ProcessContext> process_context,
+                                   CreateInstanceFunction createInstance)
     : platform_impl_(std::move(platform_impl)), options_(options),
       component_factory_(component_factory), stats_allocator_(symbol_table_) {
   // Process the option to disable extensions as early as possible,
@@ -71,7 +72,6 @@ StrippedMainBase::StrippedMainBase(const Server::Options& options, Event::TimeSy
     tls_ = std::make_unique<ThreadLocal::InstanceImpl>();
     Thread::BasicLockable& log_lock = restarter_->logLock();
     Thread::BasicLockable& access_log_lock = restarter_->accessLogLock();
-    auto local_address = Network::Utility::getLocalAddress(options_.localAddressIpVersion());
     logging_context_ = std::make_unique<Logger::Context>(options_.logLevel(), options_.logFormat(),
                                                          log_lock, options_.logFormatEscaped(),
                                                          options_.enableFineGrainLogging());
@@ -84,11 +84,10 @@ StrippedMainBase::StrippedMainBase(const Server::Options& options, Event::TimeSy
 
     stats_store_ = std::make_unique<Stats::ThreadLocalStoreImpl>(stats_allocator_);
 
-    server_ = std::make_unique<Server::InstanceImpl>(
-        *init_manager_, options_, time_system, local_address, listener_hooks, *restarter_,
-        *stats_store_, access_log_lock, component_factory, std::move(random_generator), *tls_,
-        platform_impl_->threadFactory(), platform_impl_->fileSystem(), std::move(process_context));
-
+    server_ = createInstance(*init_manager_, options_, time_system, listener_hooks, *restarter_,
+                             *stats_store_, access_log_lock, component_factory,
+                             std::move(random_generator), *tls_, platform_impl_->threadFactory(),
+                             platform_impl_->fileSystem(), std::move(process_context), nullptr);
     break;
   }
   case Server::Mode::Validate:

diff --git a/source/exe/stripped_main_base.h b/source/exe/stripped_main_base.h
@@ -36,6 +36,15 @@ class ProdComponentFactory : public Server::ComponentFactory {
 // separate for legacy reasons.
 class StrippedMainBase {
 public:
+  using CreateInstanceFunction = std::function<std::unique_ptr<Server::Instance>(
+      Init::Manager& init_manager, const Server::Options& options, Event::TimeSystem& time_system,
+      ListenerHooks& hooks, Server::HotRestart& restarter, Stats::StoreRoot& store,
+      Thread::BasicLockable& access_log_lock, Server::ComponentFactory& component_factory,
+      Random::RandomGeneratorPtr&& random_generator, ThreadLocal::Instance& tls,
+      Thread::ThreadFactory& thread_factory, Filesystem::Instance& file_system,
+      std::unique_ptr<ProcessContext> process_context,
+      Buffer::WatermarkFactorySharedPtr watermark_factory)>;
+
   static std::string hotRestartVersion(bool hot_restart_enabled);
 
   // Consumer must guarantee that all passed references are alive until this object is
@@ -44,7 +53,8 @@ class StrippedMainBase {
                    ListenerHooks& listener_hooks, Server::ComponentFactory& component_factory,
                    std::unique_ptr<Server::Platform> platform_impl,
                    std::unique_ptr<Random::RandomGenerator>&& random_generator,
-                   std::unique_ptr<ProcessContext> process_context);
+                   std::unique_ptr<ProcessContext> process_context,
+                   CreateInstanceFunction createInstance);
 
   void runServer() {
     ASSERT(options_.mode() == Server::Mode::Serve);
@@ -71,7 +81,7 @@ class StrippedMainBase {
   Stats::ThreadLocalStoreImplPtr stats_store_;
   std::unique_ptr<Logger::Context> logging_context_;
   std::unique_ptr<Init::Manager> init_manager_{std::make_unique<Init::ManagerImpl>("Server")};
-  std::unique_ptr<Server::InstanceImpl> server_;
+  std::unique_ptr<Server::Instance> server_;
 
 private:
   void configureComponentLogLevels();

diff --git a/source/server/config_validation/server.h b/source/server/config_validation/server.h
@@ -68,6 +68,7 @@ class ValidationInstance final : Logger::Loggable<Logger::Id::main>,
                      Filesystem::Instance& file_system);
 
   // Server::Instance
+  void run() override { PANIC("not implemented"); }
   OptRef<Admin> admin() override {
     return makeOptRefFromPtr(static_cast<Envoy::Server::Admin*>(admin_.get()));
   }

diff --git a/source/server/server.h b/source/server/server.h
@@ -243,7 +243,7 @@ class InstanceImpl final : Logger::Loggable<Logger::Id::main>,
 
   ~InstanceImpl() override;
 
-  void run();
+  void run() override;
 
   // Server::Instance
   OptRef<Admin> admin() override { return makeOptRefFromPtr(admin_.get()); }

diff --git a/test/mocks/server/instance.h b/test/mocks/server/instance.h
@@ -18,6 +18,7 @@ class MockInstance : public Instance {
   Secret::SecretManager& secretManager() override { return *(secret_manager_); }
 
   MOCK_METHOD(OptRef<Admin>, admin, ());
+  MOCK_METHOD(void, run, ());
   MOCK_METHOD(Api::Api&, api, ());
   MOCK_METHOD(Upstream::ClusterManager&, clusterManager, ());
   MOCK_METHOD(const Upstream::ClusterManager&, clusterManager, (), (const));