Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime: no longer using runtime singleton #20337

Merged
merged 3 commits into from
Mar 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ Minor Behavior Changes
* listener: the :ref:`ipv4_compat <envoy_api_field_core.SocketAddress.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 <envoy_v3_api_msg_extensions.filters.network.sip_proxy.tra.v3alpha.TraServiceConfig>` and :ref:`customized_affinity <envoy_v3_api_msg_extensions.filters.network.sip_proxy.v3alpha.CustomizedAffinity>`.
* 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.
Expand Down
3 changes: 1 addition & 2 deletions source/common/runtime/runtime_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
Expand Down
10 changes: 8 additions & 2 deletions source/server/config_validation/server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,14 @@ void ValidationInstance::initialize(const Options& options,
listener_manager_ =
std::make_unique<ListenerManagerImpl>(*this, *this, *this, false, quic_stat_names_);
thread_local_.registerThread(*dispatcher_, true);
runtime_singleton_ = std::make_unique<Runtime::ScopedLoaderSingleton>(
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<Runtime::ScopedLoaderSingleton>(std::move(runtime_ptr));
}

secret_manager_ = std::make_unique<Secret::SecretManagerImpl>(admin().getConfigTracker());
ssl_context_manager_ = createContextManager("ssl_context_manager", api_->timeSource());
cluster_manager_factory_ = std::make_unique<Upstream::ValidationClusterManagerFactory>(
Expand Down
8 changes: 7 additions & 1 deletion source/server/config_validation/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,12 @@ class ValidationInstance final : Logger::Loggable<Logger::Id::main>,
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 {}
Expand Down Expand Up @@ -209,6 +214,7 @@ class ValidationInstance final : Logger::Loggable<Logger::Id::main>,
std::unique_ptr<Server::ValidationAdmin> admin_;
Singleton::ManagerPtr singleton_manager_;
std::unique_ptr<Runtime::ScopedLoaderSingleton> runtime_singleton_;
std::unique_ptr<Runtime::Loader> runtime_;
Random::RandomGeneratorImpl random_generator_;
std::unique_ptr<Ssl::ContextManager> ssl_context_manager_;
Configuration::MainImpl config_;
Expand Down
4 changes: 2 additions & 2 deletions test/common/http/conn_pool_grid_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
Expand Down Expand Up @@ -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)));
Expand Down
3 changes: 1 addition & 2 deletions test/common/http/http1/codec_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
{
Expand Down
21 changes: 9 additions & 12 deletions test/common/http/http2/codec_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,11 @@ class Http2CodecImplTestFixture {
}

virtual void initialize() {
Runtime::LoaderSingleton::getExisting()->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"}});

http2OptionsFromTuple(client_http2_options_, client_settings_);
http2OptionsFromTuple(server_http2_options_, server_settings_);
client_ = std::make_unique<TestClientConnectionImpl>(
Expand Down Expand Up @@ -3599,12 +3598,10 @@ class Http2CodecMetadataTest : public Http2CodecImplTestFixture, public ::testin

protected:
void initialize() override {
Runtime::LoaderSingleton::getExisting()->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_);
Expand Down
8 changes: 4 additions & 4 deletions test/common/network/listener_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,8 @@ TEST_P(TcpListenerImplTest, GlobalConnectionLimitEnforcement) {
auto socket = std::make_shared<Network::Test::TcpListenSocketImmediateListen>(
Network::Test::getCanonicalLoopbackAddress(version_));
Network::MockTcpListenerCallbacks listener_callbacks;
Network::ListenerPtr listener = dispatcher_->createListener(
socket, listener_callbacks, *Runtime::LoaderSingleton::getExisting(), true, false);
Network::ListenerPtr listener =
dispatcher_->createListener(socket, listener_callbacks, scoped_runtime.loader(), true, false);

std::vector<Network::ClientConnectionPtr> client_connections;
std::vector<Network::ConnectionPtr> server_connections;
Expand Down Expand Up @@ -193,8 +193,8 @@ TEST_P(TcpListenerImplTest, GlobalConnectionLimitListenerOptOut) {
auto socket = std::make_shared<Network::Test::TcpListenSocketImmediateListen>(
Network::Test::getCanonicalLoopbackAddress(version_));
Network::MockTcpListenerCallbacks listener_callbacks;
Network::ListenerPtr listener = dispatcher_->createListener(
socket, listener_callbacks, *Runtime::LoaderSingleton::getExisting(), true, true);
Network::ListenerPtr listener =
dispatcher_->createListener(socket, listener_callbacks, scoped_runtime.loader(), true, true);

std::vector<Network::ClientConnectionPtr> client_connections;
std::vector<Network::ConnectionPtr> server_connections;
Expand Down
2 changes: 1 addition & 1 deletion test/common/protobuf/utility_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
1 change: 1 addition & 0 deletions test/common/quic/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
Expand Down
15 changes: 5 additions & 10 deletions test/common/quic/active_quic_listener_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -82,10 +83,6 @@ class ActiveQuicListenerTest : public testing::TestWithParam<Network::Address::I
void SetUp() override {
envoy::config::bootstrap::v3::LayeredRuntime config;
config.add_layers()->mutable_admin_layer();
loader_ = std::make_unique<Runtime::ScopedLoaderSingleton>(
Runtime::LoaderPtr{new Runtime::LoaderImpl(*dispatcher_, tls_, config, local_info_, store_,
generator_, validation_visitor_, *api_)});

listen_socket_ =
std::make_shared<Network::UdpListenSocket>(local_address_, nullptr, /*bind*/ true);
listen_socket_->addOptions(Network::SocketOptionFactory::buildIpPacketInfoOptions());
Expand Down Expand Up @@ -120,8 +117,7 @@ class ActiveQuicListenerTest : public testing::TestWithParam<Network::Address::I
.WillRepeatedly(ReturnRef(filter_chain_manager_));
quic_listener_ =
staticUniquePointerCast<ActiveQuicListener>(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_);
Expand Down Expand Up @@ -247,7 +243,6 @@ class ActiveQuicListenerTest : public testing::TestWithParam<Network::Address::I
}
// Trigger alarm to fire before listener destruction.
dispatcher_->run(Event::Dispatcher::RunType::NonBlock);
Runtime::LoaderSingleton::clear();
}

protected:
Expand Down Expand Up @@ -275,6 +270,7 @@ class ActiveQuicListenerTest : public testing::TestWithParam<Network::Address::I
handshake_timeout_);
}

TestScopedRuntime scoped_runtime_;
Network::Address::IpVersion version_;
Event::SimulatedTimeSystemHelper simulated_time_system_;
Api::ApiPtr api_;
Expand All @@ -294,7 +290,6 @@ class ActiveQuicListenerTest : public testing::TestWithParam<Network::Address::I
Network::ActiveUdpListenerFactoryPtr listener_factory_;
NiceMock<Network::MockListenSocketFactory> socket_factory_;
EnvoyQuicDispatcher* quic_dispatcher_;
std::unique_ptr<Runtime::ScopedLoaderSingleton> loader_;

NiceMock<ThreadLocal::MockInstance> tls_;
Stats::TestUtil::TestStore store_;
Expand Down Expand Up @@ -449,14 +444,14 @@ TEST_P(ActiveQuicListenerTest, QuicProcessingDisabledAndEnabled) {
dispatcher_->run(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);
Expand Down
2 changes: 1 addition & 1 deletion test/common/router/router_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"}});

Expand Down
1 change: 0 additions & 1 deletion test/common/upstream/eds_speed_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<NiceMock<Config::MockCustomConfigValidators>>()) {
TestDeprecatedV2Api::allowDeprecatedV2();
if (use_unified_mux_) {
grpc_mux_.reset(new Config::XdsMux::GrpcMuxSotw(
std::unique_ptr<Grpc::MockAsyncClient>(async_client_), dispatcher_,
Expand Down
10 changes: 0 additions & 10 deletions test/config_test/config_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,6 @@ static std::vector<absl::string_view> 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 {
Expand All @@ -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; }));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -210,11 +210,11 @@ class DecompressorFilterTest : public testing::TestWithParam<bool> {
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(
Expand Down
8 changes: 4 additions & 4 deletions test/integration/buffer_accounting_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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"));
Expand Down
Loading