From ce1ad14f7531aec92adffb177847dfd73b38b7ad Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 14 Mar 2022 15:25:42 -0400 Subject: [PATCH] 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"}, });