From 8c82329505956ffb7997e3b2448e2278c957e089 Mon Sep 17 00:00:00 2001 From: Weston Carlson Date: Tue, 2 Nov 2021 19:31:33 -0600 Subject: [PATCH 01/10] Add listener global conn limit opt out. Signed-off-by: Weston Carlson --- api/envoy/config/bootstrap/v3/bootstrap.proto | 6 +- api/envoy/config/listener/v3/listener.proto | 6 +- .../overload_manager/overload_manager.rst | 4 + envoy/event/dispatcher.h | 4 +- envoy/network/listener.h | 6 + envoy/server/configuration.h | 6 + source/common/event/dispatcher_impl.cc | 6 +- source/common/event/dispatcher_impl.h | 4 +- source/common/network/tcp_listener_impl.cc | 9 +- source/common/network/tcp_listener_impl.h | 6 +- source/server/active_tcp_listener.cc | 2 +- source/server/admin/admin.cc | 7 +- source/server/admin/admin.h | 7 +- source/server/config_validation/dispatcher.cc | 3 +- source/server/config_validation/dispatcher.h | 2 +- source/server/configuration_impl.cc | 1 + source/server/configuration_impl.h | 2 + source/server/listener_impl.cc | 2 + source/server/listener_impl.h | 2 + source/server/server.cc | 3 +- test/common/network/listener_impl_test.cc | 76 ++++++++-- .../proxy_protocol_regression_test.cc | 1 + .../proxy_protocol/proxy_protocol_test.cc | 2 + test/integration/cx_limit_integration_test.cc | 140 +++++++++++++++++- test/integration/fake_upstream.h | 1 + test/mocks/event/mocks.h | 9 +- test/mocks/network/mocks.h | 1 + test/server/connection_handler_test.cc | 12 +- 28 files changed, 282 insertions(+), 48 deletions(-) diff --git a/api/envoy/config/bootstrap/v3/bootstrap.proto b/api/envoy/config/bootstrap/v3/bootstrap.proto index 0cb494e163235..13c436bd2b4bc 100644 --- a/api/envoy/config/bootstrap/v3/bootstrap.proto +++ b/api/envoy/config/bootstrap/v3/bootstrap.proto @@ -329,7 +329,7 @@ message Bootstrap { // Administration interface :ref:`operations documentation // `. -// [#next-free-field: 6] +// [#next-free-field: 7] message Admin { option (udpa.annotations.versioning).previous_message_type = "envoy.config.bootstrap.v2.Admin"; @@ -355,6 +355,10 @@ message Admin { // Additional socket options that may not be present in Envoy source code or // precompiled binaries. repeated core.v3.SocketOption socket_options = 4; + + // Indicates whether `global_downstream_max_connections `_ + // should apply to the admin interface or not. + bool ignore_global_conn_limit = 6; } // Cluster manager :ref:`architecture overview `. diff --git a/api/envoy/config/listener/v3/listener.proto b/api/envoy/config/listener/v3/listener.proto index f065ff67160b3..8df51a4f5b49b 100644 --- a/api/envoy/config/listener/v3/listener.proto +++ b/api/envoy/config/listener/v3/listener.proto @@ -35,7 +35,7 @@ message ListenerCollection { repeated xds.core.v3.CollectionEntry entries = 1; } -// [#next-free-field: 31] +// [#next-free-field: 32] message Listener { option (udpa.annotations.versioning).previous_message_type = "envoy.api.v2.Listener"; @@ -318,4 +318,8 @@ message Listener { // Enable MPTCP (multi-path TCP) on this listener. Clients will be allowed to establish // MPTCP connections. Non-MPTCP clients will fall back to regular TCP. bool enable_mptcp = 30; + + // Whether the listener should limit connections based upon the value of + // `global_downstream_max_connections `_ + bool ignore_global_conn_limit = 31; } diff --git a/docs/root/configuration/operations/overload_manager/overload_manager.rst b/docs/root/configuration/operations/overload_manager/overload_manager.rst index 7dacd28b323b0..ffc0b75041f7f 100644 --- a/docs/root/configuration/operations/overload_manager/overload_manager.rst +++ b/docs/root/configuration/operations/overload_manager/overload_manager.rst @@ -155,6 +155,10 @@ If the value is unspecified, there is no global limit on the number of active do and Envoy will emit a warning indicating this at startup. To disable the warning without setting a limit on the number of active downstream connections, the runtime value may be set to a very large limit (~2e9). +Listeners (including the admin listener) can opt out of this global connection limit by setting +:ref:`Listener.ignore_global_conn_limit ` +to true. You may want to opt out a listener to be able to probe or collect stats from Envoy while it +is otherwise at its connection limit. If it is desired to only limit the number of downstream connections for a particular listener, per-listener limits can be set via the :ref:`listener configuration `. diff --git a/envoy/event/dispatcher.h b/envoy/event/dispatcher.h index 94cb73e5c4fbc..40715e74daa83 100644 --- a/envoy/event/dispatcher.h +++ b/envoy/event/dispatcher.h @@ -231,8 +231,8 @@ class Dispatcher : public DispatcherBase, public ScopeTracker { * @return Network::ListenerPtr a new listener that is owned by the caller. */ virtual Network::ListenerPtr createListener(Network::SocketSharedPtr&& socket, - Network::TcpListenerCallbacks& cb, - bool bind_to_port) PURE; + Network::TcpListenerCallbacks& cb, bool bind_to_port, + bool ignore_global_conn_limit) PURE; /** * Creates a logical udp listener on a specific port. diff --git a/envoy/network/listener.h b/envoy/network/listener.h index 2e4598de04516..3f400712a5a52 100644 --- a/envoy/network/listener.h +++ b/envoy/network/listener.h @@ -215,6 +215,12 @@ class ListenerConfig { * @return init manager of the listener. */ virtual Init::Manager& initManager() PURE; + + /** + * @return bool whether the listener should block connections or not based on the globally set + * limit + */ + virtual bool ignoreGlobalConnLimit() const PURE; }; /** diff --git a/envoy/server/configuration.h b/envoy/server/configuration.h index b43da186eabcd..4fc50cbed7fc6 100644 --- a/envoy/server/configuration.h +++ b/envoy/server/configuration.h @@ -141,6 +141,12 @@ class Admin { * @return Network::Address::OptionsSharedPtr the list of listener socket options. */ virtual Network::Socket::OptionsSharedPtr socketOptions() PURE; + + /** + * @return bool whether the listener should block connections or not based on the globally set + * limit + */ + virtual bool ignoreGlobalConnLimit() const PURE; }; /** diff --git a/source/common/event/dispatcher_impl.cc b/source/common/event/dispatcher_impl.cc index 1f28e9a24fb85..9f8e22ce883cb 100644 --- a/source/common/event/dispatcher_impl.cc +++ b/source/common/event/dispatcher_impl.cc @@ -171,10 +171,10 @@ Filesystem::WatcherPtr DispatcherImpl::createFilesystemWatcher() { Network::ListenerPtr DispatcherImpl::createListener(Network::SocketSharedPtr&& socket, Network::TcpListenerCallbacks& cb, - bool bind_to_port) { + bool bind_to_port, bool limit_connections) { ASSERT(isThreadSafe()); - return std::make_unique(*this, api_.randomGenerator(), - std::move(socket), cb, bind_to_port); + return std::make_unique( + *this, api_.randomGenerator(), std::move(socket), cb, bind_to_port, limit_connections); } Network::UdpListenerPtr diff --git a/source/common/event/dispatcher_impl.h b/source/common/event/dispatcher_impl.h index 513ebc9d1b2b3..67a6ae5a3a265 100644 --- a/source/common/event/dispatcher_impl.h +++ b/source/common/event/dispatcher_impl.h @@ -70,8 +70,8 @@ class DispatcherImpl : Logger::Loggable, uint32_t events) override; Filesystem::WatcherPtr createFilesystemWatcher() override; Network::ListenerPtr createListener(Network::SocketSharedPtr&& socket, - Network::TcpListenerCallbacks& cb, - bool bind_to_port) override; + Network::TcpListenerCallbacks& cb, bool bind_to_port, + bool limit_connections) override; Network::UdpListenerPtr createUdpListener(Network::SocketSharedPtr socket, Network::UdpListenerCallbacks& cb, const envoy::config::core::v3::UdpSocketConfig& config) override; diff --git a/source/common/network/tcp_listener_impl.cc b/source/common/network/tcp_listener_impl.cc index a3f79f72df6b6..b73f26ab4df0e 100644 --- a/source/common/network/tcp_listener_impl.cc +++ b/source/common/network/tcp_listener_impl.cc @@ -20,11 +20,11 @@ namespace Network { const absl::string_view TcpListenerImpl::GlobalMaxCxRuntimeKey = "overload.global_downstream_max_connections"; -bool TcpListenerImpl::rejectCxOverGlobalLimit() { +bool TcpListenerImpl::rejectCxOverGlobalLimit() const { // Enforce the global connection limit if necessary, immediately closing the accepted connection. Runtime::Loader* runtime = Runtime::LoaderSingleton::getExisting(); - if (runtime == nullptr) { + if (ignore_global_conn_limit_ || runtime == nullptr) { // The runtime singleton won't exist in most unit tests that do not need global downstream limit // enforcement. Therefore, there is no need to enforce limits if the singleton doesn't exist. // TODO(tonya11en): Revisit this once runtime is made globally available. @@ -98,9 +98,10 @@ void TcpListenerImpl::onSocketEvent(short flags) { TcpListenerImpl::TcpListenerImpl(Event::DispatcherImpl& dispatcher, Random::RandomGenerator& random, SocketSharedPtr socket, TcpListenerCallbacks& cb, - bool bind_to_port) + bool bind_to_port, bool ignore_global_conn_limit) : BaseListenerImpl(dispatcher, std::move(socket)), cb_(cb), random_(random), - bind_to_port_(bind_to_port), reject_fraction_(0.0) { + bind_to_port_(bind_to_port), reject_fraction_(0.0), + ignore_global_conn_limit_(ignore_global_conn_limit) { if (bind_to_port) { // Although onSocketEvent drains to completion, use level triggered mode to avoid potential // loss of the trigger due to transient accept errors. diff --git a/source/common/network/tcp_listener_impl.h b/source/common/network/tcp_listener_impl.h index 27ecfeec949b6..09f1b8530ec14 100644 --- a/source/common/network/tcp_listener_impl.h +++ b/source/common/network/tcp_listener_impl.h @@ -17,7 +17,8 @@ namespace Network { class TcpListenerImpl : public BaseListenerImpl { public: TcpListenerImpl(Event::DispatcherImpl& dispatcher, Random::RandomGenerator& random, - SocketSharedPtr socket, TcpListenerCallbacks& cb, bool bind_to_port); + SocketSharedPtr socket, TcpListenerCallbacks& cb, bool bind_to_port, + bool limit_connections); ~TcpListenerImpl() override { if (bind_to_port_) { socket_->ioHandle().resetFileEvents(); @@ -37,11 +38,12 @@ class TcpListenerImpl : public BaseListenerImpl { // Returns true if global connection limit has been reached and the accepted socket should be // rejected/closed. If the accepted socket is to be admitted, false is returned. - static bool rejectCxOverGlobalLimit(); + bool rejectCxOverGlobalLimit() const; Random::RandomGenerator& random_; bool bind_to_port_; UnitFloat reject_fraction_; + bool ignore_global_conn_limit_; }; } // namespace Network diff --git a/source/server/active_tcp_listener.cc b/source/server/active_tcp_listener.cc index 3e0f850c9b7a6..763a9170d4b2e 100644 --- a/source/server/active_tcp_listener.cc +++ b/source/server/active_tcp_listener.cc @@ -17,7 +17,7 @@ ActiveTcpListener::ActiveTcpListener(Network::TcpConnectionHandler& parent, : OwnedActiveStreamListenerBase(parent, parent.dispatcher(), parent.dispatcher().createListener( config.listenSocketFactory().getListenSocket(worker_index), - *this, config.bindToPort()), + *this, config.bindToPort(), config.ignoreGlobalConnLimit()), config), tcp_conn_handler_(parent) { config.connectionBalancer().registerHandler(*this); diff --git a/source/server/admin/admin.cc b/source/server/admin/admin.cc index 74c17040b261f..0039d7cf4567b 100644 --- a/source/server/admin/admin.cc +++ b/source/server/admin/admin.cc @@ -109,7 +109,6 @@ const char AdminHtmlEnd[] = R"( )"; - } // namespace ConfigTracker& AdminImpl::getConfigTracker() { return config_tracker_; } @@ -144,7 +143,8 @@ void AdminImpl::startHttpListener(const std::list& } } -AdminImpl::AdminImpl(const std::string& profile_path, Server::Instance& server) +AdminImpl::AdminImpl(const std::string& profile_path, Server::Instance& server, + bool ignore_global_conn_limit) : server_(server), request_id_extension_(Extensions::RequestId::UUIDRequestIDExtension::defaultInstance( server_.api().randomGenerator())), @@ -221,7 +221,8 @@ AdminImpl::AdminImpl(const std::string& profile_path, Server::Instance& server) }, date_provider_(server.dispatcher().timeSource()), admin_filter_chain_(std::make_shared()), - local_reply_(LocalReply::Factory::createDefault()) {} + local_reply_(LocalReply::Factory::createDefault()), + ignore_global_conn_limit_(ignore_global_conn_limit) {} Http::ServerConnectionPtr AdminImpl::createCodec(Network::Connection& connection, const Buffer::Instance& data, diff --git a/source/server/admin/admin.h b/source/server/admin/admin.h index e99330c2c8fc7..acf694cc87780 100644 --- a/source/server/admin/admin.h +++ b/source/server/admin/admin.h @@ -68,7 +68,7 @@ class AdminImpl : public Admin, public Http::ConnectionManagerConfig, Logger::Loggable { public: - AdminImpl(const std::string& profile_path, Server::Instance& server); + AdminImpl(const std::string& profile_path, Server::Instance& server, bool limit_connections); Http::Code runCallback(absl::string_view path_and_query, Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, @@ -341,7 +341,7 @@ class AdminImpl : public Admin, AdminListener(AdminImpl& parent, Stats::ScopePtr&& listener_scope) : parent_(parent), name_("admin"), scope_(std::move(listener_scope)), stats_(Http::ConnectionManagerImpl::generateListenerStats("http.admin.", *scope_)), - init_manager_(nullptr) {} + init_manager_(nullptr), ignore_global_conn_limit_(parent.ignore_global_conn_limit_) {} // Network::ListenerConfig Network::FilterChainManager& filterChainManager() override { return parent_; } @@ -370,6 +370,7 @@ class AdminImpl : public Admin, } uint32_t tcpBacklogSize() const override { return ENVOY_TCP_BACKLOG_SIZE; } Init::Manager& initManager() override { return *init_manager_; } + bool ignoreGlobalConnLimit() const override { return ignore_global_conn_limit_; } AdminImpl& parent_; const std::string name_; @@ -381,6 +382,7 @@ class AdminImpl : public Admin, private: const std::vector empty_access_logs_; std::unique_ptr init_manager_; + const bool ignore_global_conn_limit_; }; using AdminListenerPtr = std::unique_ptr; @@ -453,6 +455,7 @@ class AdminImpl : public Admin, const LocalReply::LocalReplyPtr local_reply_; const std::vector detection_extensions_{}; const absl::optional scheme_{}; + const bool ignore_global_conn_limit_; }; } // namespace Server diff --git a/source/server/config_validation/dispatcher.cc b/source/server/config_validation/dispatcher.cc index 155b482709e22..36d83abe8f1d7 100644 --- a/source/server/config_validation/dispatcher.cc +++ b/source/server/config_validation/dispatcher.cc @@ -16,7 +16,8 @@ Network::ClientConnectionPtr ValidationDispatcher::createClientConnection( } Network::ListenerPtr ValidationDispatcher::createListener(Network::SocketSharedPtr&&, - Network::TcpListenerCallbacks&, bool) { + Network::TcpListenerCallbacks&, bool, + bool) { NOT_IMPLEMENTED_GCOVR_EXCL_LINE; } diff --git a/source/server/config_validation/dispatcher.h b/source/server/config_validation/dispatcher.h index e16829cdb0b61..8902e3ffd2bad 100644 --- a/source/server/config_validation/dispatcher.h +++ b/source/server/config_validation/dispatcher.h @@ -24,7 +24,7 @@ class ValidationDispatcher : public DispatcherImpl { Network::Address::InstanceConstSharedPtr, Network::TransportSocketPtr&&, const Network::ConnectionSocket::OptionsSharedPtr& options) override; Network::ListenerPtr createListener(Network::SocketSharedPtr&&, Network::TcpListenerCallbacks&, - bool bind_to_port) override; + bool bind_to_port, bool limit_connections) override; }; } // namespace Event diff --git a/source/server/configuration_impl.cc b/source/server/configuration_impl.cc index a4556846df540..d5abc1439b7ab 100644 --- a/source/server/configuration_impl.cc +++ b/source/server/configuration_impl.cc @@ -206,6 +206,7 @@ InitialImpl::InitialImpl(const envoy::config::bootstrap::v3::Bootstrap& bootstra admin_.socket_options_, Network::SocketOptionFactory::buildLiteralOptions(admin.socket_options())); } + admin_.ignore_global_conn_limit_ = admin.ignore_global_conn_limit(); if (!bootstrap.flags_path().empty()) { flags_path_ = bootstrap.flags_path(); diff --git a/source/server/configuration_impl.h b/source/server/configuration_impl.h index 7efa5e22fda54..792a106e26f5d 100644 --- a/source/server/configuration_impl.h +++ b/source/server/configuration_impl.h @@ -192,11 +192,13 @@ class InitialImpl : public Initial { Network::Address::InstanceConstSharedPtr address() override { return address_; } Network::Socket::OptionsSharedPtr socketOptions() override { return socket_options_; } std::list accessLogs() const override { return access_logs_; } + bool ignoreGlobalConnLimit() const override { return ignore_global_conn_limit_; } std::string profile_path_; std::list access_logs_; Network::Address::InstanceConstSharedPtr address_; Network::Socket::OptionsSharedPtr socket_options_; + bool ignore_global_conn_limit_; }; AdminImpl admin_; diff --git a/source/server/listener_impl.cc b/source/server/listener_impl.cc index 4be05bd5329e8..83ae92e939205 100644 --- a/source/server/listener_impl.cc +++ b/source/server/listener_impl.cc @@ -305,6 +305,7 @@ ListenerImpl::ListenerImpl(const envoy::config::listener::v3::Listener& config, validation_visitor_( added_via_api_ ? parent_.server_.messageValidationContext().dynamicValidationVisitor() : parent_.server_.messageValidationContext().staticValidationVisitor()), + ignore_global_conn_limit_(config.ignore_global_conn_limit()), listener_init_target_(fmt::format("Listener-init-target {}", name), [this]() { dynamic_init_manager_->initialize(local_init_watcher_); }), dynamic_init_manager_(std::make_unique( @@ -394,6 +395,7 @@ ListenerImpl::ListenerImpl(ListenerImpl& origin, validation_visitor_( added_via_api_ ? parent_.server_.messageValidationContext().dynamicValidationVisitor() : parent_.server_.messageValidationContext().staticValidationVisitor()), + ignore_global_conn_limit_(config.ignore_global_conn_limit()), // listener_init_target_ is not used during in place update because we expect server started. listener_init_target_("", nullptr), dynamic_init_manager_(std::make_unique( diff --git a/source/server/listener_impl.h b/source/server/listener_impl.h index a6b8166d61a91..9eaf3102473b5 100644 --- a/source/server/listener_impl.h +++ b/source/server/listener_impl.h @@ -333,6 +333,7 @@ class ListenerImpl final : public Network::ListenerConfig, } uint32_t tcpBacklogSize() const override { return tcp_backlog_size_; } Init::Manager& initManager() override; + bool ignoreGlobalConnLimit() const override { return ignore_global_conn_limit_; } envoy::config::core::v3::TrafficDirection direction() const override { return config().traffic_direction(); } @@ -411,6 +412,7 @@ class ListenerImpl final : public Network::ListenerConfig, const uint64_t hash_; const uint32_t tcp_backlog_size_; ProtobufMessage::ValidationVisitor& validation_visitor_; + const bool ignore_global_conn_limit_; // A target is added to Server's InitManager if workers_started_ is false. Init::TargetImpl listener_init_target_; diff --git a/source/server/server.cc b/source/server/server.cc index 9bb2a226a3e31..9dee87f9a24b8 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -487,7 +487,8 @@ void InstanceImpl::initialize(Network::Address::InstanceConstSharedPtr local_add // This is needed so that we don't read the value until runtime is fully initialized. enable_reuse_port_default_ = ReusePortDefault::Runtime; } - admin_ = std::make_unique(initial_config.admin().profilePath(), *this); + admin_ = std::make_unique(initial_config.admin().profilePath(), *this, + initial_config.admin().ignoreGlobalConnLimit()); loadServerFlags(initial_config.flagsPath()); diff --git a/test/common/network/listener_impl_test.cc b/test/common/network/listener_impl_test.cc index b1f0aac6462d6..07c687b9dc416 100644 --- a/test/common/network/listener_impl_test.cc +++ b/test/common/network/listener_impl_test.cc @@ -35,7 +35,8 @@ static void errorCallbackTest(Address::IpVersion version) { auto socket = std::make_shared( Network::Test::getCanonicalLoopbackAddress(version)); Network::MockTcpListenerCallbacks listener_callbacks; - Network::ListenerPtr listener = dispatcher->createListener(socket, listener_callbacks, true); + Network::ListenerPtr listener = + dispatcher->createListener(socket, listener_callbacks, true, true); Network::ClientConnectionPtr client_connection = dispatcher->createClientConnection( socket->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), @@ -66,8 +67,10 @@ TEST_P(ListenerImplDeathTest, ErrorCallback) { class TestTcpListenerImpl : public TcpListenerImpl { public: TestTcpListenerImpl(Event::DispatcherImpl& dispatcher, Random::RandomGenerator& random_generator, - SocketSharedPtr socket, TcpListenerCallbacks& cb, bool bind_to_port) - : TcpListenerImpl(dispatcher, random_generator, std::move(socket), cb, bind_to_port) {} + SocketSharedPtr socket, TcpListenerCallbacks& cb, bool bind_to_port, + bool limit_connections) + : TcpListenerImpl(dispatcher, random_generator, std::move(socket), cb, bind_to_port, + limit_connections) {} MOCK_METHOD(Address::InstanceConstSharedPtr, getLocalAddress, (os_fd_t fd)); }; @@ -85,10 +88,10 @@ TEST_P(TcpListenerImplTest, UseActualDst) { Random::MockRandomGenerator random_generator; // Do not redirect since use_original_dst is false. Network::TestTcpListenerImpl listener(dispatcherImpl(), random_generator, socket, - listener_callbacks1, true); + listener_callbacks1, true, true); Network::MockTcpListenerCallbacks listener_callbacks2; Network::TestTcpListenerImpl listenerDst(dispatcherImpl(), random_generator, socketDst, - listener_callbacks2, false); + listener_callbacks2, false, true); Network::ClientConnectionPtr client_connection = dispatcher_->createClientConnection( socket->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), @@ -122,7 +125,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, true); + Network::ListenerPtr listener = + dispatcher_->createListener(socket, listener_callbacks, true, true); std::vector client_connections; std::vector server_connections; @@ -181,6 +185,54 @@ TEST_P(TcpListenerImplTest, GlobalConnectionLimitEnforcement) { {{"overload.global_downstream_max_connections", ""}}); } +TEST_P(TcpListenerImplTest, GlobalConnectionLimitListenerOptOut) { + // Required to manipulate runtime values when there is no test server. + TestScopedRuntime scoped_runtime; + + Runtime::LoaderSingleton::getExisting()->mergeValues( + {{"overload.global_downstream_max_connections", "1"}}); + auto socket = std::make_shared( + Network::Test::getCanonicalLoopbackAddress(version_)); + Network::MockTcpListenerCallbacks listener_callbacks; + Network::ListenerPtr listener = + dispatcher_->createListener(socket, listener_callbacks, true, false); + + std::vector client_connections; + std::vector server_connections; + StreamInfo::StreamInfoImpl stream_info(dispatcher_->timeSource(), nullptr); + EXPECT_CALL(listener_callbacks, onAccept_(_)) + .WillRepeatedly(Invoke([&](Network::ConnectionSocketPtr& accepted_socket) -> void { + server_connections.emplace_back(dispatcher_->createServerConnection( + std::move(accepted_socket), Network::Test::createRawBufferSocket(), stream_info)); + dispatcher_->exit(); + })); + + auto initiate_connections = [&](const int count) { + for (int i = 0; i < count; ++i) { + client_connections.emplace_back( + dispatcher_->createClientConnection(socket->connectionInfoProvider().localAddress(), + Network::Address::InstanceConstSharedPtr(), + Network::Test::createRawBufferSocket(), nullptr)); + client_connections.back()->connect(); + } + }; + + initiate_connections(2); + EXPECT_CALL(listener_callbacks, onReject(TcpListenerCallbacks::RejectCause::GlobalCxLimit)) + .Times(0); + dispatcher_->run(Event::Dispatcher::RunType::Block); + + for (const auto& conn : client_connections) { + conn->close(ConnectionCloseType::NoFlush); + } + for (const auto& conn : server_connections) { + conn->close(ConnectionCloseType::NoFlush); + } + + // We expect any server-side connections that get created to populate 'server_connections'. + EXPECT_EQ(2, server_connections.size()); +} + TEST_P(TcpListenerImplTest, WildcardListenerUseActualDst) { auto socket = std::make_shared( Network::Test::getCanonicalLoopbackAddress(version_)); @@ -188,7 +240,7 @@ TEST_P(TcpListenerImplTest, WildcardListenerUseActualDst) { Random::MockRandomGenerator random_generator; // Do not redirect since use_original_dst is false. Network::TestTcpListenerImpl listener(dispatcherImpl(), random_generator, socket, - listener_callbacks, true); + listener_callbacks, true, true); auto local_dst_address = Network::Utility::getAddressWithPort( *Network::Test::getCanonicalLoopbackAddress(version_), @@ -232,7 +284,7 @@ TEST_P(TcpListenerImplTest, WildcardListenerIpv4Compat) { // Do not redirect since use_original_dst is false. Network::TestTcpListenerImpl listener(dispatcherImpl(), random_generator, socket, - listener_callbacks, true); + listener_callbacks, true, true); auto listener_address = Network::Utility::getAddressWithPort( *Network::Test::getCanonicalLoopbackAddress(version_), @@ -271,7 +323,7 @@ TEST_P(TcpListenerImplTest, DisableAndEnableListener) { MockTcpListenerCallbacks listener_callbacks; MockConnectionCallbacks connection_callbacks; Random::MockRandomGenerator random_generator; - TestTcpListenerImpl listener(dispatcherImpl(), random_generator, socket, listener_callbacks, + TestTcpListenerImpl listener(dispatcherImpl(), random_generator, socket, listener_callbacks, true, true); // When listener is disabled, the timer should fire before any connection is accepted. @@ -312,7 +364,7 @@ TEST_P(TcpListenerImplTest, SetListenerRejectFractionZero) { MockTcpListenerCallbacks listener_callbacks; MockConnectionCallbacks connection_callbacks; Random::MockRandomGenerator random_generator; - TestTcpListenerImpl listener(dispatcherImpl(), random_generator, socket, listener_callbacks, + TestTcpListenerImpl listener(dispatcherImpl(), random_generator, socket, listener_callbacks, true, true); listener.setRejectFraction(UnitFloat(0)); @@ -343,7 +395,7 @@ TEST_P(TcpListenerImplTest, SetListenerRejectFractionIntermediate) { MockTcpListenerCallbacks listener_callbacks; MockConnectionCallbacks connection_callbacks; Random::MockRandomGenerator random_generator; - TestTcpListenerImpl listener(dispatcherImpl(), random_generator, socket, listener_callbacks, + TestTcpListenerImpl listener(dispatcherImpl(), random_generator, socket, listener_callbacks, true, true); listener.setRejectFraction(UnitFloat(0.5f)); @@ -406,7 +458,7 @@ TEST_P(TcpListenerImplTest, SetListenerRejectFractionAll) { MockTcpListenerCallbacks listener_callbacks; MockConnectionCallbacks connection_callbacks; Random::MockRandomGenerator random_generator; - TestTcpListenerImpl listener(dispatcherImpl(), random_generator, socket, listener_callbacks, + TestTcpListenerImpl listener(dispatcherImpl(), random_generator, socket, listener_callbacks, true, true); listener.setRejectFraction(UnitFloat(1)); diff --git a/test/extensions/common/proxy_protocol/proxy_protocol_regression_test.cc b/test/extensions/common/proxy_protocol/proxy_protocol_regression_test.cc index cb9d97d5593f8..fe2ddcde9f789 100644 --- a/test/extensions/common/proxy_protocol/proxy_protocol_regression_test.cc +++ b/test/extensions/common/proxy_protocol/proxy_protocol_regression_test.cc @@ -82,6 +82,7 @@ class ProxyProtocolRegressionTest : public testing::TestWithParammutable_listeners(0); + listener->mutable_limit_connections()->set_value(false); + }); + } + + void setAdminGlobalLimitOptOut() { + config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { + auto* admin = bootstrap.mutable_admin(); + admin->mutable_limit_connections()->set_value(false); + }); } void initialize() override { BaseIntegrationTest::initialize(); } @@ -145,7 +160,7 @@ TEST_P(ConnectionLimitIntegrationTest, TestGlobalLimit) { // Includes twice the number of connections expected because the tracking is performed via a // static variable and the fake upstream has a listener. This causes upstream connections to the // fake upstream to also be tracked as part of the global downstream connection tracking. - setGlobalLimit("4"); + setGlobalLimit(4); initialize(); }; @@ -154,9 +169,12 @@ TEST_P(ConnectionLimitIntegrationTest, TestGlobalLimit) { TEST_P(ConnectionLimitIntegrationTest, TestBothLimits) { std::function init_func = [this]() { + // Includes twice the number of connections expected because the tracking is performed via a + // static variable and the fake upstream has a listener. This causes upstream connections to the + // fake upstream to also be tracked as part of the global downstream connection tracking. + setGlobalLimit(4); // Setting the listener limit to a much higher value and making sure the right stat gets // incremented when both limits are set. - setGlobalLimit("4"); setListenerLimit(100); initialize(); }; @@ -164,5 +182,119 @@ TEST_P(ConnectionLimitIntegrationTest, TestBothLimits) { doTest(init_func, "downstream_global_cx_overflow"); } +TEST_P(ConnectionLimitIntegrationTest, TestGlobalLimitOptOut) { + // Includes 4 connections because the tracking is performed regardless of whether a specific + // listener has opted out. Since the fake upstream has a listener, we need to keep value at 4 so + // it can accept connections. (2 downstream listener conns + 2 upstream listener conns) + setGlobalLimit(4); + setGlobalLimitOptOut(); + setAdminGlobalLimitOptOut(); + setListenerLimit(100); + initialize(); + + std::vector tcp_clients; + std::vector raw_conns; + tcp_clients.emplace_back(makeTcpConnection(lookupPort("listener_0"))); + raw_conns.emplace_back(); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(raw_conns.back())); + ASSERT_TRUE(tcp_clients.back()->connected()); + + tcp_clients.emplace_back(makeTcpConnection(lookupPort("listener_0"))); + raw_conns.emplace_back(); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(raw_conns.back())); + ASSERT_TRUE(tcp_clients.back()->connected()); + + // 3rd connection should fail, not because listener_0 hit a limit, but because the + // upstream listener hit a limit (5 conns would exist when it goes to accept, so it rejects it). + // We can see that listener_0 didn't hit any limits because it's downstream_global_cx_overflow + // stat is still at 0, in contrast with the TestGlobalLimit test where it is 1 + tcp_clients.emplace_back(makeTcpConnection(lookupPort("listener_0"))); + raw_conns.emplace_back(); + ASSERT_FALSE( + fake_upstreams_[0]->waitForRawConnection(raw_conns.back(), std::chrono::milliseconds(500))); + tcp_clients.back()->waitForDisconnect(); + + // Get rid of the client that failed to connect. + tcp_clients.back()->close(); + tcp_clients.pop_back(); + + // admin connections should succeed + tcp_clients.emplace_back(makeTcpConnection(lookupPort("admin"))); + raw_conns.emplace_back(); + ASSERT_TRUE(tcp_clients.back()->connected()); + + const bool isV4 = (version_ == Network::Address::IpVersion::v4); + auto local_address = isV4 ? Network::Utility::getCanonicalIpv4LoopbackAddress() + : Network::Utility::getIpv6LoopbackAddress(); + + const std::string counter_prefix = (isV4 ? "listener.127.0.0.1_0." : "listener.[__1]_0."); + + // listner_0 does not hit any connection limits + test_server_->waitForCounterEq(counter_prefix + "downstream_global_cx_overflow", 0); + test_server_->waitForCounterEq(counter_prefix + "downstream_cx_overflow", 0); + test_server_->waitForCounterEq("listener.admin.downstream_global_cx_overflow", 0); + test_server_->waitForCounterEq("listener.admin.downstream_cx_overflow", 0); + + for (auto& tcp_client : tcp_clients) { + tcp_client->close(); + } + + tcp_clients.clear(); + raw_conns.clear(); +} + +TEST_P(ConnectionLimitIntegrationTest, TestListenerLimitWithGlobalOptOut) { + // Includes 4 connections because the tracking is performed regardless of whether a specific + // listener has opted out. Since the fake upstream has a listener, we need to keep value at 4 so + // it can accept connections. (2 downstream listener conns + 2 upstream listener conns) + setGlobalLimit(4); + setGlobalLimitOptOut(); + // Only allow 2 connections for the listener even though it has opted out of the global connection + // limit + setListenerLimit(2); + initialize(); + + std::vector tcp_clients; + std::vector raw_conns; + tcp_clients.emplace_back(makeTcpConnection(lookupPort("listener_0"))); + raw_conns.emplace_back(); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(raw_conns.back())); + ASSERT_TRUE(tcp_clients.back()->connected()); + + tcp_clients.emplace_back(makeTcpConnection(lookupPort("listener_0"))); + raw_conns.emplace_back(); + ASSERT_TRUE(fake_upstreams_[0]->waitForRawConnection(raw_conns.back())); + ASSERT_TRUE(tcp_clients.back()->connected()); + + // 3rd connection should fail because we've hit the listener connection limit, not because + // we've hit a global limit + tcp_clients.emplace_back(makeTcpConnection(lookupPort("listener_0"))); + raw_conns.emplace_back(); + ASSERT_FALSE( + fake_upstreams_[0]->waitForRawConnection(raw_conns.back(), std::chrono::milliseconds(500))); + tcp_clients.back()->waitForDisconnect(); + + // Get rid of the client that failed to connect. + tcp_clients.back()->close(); + tcp_clients.pop_back(); + + const bool isV4 = (version_ == Network::Address::IpVersion::v4); + auto local_address = isV4 ? Network::Utility::getCanonicalIpv4LoopbackAddress() + : Network::Utility::getIpv6LoopbackAddress(); + + const std::string counter_prefix = (isV4 ? "listener.127.0.0.1_0." : "listener.[__1]_0."); + + // listner_0 does hits the listener connection limit + test_server_->waitForCounterEq(counter_prefix + "downstream_global_cx_overflow", 0); + test_server_->waitForCounterEq(counter_prefix + "downstream_cx_overflow", 1); + + for (auto& tcp_client : tcp_clients) { + tcp_client->close(); + } + + tcp_clients.clear(); + raw_conns.clear(); +} + } // namespace } // namespace Envoy diff --git a/test/integration/fake_upstream.h b/test/integration/fake_upstream.h index f667bcb2a2258..0761ea68a9f9a 100644 --- a/test/integration/fake_upstream.h +++ b/test/integration/fake_upstream.h @@ -801,6 +801,7 @@ class FakeUpstream : Logger::Loggable, ResourceLimit& openConnections() override { return connection_resource_; } uint32_t tcpBacklogSize() const override { return ENVOY_TCP_BACKLOG_SIZE; } Init::Manager& initManager() override { return *init_manager_; } + bool ignoreGlobalConnLimit() const override { return false; } void setMaxConnections(const uint32_t num_connections) { connection_resource_.setMax(num_connections); diff --git a/test/mocks/event/mocks.h b/test/mocks/event/mocks.h index 8d91e35582916..f9ad32e256d77 100644 --- a/test/mocks/event/mocks.h +++ b/test/mocks/event/mocks.h @@ -70,9 +70,10 @@ class MockDispatcher : public Dispatcher { } Network::ListenerPtr createListener(Network::SocketSharedPtr&& socket, - Network::TcpListenerCallbacks& cb, - bool bind_to_port) override { - return Network::ListenerPtr{createListener_(std::move(socket), cb, bind_to_port)}; + Network::TcpListenerCallbacks& cb, bool bind_to_port, + bool limit_connections) override { + return Network::ListenerPtr{ + createListener_(std::move(socket), cb, bind_to_port, limit_connections)}; } Network::UdpListenerPtr @@ -138,7 +139,7 @@ class MockDispatcher : public Dispatcher { MOCK_METHOD(Filesystem::Watcher*, createFilesystemWatcher_, ()); MOCK_METHOD(Network::Listener*, createListener_, (Network::SocketSharedPtr && socket, Network::TcpListenerCallbacks& cb, - bool bind_to_port)); + bool bind_to_port, bool limit_connections)); MOCK_METHOD(Network::UdpListener*, createUdpListener_, (Network::SocketSharedPtr socket, Network::UdpListenerCallbacks& cb, const envoy::config::core::v3::UdpSocketConfig& config)); diff --git a/test/mocks/network/mocks.h b/test/mocks/network/mocks.h index e4f5d6c18df9e..2839b511006f2 100644 --- a/test/mocks/network/mocks.h +++ b/test/mocks/network/mocks.h @@ -432,6 +432,7 @@ class MockListenerConfig : public ListenerConfig { MOCK_METHOD(ResourceLimit&, openConnections, ()); MOCK_METHOD(uint32_t, tcpBacklogSize, (), (const)); MOCK_METHOD(Init::Manager&, initManager, ()); + MOCK_METHOD(bool, limitConnections, (), (const)); envoy::config::core::v3::TrafficDirection direction() const override { return envoy::config::core::v3::UNSPECIFIED; diff --git a/test/server/connection_handler_test.cc b/test/server/connection_handler_test.cc index 672f24e901648..3438421941038 100644 --- a/test/server/connection_handler_test.cc +++ b/test/server/connection_handler_test.cc @@ -67,7 +67,8 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggable access_log, std::shared_ptr> filter_chain_manager = nullptr, uint32_t tcp_backlog_size = ENVOY_TCP_BACKLOG_SIZE, - Network::ConnectionBalancerSharedPtr connection_balancer = nullptr) + Network::ConnectionBalancerSharedPtr connection_balancer = nullptr, + bool ignore_global_conn_limit = false) : parent_(parent), socket_(std::make_shared>()), tag_(tag), bind_to_port_(bind_to_port), tcp_backlog_size_(tcp_backlog_size), hand_off_restored_destination_connections_(hand_off_restored_destination_connections), @@ -77,7 +78,7 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggable() : connection_balancer), access_logs_({access_log}), inline_filter_chain_manager_(filter_chain_manager), - init_manager_(nullptr) { + init_manager_(nullptr), ignore_global_conn_limit_(ignore_global_conn_limit) { envoy::config::listener::v3::UdpListenerConfig udp_config; udp_listener_config_ = std::make_unique(udp_config); udp_listener_config_->listener_factory_ = @@ -137,6 +138,7 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggable access_logs_; std::shared_ptr> inline_filter_chain_manager_; std::unique_ptr init_manager_; + const bool ignore_global_conn_limit_; envoy::config::core::v3::TrafficDirection direction_; Network::UdpListenerCallbacks* udp_listener_callbacks_{}; }; @@ -211,11 +214,12 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggable> overridden_filter_chain_manager = nullptr, - uint32_t tcp_backlog_size = ENVOY_TCP_BACKLOG_SIZE) { + uint32_t tcp_backlog_size = ENVOY_TCP_BACKLOG_SIZE, bool ignore_global_conn_limit = false) { listeners_.emplace_back(std::make_unique( *this, tag, bind_to_port, hand_off_restored_destination_connections, name, socket_type, listener_filters_timeout, continue_on_listener_filters_timeout, access_log_, - overridden_filter_chain_manager, tcp_backlog_size, connection_balancer)); + overridden_filter_chain_manager, tcp_backlog_size, connection_balancer, + ignore_global_conn_limit)); if (listener == nullptr) { // Expecting listener config in place update. From dd69f184a75cc9ce7590e782fafe9dbf085a702b Mon Sep 17 00:00:00 2001 From: Weston Carlson Date: Tue, 2 Nov 2021 19:54:41 -0600 Subject: [PATCH 02/10] Update version history. Signed-off-by: Weston Carlson --- docs/root/version_history/current.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 30648e1bb1c41..64bdc81a0f794 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -61,6 +61,7 @@ New Features * http: added support for :ref:`retriable health check status codes `. * listener: added API for extensions to access :ref:`typed_filter_metadata ` configured in the listener's :ref:`metadata ` field. * listener: added support for :ref:`MPTCP ` (multipath TCP). +* listener: added support for opting out listeners from the globally set downstream connection limit via :ref:`ignore_global_conn_limit `. * oauth filter: added :ref:`cookie_names ` to allow overriding (default) cookie names (``BearerToken``, ``OauthHMAC``, and ``OauthExpires``) set by the filter. * tcp: added a :ref:`FilterState ` :ref:`hash policy `, used by :ref:`TCP proxy ` to allow hashing load balancer algorithms to hash on objects in filter state. * tcp_proxy: added support to populate upstream http connect header values from stream info. From 78135f9458bc5181d6b846c42a52f745a66d85eb Mon Sep 17 00:00:00 2001 From: Weston Carlson Date: Wed, 3 Nov 2021 08:53:02 -0600 Subject: [PATCH 03/10] Fix spelling. Signed-off-by: Weston Carlson --- ci/README.md | 1 - test/integration/cx_limit_integration_test.cc | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/ci/README.md b/ci/README.md index 0561facedf73c..12468c6f2b4ad 100644 --- a/ci/README.md +++ b/ci/README.md @@ -135,7 +135,6 @@ The `./ci/run_envoy_docker.sh './ci/do_ci.sh '` targets are: * `bazel.clang_tidy ` — build and run clang-tidy specified source files, if no files specified, runs against the diff with the last GitHub commit. * `check_format`— run `clang-format` and `buildifier` on entire source tree. * `fix_format`— run and enforce `clang-format` and `buildifier` on entire source tree. -* `check_spelling_pedantic`— run `aspell` on C++ and proto comments. * `docs`— build documentation tree in `generated/docs`. ## On Windows diff --git a/test/integration/cx_limit_integration_test.cc b/test/integration/cx_limit_integration_test.cc index f18db8e083f5d..ac8b186b27b6e 100644 --- a/test/integration/cx_limit_integration_test.cc +++ b/test/integration/cx_limit_integration_test.cc @@ -229,7 +229,7 @@ TEST_P(ConnectionLimitIntegrationTest, TestGlobalLimitOptOut) { const std::string counter_prefix = (isV4 ? "listener.127.0.0.1_0." : "listener.[__1]_0."); - // listner_0 does not hit any connection limits + // listener_0 does not hit any connection limits test_server_->waitForCounterEq(counter_prefix + "downstream_global_cx_overflow", 0); test_server_->waitForCounterEq(counter_prefix + "downstream_cx_overflow", 0); test_server_->waitForCounterEq("listener.admin.downstream_global_cx_overflow", 0); @@ -284,7 +284,7 @@ TEST_P(ConnectionLimitIntegrationTest, TestListenerLimitWithGlobalOptOut) { const std::string counter_prefix = (isV4 ? "listener.127.0.0.1_0." : "listener.[__1]_0."); - // listner_0 does hits the listener connection limit + // listener_0 does hits the listener connection limit test_server_->waitForCounterEq(counter_prefix + "downstream_global_cx_overflow", 0); test_server_->waitForCounterEq(counter_prefix + "downstream_cx_overflow", 1); From 5e24f3de67d11190350d8783a9c331aaeb13ce40 Mon Sep 17 00:00:00 2001 From: Weston Carlson Date: Wed, 3 Nov 2021 12:59:45 -0600 Subject: [PATCH 04/10] Add new listener field to various tests. Signed-off-by: Weston Carlson --- ci/README.md | 1 + test/common/http/codec_client_test.cc | 3 +- test/common/network/connection_impl_test.cc | 6 ++-- test/common/network/listener_impl_test.cc | 26 +++++++-------- .../dns_resolver/cares/dns_impl_test.cc | 2 +- .../transport_sockets/tls/ssl_socket_test.cc | 33 ++++++++++--------- test/integration/cx_limit_integration_test.cc | 4 +-- test/mocks/event/mocks.h | 6 ++-- test/mocks/event/wrapped_dispatcher.h | 6 ++-- test/mocks/network/mocks.h | 2 +- test/server/connection_handler_test.cc | 4 +-- 11 files changed, 49 insertions(+), 44 deletions(-) diff --git a/ci/README.md b/ci/README.md index 12468c6f2b4ad..fd4a3b2d63532 100644 --- a/ci/README.md +++ b/ci/README.md @@ -135,6 +135,7 @@ The `./ci/run_envoy_docker.sh './ci/do_ci.sh '` targets are: * `bazel.clang_tidy ` — build and run clang-tidy specified source files, if no files specified, runs against the diff with the last GitHub commit. * `check_format`— run `clang-format` and `buildifier` on entire source tree. * `fix_format`— run and enforce `clang-format` and `buildifier` on entire source tree. +* `format_pre`— run validation and linting tools. * `docs`— build documentation tree in `generated/docs`. ## On Windows diff --git a/test/common/http/codec_client_test.cc b/test/common/http/codec_client_test.cc index 87fecd8ebe183..b3c873786e3a7 100644 --- a/test/common/http/codec_client_test.cc +++ b/test/common/http/codec_client_test.cc @@ -291,7 +291,8 @@ class CodecNetworkTest : public Event::TestUsingSimulatedTime, Network::ClientConnectionPtr client_connection = dispatcher_->createClientConnection( socket->connectionInfoProvider().localAddress(), source_address_, Network::Test::createRawBufferSocket(), nullptr); - upstream_listener_ = dispatcher_->createListener(std::move(socket), listener_callbacks_, true); + upstream_listener_ = + dispatcher_->createListener(std::move(socket), listener_callbacks_, true, false); client_connection_ = client_connection.get(); client_connection_->addConnectionCallbacks(client_callbacks_); diff --git a/test/common/network/connection_impl_test.cc b/test/common/network/connection_impl_test.cc index 7d2e600272484..838009c717f2d 100644 --- a/test/common/network/connection_impl_test.cc +++ b/test/common/network/connection_impl_test.cc @@ -144,7 +144,7 @@ class ConnectionImplTest : public testing::TestWithParam { } socket_ = std::make_shared( Network::Test::getCanonicalLoopbackAddress(GetParam())); - listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true); + listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true, false); client_connection_ = std::make_unique( *dispatcher_, socket_->connectionInfoProvider().localAddress(), source_address_, Network::Test::createRawBufferSocket(), socket_options_); @@ -1363,7 +1363,7 @@ TEST_P(ConnectionImplTest, BindFailureTest) { dispatcher_ = api_->allocateDispatcher("test_thread"); socket_ = std::make_shared( Network::Test::getCanonicalLoopbackAddress(GetParam())); - listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true); + listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true, false); client_connection_ = dispatcher_->createClientConnection( socket_->connectionInfoProvider().localAddress(), source_address_, @@ -2834,7 +2834,7 @@ class ReadBufferLimitTest : public ConnectionImplTest { dispatcher_ = api_->allocateDispatcher("test_thread"); socket_ = std::make_shared( Network::Test::getCanonicalLoopbackAddress(GetParam())); - listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true); + listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true, false); client_connection_ = dispatcher_->createClientConnection(socket_->connectionInfoProvider().localAddress(), diff --git a/test/common/network/listener_impl_test.cc b/test/common/network/listener_impl_test.cc index 07c687b9dc416..2e12e648be718 100644 --- a/test/common/network/listener_impl_test.cc +++ b/test/common/network/listener_impl_test.cc @@ -36,7 +36,7 @@ static void errorCallbackTest(Address::IpVersion version) { Network::Test::getCanonicalLoopbackAddress(version)); Network::MockTcpListenerCallbacks listener_callbacks; Network::ListenerPtr listener = - dispatcher->createListener(socket, listener_callbacks, true, true); + dispatcher->createListener(socket, listener_callbacks, true, false); Network::ClientConnectionPtr client_connection = dispatcher->createClientConnection( socket->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), @@ -68,9 +68,9 @@ class TestTcpListenerImpl : public TcpListenerImpl { public: TestTcpListenerImpl(Event::DispatcherImpl& dispatcher, Random::RandomGenerator& random_generator, SocketSharedPtr socket, TcpListenerCallbacks& cb, bool bind_to_port, - bool limit_connections) + bool ignore_global_conn_limit) : TcpListenerImpl(dispatcher, random_generator, std::move(socket), cb, bind_to_port, - limit_connections) {} + ignore_global_conn_limit) {} MOCK_METHOD(Address::InstanceConstSharedPtr, getLocalAddress, (os_fd_t fd)); }; @@ -88,10 +88,10 @@ TEST_P(TcpListenerImplTest, UseActualDst) { Random::MockRandomGenerator random_generator; // Do not redirect since use_original_dst is false. Network::TestTcpListenerImpl listener(dispatcherImpl(), random_generator, socket, - listener_callbacks1, true, true); + listener_callbacks1, true, false); Network::MockTcpListenerCallbacks listener_callbacks2; Network::TestTcpListenerImpl listenerDst(dispatcherImpl(), random_generator, socketDst, - listener_callbacks2, false, true); + listener_callbacks2, false, false); Network::ClientConnectionPtr client_connection = dispatcher_->createClientConnection( socket->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), @@ -126,7 +126,7 @@ TEST_P(TcpListenerImplTest, GlobalConnectionLimitEnforcement) { Network::Test::getCanonicalLoopbackAddress(version_)); Network::MockTcpListenerCallbacks listener_callbacks; Network::ListenerPtr listener = - dispatcher_->createListener(socket, listener_callbacks, true, true); + dispatcher_->createListener(socket, listener_callbacks, true, false); std::vector client_connections; std::vector server_connections; @@ -195,7 +195,7 @@ TEST_P(TcpListenerImplTest, GlobalConnectionLimitListenerOptOut) { Network::Test::getCanonicalLoopbackAddress(version_)); Network::MockTcpListenerCallbacks listener_callbacks; Network::ListenerPtr listener = - dispatcher_->createListener(socket, listener_callbacks, true, false); + dispatcher_->createListener(socket, listener_callbacks, true, true); std::vector client_connections; std::vector server_connections; @@ -240,7 +240,7 @@ TEST_P(TcpListenerImplTest, WildcardListenerUseActualDst) { Random::MockRandomGenerator random_generator; // Do not redirect since use_original_dst is false. Network::TestTcpListenerImpl listener(dispatcherImpl(), random_generator, socket, - listener_callbacks, true, true); + listener_callbacks, true, false); auto local_dst_address = Network::Utility::getAddressWithPort( *Network::Test::getCanonicalLoopbackAddress(version_), @@ -284,7 +284,7 @@ TEST_P(TcpListenerImplTest, WildcardListenerIpv4Compat) { // Do not redirect since use_original_dst is false. Network::TestTcpListenerImpl listener(dispatcherImpl(), random_generator, socket, - listener_callbacks, true, true); + listener_callbacks, true, false); auto listener_address = Network::Utility::getAddressWithPort( *Network::Test::getCanonicalLoopbackAddress(version_), @@ -324,7 +324,7 @@ TEST_P(TcpListenerImplTest, DisableAndEnableListener) { MockConnectionCallbacks connection_callbacks; Random::MockRandomGenerator random_generator; TestTcpListenerImpl listener(dispatcherImpl(), random_generator, socket, listener_callbacks, true, - true); + false); // When listener is disabled, the timer should fire before any connection is accepted. listener.disable(); @@ -365,7 +365,7 @@ TEST_P(TcpListenerImplTest, SetListenerRejectFractionZero) { MockConnectionCallbacks connection_callbacks; Random::MockRandomGenerator random_generator; TestTcpListenerImpl listener(dispatcherImpl(), random_generator, socket, listener_callbacks, true, - true); + false); listener.setRejectFraction(UnitFloat(0)); @@ -396,7 +396,7 @@ TEST_P(TcpListenerImplTest, SetListenerRejectFractionIntermediate) { MockConnectionCallbacks connection_callbacks; Random::MockRandomGenerator random_generator; TestTcpListenerImpl listener(dispatcherImpl(), random_generator, socket, listener_callbacks, true, - true); + false); listener.setRejectFraction(UnitFloat(0.5f)); @@ -459,7 +459,7 @@ TEST_P(TcpListenerImplTest, SetListenerRejectFractionAll) { MockConnectionCallbacks connection_callbacks; Random::MockRandomGenerator random_generator; TestTcpListenerImpl listener(dispatcherImpl(), random_generator, socket, listener_callbacks, true, - true); + false); listener.setRejectFraction(UnitFloat(1)); diff --git a/test/extensions/network/dns_resolver/cares/dns_impl_test.cc b/test/extensions/network/dns_resolver/cares/dns_impl_test.cc index d60dc911e65f6..14f6cad7ec0c6 100644 --- a/test/extensions/network/dns_resolver/cares/dns_impl_test.cc +++ b/test/extensions/network/dns_resolver/cares/dns_impl_test.cc @@ -587,7 +587,7 @@ class DnsImplTest : public testing::TestWithParam { server_ = std::make_unique(*dispatcher_); socket_ = std::make_shared( Network::Test::getCanonicalLoopbackAddress(GetParam())); - listener_ = dispatcher_->createListener(socket_, *server_, true); + listener_ = dispatcher_->createListener(socket_, *server_, true, false); updateDnsResolverOptions(); // Create a resolver options on stack here to emulate what actually happens in envoy bootstrap. diff --git a/test/extensions/transport_sockets/tls/ssl_socket_test.cc b/test/extensions/transport_sockets/tls/ssl_socket_test.cc index 4685f03829a4f..0fa30bf0c8da2 100644 --- a/test/extensions/transport_sockets/tls/ssl_socket_test.cc +++ b/test/extensions/transport_sockets/tls/ssl_socket_test.cc @@ -342,7 +342,7 @@ void testUtil(const TestUtilOptions& options) { auto socket = std::make_shared( Network::Test::getCanonicalLoopbackAddress(options.version())); Network::MockTcpListenerCallbacks callbacks; - Network::ListenerPtr listener = dispatcher->createListener(socket, callbacks, true); + Network::ListenerPtr listener = dispatcher->createListener(socket, callbacks, true, false); envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext client_tls_context; TestUtility::loadFromYaml(TestEnvironment::substitute(options.clientCtxYaml()), @@ -677,7 +677,7 @@ void testUtilV2(const TestUtilOptionsV2& options) { auto socket = std::make_shared( Network::Test::getCanonicalLoopbackAddress(options.version())); NiceMock callbacks; - Network::ListenerPtr listener = dispatcher->createListener(socket, callbacks, true); + Network::ListenerPtr listener = dispatcher->createListener(socket, callbacks, true, false); Stats::TestUtil::TestStore client_stats_store; Api::ApiPtr client_api = Api::createApiForTest(client_stats_store, time_system); @@ -2469,7 +2469,7 @@ TEST_P(SslSocketTest, FlushCloseDuringHandshake) { auto socket = std::make_shared( Network::Test::getCanonicalLoopbackAddress(GetParam())); Network::MockTcpListenerCallbacks callbacks; - Network::ListenerPtr listener = dispatcher_->createListener(socket, callbacks, true); + Network::ListenerPtr listener = dispatcher_->createListener(socket, callbacks, true, false); Network::ClientConnectionPtr client_connection = dispatcher_->createClientConnection( socket->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), @@ -2524,7 +2524,8 @@ TEST_P(SslSocketTest, HalfClose) { auto socket = std::make_shared( Network::Test::getCanonicalLoopbackAddress(GetParam())); Network::MockTcpListenerCallbacks listener_callbacks; - Network::ListenerPtr listener = dispatcher_->createListener(socket, listener_callbacks, true); + Network::ListenerPtr listener = + dispatcher_->createListener(socket, listener_callbacks, true, false); std::shared_ptr server_read_filter(new Network::MockReadFilter()); std::shared_ptr client_read_filter(new Network::MockReadFilter()); @@ -2605,7 +2606,8 @@ TEST_P(SslSocketTest, ShutdownWithCloseNotify) { auto socket = std::make_shared( Network::Test::getCanonicalLoopbackAddress(GetParam())); Network::MockTcpListenerCallbacks listener_callbacks; - Network::ListenerPtr listener = dispatcher_->createListener(socket, listener_callbacks, true); + Network::ListenerPtr listener = + dispatcher_->createListener(socket, listener_callbacks, true, false); std::shared_ptr server_read_filter(new Network::MockReadFilter()); std::shared_ptr client_read_filter(new Network::MockReadFilter()); @@ -2692,7 +2694,8 @@ TEST_P(SslSocketTest, ShutdownWithoutCloseNotify) { auto socket = std::make_shared( Network::Test::getCanonicalLoopbackAddress(GetParam())); Network::MockTcpListenerCallbacks listener_callbacks; - Network::ListenerPtr listener = dispatcher_->createListener(socket, listener_callbacks, true); + Network::ListenerPtr listener = + dispatcher_->createListener(socket, listener_callbacks, true, false); std::shared_ptr server_read_filter(new Network::MockReadFilter()); std::shared_ptr client_read_filter(new Network::MockReadFilter()); @@ -2795,7 +2798,7 @@ TEST_P(SslSocketTest, ClientAuthMultipleCAs) { auto socket = std::make_shared( Network::Test::getCanonicalLoopbackAddress(GetParam())); Network::MockTcpListenerCallbacks callbacks; - Network::ListenerPtr listener = dispatcher_->createListener(socket, callbacks, true); + Network::ListenerPtr listener = dispatcher_->createListener(socket, callbacks, true, false); const std::string client_ctx_yaml = R"EOF( common_tls_context: @@ -2892,8 +2895,8 @@ void testTicketSessionResumption(const std::string& server_ctx_yaml1, Network::Test::getCanonicalLoopbackAddress(ip_version)); NiceMock callbacks; Event::DispatcherPtr dispatcher(server_api->allocateDispatcher("test_thread")); - Network::ListenerPtr listener1 = dispatcher->createListener(socket1, callbacks, true); - Network::ListenerPtr listener2 = dispatcher->createListener(socket2, callbacks, true); + Network::ListenerPtr listener1 = dispatcher->createListener(socket1, callbacks, true, false); + Network::ListenerPtr listener2 = dispatcher->createListener(socket2, callbacks, true, false); envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext client_tls_context; TestUtility::loadFromYaml(TestEnvironment::substitute(client_ctx_yaml), client_tls_context); @@ -3031,7 +3034,7 @@ void testSupportForStatelessSessionResumption(const std::string& server_ctx_yaml Network::Test::getCanonicalLoopbackAddress(ip_version)); NiceMock callbacks; Event::DispatcherPtr dispatcher(server_api->allocateDispatcher("test_thread")); - Network::ListenerPtr listener = dispatcher->createListener(tcp_socket, callbacks, true); + Network::ListenerPtr listener = dispatcher->createListener(tcp_socket, callbacks, true, false); envoy::extensions::transport_sockets::tls::v3::UpstreamTlsContext client_tls_context; TestUtility::loadFromYaml(TestEnvironment::substitute(client_ctx_yaml), client_tls_context); @@ -3473,8 +3476,8 @@ TEST_P(SslSocketTest, ClientAuthCrossListenerSessionResumption) { auto socket2 = std::make_shared( Network::Test::getCanonicalLoopbackAddress(GetParam())); Network::MockTcpListenerCallbacks callbacks; - Network::ListenerPtr listener = dispatcher_->createListener(socket, callbacks, true); - Network::ListenerPtr listener2 = dispatcher_->createListener(socket2, callbacks, true); + Network::ListenerPtr listener = dispatcher_->createListener(socket, callbacks, true, false); + Network::ListenerPtr listener2 = dispatcher_->createListener(socket2, callbacks, true, false); const std::string client_ctx_yaml = R"EOF( common_tls_context: tls_certificates: @@ -3591,7 +3594,7 @@ void SslSocketTest::testClientSessionResumption(const std::string& server_ctx_ya NiceMock callbacks; Api::ApiPtr api = Api::createApiForTest(server_stats_store, time_system_); Event::DispatcherPtr dispatcher(server_api->allocateDispatcher("test_thread")); - Network::ListenerPtr listener = dispatcher->createListener(socket, callbacks, true); + Network::ListenerPtr listener = dispatcher->createListener(socket, callbacks, true, false); Network::ConnectionPtr server_connection; Network::MockConnectionCallbacks server_connection_callbacks; @@ -3850,7 +3853,7 @@ TEST_P(SslSocketTest, SslError) { auto socket = std::make_shared( Network::Test::getCanonicalLoopbackAddress(GetParam())); Network::MockTcpListenerCallbacks callbacks; - Network::ListenerPtr listener = dispatcher_->createListener(socket, callbacks, true); + Network::ListenerPtr listener = dispatcher_->createListener(socket, callbacks, true, false); Network::ClientConnectionPtr client_connection = dispatcher_->createClientConnection( socket->connectionInfoProvider().localAddress(), Network::Address::InstanceConstSharedPtr(), @@ -4935,7 +4938,7 @@ class SslReadBufferLimitTest : public SslSocketTest { socket_ = std::make_shared( Network::Test::getCanonicalLoopbackAddress(GetParam())); - listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true); + listener_ = dispatcher_->createListener(socket_, listener_callbacks_, true, false); TestUtility::loadFromYaml(TestEnvironment::substitute(client_ctx_yaml_), upstream_tls_context_); auto client_cfg = diff --git a/test/integration/cx_limit_integration_test.cc b/test/integration/cx_limit_integration_test.cc index ac8b186b27b6e..ef668acfa2a94 100644 --- a/test/integration/cx_limit_integration_test.cc +++ b/test/integration/cx_limit_integration_test.cc @@ -39,14 +39,14 @@ class ConnectionLimitIntegrationTest : public testing::TestWithParammutable_listeners(0); - listener->mutable_limit_connections()->set_value(false); + listener->set_ignore_global_conn_limit(true); }); } void setAdminGlobalLimitOptOut() { config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { auto* admin = bootstrap.mutable_admin(); - admin->mutable_limit_connections()->set_value(false); + admin->set_ignore_global_conn_limit(true); }); } diff --git a/test/mocks/event/mocks.h b/test/mocks/event/mocks.h index f9ad32e256d77..a155a75e75d10 100644 --- a/test/mocks/event/mocks.h +++ b/test/mocks/event/mocks.h @@ -71,9 +71,9 @@ class MockDispatcher : public Dispatcher { Network::ListenerPtr createListener(Network::SocketSharedPtr&& socket, Network::TcpListenerCallbacks& cb, bool bind_to_port, - bool limit_connections) override { + bool ignore_global_conn_limit) override { return Network::ListenerPtr{ - createListener_(std::move(socket), cb, bind_to_port, limit_connections)}; + createListener_(std::move(socket), cb, bind_to_port, ignore_global_conn_limit)}; } Network::UdpListenerPtr @@ -139,7 +139,7 @@ class MockDispatcher : public Dispatcher { MOCK_METHOD(Filesystem::Watcher*, createFilesystemWatcher_, ()); MOCK_METHOD(Network::Listener*, createListener_, (Network::SocketSharedPtr && socket, Network::TcpListenerCallbacks& cb, - bool bind_to_port, bool limit_connections)); + bool bind_to_port, bool ignore_global_conn_limit)); MOCK_METHOD(Network::UdpListener*, createUdpListener_, (Network::SocketSharedPtr socket, Network::UdpListenerCallbacks& cb, const envoy::config::core::v3::UdpSocketConfig& config)); diff --git a/test/mocks/event/wrapped_dispatcher.h b/test/mocks/event/wrapped_dispatcher.h index c36705cd457b3..634235cbbd632 100644 --- a/test/mocks/event/wrapped_dispatcher.h +++ b/test/mocks/event/wrapped_dispatcher.h @@ -60,9 +60,9 @@ class WrappedDispatcher : public Dispatcher { } Network::ListenerPtr createListener(Network::SocketSharedPtr&& socket, - Network::TcpListenerCallbacks& cb, - bool bind_to_port) override { - return impl_.createListener(std::move(socket), cb, bind_to_port); + Network::TcpListenerCallbacks& cb, bool bind_to_port, + bool ignore_global_conn_limit) override { + return impl_.createListener(std::move(socket), cb, bind_to_port, ignore_global_conn_limit); } Network::UdpListenerPtr diff --git a/test/mocks/network/mocks.h b/test/mocks/network/mocks.h index 2839b511006f2..39a829e5772ec 100644 --- a/test/mocks/network/mocks.h +++ b/test/mocks/network/mocks.h @@ -432,7 +432,7 @@ class MockListenerConfig : public ListenerConfig { MOCK_METHOD(ResourceLimit&, openConnections, ()); MOCK_METHOD(uint32_t, tcpBacklogSize, (), (const)); MOCK_METHOD(Init::Manager&, initManager, ()); - MOCK_METHOD(bool, limitConnections, (), (const)); + MOCK_METHOD(bool, ignoreGlobalConnLimit, (), (const)); envoy::config::core::v3::TrafficDirection direction() const override { return envoy::config::core::v3::UNSPECIFIED; diff --git a/test/server/connection_handler_test.cc b/test/server/connection_handler_test.cc index 3438421941038..e7f2a362e180b 100644 --- a/test/server/connection_handler_test.cc +++ b/test/server/connection_handler_test.cc @@ -230,9 +230,9 @@ class ConnectionHandlerTest : public testing::Test, protected Logger::Loggablesocket_factory_, getListenSocket(_)) .WillOnce(Return(listeners_.back()->socket_)); if (socket_type == Network::Socket::Type::Stream) { - EXPECT_CALL(dispatcher_, createListener_(_, _, _)) + EXPECT_CALL(dispatcher_, createListener_(_, _, _, _)) .WillOnce(Invoke([listener, listener_callbacks](Network::SocketSharedPtr&&, - Network::TcpListenerCallbacks& cb, + Network::TcpListenerCallbacks& cb, bool, bool) -> Network::Listener* { if (listener_callbacks != nullptr) { *listener_callbacks = &cb; From 83351d107aa3e30a80bda835b9c3e8966f570d7c Mon Sep 17 00:00:00 2001 From: Weston Carlson Date: Wed, 3 Nov 2021 14:58:30 -0600 Subject: [PATCH 05/10] Fix admin related tests. Signed-off-by: Weston Carlson --- source/common/event/dispatcher_impl.cc | 5 +++-- source/common/event/dispatcher_impl.h | 2 +- source/common/network/tcp_listener_impl.h | 2 +- source/server/admin/admin.h | 3 ++- source/server/config_validation/dispatcher.h | 2 +- test/server/admin/admin_instance.cc | 2 +- test/server/admin/admin_test.cc | 4 ++-- test/server/admin/profiling_handler_test.cc | 2 +- 8 files changed, 12 insertions(+), 10 deletions(-) diff --git a/source/common/event/dispatcher_impl.cc b/source/common/event/dispatcher_impl.cc index 9f8e22ce883cb..a94b537ff45e7 100644 --- a/source/common/event/dispatcher_impl.cc +++ b/source/common/event/dispatcher_impl.cc @@ -171,10 +171,11 @@ Filesystem::WatcherPtr DispatcherImpl::createFilesystemWatcher() { Network::ListenerPtr DispatcherImpl::createListener(Network::SocketSharedPtr&& socket, Network::TcpListenerCallbacks& cb, - bool bind_to_port, bool limit_connections) { + bool bind_to_port, + bool ignore_global_conn_limit) { ASSERT(isThreadSafe()); return std::make_unique( - *this, api_.randomGenerator(), std::move(socket), cb, bind_to_port, limit_connections); + *this, api_.randomGenerator(), std::move(socket), cb, bind_to_port, ignore_global_conn_limit); } Network::UdpListenerPtr diff --git a/source/common/event/dispatcher_impl.h b/source/common/event/dispatcher_impl.h index 67a6ae5a3a265..068638b727252 100644 --- a/source/common/event/dispatcher_impl.h +++ b/source/common/event/dispatcher_impl.h @@ -71,7 +71,7 @@ class DispatcherImpl : Logger::Loggable, Filesystem::WatcherPtr createFilesystemWatcher() override; Network::ListenerPtr createListener(Network::SocketSharedPtr&& socket, Network::TcpListenerCallbacks& cb, bool bind_to_port, - bool limit_connections) override; + bool ignore_global_conn_limit) override; Network::UdpListenerPtr createUdpListener(Network::SocketSharedPtr socket, Network::UdpListenerCallbacks& cb, const envoy::config::core::v3::UdpSocketConfig& config) override; diff --git a/source/common/network/tcp_listener_impl.h b/source/common/network/tcp_listener_impl.h index 09f1b8530ec14..d8b06527dc977 100644 --- a/source/common/network/tcp_listener_impl.h +++ b/source/common/network/tcp_listener_impl.h @@ -18,7 +18,7 @@ class TcpListenerImpl : public BaseListenerImpl { public: TcpListenerImpl(Event::DispatcherImpl& dispatcher, Random::RandomGenerator& random, SocketSharedPtr socket, TcpListenerCallbacks& cb, bool bind_to_port, - bool limit_connections); + bool ignore_global_conn_limit); ~TcpListenerImpl() override { if (bind_to_port_) { socket_->ioHandle().resetFileEvents(); diff --git a/source/server/admin/admin.h b/source/server/admin/admin.h index acf694cc87780..f26f2ace38afa 100644 --- a/source/server/admin/admin.h +++ b/source/server/admin/admin.h @@ -68,7 +68,8 @@ class AdminImpl : public Admin, public Http::ConnectionManagerConfig, Logger::Loggable { public: - AdminImpl(const std::string& profile_path, Server::Instance& server, bool limit_connections); + AdminImpl(const std::string& profile_path, Server::Instance& server, + bool ignore_global_conn_limit); Http::Code runCallback(absl::string_view path_and_query, Http::ResponseHeaderMap& response_headers, Buffer::Instance& response, diff --git a/source/server/config_validation/dispatcher.h b/source/server/config_validation/dispatcher.h index 8902e3ffd2bad..0a280d0b98a50 100644 --- a/source/server/config_validation/dispatcher.h +++ b/source/server/config_validation/dispatcher.h @@ -24,7 +24,7 @@ class ValidationDispatcher : public DispatcherImpl { Network::Address::InstanceConstSharedPtr, Network::TransportSocketPtr&&, const Network::ConnectionSocket::OptionsSharedPtr& options) override; Network::ListenerPtr createListener(Network::SocketSharedPtr&&, Network::TcpListenerCallbacks&, - bool bind_to_port, bool limit_connections) override; + bool bind_to_port, bool ignore_global_conn_limit) override; }; } // namespace Event diff --git a/test/server/admin/admin_instance.cc b/test/server/admin/admin_instance.cc index d63b0887c7467..e05656d9b336d 100644 --- a/test/server/admin/admin_instance.cc +++ b/test/server/admin/admin_instance.cc @@ -9,7 +9,7 @@ namespace Server { AdminInstanceTest::AdminInstanceTest() : address_out_path_(TestEnvironment::temporaryPath("admin.address")), cpu_profile_path_(TestEnvironment::temporaryPath("envoy.prof")), - admin_(cpu_profile_path_, server_), request_headers_{{":path", "/"}}, + admin_(cpu_profile_path_, server_, false), request_headers_{{":path", "/"}}, admin_filter_(admin_.createCallbackFunction()) { std::list access_logs; Filesystem::FilePathAndType file_info{Filesystem::DestinationType::File, "/dev/null"}; diff --git a/test/server/admin/admin_test.cc b/test/server/admin/admin_test.cc index 8a0fce0464993..99b93afda1d4e 100644 --- a/test/server/admin/admin_test.cc +++ b/test/server/admin/admin_test.cc @@ -67,7 +67,7 @@ TEST_P(AdminInstanceTest, WriteAddressToFile) { TEST_P(AdminInstanceTest, AdminAddress) { std::string address_out_path = TestEnvironment::temporaryPath("admin.address"); - AdminImpl admin_address_out_path(cpu_profile_path_, server_); + AdminImpl admin_address_out_path(cpu_profile_path_, server_, false); std::list access_logs; Filesystem::FilePathAndType file_info{Filesystem::DestinationType::File, "/dev/null"}; access_logs.emplace_back(new Extensions::AccessLoggers::File::FileAccessLog( @@ -82,7 +82,7 @@ TEST_P(AdminInstanceTest, AdminAddress) { TEST_P(AdminInstanceTest, AdminBadAddressOutPath) { std::string bad_path = TestEnvironment::temporaryPath("some/unlikely/bad/path/admin.address"); - AdminImpl admin_bad_address_out_path(cpu_profile_path_, server_); + AdminImpl admin_bad_address_out_path(cpu_profile_path_, server_, false); std::list access_logs; Filesystem::FilePathAndType file_info{Filesystem::DestinationType::File, "/dev/null"}; access_logs.emplace_back(new Extensions::AccessLoggers::File::FileAccessLog( diff --git a/test/server/admin/profiling_handler_test.cc b/test/server/admin/profiling_handler_test.cc index 709675e00a11f..aefa0a6b10d7a 100644 --- a/test/server/admin/profiling_handler_test.cc +++ b/test/server/admin/profiling_handler_test.cc @@ -67,7 +67,7 @@ TEST_P(AdminInstanceTest, AdminHeapProfiler) { TEST_P(AdminInstanceTest, AdminBadProfiler) { Buffer::OwnedImpl data; AdminImpl admin_bad_profile_path(TestEnvironment::temporaryPath("some/unlikely/bad/path.prof"), - server_); + server_, false); Http::TestResponseHeaderMapImpl header_map; const absl::string_view post = Http::Headers::get().MethodValues.Post; request_headers_.setMethod(post); From 3a5311e8e5e2b10b84168e139765e5ebd7cb80ad Mon Sep 17 00:00:00 2001 From: Weston Carlson Date: Wed, 3 Nov 2021 18:00:55 -0600 Subject: [PATCH 06/10] Update func doc string. Signed-off-by: Weston Carlson --- envoy/event/dispatcher.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/envoy/event/dispatcher.h b/envoy/event/dispatcher.h index 40715e74daa83..42516a1a69569 100644 --- a/envoy/event/dispatcher.h +++ b/envoy/event/dispatcher.h @@ -228,6 +228,8 @@ class Dispatcher : public DispatcherBase, public ScopeTracker { * @param socket supplies the socket to listen on. * @param cb supplies the callbacks to invoke for listener events. * @param bind_to_port controls whether the listener binds to a transport port or not. + * @param ignore_global_conn_limit controls whether the listener is limitted by the global + * connection limit * @return Network::ListenerPtr a new listener that is owned by the caller. */ virtual Network::ListenerPtr createListener(Network::SocketSharedPtr&& socket, From 377b1e974a5a0f9a0783ef8d48b536a6d54cc154 Mon Sep 17 00:00:00 2001 From: Weston Carlson Date: Fri, 5 Nov 2021 02:51:00 +0000 Subject: [PATCH 07/10] Improve docs. Signed-off-by: Weston Carlson --- api/envoy/config/bootstrap/v3/bootstrap.proto | 2 +- api/envoy/config/listener/v3/listener.proto | 2 +- .../operations/overload_manager/overload_manager.rst | 10 +++++++--- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/api/envoy/config/bootstrap/v3/bootstrap.proto b/api/envoy/config/bootstrap/v3/bootstrap.proto index 13c436bd2b4bc..d056ba7469550 100644 --- a/api/envoy/config/bootstrap/v3/bootstrap.proto +++ b/api/envoy/config/bootstrap/v3/bootstrap.proto @@ -356,7 +356,7 @@ message Admin { // precompiled binaries. repeated core.v3.SocketOption socket_options = 4; - // Indicates whether `global_downstream_max_connections `_ + // Indicates whether :ref:`global_downstream_max_connections ` // should apply to the admin interface or not. bool ignore_global_conn_limit = 6; } diff --git a/api/envoy/config/listener/v3/listener.proto b/api/envoy/config/listener/v3/listener.proto index 8df51a4f5b49b..f189cd1993eef 100644 --- a/api/envoy/config/listener/v3/listener.proto +++ b/api/envoy/config/listener/v3/listener.proto @@ -320,6 +320,6 @@ message Listener { bool enable_mptcp = 30; // Whether the listener should limit connections based upon the value of - // `global_downstream_max_connections `_ + // :ref:`global_downstream_max_connections ` bool ignore_global_conn_limit = 31; } diff --git a/docs/root/configuration/operations/overload_manager/overload_manager.rst b/docs/root/configuration/operations/overload_manager/overload_manager.rst index ffc0b75041f7f..562b27833f9c7 100644 --- a/docs/root/configuration/operations/overload_manager/overload_manager.rst +++ b/docs/root/configuration/operations/overload_manager/overload_manager.rst @@ -143,6 +143,8 @@ Note in the example that the minimum idle time is specified as an absolute durat would be computed based on the maximum (specified elsewhere). So if ``idle_timeout`` is again 600 seconds, then the minimum timer value would be :math:`10\% \cdot 600s = 60s`. +.. _config_overload_manager_limiting_connections: + Limiting Active Connections --------------------------- @@ -155,10 +157,12 @@ If the value is unspecified, there is no global limit on the number of active do and Envoy will emit a warning indicating this at startup. To disable the warning without setting a limit on the number of active downstream connections, the runtime value may be set to a very large limit (~2e9). -Listeners (including the admin listener) can opt out of this global connection limit by setting +Listeners can opt out of this global connection limit by setting :ref:`Listener.ignore_global_conn_limit ` -to true. You may want to opt out a listener to be able to probe or collect stats from Envoy while it -is otherwise at its connection limit. +to true. Similarly, you can opt out the admin listener by setting +:ref:`Admin.ignore_global_conn_limit `. +You may want to opt out a listener to be able to probe Envoy or collect stats while it is otherwise at its +connection limit. If it is desired to only limit the number of downstream connections for a particular listener, per-listener limits can be set via the :ref:`listener configuration `. From 906e73fbd4d4c89e142f66c3b816469b6805878c Mon Sep 17 00:00:00 2001 From: Weston Carlson Date: Tue, 9 Nov 2021 23:41:25 +0000 Subject: [PATCH 08/10] Address doc related PR feedback. Signed-off-by: Weston Carlson --- api/envoy/config/listener/v3/listener.proto | 2 +- envoy/event/dispatcher.h | 4 ++-- envoy/network/listener.h | 4 ++-- envoy/server/configuration.h | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/api/envoy/config/listener/v3/listener.proto b/api/envoy/config/listener/v3/listener.proto index f189cd1993eef..a207875cae237 100644 --- a/api/envoy/config/listener/v3/listener.proto +++ b/api/envoy/config/listener/v3/listener.proto @@ -320,6 +320,6 @@ message Listener { bool enable_mptcp = 30; // Whether the listener should limit connections based upon the value of - // :ref:`global_downstream_max_connections ` + // :ref:`global_downstream_max_connections `. bool ignore_global_conn_limit = 31; } diff --git a/envoy/event/dispatcher.h b/envoy/event/dispatcher.h index 42516a1a69569..86fd2855a1512 100644 --- a/envoy/event/dispatcher.h +++ b/envoy/event/dispatcher.h @@ -228,8 +228,8 @@ class Dispatcher : public DispatcherBase, public ScopeTracker { * @param socket supplies the socket to listen on. * @param cb supplies the callbacks to invoke for listener events. * @param bind_to_port controls whether the listener binds to a transport port or not. - * @param ignore_global_conn_limit controls whether the listener is limitted by the global - * connection limit + * @param ignore_global_conn_limit controls whether the listener is limited by the global + * connection limit. * @return Network::ListenerPtr a new listener that is owned by the caller. */ virtual Network::ListenerPtr createListener(Network::SocketSharedPtr&& socket, diff --git a/envoy/network/listener.h b/envoy/network/listener.h index 3f400712a5a52..10136e9f97a08 100644 --- a/envoy/network/listener.h +++ b/envoy/network/listener.h @@ -217,8 +217,8 @@ class ListenerConfig { virtual Init::Manager& initManager() PURE; /** - * @return bool whether the listener should block connections or not based on the globally set - * limit + * @return bool whether the listener should avoid blocking connections based on the globally set + * limit. */ virtual bool ignoreGlobalConnLimit() const PURE; }; diff --git a/envoy/server/configuration.h b/envoy/server/configuration.h index 4fc50cbed7fc6..32c7e3f661b17 100644 --- a/envoy/server/configuration.h +++ b/envoy/server/configuration.h @@ -143,8 +143,8 @@ class Admin { virtual Network::Socket::OptionsSharedPtr socketOptions() PURE; /** - * @return bool whether the listener should block connections or not based on the globally set - * limit + * @return bool whether the listener should avoid blocking connections based on the globally set + * limit. */ virtual bool ignoreGlobalConnLimit() const PURE; }; From a6962196be050e953351f51390a0d42bff3966e9 Mon Sep 17 00:00:00 2001 From: Weston Carlson Date: Mon, 15 Nov 2021 15:49:33 +0000 Subject: [PATCH 09/10] Make field const. Cleanup tests. Signed-off-by: Weston Carlson --- .../operations/overload_manager/overload_manager.rst | 3 ++- source/common/network/tcp_listener_impl.h | 2 +- test/integration/cx_limit_integration_test.cc | 9 --------- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/docs/root/configuration/operations/overload_manager/overload_manager.rst b/docs/root/configuration/operations/overload_manager/overload_manager.rst index 562b27833f9c7..48f5aa9499da3 100644 --- a/docs/root/configuration/operations/overload_manager/overload_manager.rst +++ b/docs/root/configuration/operations/overload_manager/overload_manager.rst @@ -162,7 +162,8 @@ Listeners can opt out of this global connection limit by setting to true. Similarly, you can opt out the admin listener by setting :ref:`Admin.ignore_global_conn_limit `. You may want to opt out a listener to be able to probe Envoy or collect stats while it is otherwise at its -connection limit. +connection limit. Note that connections to listeners that opt out are still tracked and count towards the +global limit. If it is desired to only limit the number of downstream connections for a particular listener, per-listener limits can be set via the :ref:`listener configuration `. diff --git a/source/common/network/tcp_listener_impl.h b/source/common/network/tcp_listener_impl.h index d8b06527dc977..f9978b73543ef 100644 --- a/source/common/network/tcp_listener_impl.h +++ b/source/common/network/tcp_listener_impl.h @@ -43,7 +43,7 @@ class TcpListenerImpl : public BaseListenerImpl { Random::RandomGenerator& random_; bool bind_to_port_; UnitFloat reject_fraction_; - bool ignore_global_conn_limit_; + const bool ignore_global_conn_limit_; }; } // namespace Network diff --git a/test/integration/cx_limit_integration_test.cc b/test/integration/cx_limit_integration_test.cc index ef668acfa2a94..1be10dcd5c97f 100644 --- a/test/integration/cx_limit_integration_test.cc +++ b/test/integration/cx_limit_integration_test.cc @@ -110,9 +110,6 @@ class ConnectionLimitIntegrationTest : public testing::TestWithParamconnected()); const bool isV4 = (version_ == Network::Address::IpVersion::v4); - auto local_address = isV4 ? Network::Utility::getCanonicalIpv4LoopbackAddress() - : Network::Utility::getIpv6LoopbackAddress(); - const std::string counter_prefix = (isV4 ? "listener.127.0.0.1_0." : "listener.[__1]_0."); test_server_->waitForCounterEq(counter_prefix + check_stat, 1); @@ -224,9 +221,6 @@ TEST_P(ConnectionLimitIntegrationTest, TestGlobalLimitOptOut) { ASSERT_TRUE(tcp_clients.back()->connected()); const bool isV4 = (version_ == Network::Address::IpVersion::v4); - auto local_address = isV4 ? Network::Utility::getCanonicalIpv4LoopbackAddress() - : Network::Utility::getIpv6LoopbackAddress(); - const std::string counter_prefix = (isV4 ? "listener.127.0.0.1_0." : "listener.[__1]_0."); // listener_0 does not hit any connection limits @@ -279,9 +273,6 @@ TEST_P(ConnectionLimitIntegrationTest, TestListenerLimitWithGlobalOptOut) { tcp_clients.pop_back(); const bool isV4 = (version_ == Network::Address::IpVersion::v4); - auto local_address = isV4 ? Network::Utility::getCanonicalIpv4LoopbackAddress() - : Network::Utility::getIpv6LoopbackAddress(); - const std::string counter_prefix = (isV4 ? "listener.127.0.0.1_0." : "listener.[__1]_0."); // listener_0 does hits the listener connection limit From 9036f88a810ef315a2259b7334239a0c8be1f9de Mon Sep 17 00:00:00 2001 From: Weston Carlson Date: Mon, 15 Nov 2021 19:15:26 +0000 Subject: [PATCH 10/10] Kick CI Signed-off-by: Weston Carlson