diff --git a/include/envoy/server/BUILD b/include/envoy/server/BUILD index f998508ad8204..67b5d12544f08 100644 --- a/include/envoy/server/BUILD +++ b/include/envoy/server/BUILD @@ -57,7 +57,10 @@ envoy_cc_library( envoy_cc_library( name = "hot_restart_interface", hdrs = ["hot_restart.h"], - deps = ["//include/envoy/event:dispatcher_interface"], + deps = [ + "//include/envoy/event:dispatcher_interface", + "//include/envoy/thread:thread_interface", + ], ) envoy_cc_library( diff --git a/include/envoy/server/hot_restart.h b/include/envoy/server/hot_restart.h index 67471797a85b5..997afff006217 100644 --- a/include/envoy/server/hot_restart.h +++ b/include/envoy/server/hot_restart.h @@ -5,6 +5,8 @@ #include "envoy/common/pure.h" #include "envoy/event/dispatcher.h" +#include "envoy/stats/stats.h" +#include "envoy/thread/thread.h" namespace Envoy { namespace Server { @@ -78,6 +80,21 @@ class HotRestart { * perform a full or hot restart. */ virtual std::string version() PURE; + + /** + * @return Thread::BasicLockable& a lock for logging. + */ + virtual Thread::BasicLockable& logLock() PURE; + + /** + * @return Thread::BasicLockable& a lock for access logs. + */ + virtual Thread::BasicLockable& accessLogLock() PURE; + + /** + * @returns an allocator for stats. + */ + virtual Stats::RawStatDataAllocator& statsAllocator() PURE; }; } // namespace Server diff --git a/include/envoy/stats/stats.h b/include/envoy/stats/stats.h index 757bf9435e810..3e7997052e6d9 100644 --- a/include/envoy/stats/stats.h +++ b/include/envoy/stats/stats.h @@ -281,5 +281,29 @@ class StoreRoot : public Store { typedef std::unique_ptr StoreRootPtr; +struct RawStatData; + +/** + * Abstract interface for allocating a RawStatData. + */ +class RawStatDataAllocator { +public: + virtual ~RawStatDataAllocator() {} + + /** + * @return RawStatData* a raw stat data block for a given stat name or nullptr if there is no + * more memory available for stats. The allocator should return a reference counted + * data location by name if one already exists with the same name. This is used for + * intra-process scope swapping as well as inter-process hot restart. + */ + virtual RawStatData* alloc(const std::string& name) PURE; + + /** + * Free a raw stat data block. The allocator should handle reference counting and only truly + * free the block if it is no longer needed. + */ + virtual void free(RawStatData& data) PURE; +}; + } // namespace Stats } // namespace Envoy diff --git a/source/common/stats/stats_impl.h b/source/common/stats/stats_impl.h index 2a8d4aacbfa52..5fbbe1f4a6991 100644 --- a/source/common/stats/stats_impl.h +++ b/source/common/stats/stats_impl.h @@ -191,28 +191,6 @@ struct RawStatData { static size_t& initializeAndGetMutableMaxObjNameLength(size_t configured_size); }; -/** - * Abstract interface for allocating a RawStatData. - */ -class RawStatDataAllocator { -public: - virtual ~RawStatDataAllocator() {} - - /** - * @return RawStatData* a raw stat data block for a given stat name or nullptr if there is no more - * memory available for stats. The allocator may return a reference counted data location - * by name if one already exists with the same name. This is used for intra-process - * scope swapping as well as inter-process hot restart. - */ - virtual RawStatData* alloc(const std::string& name) PURE; - - /** - * Free a raw stat data block. The allocator should handle reference counting and only truly - * free the block if it is no longer needed. - */ - virtual void free(RawStatData& data) PURE; -}; - /** * Implementation of the Metric interface. Virtual inheritance is used because the interfaces that * will inherit from Metric will have other base classes that will also inherit from Metric. diff --git a/source/exe/BUILD b/source/exe/BUILD index ba6db1d96a307..03720cbde6325 100644 --- a/source/exe/BUILD +++ b/source/exe/BUILD @@ -78,14 +78,9 @@ envoy_cc_library( deps = [ ":envoy_main_common_lib", ":extra_protocol_proxies_lib", - "//source/server:hot_restart_lib", - "//source/server:options_lib", "//source/server/config/http:lightstep_lib", "//source/server/config/http:zipkin_lib", - ] + select({ - "//bazel:disable_signal_trace": [], - "//conditions:default": [":sigaction_lib"], - }), + ], ) envoy_cc_library( @@ -100,7 +95,10 @@ envoy_cc_library( "//source/server:hot_restart_nop_lib", "//source/server:proto_descriptors_lib", "//source/server/config_validation:server_lib", - ], + ] + select({ + "//bazel:disable_signal_trace": [], + "//conditions:default": [":sigaction_lib"], + }), ) envoy_cc_library( diff --git a/source/exe/main.cc b/source/exe/main.cc index 0e027330a74db..5d14f7744e206 100644 --- a/source/exe/main.cc +++ b/source/exe/main.cc @@ -1,20 +1,5 @@ -#include -#include - #include "exe/main_common.h" -#ifdef ENVOY_HANDLE_SIGNALS -#include "exe/signal_action.h" -#endif - -#ifdef ENVOY_HOT_RESTART -#include "server/hot_restart_impl.h" -#endif - -#include "server/options_impl.h" - -#include "spdlog/spdlog.h" - // NOLINT(namespace-envoy) /** @@ -25,31 +10,28 @@ * after setting up command line options. */ int main(int argc, char** argv) { -#ifdef ENVOY_HANDLE_SIGNALS - // Enabled by default. Control with "bazel --define=signal_trace=disabled" - Envoy::SignalAction handle_sigs; -#endif - #ifdef ENVOY_HOT_RESTART - // Enabled by default, except on OS X. Control with "bazel --define=hot_restart=disabled" - const Envoy::OptionsImpl::HotRestartVersionCb hot_restart_version_cb = - [](uint64_t max_num_stats, uint64_t max_stat_name_len) { - return Envoy::Server::HotRestartImpl::hotRestartVersion(max_num_stats, max_stat_name_len); - }; + constexpr bool enable_hot_restart = true; #else - const Envoy::OptionsImpl::HotRestartVersionCb hot_restart_version_cb = [](uint64_t, uint64_t) { - return "disabled"; - }; + constexpr bool enable_hot_restart = false; #endif - std::unique_ptr options; + std::unique_ptr main_common; + + // Initialize the server's main context under a try/catch loop and simply return EXIT_FAILURE + // as needed. Whatever code in the initialization path that fails is expected to log an error + // message so the user can diagnose. try { - options = std::make_unique(argc, argv, hot_restart_version_cb, - spdlog::level::info); + main_common = std::make_unique(argc, argv, enable_hot_restart); } catch (const Envoy::NoServingException& e) { - return 0; + return EXIT_SUCCESS; } catch (const Envoy::MalformedArgvException& e) { - return 1; + return EXIT_FAILURE; + } catch (const Envoy::EnvoyException& e) { + return EXIT_FAILURE; } - return Envoy::main_common(*options); + + // Run the server listener loop outside try/catch blocks, so that unexpected exceptions + // show up as a core-dumps for easier diagnostis. + return main_common->run() ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/source/exe/main_common.cc b/source/exe/main_common.cc index 46a05530d0b3b..715f3407414e4 100644 --- a/source/exe/main_common.cc +++ b/source/exe/main_common.cc @@ -1,3 +1,5 @@ +#include "exe/main_common.h" + #include #include @@ -5,7 +7,6 @@ #include "common/event/libevent.h" #include "common/network/utility.h" #include "common/stats/stats_impl.h" -#include "common/stats/thread_local_store.h" #include "server/config_validation/server.h" #include "server/drain_manager_impl.h" @@ -22,79 +23,107 @@ #include "ares.h" namespace Envoy { -namespace Server { - -class ProdComponentFactory : public ComponentFactory { -public: - // Server::DrainManagerFactory - DrainManagerPtr createDrainManager(Instance& server) override { - return DrainManagerPtr{ - // The global drain manager only triggers on listener modification, which effectively is - // hot restart at the global level. The per-listener drain managers decide whether to - // to include /healthcheck/fail status. - new DrainManagerImpl(server, envoy::api::v2::Listener_DrainType_MODIFY_ONLY)}; - } - Runtime::LoaderPtr createRuntime(Server::Instance& server, - Server::Configuration::Initial& config) override { - return Server::InstanceUtil::createRuntime(server, config); - } -}; +Server::DrainManagerPtr ProdComponentFactory::createDrainManager(Server::Instance& server) { + // The global drain manager only triggers on listener modification, which effectively is + // hot restart at the global level. The per-listener drain managers decide whether to + // to include /healthcheck/fail status. + return std::make_unique(server, + envoy::api::v2::Listener_DrainType_MODIFY_ONLY); +} -} // namespace Server +Runtime::LoaderPtr ProdComponentFactory::createRuntime(Server::Instance& server, + Server::Configuration::Initial& config) { + return Server::InstanceUtil::createRuntime(server, config); +} -int main_common(OptionsImpl& options) { - Stats::RawStatData::configure(options); +MainCommonBase::MainCommonBase(OptionsImpl& options, bool hot_restart) : options_(options) { + ares_library_init(ARES_LIB_INIT_ALL); + Event::Libevent::Global::initialize(); + RELEASE_ASSERT(Envoy::Server::validateProtoDescriptors()); + switch (options_.mode()) { + case Server::Mode::Serve: { #ifdef ENVOY_HOT_RESTART - std::unique_ptr restarter; - try { - restarter.reset(new Server::HotRestartImpl(options)); - } catch (Envoy::EnvoyException& e) { - std::cerr << "unable to initialize hot restart: " << e.what() << std::endl; - return 1; + if (hot_restart) { + restarter_.reset(new Server::HotRestartImpl(options_)); + } +#endif + if (!hot_restart) { + restarter_.reset(new Server::HotRestartNopImpl()); + } + + Stats::RawStatData::configure(options_); + tls_.reset(new ThreadLocal::InstanceImpl); + Thread::BasicLockable& log_lock = restarter_->logLock(); + Thread::BasicLockable& access_log_lock = restarter_->accessLogLock(); + auto local_address = Network::Utility::getLocalAddress(options_.localAddressIpVersion()); + Logger::Registry::initialize(options_.logLevel(), log_lock); + + stats_store_.reset(new Stats::ThreadLocalStoreImpl(restarter_->statsAllocator())); + server_.reset(new Server::InstanceImpl(options_, local_address, default_test_hooks_, + *restarter_, *stats_store_, access_log_lock, + component_factory_, *tls_)); + break; } + case Server::Mode::Validate: + break; + } +} - Thread::BasicLockable& log_lock = restarter->logLock(); - Thread::BasicLockable& access_log_lock = restarter->accessLogLock(); - Stats::RawStatDataAllocator& stats_allocator = *restarter; -#else - std::unique_ptr restarter; - restarter.reset(new Server::HotRestartNopImpl()); - - Thread::MutexBasicLockable log_lock, access_log_lock; - Stats::HeapRawStatDataAllocator stats_allocator; -#endif +MainCommonBase::~MainCommonBase() { ares_library_cleanup(); } - RELEASE_ASSERT(Envoy::Server::validateProtoDescriptors()); - Event::Libevent::Global::initialize(); - Server::ProdComponentFactory component_factory; - auto local_address = Network::Utility::getLocalAddress(options.localAddressIpVersion()); - switch (options.mode()) { +bool MainCommonBase::run() { + switch (options_.mode()) { case Server::Mode::Serve: - break; - case Server::Mode::Validate: - Thread::MutexBasicLockable log_lock; - Logger::Registry::initialize(options.logLevel(), log_lock); - return Server::validateConfig(options, local_address, component_factory) ? 0 : 1; + server_->run(); + return true; + case Server::Mode::Validate: { + auto local_address = Network::Utility::getLocalAddress(options_.localAddressIpVersion()); + return Server::validateConfig(options_, local_address, component_factory_); } + } + NOT_REACHED; +} - ares_library_init(ARES_LIB_INIT_ALL); +MainCommon::MainCommon(int argc, char** argv, bool hot_restart) + : options_(computeOptions(argc, argv, hot_restart)), base_(*options_, hot_restart) {} + +std::unique_ptr MainCommon::computeOptions(int argc, char** argv, bool hot_restart) { + OptionsImpl::HotRestartVersionCb hot_restart_version_cb = [](uint64_t, uint64_t) { + return "disabled"; + }; - Logger::Registry::initialize(options.logLevel(), log_lock); - DefaultTestHooks default_test_hooks; - ThreadLocal::InstanceImpl tls; - Stats::ThreadLocalStoreImpl stats_store(stats_allocator); +#ifdef ENVOY_HOT_RESTART + if (hot_restart) { + // Enabled by default, except on OS X. Control with "bazel --define=hot_restart=disabled" + hot_restart_version_cb = [](uint64_t max_num_stats, uint64_t max_stat_name_len) { + return Server::HotRestartImpl::hotRestartVersion(max_num_stats, max_stat_name_len); + }; + } +#else + // Hot-restart should not be specified if the support is not compiled in. + RELEASE_ASSERT(!hot_restart); +#endif + return std::make_unique(argc, argv, hot_restart_version_cb, spdlog::level::info); +} + +// Legacy implementation of main_common. +// +// TODO(jmarantz): Remove this when all callers are removed. At that time, MainCommonBase +// and MainCommon can be merged. The current theory is that only Google calls this. +int main_common(OptionsImpl& options) { try { - Server::InstanceImpl server(options, local_address, default_test_hooks, *restarter, stats_store, - access_log_lock, component_factory, tls); - server.run(); - } catch (const EnvoyException& e) { - ares_library_cleanup(); - return 1; +#if ENVOY_HOT_RESTART + MainCommonBase main_common(options, true); +#else + MainCommonBase main_common(options, false); +#endif + return main_common.run() ? EXIT_SUCCESS : EXIT_FAILURE; + } catch (EnvoyException& e) { + return EXIT_FAILURE; } - ares_library_cleanup(); - return 0; + return EXIT_SUCCESS; } } // namespace Envoy diff --git a/source/exe/main_common.h b/source/exe/main_common.h index 89f576ffba6e6..2117f1c40fe89 100644 --- a/source/exe/main_common.h +++ b/source/exe/main_common.h @@ -1,8 +1,59 @@ #pragma once +#include "common/stats/thread_local_store.h" +#include "common/thread_local/thread_local_impl.h" + #include "server/options_impl.h" +#include "server/server.h" + +#ifdef ENVOY_HANDLE_SIGNALS +#include "exe/signal_action.h" +#endif namespace Envoy { + +class ProdComponentFactory : public Server::ComponentFactory { +public: + // Server::DrainManagerFactory + Server::DrainManagerPtr createDrainManager(Server::Instance& server) override; + Runtime::LoaderPtr createRuntime(Server::Instance& server, + Server::Configuration::Initial& config) override; +}; + +class MainCommonBase { +public: + MainCommonBase(OptionsImpl& options, bool hot_restart); + ~MainCommonBase(); + + bool run(); + +protected: + Envoy::OptionsImpl& options_; + ProdComponentFactory component_factory_; + DefaultTestHooks default_test_hooks_; + std::unique_ptr tls_; + std::unique_ptr restarter_; + std::unique_ptr stats_store_; + std::unique_ptr server_; +}; + +class MainCommon { +public: + MainCommon(int argc, char** argv, bool hot_restart); + bool run() { return base_.run(); } + +private: + static std::unique_ptr computeOptions(int argc, char** argv, + bool hot_restart); + +#ifdef ENVOY_HANDLE_SIGNALS + Envoy::SignalAction handle_sigs; +#endif + + std::unique_ptr options_; + MainCommonBase base_; +}; + /** * This is the real main body that executes after site-specific * main() runs. @@ -10,6 +61,6 @@ namespace Envoy { * @param options Options object initialized by site-specific code * @return int Return code that should be returned from the actual main() */ -int main_common(Envoy::OptionsImpl& options); +int main_common(OptionsImpl& options); } // namespace Envoy diff --git a/source/server/hot_restart_impl.h b/source/server/hot_restart_impl.h index c851a54270ed4..a00573b5b512a 100644 --- a/source/server/hot_restart_impl.h +++ b/source/server/hot_restart_impl.h @@ -120,9 +120,6 @@ class HotRestartImpl : public HotRestart, public: HotRestartImpl(Options& options); - Thread::BasicLockable& logLock() { return log_lock_; } - Thread::BasicLockable& accessLogLock() { return access_log_lock_; } - // Server::HotRestart void drainParentListeners() override; int duplicateParentListenSocket(const std::string& address) override; @@ -132,6 +129,9 @@ class HotRestartImpl : public HotRestart, void terminateParent() override; void shutdown() override; std::string version() override; + Thread::BasicLockable& logLock() override { return log_lock_; } + Thread::BasicLockable& accessLogLock() override { return access_log_lock_; } + Stats::RawStatDataAllocator& statsAllocator() override { return *this; } /** * envoy --hot_restart_version doesn't initialize Envoy, but computes the version string diff --git a/source/server/hot_restart_nop_impl.h b/source/server/hot_restart_nop_impl.h index bbf479733ab99..73568945fdb93 100644 --- a/source/server/hot_restart_nop_impl.h +++ b/source/server/hot_restart_nop_impl.h @@ -4,6 +4,8 @@ #include "envoy/server/hot_restart.h" +#include "common/common/thread.h" + namespace Envoy { namespace Server { @@ -12,8 +14,9 @@ namespace Server { */ class HotRestartNopImpl : public Server::HotRestart { public: - HotRestartNopImpl(){}; + HotRestartNopImpl() {} + // Server::HotRestart void drainParentListeners() override {} int duplicateParentListenSocket(const std::string&) override { return -1; } void getParentStats(GetParentStatsInfo& info) override { memset(&info, 0, sizeof(info)); } @@ -22,6 +25,14 @@ class HotRestartNopImpl : public Server::HotRestart { void terminateParent() override {} void shutdown() override {} std::string version() override { return "disabled"; } + Thread::BasicLockable& logLock() override { return log_lock_; } + Thread::BasicLockable& accessLogLock() override { return access_log_lock_; } + Stats::RawStatDataAllocator& statsAllocator() override { return stats_allocator_; } + +private: + Thread::MutexBasicLockable log_lock_; + Thread::MutexBasicLockable access_log_lock_; + Stats::HeapRawStatDataAllocator stats_allocator_; }; } // namespace Server diff --git a/source/server/server.cc b/source/server/server.cc index bd695f155ed62..7c5faaa7ff32e 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -51,7 +51,7 @@ InstanceImpl::InstanceImpl(Options& options, Network::Address::InstanceConstShar handler_(new ConnectionHandlerImpl(ENVOY_LOGGER(), *dispatcher_)), listener_component_factory_(*this), worker_factory_(thread_local_, *api_, hooks), dns_resolver_(dispatcher_->createDnsResolver({})), - access_log_manager_(*api_, *dispatcher_, access_log_lock, store) { + access_log_manager_(*api_, *dispatcher_, access_log_lock, store), terminated_(false) { try { if (!options.logPath().empty()) { @@ -70,19 +70,13 @@ InstanceImpl::InstanceImpl(Options& options, Network::Address::InstanceConstShar ENVOY_LOG(critical, "error initializing configuration '{}': {}", options.configPath(), e.what()); - // Invoke shutdown methods called in run(). - thread_local_.shutdownGlobalThreading(); - stats_store_.shutdownThreading(); - thread_local_.shutdownThread(); - - // Invoke the shutdown method called in the destructor. - restarter_.shutdown(); + terminate(); throw; } } InstanceImpl::~InstanceImpl() { - restarter_.shutdown(); + terminate(); // Stop logging to file before all the AccessLogManager and its dependencies are // destructed to avoid crashing at shutdown. @@ -364,6 +358,15 @@ void InstanceImpl::run() { guard_dog_->stopWatching(watchdog); watchdog.reset(); + terminate(); +} + +void InstanceImpl::terminate() { + if (terminated_) { + return; + } + terminated_ = true; + // Before starting to shutdown anything else, stop slot destruction updates. thread_local_.shutdownGlobalThreading(); @@ -371,16 +374,21 @@ void InstanceImpl::run() { stats_store_.shutdownThreading(); // Shutdown all the workers now that the main dispatch loop is done. - listener_manager_->stopWorkers(); + if (listener_manager_.get() != nullptr) { + listener_manager_->stopWorkers(); + } // Only flush if we have not been hot restarted. if (stat_flush_timer_) { flushStats(); } - config_->clusterManager().shutdown(); + if (config_.get() != nullptr) { + config_->clusterManager().shutdown(); + } handler_.reset(); thread_local_.shutdownThread(); + restarter_.shutdown(); ENVOY_LOG(info, "exiting"); ENVOY_FLUSH_LOG(); } diff --git a/source/server/server.h b/source/server/server.h index b742d5ac41550..fa73bd9f6fb5e 100644 --- a/source/server/server.h +++ b/source/server/server.h @@ -171,6 +171,7 @@ class InstanceImpl : Logger::Loggable, public Instance { void loadServerFlags(const Optional& flags_path); uint64_t numConnections(); void startWorkers(); + void terminate(); Options& options_; HotRestart& restarter_; @@ -199,6 +200,7 @@ class InstanceImpl : Logger::Loggable, public Instance { std::unique_ptr cluster_manager_factory_; InitManagerImpl init_manager_; std::unique_ptr guard_dog_; + bool terminated_; }; } // Server diff --git a/test/config/integration/BUILD b/test/config/integration/BUILD index 8e89dbb7cdd93..c598858a65115 100644 --- a/test/config/integration/BUILD +++ b/test/config/integration/BUILD @@ -39,3 +39,8 @@ filegroup( "server.json", ], ) + +filegroup( + name = "google_com_proxy_port_0", + srcs = ["google_com_proxy_port_0.v2.yaml"], +) diff --git a/test/config/integration/google_com_proxy_port_0.v2.yaml b/test/config/integration/google_com_proxy_port_0.v2.yaml new file mode 100644 index 0000000000000..347ad50734a6c --- /dev/null +++ b/test/config/integration/google_com_proxy_port_0.v2.yaml @@ -0,0 +1,32 @@ +admin: + access_log_path: /dev/null + address: + socket_address: { address: 0.0.0.0, port_value: 0 } + +static_resources: + listeners: + - name: listener_0 + address: + socket_address: { address: 0.0.0.0, port_value: 0 } + filter_chains: + - filters: + - name: envoy.http_connection_manager + config: + stat_prefix: ingress_http + codec_type: AUTO + route_config: + name: local_route + virtual_hosts: + - name: local_service + domains: ["*"] + routes: + - match: { prefix: "/" } + route: { host_rewrite: www.google.com, cluster: service_google } + clusters: + - name: service_google + connect_timeout: 0.25s + type: LOGICAL_DNS + # Comment out the following line to test on v6 networks + dns_lookup_family: V4_ONLY + lb_policy: ROUND_ROBIN + hosts: [{ socket_address: { address: google.com, port_value: 443 }}] diff --git a/test/exe/BUILD b/test/exe/BUILD index 1e8d6745ce832..3d140e64778c8 100644 --- a/test/exe/BUILD +++ b/test/exe/BUILD @@ -23,6 +23,16 @@ sh_test( data = ["//source/exe:envoy-static"], ) +envoy_cc_test( + name = "main_common_test", + srcs = ["main_common_test.cc"], + data = ["//test/config/integration:google_com_proxy_port_0"], + deps = [ + "//source/exe:envoy_main_common_lib", + "//test/test_common:environment_lib", + ], +) + envoy_cc_test( name = "signals_test", srcs = ["signals_test.cc"], diff --git a/test/exe/main_common_test.cc b/test/exe/main_common_test.cc new file mode 100644 index 0000000000000..13fb3ff4cc875 --- /dev/null +++ b/test/exe/main_common_test.cc @@ -0,0 +1,75 @@ +// MainCommonTest works fine in coverage tests, but it appears to break SignalsTest when +// run in the same process. It appears that MainCommon doesn't completely clean up after +// itself, possibly due to a bug in SignalAction. So for now, we can test MainCommon +// but can't measure its test coverage. +// +// TODO(issues/2580): Fix coverage tests when MainCommonTest is enabled. +#ifndef ENVOY_CONFIG_COVERAGE + +#include "exe/main_common.h" + +#include "server/options_impl.h" + +#include "test/test_common/environment.h" + +#include "gtest/gtest.h" + +#ifdef ENVOY_HANDLE_SIGNALS +#include "exe/signal_action.h" +#endif + +#ifdef ENVOY_HOT_RESTART +#include "server/hot_restart_impl.h" +#endif + +namespace Envoy { + +TEST(MainCommon, ConstructDestruct) { + std::string config_file = Envoy::TestEnvironment::getCheckedEnvVar("TEST_RUNDIR") + + "/test/config/integration/google_com_proxy_port_0.v2.yaml"; + const char* argv[] = {"envoy-static", "-c", config_file.c_str(), nullptr}; + MainCommon main_common(3, const_cast(argv), false); +} + +TEST(MainCommon, LegacyMain) { + // Testing the legacy path is difficult because if we give it a valid config, it will + // never exit. So just give it an empty config and let it fail. + int argc = 1; + const char* argv[] = {"envoy_static", nullptr}; + +#ifdef ENVOY_HANDLE_SIGNALS + // Enabled by default. Control with "bazel --define=signal_trace=disabled" + Envoy::SignalAction handle_sigs; +#endif + +#ifdef ENVOY_HOT_RESTART + // Enabled by default, except on OS X. Control with "bazel --define=hot_restart=disabled" + const Envoy::OptionsImpl::HotRestartVersionCb hot_restart_version_cb = + [](uint64_t max_num_stats, uint64_t max_stat_name_len) { + return Envoy::Server::HotRestartImpl::hotRestartVersion(max_num_stats, max_stat_name_len); + }; +#else + const Envoy::OptionsImpl::HotRestartVersionCb hot_restart_version_cb = [](uint64_t, uint64_t) { + return "disabled"; + }; +#endif + + std::unique_ptr options; + int return_code = -1; + try { + options = std::make_unique(argc, const_cast(argv), + hot_restart_version_cb, spdlog::level::info); + } catch (const Envoy::NoServingException& e) { + return_code = EXIT_SUCCESS; + } catch (const Envoy::MalformedArgvException& e) { + return_code = EXIT_FAILURE; + } + if (return_code == -1) { + return_code = Envoy::main_common(*options); + } + EXPECT_EQ(EXIT_FAILURE, return_code); +} + +} // namespace Envoy + +#endif // ENVOY_CONFIG_COVERAGE diff --git a/test/mocks/server/mocks.cc b/test/mocks/server/mocks.cc index 16d0bf7b7e63d..41c26093b72c6 100644 --- a/test/mocks/server/mocks.cc +++ b/test/mocks/server/mocks.cc @@ -47,7 +47,11 @@ MockGuardDog::MockGuardDog() : watch_dog_(new NiceMock()) { } MockGuardDog::~MockGuardDog() {} -MockHotRestart::MockHotRestart() {} +MockHotRestart::MockHotRestart() { + ON_CALL(*this, logLock()).WillByDefault(ReturnRef(log_lock_)); + ON_CALL(*this, accessLogLock()).WillByDefault(ReturnRef(access_log_lock_)); + ON_CALL(*this, statsAllocator()).WillByDefault(ReturnRef(stats_allocator_)); +} MockHotRestart::~MockHotRestart() {} MockListenerComponentFactory::MockListenerComponentFactory() diff --git a/test/mocks/server/mocks.h b/test/mocks/server/mocks.h index 620d759094cf5..89b5f98fcd367 100644 --- a/test/mocks/server/mocks.h +++ b/test/mocks/server/mocks.h @@ -14,6 +14,7 @@ #include "envoy/server/transport_socket_config.h" #include "envoy/server/worker.h" #include "envoy/ssl/context_manager.h" +#include "envoy/thread/thread.h" #include "common/ssl/context_manager_impl.h" #include "common/stats/stats_impl.h" @@ -135,6 +136,14 @@ class MockHotRestart : public HotRestart { MOCK_METHOD0(terminateParent, void()); MOCK_METHOD0(shutdown, void()); MOCK_METHOD0(version, std::string()); + MOCK_METHOD0(logLock, Thread::BasicLockable&()); + MOCK_METHOD0(accessLogLock, Thread::BasicLockable&()); + MOCK_METHOD0(statsAllocator, Stats::RawStatDataAllocator&()); + +private: + Thread::MutexBasicLockable log_lock_; + Thread::MutexBasicLockable access_log_lock_; + Stats::HeapRawStatDataAllocator stats_allocator_; }; class MockListenerComponentFactory : public ListenerComponentFactory { diff --git a/test/run_envoy_bazel_coverage.sh b/test/run_envoy_bazel_coverage.sh index 353fcc8977a52..081975b441e16 100755 --- a/test/run_envoy_bazel_coverage.sh +++ b/test/run_envoy_bazel_coverage.sh @@ -34,9 +34,9 @@ BAZEL_TEST_OPTIONS="${BAZEL_TEST_OPTIONS} -c dbg --copt=-DNDEBUG" # a single coverage test binary and do not require the "bazel coverage" support # for collecting multiple traces and glueing them together. "${BAZEL_COVERAGE}" --batch test "${COVERAGE_TARGET}" ${BAZEL_TEST_OPTIONS} \ - --cache_test_results=no --cxxopt="--coverage" --linkopt="--coverage" \ - --define ENVOY_CONFIG_COVERAGE=1 \ - --test_output=all --strategy=Genrule=standalone --spawn_strategy=standalone + --cache_test_results=no --cxxopt="--coverage" --cxxopt="-DENVOY_CONFIG_COVERAGE=1" \ + --linkopt="--coverage" --define ENVOY_CONFIG_COVERAGE=1 --test_output=streamed \ + --strategy=Genrule=standalone --spawn_strategy=standalone --test_timeout=1000 # The Bazel build has a lot of whack in it, in particular generated files, headers from external # deps, etc. So, we exclude this from gcov to avoid false reporting of these files in the html and @@ -84,3 +84,4 @@ if test ${COVERAGE_FAILED} -eq 1; then else echo Code coverage ${COVERAGE_VALUE} is good and higher than limit of ${COVERAGE_THRESHOLD} fi +echo "HTML coverage report is in ${COVERAGE_DIR}/coverage.html" diff --git a/test/server/server_test.cc b/test/server/server_test.cc index a9ab8b09c99f6..eb66c382d837f 100644 --- a/test/server/server_test.cc +++ b/test/server/server_test.cc @@ -107,14 +107,6 @@ class ServerInstanceImplTest : public testing::TestWithParamapi().fileExists("/dev/null")); } - void TearDown() override { - if (server_) { - server_->threadLocal().shutdownGlobalThreading(); - server_->clusterManager().shutdown(); - server_->threadLocal().shutdownThread(); - } - } - Network::Address::IpVersion version_; testing::NiceMock options_; DefaultTestHooks hooks_;