From ddb9a942a9ec4bdf83022e8b3bb2431a27e2775d Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 7 Mar 2022 12:49:00 -0500 Subject: [PATCH 1/3] cleanup Signed-off-by: Alyssa Wilk --- source/server/config_validation/server.cc | 10 +++- source/server/config_validation/server.h | 8 ++- test/common/http/conn_pool_grid_test.cc | 4 +- test/common/http/http2/codec_impl_test.cc | 4 +- test/common/network/listener_impl_test.cc | 4 +- test/common/quic/active_quic_listener_test.cc | 5 -- test/common/router/router_test.cc | 2 +- test/config_test/config_test.cc | 10 ---- test/server/config_validation/server_test.cc | 52 +++++-------------- test/server/connection_handler_test.cc | 2 +- test/test_common/test_runtime.h | 24 ++++++--- 11 files changed, 54 insertions(+), 71 deletions(-) diff --git a/source/server/config_validation/server.cc b/source/server/config_validation/server.cc index 5cb7c97b8e0f..f99d8242c444 100644 --- a/source/server/config_validation/server.cc +++ b/source/server/config_validation/server.cc @@ -100,8 +100,14 @@ void ValidationInstance::initialize(const Options& options, listener_manager_ = std::make_unique(*this, *this, *this, false, quic_stat_names_); thread_local_.registerThread(*dispatcher_, true); - runtime_singleton_ = std::make_unique( - component_factory.createRuntime(*this, initial_config)); + + Runtime::LoaderPtr runtime_ptr = component_factory.createRuntime(*this, initial_config); + if (runtime_ptr->snapshot().getBoolean("envoy.restart_features.no_runtime_singleton", false)) { + runtime_ = std::move(runtime_ptr); + } else { + runtime_singleton_ = std::make_unique(std::move(runtime_ptr)); + } + secret_manager_ = std::make_unique(admin().getConfigTracker()); ssl_context_manager_ = createContextManager("ssl_context_manager", api_->timeSource()); cluster_manager_factory_ = std::make_unique( diff --git a/source/server/config_validation/server.h b/source/server/config_validation/server.h index 1c5c4cae80b4..70fc5c7e06e1 100644 --- a/source/server/config_validation/server.h +++ b/source/server/config_validation/server.h @@ -94,7 +94,12 @@ class ValidationInstance final : Logger::Loggable, ServerLifecycleNotifier& lifecycleNotifier() override { return *this; } ListenerManager& listenerManager() override { return *listener_manager_; } Secret::SecretManager& secretManager() override { return *secret_manager_; } - Runtime::Loader& runtime() override { return runtime_singleton_->instance(); } + Runtime::Loader& runtime() override { + if (runtime_singleton_) { + return runtime_singleton_->instance(); + } + return *runtime_; + } void shutdown() override; bool isShutdown() override { return false; } void shutdownAdmin() override {} @@ -209,6 +214,7 @@ class ValidationInstance final : Logger::Loggable, std::unique_ptr admin_; Singleton::ManagerPtr singleton_manager_; std::unique_ptr runtime_singleton_; + std::unique_ptr runtime_; Random::RandomGeneratorImpl random_generator_; std::unique_ptr ssl_context_manager_; Configuration::MainImpl config_; diff --git a/test/common/http/conn_pool_grid_test.cc b/test/common/http/conn_pool_grid_test.cc index 945279c94fa8..0ffc9061f973 100644 --- a/test/common/http/conn_pool_grid_test.cc +++ b/test/common/http/conn_pool_grid_test.cc @@ -820,7 +820,7 @@ TEST_F(ConnectivityGridTest, RealGrid) { TEST_F(ConnectivityGridTest, ConnectionCloseDuringCreation) { TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( + scoped_runtime.mergeValues( {{"envoy.reloadable_features.postpone_h3_client_connect_to_next_loop", "false"}}); initialize(); EXPECT_CALL(*cluster_, connectTimeout()).WillRepeatedly(Return(std::chrono::seconds(10))); @@ -891,7 +891,7 @@ TEST_F(ConnectivityGridTest, ConnectionCloseDuringCreation) { TEST_F(ConnectivityGridTest, ConnectionCloseDuringAysnConnect) { TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( + scoped_runtime.mergeValues( {{"envoy.reloadable_features.postpone_h3_client_connect_to_next_loop", "true"}}); initialize(); EXPECT_CALL(*cluster_, connectTimeout()).WillRepeatedly(Return(std::chrono::seconds(10))); diff --git a/test/common/http/http2/codec_impl_test.cc b/test/common/http/http2/codec_impl_test.cc index f64c0b4fffc4..4d48e14c7c87 100644 --- a/test/common/http/http2/codec_impl_test.cc +++ b/test/common/http/http2/codec_impl_test.cc @@ -136,7 +136,7 @@ class Http2CodecImplTestFixture { } virtual void initialize() { - Runtime::LoaderSingleton::getExisting()->mergeValues( + scoped_runtime_->mergeValues( {{"envoy.reloadable_features.http2_new_codec_wrapper", enable_new_codec_wrapper_ ? "true" : "false"}}); Runtime::LoaderSingleton::getExisting()->mergeValues( @@ -3599,7 +3599,7 @@ class Http2CodecMetadataTest : public Http2CodecImplTestFixture, public ::testin protected: void initialize() override { - Runtime::LoaderSingleton::getExisting()->mergeValues( + scoped_runtime_->mergeValues( {{"envoy.reloadable_features.http2_new_codec_wrapper", enable_new_codec_wrapper_ ? "true" : "false"}}); Runtime::LoaderSingleton::getExisting()->mergeValues( diff --git a/test/common/network/listener_impl_test.cc b/test/common/network/listener_impl_test.cc index fde37f55c4d0..6e1d46bf6475 100644 --- a/test/common/network/listener_impl_test.cc +++ b/test/common/network/listener_impl_test.cc @@ -129,7 +129,7 @@ TEST_P(TcpListenerImplTest, GlobalConnectionLimitEnforcement) { Network::Test::getCanonicalLoopbackAddress(version_)); Network::MockTcpListenerCallbacks listener_callbacks; Network::ListenerPtr listener = dispatcher_->createListener( - socket, listener_callbacks, *Runtime::LoaderSingleton::getExisting(), true, false); + socket, listener_callbacks, scoped_runtime.loader(), true, false); std::vector client_connections; std::vector server_connections; @@ -194,7 +194,7 @@ TEST_P(TcpListenerImplTest, GlobalConnectionLimitListenerOptOut) { Network::Test::getCanonicalLoopbackAddress(version_)); Network::MockTcpListenerCallbacks listener_callbacks; Network::ListenerPtr listener = dispatcher_->createListener( - socket, listener_callbacks, *Runtime::LoaderSingleton::getExisting(), true, true); + socket, listener_callbacks, scoped_runtime.loader(), true, true); std::vector client_connections; std::vector server_connections; diff --git a/test/common/quic/active_quic_listener_test.cc b/test/common/quic/active_quic_listener_test.cc index 6242cd77f389..c7ccc377dca4 100644 --- a/test/common/quic/active_quic_listener_test.cc +++ b/test/common/quic/active_quic_listener_test.cc @@ -82,10 +82,6 @@ class ActiveQuicListenerTest : public testing::TestWithParammutable_admin_layer(); - loader_ = std::make_unique( - Runtime::LoaderPtr{new Runtime::LoaderImpl(*dispatcher_, tls_, config, local_info_, store_, - generator_, validation_visitor_, *api_)}); - listen_socket_ = std::make_shared(local_address_, nullptr, /*bind*/ true); listen_socket_->addOptions(Network::SocketOptionFactory::buildIpPacketInfoOptions()); @@ -294,7 +290,6 @@ class ActiveQuicListenerTest : public testing::TestWithParam socket_factory_; EnvoyQuicDispatcher* quic_dispatcher_; - std::unique_ptr loader_; NiceMock tls_; Stats::TestUtil::TestStore store_; diff --git a/test/common/router/router_test.cc b/test/common/router/router_test.cc index ff6a434ad500..b15451c3d364 100644 --- a/test/common/router/router_test.cc +++ b/test/common/router/router_test.cc @@ -1144,7 +1144,7 @@ TEST_F(RouterTest, ResetDuringEncodeHeaders) { TEST_F(RouterTest, UpstreamTimeoutNoStatsEmissionWhenRuntimeGuardFalse) { TestScopedRuntime scoped_runtime; - Runtime::LoaderSingleton::getExisting()->mergeValues( + scoped_runtime.mergeValues( {{"envoy.reloadable_features.do_not_await_headers_on_upstream_timeout_to_emit_stats", "false"}}); diff --git a/test/config_test/config_test.cc b/test/config_test/config_test.cc index 13092ff3fbc9..0f01ac435b3c 100644 --- a/test/config_test/config_test.cc +++ b/test/config_test/config_test.cc @@ -51,15 +51,6 @@ static std::vector unsuported_win32_configs = { #endif }; -class ScopedRuntimeInjector { -public: - ScopedRuntimeInjector(Runtime::Loader& runtime) { - Runtime::LoaderSingleton::initialize(&runtime); - } // namespace - - ~ScopedRuntimeInjector() { Runtime::LoaderSingleton::clear(); } -}; // namespace ConfigTest - } // namespace class ConfigTest { @@ -82,7 +73,6 @@ class ConfigTest { // production code. Note that this test is actually more strict than production because // in production runtime is not setup until after the bootstrap config is loaded. This seems // better for configuration tests. - ScopedRuntimeInjector scoped_runtime(server_.runtime()); ON_CALL(server_.runtime_loader_.snapshot_, deprecatedFeatureEnabled(_, _)) .WillByDefault(Invoke([](absl::string_view, bool default_value) { return default_value; })); diff --git a/test/server/config_validation/server_test.cc b/test/server/config_validation/server_test.cc index f23063f52d16..d2a064ea2d93 100644 --- a/test/server/config_validation/server_test.cc +++ b/test/server/config_validation/server_test.cc @@ -81,36 +81,6 @@ class RuntimeFeatureValidationServerTest : public ValidationServerTest { } return files; } - - class TestConfigFactory : public Configuration::NamedNetworkFilterConfigFactory { - public: - std::string name() const override { return "envoy.filters.network.test"; } - - Network::FilterFactoryCb createFilterFactoryFromProto(const Protobuf::Message&, - Configuration::FactoryContext&) override { - // Validate that the validation server loaded the runtime data and installed the singleton. - auto* runtime = Runtime::LoaderSingleton::getExisting(); - if (runtime == nullptr) { - throw EnvoyException("Runtime::LoaderSingleton == nullptr"); - } - - if (!runtime->threadsafeSnapshot()->getBoolean("test.runtime.loaded", false)) { - throw EnvoyException( - "Found Runtime::LoaderSingleton, got wrong value for test.runtime.loaded"); - } - - return [](Network::FilterManager&) {}; - } - - ProtobufTypes::MessagePtr createEmptyConfigProto() override { - return ProtobufTypes::MessagePtr{new ProtobufWkt::Struct()}; - } - - bool isTerminalFilterByProto(const Protobuf::Message&, - Server::Configuration::FactoryContext&) override { - return true; - } - }; }; TEST_P(ValidationServerTest, Validate) { @@ -192,15 +162,19 @@ INSTANTIATE_TEST_SUITE_P(AllConfigs, ValidationServerTest_1, ::testing::ValuesIn(ValidationServerTest_1::getAllConfigFiles())); TEST_P(RuntimeFeatureValidationServerTest, ValidRuntimeLoaderSingleton) { - TestConfigFactory factory; - Registry::InjectFactory registration(factory); - - auto local_address = Network::Utility::getLocalAddress(options_.localAddressIpVersion()); - - // If this fails, it's likely because TestConfigFactory threw an exception related to the - // runtime loader. - ASSERT_TRUE(validateConfig(options_, local_address, component_factory_, - Thread::threadFactoryForTest(), Filesystem::fileSystemForTest())); + Thread::MutexBasicLockable access_log_lock; + Stats::IsolatedStoreImpl stats_store; + DangerousDeprecatedTestTime time_system; + ValidationInstance server(options_, time_system.timeSystem(), + Network::Address::InstanceConstSharedPtr(), stats_store, + access_log_lock, component_factory_, Thread::threadFactoryForTest(), + Filesystem::fileSystemForTest()); + EXPECT_TRUE(server.runtime().snapshot().getBoolean("test.runtime.loaded")); + server.registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit, [] { FAIL(); }); + server.registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit, + [](Event::PostCb) { FAIL(); }); + server.setSinkPredicates(std::make_unique>()); + server.shutdown(); } // Test the admin handler stubs used in validation diff --git a/test/server/connection_handler_test.cc b/test/server/connection_handler_test.cc index 810bbc764a23..098e01818729 100644 --- a/test/server/connection_handler_test.cc +++ b/test/server/connection_handler_test.cc @@ -344,7 +344,7 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggable access_log_; TestScopedRuntime scoped_runtime_; - Runtime::Loader& runtime_{*Runtime::LoaderSingleton::getExisting()}; + Runtime::Loader& runtime_{scoped_runtime_.loader()}; }; // Verify that if a listener is removed while a rebalanced connection is in flight, we correctly diff --git a/test/test_common/test_runtime.h b/test/test_common/test_runtime.h index 59d109f39377..540b375b2005 100644 --- a/test/test_common/test_runtime.h +++ b/test/test_common/test_runtime.h @@ -32,12 +32,23 @@ class TestScopedRuntime { // The existence of an admin layer is required for mergeValues() to work. config.add_layers()->mutable_admin_layer(); - loader_ = std::make_unique( + Runtime::LoaderPtr runtime_ptr = std::make_unique(dispatcher_, tls_, config, local_info_, store_, - generator_, validation_visitor_, *api_)); + generator_, validation_visitor_, *api_); + // This will ignore values set in test, but just use flag defaults! + if (Runtime::runtimeFeatureEnabled("envoy.restart_features.no_runtime_singleton") { + runtime_ = std::move(runtime_ptr); + } else { + runtime_singleton_ = std::make_unique(std::move(runtime_ptr)); + } } - Runtime::Loader& loader() { return *Runtime::LoaderSingleton::getExisting(); } + Runtime::Loader& loader() { + if (runtime_singleton_) { + return runtime_singleton_->instance(); + } + return *runtime_; + } void mergeValues(const absl::node_hash_map& values) { loader().mergeValues(values); @@ -52,14 +63,15 @@ class TestScopedRuntime { Api::ApiPtr api_; testing::NiceMock local_info_; testing::NiceMock validation_visitor_; - std::unique_ptr loader_; + std::unique_ptr runtime_singleton_; + std::unique_ptr runtime_; }; class TestDeprecatedV2Api : public TestScopedRuntime { public: TestDeprecatedV2Api() { allowDeprecatedV2(); } - static void allowDeprecatedV2() { - Runtime::LoaderSingleton::getExisting()->mergeValues({ + void allowDeprecatedV2() { + loader()->mergeValues({ {"envoy.test_only.broken_in_production.enable_deprecated_v2_api", "true"}, {"envoy.features.enable_all_deprecated_features", "true"}, }); From ce1ad14f7531aec92adffb177847dfd73b38b7ad Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 14 Mar 2022 15:25:42 -0400 Subject: [PATCH 2/3] Switch to no global runtime Signed-off-by: Alyssa Wilk --- docs/root/version_history/current.rst | 1 + source/common/runtime/runtime_features.cc | 3 +-- test/common/http/http2/codec_impl_test.cc | 18 ++++++------------ test/common/network/listener_impl_test.cc | 8 ++++---- test/common/protobuf/utility_test.cc | 2 +- test/common/quic/BUILD | 1 + test/common/quic/active_quic_listener_test.cc | 10 +++++----- test/common/upstream/eds_speed_test.cc | 1 - .../buffer_accounting_integration_test.cc | 8 ++++---- test/server/config_validation/server_test.cc | 2 +- .../test_data/runtime_config.yaml | 13 ------------- test/server/listener_manager_impl_test.cc | 4 ++-- test/test_common/test_runtime.h | 9 ++++----- 13 files changed, 30 insertions(+), 50 deletions(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 2d208b2f7cb0..6ec729c72c0f 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -27,6 +27,7 @@ Minor Behavior Changes * listener: the :ref:`ipv4_compat ` flag can only be set on Ipv6 address and Ipv4-mapped Ipv6 address. A runtime guard is added ``envoy.reloadable_features.strict_check_on_ipv4_compat`` and the default is true. * perf: ssl contexts are now tracked without scan based garbage collection and greatly improved the performance on secret update. * router: record upstream request timeouts for all the cases and not just for those requests which are awaiting headers. This behavioral change can be temporarily reverted by setting runtime guard ``envoy.reloadable_features.do_not_await_headers_on_upstream_timeout_to_emit_stats`` to false. +* runtime: removed global runtime as Envoy default. This behavioral change can be reverted by setting runtime guard ``envoy.restart_features.no_runtime_singleton`` to false. * sip-proxy: add customized affinity support by adding :ref:`tra_service_config ` and :ref:`customized_affinity `. * sip-proxy: add support for the ``503`` response code. When there is something wrong occurred, send ``503 Service Unavailable`` back to downstream. * stateful session http filter: only enable cookie based session state when request path matches the configured cookie path. diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 9c2cb5c27897..f8161371dc3b 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -66,6 +66,7 @@ RUNTIME_GUARD(envoy_reloadable_features_update_grpc_response_error_tag); RUNTIME_GUARD(envoy_reloadable_features_use_dns_ttl); RUNTIME_GUARD(envoy_reloadable_features_validate_connect); RUNTIME_GUARD(envoy_restart_features_explicit_wildcard_resource); +RUNTIME_GUARD(envoy_restart_features_no_runtime_singleton); RUNTIME_GUARD(envoy_restart_features_use_apple_api_for_dns_lookups); // Begin false flags. These should come with a TODO to flip true. @@ -75,8 +76,6 @@ FALSE_RUNTIME_GUARD(envoy_reloadable_features_test_feature_false); FALSE_RUNTIME_GUARD(envoy_reloadable_features_allow_multiple_dns_addresses); // TODO(adisuissa) reset to true to enable unified mux by default FALSE_RUNTIME_GUARD(envoy_reloadable_features_unified_mux); -// TODO(alyssar) flip false once issue complete. -FALSE_RUNTIME_GUARD(envoy_restart_features_no_runtime_singleton); // TODO(kbaichoo): Make this enabled by default when fairness and chunking // are implemented, and we've had more cpu time. FALSE_RUNTIME_GUARD(envoy_reloadable_features_defer_processing_backedup_streams); diff --git a/test/common/http/http2/codec_impl_test.cc b/test/common/http/http2/codec_impl_test.cc index 4d48e14c7c87..3d28ae0f7aab 100644 --- a/test/common/http/http2/codec_impl_test.cc +++ b/test/common/http/http2/codec_impl_test.cc @@ -136,12 +136,8 @@ class Http2CodecImplTestFixture { } virtual void initialize() { - scoped_runtime_->mergeValues( - {{"envoy.reloadable_features.http2_new_codec_wrapper", - enable_new_codec_wrapper_ ? "true" : "false"}}); - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{std::string(Runtime::defer_processing_backedup_streams), - defer_processing_backedup_streams_ ? "true" : "false"}}); + scoped_runtime_.mergeValues({{"envoy.reloadable_features.http2_new_codec_wrapper", + enable_new_codec_wrapper_ ? "true" : "false"}}); http2OptionsFromTuple(client_http2_options_, client_settings_); http2OptionsFromTuple(server_http2_options_, server_settings_); client_ = std::make_unique( @@ -3599,12 +3595,10 @@ class Http2CodecMetadataTest : public Http2CodecImplTestFixture, public ::testin protected: void initialize() override { - scoped_runtime_->mergeValues( - {{"envoy.reloadable_features.http2_new_codec_wrapper", - enable_new_codec_wrapper_ ? "true" : "false"}}); - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{std::string(Runtime::defer_processing_backedup_streams), - defer_processing_backedup_streams_ ? "true" : "false"}}); + scoped_runtime_.mergeValues({{"envoy.reloadable_features.http2_new_codec_wrapper", + enable_new_codec_wrapper_ ? "true" : "false"}}); + scoped_runtime_.mergeValues({{std::string(Runtime::defer_processing_backedup_streams), + defer_processing_backedup_streams_ ? "true" : "false"}}); allow_metadata_ = true; http2OptionsFromTuple(client_http2_options_, client_settings_); http2OptionsFromTuple(server_http2_options_, server_settings_); diff --git a/test/common/network/listener_impl_test.cc b/test/common/network/listener_impl_test.cc index 6e1d46bf6475..f66202e75002 100644 --- a/test/common/network/listener_impl_test.cc +++ b/test/common/network/listener_impl_test.cc @@ -128,8 +128,8 @@ TEST_P(TcpListenerImplTest, GlobalConnectionLimitEnforcement) { auto socket = std::make_shared( Network::Test::getCanonicalLoopbackAddress(version_)); Network::MockTcpListenerCallbacks listener_callbacks; - Network::ListenerPtr listener = dispatcher_->createListener( - socket, listener_callbacks, scoped_runtime.loader(), true, false); + Network::ListenerPtr listener = + dispatcher_->createListener(socket, listener_callbacks, scoped_runtime.loader(), true, false); std::vector client_connections; std::vector server_connections; @@ -193,8 +193,8 @@ TEST_P(TcpListenerImplTest, GlobalConnectionLimitListenerOptOut) { auto socket = std::make_shared( Network::Test::getCanonicalLoopbackAddress(version_)); Network::MockTcpListenerCallbacks listener_callbacks; - Network::ListenerPtr listener = dispatcher_->createListener( - socket, listener_callbacks, scoped_runtime.loader(), true, true); + Network::ListenerPtr listener = + dispatcher_->createListener(socket, listener_callbacks, scoped_runtime.loader(), true, true); std::vector client_connections; std::vector server_connections; diff --git a/test/common/protobuf/utility_test.cc b/test/common/protobuf/utility_test.cc index f3d63cabb95a..1e0675350db4 100644 --- a/test/common/protobuf/utility_test.cc +++ b/test/common/protobuf/utility_test.cc @@ -1732,7 +1732,7 @@ class DeprecatedFieldsTest : public testing::Test, protected RuntimeStatsHelper }; TEST_F(DeprecatedFieldsTest, NoCrashIfRuntimeMissing) { - loader_.reset(); + runtime_.reset(); envoy::test::deprecation_test::Base base; base.set_not_deprecated("foo"); diff --git a/test/common/quic/BUILD b/test/common/quic/BUILD index 3074476f09af..d813179ab006 100644 --- a/test/common/quic/BUILD +++ b/test/common/quic/BUILD @@ -209,6 +209,7 @@ envoy_cc_test( "//test/mocks/server:instance_mocks", "//test/test_common:network_utility_lib", "//test/test_common:simulated_time_system_lib", + "//test/test_common:test_runtime_lib", "@com_github_google_quiche//:quic_test_tools_crypto_server_config_peer_lib", "@envoy_api//envoy/config/listener/v3:pkg_cc_proto", ], diff --git a/test/common/quic/active_quic_listener_test.cc b/test/common/quic/active_quic_listener_test.cc index c7ccc377dca4..971afaa2c239 100644 --- a/test/common/quic/active_quic_listener_test.cc +++ b/test/common/quic/active_quic_listener_test.cc @@ -26,6 +26,7 @@ #include "test/test_common/environment.h" #include "test/test_common/network_utility.h" #include "test/test_common/simulated_time_system.h" +#include "test/test_common/test_runtime.h" #include "test/test_common/utility.h" #include "absl/time/time.h" @@ -116,8 +117,7 @@ class ActiveQuicListenerTest : public testing::TestWithParam(listener_factory_->createActiveUdpListener( - Runtime::LoaderSingleton::get(), 0, connection_handler_, *dispatcher_, - listener_config_)); + scoped_runtime_.loader(), 0, connection_handler_, *dispatcher_, listener_config_)); quic_dispatcher_ = ActiveQuicListenerPeer::quicDispatcher(*quic_listener_); quic::QuicCryptoServerConfig& crypto_config = ActiveQuicListenerPeer::cryptoConfig(*quic_listener_); @@ -243,7 +243,6 @@ class ActiveQuicListenerTest : public testing::TestWithParamrun(Event::Dispatcher::RunType::NonBlock); - Runtime::LoaderSingleton::clear(); } protected: @@ -271,6 +270,7 @@ class ActiveQuicListenerTest : public testing::TestWithParamrun(Event::Dispatcher::RunType::NonBlock); EXPECT_EQ(quic_dispatcher_->NumSessions(), 1); - Runtime::LoaderSingleton::getExisting()->mergeValues({{"quic.enabled", " false"}}); + scoped_runtime_.mergeValues({{"quic.enabled", " false"}}); sendCHLO(quic::test::TestConnectionId(2)); dispatcher_->run(Event::Dispatcher::RunType::NonBlock); // If listener was enabled, there should have been session created for active connection. EXPECT_EQ(quic_dispatcher_->NumSessions(), 1); EXPECT_FALSE(ActiveQuicListenerPeer::enabled(*quic_listener_)); - Runtime::LoaderSingleton::getExisting()->mergeValues({{"quic.enabled", " true"}}); + scoped_runtime_.mergeValues({{"quic.enabled", " true"}}); sendCHLO(quic::test::TestConnectionId(2)); dispatcher_->run(Event::Dispatcher::RunType::NonBlock); EXPECT_EQ(quic_dispatcher_->NumSessions(), 2); diff --git a/test/common/upstream/eds_speed_test.cc b/test/common/upstream/eds_speed_test.cc index 305f183ef94b..23a4bba9e5c6 100644 --- a/test/common/upstream/eds_speed_test.cc +++ b/test/common/upstream/eds_speed_test.cc @@ -47,7 +47,6 @@ class EdsSpeedTest { subscription_stats_(Config::Utility::generateStats(stats_)), api_(Api::createApiForTest(stats_)), async_client_(new Grpc::MockAsyncClient()), config_validators_(std::make_unique>()) { - TestDeprecatedV2Api::allowDeprecatedV2(); if (use_unified_mux_) { grpc_mux_.reset(new Config::XdsMux::GrpcMuxSotw( std::unique_ptr(async_client_), dispatcher_, diff --git a/test/integration/buffer_accounting_integration_test.cc b/test/integration/buffer_accounting_integration_test.cc index 85ee8e20bb6d..1e7ad5b97577 100644 --- a/test/integration/buffer_accounting_integration_test.cc +++ b/test/integration/buffer_accounting_integration_test.cc @@ -740,9 +740,9 @@ INSTANTIATE_TEST_SUITE_P( TEST_P(Http2DeferredProcessingIntegrationTest, CanBufferInDownstreamCodec) { config_helper_.setBufferLimits(1000, 1000); + config_helper_.addRuntimeOverride(std::string(Runtime::defer_processing_backedup_streams), + "true"); initialize(); - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{std::string(Runtime::defer_processing_backedup_streams), "true"}}); // Stop writes to the upstream. write_matcher_->setDestinationPort(fake_upstreams_[0]->localAddress()->ip()->port()); @@ -784,9 +784,9 @@ TEST_P(Http2DeferredProcessingIntegrationTest, CanBufferInDownstreamCodec) { TEST_P(Http2DeferredProcessingIntegrationTest, CanBufferInUpstreamCodec) { config_helper_.setBufferLimits(1000, 1000); + config_helper_.addRuntimeOverride(std::string(Runtime::defer_processing_backedup_streams), + "true"); initialize(); - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{std::string(Runtime::defer_processing_backedup_streams), "true"}}); // Stop writes to the downstream. write_matcher_->setSourcePort(lookupPort("http")); diff --git a/test/server/config_validation/server_test.cc b/test/server/config_validation/server_test.cc index d2a064ea2d93..71dfe1673ac0 100644 --- a/test/server/config_validation/server_test.cc +++ b/test/server/config_validation/server_test.cc @@ -169,7 +169,7 @@ TEST_P(RuntimeFeatureValidationServerTest, ValidRuntimeLoaderSingleton) { Network::Address::InstanceConstSharedPtr(), stats_store, access_log_lock, component_factory_, Thread::threadFactoryForTest(), Filesystem::fileSystemForTest()); - EXPECT_TRUE(server.runtime().snapshot().getBoolean("test.runtime.loaded")); + EXPECT_TRUE(server.runtime().snapshot().getBoolean("test.runtime.loaded", false)); server.registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit, [] { FAIL(); }); server.registerCallback(ServerLifecycleNotifier::Stage::ShutdownExit, [](Event::PostCb) { FAIL(); }); diff --git a/test/server/config_validation/test_data/runtime_config.yaml b/test/server/config_validation/test_data/runtime_config.yaml index a044a519a57b..6a018409562d 100644 --- a/test/server/config_validation/test_data/runtime_config.yaml +++ b/test/server/config_validation/test_data/runtime_config.yaml @@ -6,19 +6,6 @@ layered_runtime: - name: static-layer static_layer: "test.runtime.loaded": true -static_resources: - listeners: - - name: "test.listener" - address: - socket_address: - protocol: TCP - address: 0.0.0.0 - port_value: 0 - filter_chains: - - filters: - - name: envoy.filters.network.test - typed_config: - "@type": type.googleapis.com/google.protobuf.Struct admin: address: socket_address: diff --git a/test/server/listener_manager_impl_test.cc b/test/server/listener_manager_impl_test.cc index 39d951d558a7..f996c74889d6 100644 --- a/test/server/listener_manager_impl_test.cc +++ b/test/server/listener_manager_impl_test.cc @@ -566,7 +566,7 @@ TEST_F(ListenerManagerImplTest, RejectIpv4CompatOnIpv4Address) { TEST_F(ListenerManagerImplTest, AcceptIpv4CompatOnIpv4Address) { auto scoped_runtime_guard = std::make_unique(); - Runtime::LoaderSingleton::getExisting()->mergeValues( + scoped_runtime_guard->mergeValues( {{"envoy.reloadable_features.strict_check_on_ipv4_compat", "false"}}); const std::string yaml = R"EOF( name: "foo" @@ -631,7 +631,7 @@ TEST_F(ListenerManagerImplTest, AcceptIpv4CompatOnNonCanonicalIpv6AnyAddress) { TEST_F(ListenerManagerImplTest, AcceptIpv4CompatOnNonIpv4MappedIpv6address) { auto scoped_runtime_guard = std::make_unique(); - Runtime::LoaderSingleton::getExisting()->mergeValues( + scoped_runtime_guard->mergeValues( {{"envoy.reloadable_features.strict_check_on_ipv4_compat", "false"}}); const std::string yaml = R"EOF( name: "foo" diff --git a/test/test_common/test_runtime.h b/test/test_common/test_runtime.h index 540b375b2005..1da78f39442e 100644 --- a/test/test_common/test_runtime.h +++ b/test/test_common/test_runtime.h @@ -32,11 +32,10 @@ class TestScopedRuntime { // The existence of an admin layer is required for mergeValues() to work. config.add_layers()->mutable_admin_layer(); - Runtime::LoaderPtr runtime_ptr = - std::make_unique(dispatcher_, tls_, config, local_info_, store_, - generator_, validation_visitor_, *api_); + Runtime::LoaderPtr runtime_ptr = std::make_unique( + dispatcher_, tls_, config, local_info_, store_, generator_, validation_visitor_, *api_); // This will ignore values set in test, but just use flag defaults! - if (Runtime::runtimeFeatureEnabled("envoy.restart_features.no_runtime_singleton") { + if (Runtime::runtimeFeatureEnabled("envoy.restart_features.no_runtime_singleton")) { runtime_ = std::move(runtime_ptr); } else { runtime_singleton_ = std::make_unique(std::move(runtime_ptr)); @@ -71,7 +70,7 @@ class TestDeprecatedV2Api : public TestScopedRuntime { public: TestDeprecatedV2Api() { allowDeprecatedV2(); } void allowDeprecatedV2() { - loader()->mergeValues({ + loader().mergeValues({ {"envoy.test_only.broken_in_production.enable_deprecated_v2_api", "true"}, {"envoy.features.enable_all_deprecated_features", "true"}, }); From 30b61cf2dbdab5ebe58f88c48e3afaf384e96d87 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Tue, 15 Mar 2022 15:40:27 -0400 Subject: [PATCH 3/3] fix bad merge Signed-off-by: Alyssa Wilk --- test/common/http/http1/codec_impl_test.cc | 3 +-- test/common/http/http2/codec_impl_test.cc | 3 +++ .../filters/http/decompressor/decompressor_filter_test.cc | 4 ++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/test/common/http/http1/codec_impl_test.cc b/test/common/http/http1/codec_impl_test.cc index c1ef54a8f42b..59ced124d59c 100644 --- a/test/common/http/http1/codec_impl_test.cc +++ b/test/common/http/http1/codec_impl_test.cc @@ -3035,8 +3035,7 @@ TEST_F(Http1ServerConnectionImplTest, RuntimeLazyReadDisableTest) { connection_.dispatcher_.clearDeferredDeleteList(); } - Runtime::LoaderSingleton::getExisting()->mergeValues( - {{"envoy.reloadable_features.http1_lazy_read_disable", "false"}}); + scoped_runtime.mergeValues({{"envoy.reloadable_features.http1_lazy_read_disable", "false"}}); // Always call readDisable if lazy read disable flag is set to false. { diff --git a/test/common/http/http2/codec_impl_test.cc b/test/common/http/http2/codec_impl_test.cc index 3d28ae0f7aab..efb057c6dc86 100644 --- a/test/common/http/http2/codec_impl_test.cc +++ b/test/common/http/http2/codec_impl_test.cc @@ -138,6 +138,9 @@ class Http2CodecImplTestFixture { virtual void initialize() { scoped_runtime_.mergeValues({{"envoy.reloadable_features.http2_new_codec_wrapper", enable_new_codec_wrapper_ ? "true" : "false"}}); + scoped_runtime_.mergeValues({{std::string(Runtime::defer_processing_backedup_streams), + defer_processing_backedup_streams_ ? "true" : "false"}}); + http2OptionsFromTuple(client_http2_options_, client_settings_); http2OptionsFromTuple(server_http2_options_, server_settings_); client_ = std::make_unique( diff --git a/test/extensions/filters/http/decompressor/decompressor_filter_test.cc b/test/extensions/filters/http/decompressor/decompressor_filter_test.cc index 97c3d624c733..6d9f2b787a20 100644 --- a/test/extensions/filters/http/decompressor/decompressor_filter_test.cc +++ b/test/extensions/filters/http/decompressor/decompressor_filter_test.cc @@ -210,11 +210,11 @@ class DecompressorFilterTest : public testing::TestWithParam { const std::string& final_accept_encoding) { TestScopedRuntime scoped_runtime; if (legacy) { - Runtime::LoaderSingleton::getExisting()->mergeValues( + scoped_runtime.mergeValues( {{"envoy.reloadable_features.append_to_accept_content_encoding_only_once", "false"}}); } else { - Runtime::LoaderSingleton::getExisting()->mergeValues( + scoped_runtime.mergeValues( {{"envoy.reloadable_features.append_to_accept_content_encoding_only_once", "true"}}); } setUpFilter(R"EOF(