From a423c3551665f019520306407f247d21b788326b Mon Sep 17 00:00:00 2001 From: Trevor Schroeder Date: Thu, 17 May 2018 13:05:14 -0400 Subject: [PATCH 1/3] Add setters for all OptionsImpl fields. This allows modifying options in a custom main() before instantiating the server but leaving them readonly within the server's context. Signed-off-by: Trevor Schroeder --- source/server/options_impl.h | 35 +++++++++++++++++++++++ test/server/options_impl_test.cc | 49 ++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+) diff --git a/source/server/options_impl.h b/source/server/options_impl.h index befb0261605a1..8950d72e96369 100644 --- a/source/server/options_impl.h +++ b/source/server/options_impl.h @@ -32,30 +32,65 @@ class OptionsImpl : public Server::Options { // Server::Options uint64_t baseId() const override { return base_id_; } + void setBaseId(uint64_t base_id) { base_id_ = base_id; }; uint32_t concurrency() const override { return concurrency_; } + void setConcurrency(uint32_t concurrency) { concurrency_ = concurrency; } const std::string& configPath() const override { return config_path_; } + void setConfigPath(const std::string& config_path) { config_path_ = config_path; } const std::string& configYaml() const override { return config_yaml_; } + void setConfigYaml(const std::string& config_yaml) { config_yaml_ = config_yaml; } bool v2ConfigOnly() const override { return v2_config_only_; } + void setV2ConfigOnly(bool v2_config_only) { v2_config_only_ = v2_config_only; } const std::string& adminAddressPath() const override { return admin_address_path_; } + void setAdminAddressPath(const std::string& admin_address_path) { + admin_address_path_ = admin_address_path; + } Network::Address::IpVersion localAddressIpVersion() const override { return local_address_ip_version_; } + void setLocalAddressIpVersion(Network::Address::IpVersion local_address_ip_version) { + local_address_ip_version_ = local_address_ip_version; + } std::chrono::seconds drainTime() const override { return drain_time_; } + void setDrainTime(std::chrono::seconds drain_time) { drain_time_ = drain_time; } spdlog::level::level_enum logLevel() const override { return log_level_; } + void setLogLevel(spdlog::level::level_enum log_level) { log_level_ = log_level; } const std::string& logFormat() const override { return log_format_; } + void setLogFormat(const std::string& log_format) { log_format_ = log_format; } const std::string& logPath() const override { return log_path_; } + void setLogPath(const std::string& log_path) { log_path_ = log_path; } std::chrono::seconds parentShutdownTime() const override { return parent_shutdown_time_; } + void setParentShutdownTime(std::chrono::seconds parent_shutdown_time) { + parent_shutdown_time_ = parent_shutdown_time; + } uint64_t restartEpoch() const override { return restart_epoch_; } + void setRestartEpoch(uint64_t restart_epoch) { restart_epoch_ = restart_epoch; } Server::Mode mode() const override { return mode_; } + void setMode(Server::Mode mode) { mode_ = mode; } std::chrono::milliseconds fileFlushIntervalMsec() const override { return file_flush_interval_msec_; } + void setFileFlushIntervalMsec(std::chrono::milliseconds file_flush_interval_msec) { + file_flush_interval_msec_ = file_flush_interval_msec; + } const std::string& serviceClusterName() const override { return service_cluster_; } + void setServiceClusterName(const std::string& service_cluster) { + service_cluster_ = service_cluster; + } const std::string& serviceNodeName() const override { return service_node_; } + void setServiceNodeName(const std::string& service_node) { service_node_ = service_node; } const std::string& serviceZone() const override { return service_zone_; } + void setServiceZone(const std::string& service_zone) { service_zone_ = service_zone; } uint64_t maxStats() const override { return max_stats_; } + void setMaxStats(uint64_t max_stats) { max_stats_ = max_stats; } uint64_t maxObjNameLength() const override { return max_obj_name_length_; } + void setMaxObjNameLength(uint64_t max_obj_name_length) { + max_obj_name_length_ = max_obj_name_length; + } bool hotRestartDisabled() const override { return hot_restart_disabled_; } + void setHotRestartDisabled(bool hot_restart_disabled) { + hot_restart_disabled_ = hot_restart_disabled; + } private: uint64_t base_id_; diff --git a/test/server/options_impl_test.cc b/test/server/options_impl_test.cc index 6b28b9ed56c8f..d627e3a59eac4 100644 --- a/test/server/options_impl_test.cc +++ b/test/server/options_impl_test.cc @@ -83,6 +83,55 @@ TEST(OptionsImplTest, All) { EXPECT_EQ(true, options->hotRestartDisabled()); } +TEST(OptionsImplTest, SetAll) { + std::unique_ptr options = createOptionsImpl("envoy -c hello"); + bool v2_config_only = options->v2ConfigOnly(); + bool hot_restart_disabled = options->v2ConfigOnly(); + options->setBaseId(109876); + options->setConcurrency(42); + options->setConfigPath("foo"); + options->setConfigYaml("bogus:"); + options->setV2ConfigOnly(!options->v2ConfigOnly()); + options->setAdminAddressPath("path"); + options->setLocalAddressIpVersion(Network::Address::IpVersion::v6); + options->setDrainTime(std::chrono::seconds(42)); + options->setLogLevel(spdlog::level::trace); + options->setLogFormat("%L %n %v"); + options->setLogPath("/foo/bar"); + options->setParentShutdownTime(std::chrono::seconds(43)); + options->setRestartEpoch(44); + options->setFileFlushIntervalMsec(std::chrono::milliseconds(45)); + options->setMode(Server::Mode::Validate); + options->setServiceClusterName("cluster_foo"); + options->setServiceNodeName("node_foo"); + options->setServiceZone("zone_foo"); + options->setMaxStats(12345); + options->setMaxObjNameLength(54321); + options->setHotRestartDisabled(!options->hotRestartDisabled()); + + EXPECT_EQ(109876, options->baseId()); + EXPECT_EQ(42U, options->concurrency()); + EXPECT_EQ("foo", options->configPath()); + EXPECT_EQ("bogus:", options->configYaml()); + EXPECT_EQ(!v2_config_only, options->v2ConfigOnly()); + EXPECT_EQ("path", options->adminAddressPath()); + EXPECT_EQ(Network::Address::IpVersion::v6, options->localAddressIpVersion()); + EXPECT_EQ(std::chrono::seconds(42), options->drainTime()); + EXPECT_EQ(spdlog::level::trace, options->logLevel()); + EXPECT_EQ("%L %n %v", options->logFormat()); + EXPECT_EQ("/foo/bar", options->logPath()); + EXPECT_EQ(std::chrono::seconds(43), options->parentShutdownTime()); + EXPECT_EQ(44, options->restartEpoch()); + EXPECT_EQ(std::chrono::milliseconds(45), options->fileFlushIntervalMsec()); + EXPECT_EQ(Server::Mode::Validate, options->mode()); + EXPECT_EQ("cluster_foo", options->serviceClusterName()); + EXPECT_EQ("node_foo", options->serviceNodeName()); + EXPECT_EQ("zone_foo", options->serviceZone()); + EXPECT_EQ(12345U, options->maxStats()); + EXPECT_EQ(54321U, options->maxObjNameLength()); + EXPECT_EQ(!hot_restart_disabled, options->hotRestartDisabled()); +} + TEST(OptionsImplTest, DefaultParams) { std::unique_ptr options = createOptionsImpl("envoy -c hello"); EXPECT_EQ(std::chrono::seconds(600), options->drainTime()); From 25815d9b0fae9fb3900132f7d0e4af34b003622b Mon Sep 17 00:00:00 2001 From: Trevor Schroeder Date: Thu, 17 May 2018 16:54:56 -0400 Subject: [PATCH 2/3] Sort setters above public Options interface methods. Signed-off-by: Trevor Schroeder --- source/server/options_impl.h | 48 +++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/source/server/options_impl.h b/source/server/options_impl.h index 8950d72e96369..f0f8793827981 100644 --- a/source/server/options_impl.h +++ b/source/server/options_impl.h @@ -30,42 +30,26 @@ class OptionsImpl : public Server::Options { OptionsImpl(int argc, char** argv, const HotRestartVersionCb& hot_restart_version_cb, spdlog::level::level_enum default_log_level); - // Server::Options - uint64_t baseId() const override { return base_id_; } + // Setters for option fields. These are not part of the Options interface. void setBaseId(uint64_t base_id) { base_id_ = base_id; }; - uint32_t concurrency() const override { return concurrency_; } void setConcurrency(uint32_t concurrency) { concurrency_ = concurrency; } - const std::string& configPath() const override { return config_path_; } void setConfigPath(const std::string& config_path) { config_path_ = config_path; } - const std::string& configYaml() const override { return config_yaml_; } void setConfigYaml(const std::string& config_yaml) { config_yaml_ = config_yaml; } - bool v2ConfigOnly() const override { return v2_config_only_; } void setV2ConfigOnly(bool v2_config_only) { v2_config_only_ = v2_config_only; } - const std::string& adminAddressPath() const override { return admin_address_path_; } void setAdminAddressPath(const std::string& admin_address_path) { admin_address_path_ = admin_address_path; } - Network::Address::IpVersion localAddressIpVersion() const override { - return local_address_ip_version_; - } void setLocalAddressIpVersion(Network::Address::IpVersion local_address_ip_version) { local_address_ip_version_ = local_address_ip_version; } - std::chrono::seconds drainTime() const override { return drain_time_; } void setDrainTime(std::chrono::seconds drain_time) { drain_time_ = drain_time; } - spdlog::level::level_enum logLevel() const override { return log_level_; } void setLogLevel(spdlog::level::level_enum log_level) { log_level_ = log_level; } - const std::string& logFormat() const override { return log_format_; } void setLogFormat(const std::string& log_format) { log_format_ = log_format; } - const std::string& logPath() const override { return log_path_; } void setLogPath(const std::string& log_path) { log_path_ = log_path; } - std::chrono::seconds parentShutdownTime() const override { return parent_shutdown_time_; } void setParentShutdownTime(std::chrono::seconds parent_shutdown_time) { parent_shutdown_time_ = parent_shutdown_time; } - uint64_t restartEpoch() const override { return restart_epoch_; } void setRestartEpoch(uint64_t restart_epoch) { restart_epoch_ = restart_epoch; } - Server::Mode mode() const override { return mode_; } void setMode(Server::Mode mode) { mode_ = mode; } std::chrono::milliseconds fileFlushIntervalMsec() const override { return file_flush_interval_msec_; @@ -73,25 +57,43 @@ class OptionsImpl : public Server::Options { void setFileFlushIntervalMsec(std::chrono::milliseconds file_flush_interval_msec) { file_flush_interval_msec_ = file_flush_interval_msec; } - const std::string& serviceClusterName() const override { return service_cluster_; } void setServiceClusterName(const std::string& service_cluster) { service_cluster_ = service_cluster; } - const std::string& serviceNodeName() const override { return service_node_; } void setServiceNodeName(const std::string& service_node) { service_node_ = service_node; } - const std::string& serviceZone() const override { return service_zone_; } void setServiceZone(const std::string& service_zone) { service_zone_ = service_zone; } - uint64_t maxStats() const override { return max_stats_; } void setMaxStats(uint64_t max_stats) { max_stats_ = max_stats; } - uint64_t maxObjNameLength() const override { return max_obj_name_length_; } void setMaxObjNameLength(uint64_t max_obj_name_length) { max_obj_name_length_ = max_obj_name_length; } - bool hotRestartDisabled() const override { return hot_restart_disabled_; } void setHotRestartDisabled(bool hot_restart_disabled) { hot_restart_disabled_ = hot_restart_disabled; } + // Server::Options + uint64_t baseId() const override { return base_id_; } + uint32_t concurrency() const override { return concurrency_; } + const std::string& configPath() const override { return config_path_; } + const std::string& configYaml() const override { return config_yaml_; } + bool v2ConfigOnly() const override { return v2_config_only_; } + const std::string& adminAddressPath() const override { return admin_address_path_; } + Network::Address::IpVersion localAddressIpVersion() const override { + return local_address_ip_version_; + } + std::chrono::seconds drainTime() const override { return drain_time_; } + spdlog::level::level_enum logLevel() const override { return log_level_; } + const std::string& logFormat() const override { return log_format_; } + const std::string& logPath() const override { return log_path_; } + std::chrono::seconds parentShutdownTime() const override { return parent_shutdown_time_; } + uint64_t restartEpoch() const override { return restart_epoch_; } + Server::Mode mode() const override { return mode_; } + const std::string& serviceClusterName() const override { return service_cluster_; } + const std::string& serviceNodeName() const override { return service_node_; } + const std::string& serviceZone() const override { return service_zone_; } + uint64_t maxStats() const override { return max_stats_; } + uint64_t maxObjNameLength() const override { return max_obj_name_length_; } + bool hotRestartDisabled() const override { return hot_restart_disabled_; } + private: uint64_t base_id_; uint32_t concurrency_; From 416e595f43e9792d923bb26e4c39adca1139391b Mon Sep 17 00:00:00 2001 From: Trevor Schroeder Date: Thu, 17 May 2018 17:17:21 -0400 Subject: [PATCH 3/3] Put fileFlushInterval back in the right place. Signed-off-by: Trevor Schroeder --- source/server/options_impl.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/source/server/options_impl.h b/source/server/options_impl.h index f0f8793827981..4c0ba07007501 100644 --- a/source/server/options_impl.h +++ b/source/server/options_impl.h @@ -51,9 +51,6 @@ class OptionsImpl : public Server::Options { } void setRestartEpoch(uint64_t restart_epoch) { restart_epoch_ = restart_epoch; } void setMode(Server::Mode mode) { mode_ = mode; } - std::chrono::milliseconds fileFlushIntervalMsec() const override { - return file_flush_interval_msec_; - } void setFileFlushIntervalMsec(std::chrono::milliseconds file_flush_interval_msec) { file_flush_interval_msec_ = file_flush_interval_msec; } @@ -87,6 +84,9 @@ class OptionsImpl : public Server::Options { std::chrono::seconds parentShutdownTime() const override { return parent_shutdown_time_; } uint64_t restartEpoch() const override { return restart_epoch_; } Server::Mode mode() const override { return mode_; } + std::chrono::milliseconds fileFlushIntervalMsec() const override { + return file_flush_interval_msec_; + } const std::string& serviceClusterName() const override { return service_cluster_; } const std::string& serviceNodeName() const override { return service_node_; } const std::string& serviceZone() const override { return service_zone_; }