diff --git a/changelogs/current/minor_behavior_changes/happy_eyeballs__sort-address-list-once.rst b/changelogs/current/minor_behavior_changes/happy_eyeballs__sort-address-list-once.rst new file mode 100644 index 0000000000000..0081d98f1adbb --- /dev/null +++ b/changelogs/current/minor_behavior_changes/happy_eyeballs__sort-address-list-once.rst @@ -0,0 +1,3 @@ +The happy eyeballs sorting of a multi-address host's address list now happens once when the +address list is created or refreshed, instead of on every upstream connection attempt. The order +in which connection attempts are made is unchanged. diff --git a/source/common/network/happy_eyeballs_connection_impl.cc b/source/common/network/happy_eyeballs_connection_impl.cc index 6785ab546ff4c..4bc66c2f7754e 100644 --- a/source/common/network/happy_eyeballs_connection_impl.cc +++ b/source/common/network/happy_eyeballs_connection_impl.cc @@ -10,22 +10,23 @@ namespace Envoy { namespace Network { HappyEyeballsConnectionProvider::HappyEyeballsConnectionProvider( - Event::Dispatcher& dispatcher, const std::vector& address_list, + Event::Dispatcher& dispatcher, + const Upstream::HostDescription::SharedConstAddressVector& sorted_address_list, const std::shared_ptr& upstream_local_address_selector, UpstreamTransportSocketFactory& socket_factory, TransportSocketOptionsConstSharedPtr transport_socket_options, const Upstream::HostDescriptionConstSharedPtr& host, - const ConnectionSocket::OptionsSharedPtr options, - const envoy::config::cluster::v3::UpstreamConnectionOptions::HappyEyeballsConfig& - happy_eyeballs_config) - : dispatcher_(dispatcher), address_list_(sortAddresses(address_list, happy_eyeballs_config)), + const ConnectionSocket::OptionsSharedPtr options) + : dispatcher_(dispatcher), address_list_(sorted_address_list), upstream_local_address_selector_(upstream_local_address_selector), socket_factory_(socket_factory), transport_socket_options_(transport_socket_options), - host_(host), options_(options) {} + host_(host), options_(options) { + ASSERT(address_list_ != nullptr && !address_list_->empty()); +} bool HappyEyeballsConnectionProvider::hasNextConnection() { - return next_address_ < address_list_.size(); + return next_address_ < address_list_->size(); } ClientConnectionPtr HappyEyeballsConnectionProvider::createNextConnection(const uint64_t id) { @@ -37,8 +38,8 @@ ClientConnectionPtr HappyEyeballsConnectionProvider::createNextConnection(const first_connection_created_ = true; ASSERT(hasNextConnection()); ENVOY_LOG_EVENT(debug, "happy_eyeballs_cx_attempt", "C[{}] address={}", id, - address_list_[next_address_]->asStringView()); - auto& address = address_list_[next_address_++]; + (*address_list_)[next_address_]->asStringView()); + auto& address = (*address_list_)[next_address_++]; auto upstream_local_address = upstream_local_address_selector_->getUpstreamLocalAddress( address, options_, makeOptRefFromPtr(transport_socket_options_.get())); @@ -50,7 +51,7 @@ ClientConnectionPtr HappyEyeballsConnectionProvider::createNextConnection(const size_t HappyEyeballsConnectionProvider::nextConnection() { return next_address_; } -size_t HappyEyeballsConnectionProvider::totalConnections() { return address_list_.size(); } +size_t HappyEyeballsConnectionProvider::totalConnections() { return address_list_->size(); } namespace { diff --git a/source/common/network/happy_eyeballs_connection_impl.h b/source/common/network/happy_eyeballs_connection_impl.h index 05758d290cf80..425981d36eb2a 100644 --- a/source/common/network/happy_eyeballs_connection_impl.h +++ b/source/common/network/happy_eyeballs_connection_impl.h @@ -12,22 +12,22 @@ namespace Network { /** * Implementation of ConnectionProvider for HappyEyeballs. It provides client * connections to multiple addresses in an specific order complying to - * HappyEyeballs. + * HappyEyeballs. The address list passed to the constructor must already be + * sorted with sortAddresses(); the host computes this once when its address + * list is created or refreshed rather than on every connection attempt. */ class HappyEyeballsConnectionProvider : public ConnectionProvider, Logger::Loggable { public: HappyEyeballsConnectionProvider( Event::Dispatcher& dispatcher, - const std::vector& address_list, + const Upstream::HostDescription::SharedConstAddressVector& sorted_address_list, const std::shared_ptr& upstream_local_address_selector, UpstreamTransportSocketFactory& socket_factory, TransportSocketOptionsConstSharedPtr transport_socket_options, const Upstream::HostDescriptionConstSharedPtr& host, - const ConnectionSocket::OptionsSharedPtr options, - const envoy::config::cluster::v3::UpstreamConnectionOptions::HappyEyeballsConfig& - happy_eyeballs_config); + const ConnectionSocket::OptionsSharedPtr options); bool hasNextConnection() override; ClientConnectionPtr createNextConnection(const uint64_t id) override; size_t nextConnection() override; @@ -44,8 +44,8 @@ class HappyEyeballsConnectionProvider : public ConnectionProvider, private: Event::Dispatcher& dispatcher_; - // List of addresses to attempt to connect to. - const std::vector address_list_; + // List of addresses to attempt to connect to, pre-sorted with sortAddresses(). + const Upstream::HostDescription::SharedConstAddressVector address_list_; const Upstream::UpstreamLocalAddressSelectorConstSharedPtr upstream_local_address_selector_; UpstreamTransportSocketFactory& socket_factory_; TransportSocketOptionsConstSharedPtr transport_socket_options_; @@ -69,28 +69,26 @@ class HappyEyeballsConnectionProvider : public ConnectionProvider, * they are applied to each open connection and applied when creating new ones. * * See the Happy Eyeballs RFC at https://datatracker.ietf.org/doc/html/rfc6555 - * TODO(RyanTheOptimist): Implement the Happy Eyeballs address sorting algorithm - * either in the class or in the resolution code. + * The address list must already be sorted with + * HappyEyeballsConnectionProvider::sortAddresses(), which the host does once + * when the address list is created or refreshed. */ class HappyEyeballsConnectionImpl : public MultiConnectionBaseImpl, Logger::Loggable { public: HappyEyeballsConnectionImpl( Event::Dispatcher& dispatcher, - const std::vector& address_list, + const Upstream::HostDescription::SharedConstAddressVector& sorted_address_list, const std::shared_ptr& upstream_local_address_selector, UpstreamTransportSocketFactory& socket_factory, TransportSocketOptionsConstSharedPtr transport_socket_options, const Upstream::HostDescriptionConstSharedPtr& host, - const ConnectionSocket::OptionsSharedPtr options, - const envoy::config::cluster::v3::UpstreamConnectionOptions::HappyEyeballsConfig& - happy_eyeballs_config) - : MultiConnectionBaseImpl(dispatcher, - std::make_unique( - dispatcher, address_list, upstream_local_address_selector, - socket_factory, transport_socket_options, host, options, - happy_eyeballs_config)) {} + const ConnectionSocket::OptionsSharedPtr options) + : MultiConnectionBaseImpl( + dispatcher, std::make_unique( + dispatcher, sorted_address_list, upstream_local_address_selector, + socket_factory, transport_socket_options, host, options)) {} }; } // namespace Network diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 763fd02e58b2b..96e06b2efc715 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -519,6 +519,7 @@ HostDescriptionImpl::HostDescriptionImpl( uint32_t priority, const AddressVector& address_list, absl::string_view stat_name) : HostDescriptionImplBase(cluster, hostname, dest_address, endpoint_metadata, locality_metadata, locality, health_check_config, priority, creation_status), + sorted_address_list_or_null_(makeSortedAddressListOrNull(*cluster, address_list)), address_(dest_address), address_list_or_null_(makeAddressListOrNull(dest_address, address_list)), health_check_address_(resolveHealthCheckAddress(health_check_config, dest_address)), @@ -565,6 +566,20 @@ HostDescription::SharedConstAddressVector HostDescriptionImplBase::makeAddressLi return std::make_shared(address_list); } +HostDescription::SharedConstAddressVector +HostDescriptionImplBase::makeSortedAddressListOrNull(const ClusterInfo& cluster, + const AddressVector& address_list) { + if (address_list.size() <= 1) { + return {}; + } + const envoy::config::cluster::v3::UpstreamConnectionOptions::HappyEyeballsConfig& + happy_eyeballs_config = + cluster.happyEyeballsConfig().has_value() ? *cluster.happyEyeballsConfig() + : defaultHappyEyeballsConfig(); + return std::make_shared( + Network::HappyEyeballsConnectionProvider::sortAddresses(address_list, happy_eyeballs_config)); +} + Network::UpstreamTransportSocketFactory& HostDescriptionImplBase::resolveTransportSocketFactory( const Network::Address::InstanceConstSharedPtr& dest_address, const envoy::config::core::v3::Metadata* endpoint_metadata, @@ -590,8 +605,8 @@ Host::CreateConnectionData HostImplBase::createConnection( ? resolveTransportSocketFactory(address(), metadata().get(), transport_socket_options) : transportSocketFactory(); - return createConnection(dispatcher, cluster(), address(), addressListOrNull(), factory, options, - transport_socket_options, shared_from_this()); + return createConnection(dispatcher, cluster(), address(), sortedAddressListOrNull(), factory, + options, transport_socket_options, shared_from_this()); } void HostImplBase::setEdsHealthFlag(envoy::config::core::v3::HealthStatus health_status) { @@ -637,7 +652,7 @@ Host::CreateConnectionData HostImplBase::createOrcaReportingConnection( Network::UpstreamTransportSocketFactory& factory, Network::Address::InstanceConstSharedPtr orca_address) const { return createOrcaConnection(dispatcher, std::move(transport_socket_options), factory, - std::move(orca_address), address(), addressListOrNull(), + std::move(orca_address), address(), sortedAddressListOrNull(), shared_from_this()); } @@ -647,13 +662,13 @@ Host::CreateConnectionData HostImplBase::createOrcaConnection( Network::UpstreamTransportSocketFactory& factory, Network::Address::InstanceConstSharedPtr orca_address, const Network::Address::InstanceConstSharedPtr& host_address, - const SharedConstAddressVector& address_list, HostDescriptionConstSharedPtr host) const { + const SharedConstAddressVector& sorted_address_list, HostDescriptionConstSharedPtr host) const { // The original-port address list applies only when dialing the host's own address. Compare // by value: pointer identity doesn't survive LogicalHost re-resolution. const bool use_address_list = *orca_address == *host_address; return createConnection(dispatcher, cluster(), orca_address, - use_address_list ? address_list : SharedConstAddressVector{}, factory, - /*options=*/nullptr, transport_socket_options, std::move(host)); + use_address_list ? sorted_address_list : SharedConstAddressVector{}, + factory, /*options=*/nullptr, transport_socket_options, std::move(host)); } std::optional HostImplBase::maybeGetProxyRedirectAddress( @@ -713,7 +728,7 @@ std::optional HostImplBase::maybeGetPr Host::CreateConnectionData HostImplBase::createConnection( Event::Dispatcher& dispatcher, const ClusterInfo& cluster, const Network::Address::InstanceConstSharedPtr& address, - const SharedConstAddressVector& address_list_or_null, + const SharedConstAddressVector& sorted_address_list, Network::UpstreamTransportSocketFactory& socket_factory, const Network::ConnectionSocket::OptionsSharedPtr& options, Network::TransportSocketOptionsConstSharedPtr transport_socket_options, @@ -736,15 +751,11 @@ Host::CreateConnectionData HostImplBase::createConnection( proxy_address.value(), upstream_local_address.address_, socket_factory.createTransportSocket(transport_socket_options, host), upstream_local_address.socket_options_, transport_socket_options); - } else if (address_list_or_null != nullptr && address_list_or_null->size() > 1) { + } else if (sorted_address_list != nullptr && sorted_address_list->size() > 1) { ENVOY_LOG(debug, "Upstream using happy eyeballs config."); - const envoy::config::cluster::v3::UpstreamConnectionOptions::HappyEyeballsConfig& - happy_eyeballs_config = - cluster.happyEyeballsConfig().has_value() ? *cluster.happyEyeballsConfig() - : defaultHappyEyeballsConfig(); connection = std::make_unique( - dispatcher, *address_list_or_null, source_address_selector, socket_factory, - transport_socket_options, host, options, happy_eyeballs_config); + dispatcher, sorted_address_list, source_address_selector, socket_factory, + transport_socket_options, host, options); } else { auto upstream_local_address = source_address_selector->getUpstreamLocalAddress( address, options, makeOptRefFromPtr(transport_socket_options.get())); diff --git a/source/common/upstream/upstream_impl.h b/source/common/upstream/upstream_impl.h index d253f0794c6f7..9afe2243da32e 100644 --- a/source/common/upstream/upstream_impl.h +++ b/source/common/upstream/upstream_impl.h @@ -279,6 +279,17 @@ class HostDescriptionImplBase : virtual public HostDescription, makeAddressListOrNull(const Network::Address::InstanceConstSharedPtr& address, const AddressVector& address_list); + /** + * @return nullptr if address_list has fewer than 2 addresses (happy eyeballs does not + * apply), otherwise a shared_ptr to a copy of address_list sorted with + * Network::HappyEyeballsConnectionProvider::sortAddresses() using the cluster's happy + * eyeballs config, or the default config if the cluster does not specify one. This is + * computed once when the address list is created or refreshed so that connection + * attempts do not re-sort it. + */ + static SharedConstAddressVector makeSortedAddressListOrNull(const ClusterInfo& cluster, + const AddressVector& address_list); + private: ClusterInfoConstSharedPtr cluster_; const std::string hostname_; @@ -347,6 +358,11 @@ class HostDescriptionImpl : public HostDescriptionImplBase { const envoy::config::endpoint::v3::Endpoint::HealthCheckConfig& health_check_config, uint32_t priority, const AddressVector& address_list = {}, absl::string_view stat_name = {}); + // Happy eyeballs sorted copy of the address list, or nullptr if the host does not have + // multiple addresses. Set at construction and never changed; read by + // HostImpl::sortedAddressListOrNull(). + const SharedConstAddressVector sorted_address_list_or_null_; + private: // No locks are required in this implementation: all address-related member // variables are set at construction and never change. See @@ -478,10 +494,17 @@ class HostImplBase : public Host, } protected: + /** + * @return the address list sorted for happy eyeballs connection attempts, or nullptr if + * the host does not have multiple addresses. The list is computed once when the address + * list is created or refreshed rather than on every connection attempt. + */ + virtual SharedConstAddressVector sortedAddressListOrNull() const PURE; + static CreateConnectionData createConnection(Event::Dispatcher& dispatcher, const ClusterInfo& cluster, const Network::Address::InstanceConstSharedPtr& address, - const SharedConstAddressVector& address_list, + const SharedConstAddressVector& sorted_address_list, Network::UpstreamTransportSocketFactory& socket_factory, const Network::ConnectionSocket::OptionsSharedPtr& options, Network::TransportSocketOptionsConstSharedPtr transport_socket_options, @@ -497,7 +520,7 @@ class HostImplBase : public Host, Network::UpstreamTransportSocketFactory& factory, Network::Address::InstanceConstSharedPtr orca_address, const Network::Address::InstanceConstSharedPtr& host_address, - const SharedConstAddressVector& address_list, + const SharedConstAddressVector& sorted_address_list, HostDescriptionConstSharedPtr host) const; private: @@ -556,6 +579,11 @@ class HostImpl : public HostImplBase, public HostDescriptionImpl { HostDescriptionImpl(creation_status, cluster, hostname, address, endpoint_metadata, locality_metadata, locality, health_check_config, priority, address_list, stat_name) {} + + // Upstream::HostImplBase + SharedConstAddressVector sortedAddressListOrNull() const override { + return sorted_address_list_or_null_; + } }; class HostsPerLocalityImpl : public HostsPerLocality { diff --git a/source/extensions/clusters/common/logical_host.cc b/source/extensions/clusters/common/logical_host.cc index e8d76fef0ad3e..284c6a3172d1e 100644 --- a/source/extensions/clusters/common/logical_host.cc +++ b/source/extensions/clusters/common/logical_host.cc @@ -38,7 +38,8 @@ LogicalHost::LogicalHost( lb_endpoint.endpoint().health_check_config(), locality_lb_endpoint.priority(), creation_status), override_transport_socket_options_(override_transport_socket_options), address_(address), - address_list_or_null_(makeAddressListOrNull(address, address_list)) { + address_list_or_null_(makeAddressListOrNull(address, address_list)), + sorted_address_list_or_null_(makeSortedAddressListOrNull(*cluster, address_list)) { health_check_address_ = resolveHealthCheckAddress(lb_endpoint.endpoint().health_check_config(), address); } @@ -59,10 +60,13 @@ void LogicalHost::setNewAddresses(const Network::Address::InstanceConstSharedPtr shared_address_list = std::make_shared(address_list); ASSERT(*address_list.front() == *address); } + SharedConstAddressVector sorted_address_list = + makeSortedAddressListOrNull(cluster(), address_list); { absl::MutexLock lock(address_lock_); address_ = address; address_list_or_null_ = std::move(shared_address_list); + sorted_address_list_or_null_ = std::move(sorted_address_list); health_check_address_ = std::move(health_check_address); } } @@ -72,6 +76,11 @@ HostDescription::SharedConstAddressVector LogicalHost::addressListOrNull() const return address_list_or_null_; } +HostDescription::SharedConstAddressVector LogicalHost::sortedAddressListOrNull() const { + absl::MutexLock lock(address_lock_); + return sorted_address_list_or_null_; +} + Network::Address::InstanceConstSharedPtr LogicalHost::address() const { absl::MutexLock lock(address_lock_); return address_; @@ -86,11 +95,11 @@ Upstream::Host::CreateConnectionData LogicalHost::createConnection( Event::Dispatcher& dispatcher, const Network::ConnectionSocket::OptionsSharedPtr& options, Network::TransportSocketOptionsConstSharedPtr transport_socket_options) const { Network::Address::InstanceConstSharedPtr address; - SharedConstAddressVector address_list_or_null; + SharedConstAddressVector sorted_address_list_or_null; { absl::MutexLock lock(address_lock_); address = address_; - address_list_or_null = address_list_or_null_; + sorted_address_list_or_null = sorted_address_list_or_null_; } // Use override_transport_socket_options if set, otherwise use the passed options. @@ -109,8 +118,8 @@ Upstream::Host::CreateConnectionData LogicalHost::createConnection( : transportSocketFactory(); return HostImplBase::createConnection( - dispatcher, cluster(), address, address_list_or_null, factory, options, effective_options, - std::make_shared(address, shared_from_this())); + dispatcher, cluster(), address, sorted_address_list_or_null, factory, options, + effective_options, std::make_shared(address, shared_from_this())); } Upstream::Host::CreateConnectionData LogicalHost::createOrcaReportingConnection( @@ -119,17 +128,17 @@ Upstream::Host::CreateConnectionData LogicalHost::createOrcaReportingConnection( Network::UpstreamTransportSocketFactory& factory, Network::Address::InstanceConstSharedPtr orca_address) const { Network::Address::InstanceConstSharedPtr host_address; - SharedConstAddressVector address_list_or_null; + SharedConstAddressVector sorted_address_list_or_null; { absl::MutexLock lock(address_lock_); host_address = address_; - address_list_or_null = address_list_or_null_; + sorted_address_list_or_null = sorted_address_list_or_null_; } // The caller's options pass through unchanged; as with health checks, // override_transport_socket_options_ is not consulted here. return createOrcaConnection( dispatcher, transport_socket_options, factory, orca_address, host_address, - address_list_or_null, + sorted_address_list_or_null, std::make_shared(orca_address, shared_from_this())); } diff --git a/source/extensions/clusters/common/logical_host.h b/source/extensions/clusters/common/logical_host.h index 97640bd43cff3..56f16caf65dfc 100644 --- a/source/extensions/clusters/common/logical_host.h +++ b/source/extensions/clusters/common/logical_host.h @@ -59,6 +59,11 @@ class LogicalHost : public HostImplBase, public HostDescriptionImplBase { absl::string_view observabilityName() const override { return {}; } Network::Address::InstanceConstSharedPtr orcaReportingAddress() const override; + // Upstream::HostImplBase + // Public (protected in the base class) so that tests can verify the sorted list is + // kept in sync with the raw list across address refreshes. + SharedConstAddressVector sortedAddressListOrNull() const override; + protected: LogicalHost( const ClusterInfoConstSharedPtr& cluster, const std::string& hostname, @@ -74,6 +79,9 @@ class LogicalHost : public HostImplBase, public HostDescriptionImplBase { // The first entry in the address_list_ should match the value in address_. Network::Address::InstanceConstSharedPtr address_ ABSL_GUARDED_BY(address_lock_); SharedConstAddressVector address_list_or_null_ ABSL_GUARDED_BY(address_lock_); + // Happy eyeballs sorted copy of address_list_or_null_, updated together with it so that + // the raw and sorted lists stay consistent. nullptr unless there are multiple addresses. + SharedConstAddressVector sorted_address_list_or_null_ ABSL_GUARDED_BY(address_lock_); Network::Address::InstanceConstSharedPtr health_check_address_ ABSL_GUARDED_BY(address_lock_); mutable absl::Mutex address_lock_; }; diff --git a/test/common/network/BUILD b/test/common/network/BUILD index b909fb7485f19..bed4b6fe9bd48 100644 --- a/test/common/network/BUILD +++ b/test/common/network/BUILD @@ -133,6 +133,22 @@ envoy_cc_test( ], ) +envoy_cc_benchmark_binary( + name = "happy_eyeballs_speed_test", + srcs = ["happy_eyeballs_speed_test.cc"], + rbe_pool = "6gig", + deps = [ + "//source/common/network:address_lib", + "//source/common/network:happy_eyeballs_connection_impl_lib", + "@benchmark", + ], +) + +envoy_benchmark_test( + name = "happy_eyeballs_speed_test_benchmark_test", + benchmark_binary = "happy_eyeballs_speed_test", +) + envoy_cc_test( name = "multi_connection_base_impl_test", srcs = ["multi_connection_base_impl_test.cc"], diff --git a/test/common/network/happy_eyeballs_speed_test.cc b/test/common/network/happy_eyeballs_speed_test.cc new file mode 100644 index 0000000000000..6287f8dd7a042 --- /dev/null +++ b/test/common/network/happy_eyeballs_speed_test.cc @@ -0,0 +1,38 @@ +// Measures the cost of sorting an upstream address list with address families +// interleaved as per RFC 8305 (happy eyeballs v2). This sort used to run on +// every upstream connection attempt; it now runs once when a host's address +// list is created or refreshed. + +#include "source/common/common/fmt.h" +#include "source/common/network/address_impl.h" +#include "source/common/network/happy_eyeballs_connection_impl.h" + +#include "benchmark/benchmark.h" + +namespace Envoy { +namespace Network { + +static void happyEyeballsSortAddresses(benchmark::State& state) { + const uint64_t num_addresses = state.range(0); + std::vector address_list; + address_list.reserve(num_addresses); + // Build a dual family list interleaving IPv6 and IPv4 addresses. + for (uint64_t i = 0; i < num_addresses; i++) { + if (i % 2 == 0) { + address_list.push_back( + std::make_shared(fmt::format("2001:db8::{}", i + 1), 443)); + } else { + address_list.push_back( + std::make_shared(fmt::format("10.0.0.{}", i + 1))); + } + } + envoy::config::cluster::v3::UpstreamConnectionOptions::HappyEyeballsConfig config; + for (auto _ : state) { + UNREFERENCED_PARAMETER(_); + benchmark::DoNotOptimize(HappyEyeballsConnectionProvider::sortAddresses(address_list, config)); + } +} +BENCHMARK(happyEyeballsSortAddresses)->Arg(2)->Arg(8)->Arg(16); + +} // namespace Network +} // namespace Envoy diff --git a/test/common/upstream/upstream_impl_test.cc b/test/common/upstream/upstream_impl_test.cc index d825e639cbf43..2fc4f6186ae4c 100644 --- a/test/common/upstream/upstream_impl_test.cc +++ b/test/common/upstream/upstream_impl_test.cc @@ -2383,6 +2383,68 @@ TEST_F(HostImplTest, CreateConnectionHappyEyeballsWithEmptyConfig) { EXPECT_EQ(host, connection->stream_info_.upstreamInfo()->upstreamHost()); } +// Verifies that the happy eyeballs sort of the address list runs once when the host is +// created, and not again on each connection attempt. +TEST_F(HostImplTest, HappyEyeballsSortsAddressListOncePerHost) { + MockClusterMockPrioritySet cluster; + envoy::config::core::v3::Metadata metadata; + Config::Metadata::mutableMetadataValue(metadata, Config::MetadataFilters::get().ENVOY_LB, + Config::MetadataEnvoyLbKeys::get().CANARY) + .set_bool_value(true); + envoy::config::core::v3::Locality locality; + locality.set_region("oceania"); + locality.set_zone("hello"); + locality.set_sub_zone("world"); + Network::Address::InstanceConstSharedPtr address = + *Network::Utility::resolveUrl("tcp://[1:2:3::4]:8"); + AddressVector address_list = { + address, + *Network::Utility::resolveUrl("tcp://10.0.0.1:1235"), + }; + + // Creating the host sorts the address list exactly once. + std::shared_ptr host; + EXPECT_LOG_CONTAINS_N_TIMES("trace", "sort address with happy_eyeballs config", 1, { + host = std::shared_ptr(*HostImpl::create( + cluster.info_, "lyft.com", address, + std::make_shared(metadata), nullptr, 1, + std::make_shared(locality), + envoy::config::endpoint::v3::Endpoint::HealthCheckConfig::default_instance(), 1, + envoy::config::core::v3::UNKNOWN, address_list)); + }); + + testing::StrictMock dispatcher; + Network::TransportSocketOptionsConstSharedPtr transport_socket_options; + Network::ConnectionSocket::OptionsSharedPtr options; + + auto connection1 = new testing::StrictMock(); + EXPECT_CALL(*connection1, setBufferLimits(0)); + EXPECT_CALL(*connection1, addConnectionCallbacks(_)); + EXPECT_CALL(*connection1, connectionInfoSetter()); + EXPECT_CALL(*connection1, streamInfo()); + auto connection2 = new testing::StrictMock(); + EXPECT_CALL(*connection2, setBufferLimits(0)); + EXPECT_CALL(*connection2, addConnectionCallbacks(_)); + EXPECT_CALL(*connection2, connectionInfoSetter()); + EXPECT_CALL(*connection2, streamInfo()); + // Both connections should be created with the first address in the list. + EXPECT_CALL(dispatcher, createClientConnection_(address_list[0], _, _, _)) + .WillOnce(Return(connection1)) + .WillOnce(Return(connection2)); + EXPECT_CALL(dispatcher, createTimer_(_)).Times(2); + + // Creating connections reuses the sorted list and does not sort again. + Envoy::Upstream::Host::CreateConnectionData connection_data1; + Envoy::Upstream::Host::CreateConnectionData connection_data2; + EXPECT_LOG_CONTAINS_N_TIMES("trace", "sort address with happy_eyeballs config", 0, { + connection_data1 = host->createConnection(dispatcher, options, transport_socket_options); + connection_data2 = host->createConnection(dispatcher, options, transport_socket_options); + }); + // The created connections will be wrapped in HappyEyeballsConnectionImpls. + EXPECT_NE(connection1, connection_data1.connection_.get()); + EXPECT_NE(connection2, connection_data2.connection_.get()); +} + TEST_F(HostImplTest, HealthFlags) { MockClusterMockPrioritySet cluster; HostSharedPtr host = makeTestHost(cluster.info_, "tcp://10.0.0.1:1234", 1); diff --git a/test/extensions/clusters/common/logical_host_test.cc b/test/extensions/clusters/common/logical_host_test.cc index cd55893d16fee..e7f46b2f0dfad 100644 --- a/test/extensions/clusters/common/logical_host_test.cc +++ b/test/extensions/clusters/common/logical_host_test.cc @@ -80,6 +80,53 @@ TEST_F(RealHostDescriptionTest, UnitTest) { description_.setOutlierDetector(std::move(detector_host)); } +// Verifies that the happy eyeballs sorted address list is computed at construction and +// recomputed by setNewAddresses, while addressListOrNull() keeps the original order. +TEST(LogicalHostSortedAddressListTest, SortedListFollowsAddressUpdates) { + auto cluster_info = std::make_shared>(); + const Network::Address::InstanceConstSharedPtr v4_1 = + *Network::Utility::resolveUrl("tcp://10.0.0.1:1234"); + const Network::Address::InstanceConstSharedPtr v4_2 = + *Network::Utility::resolveUrl("tcp://10.0.0.2:1234"); + const Network::Address::InstanceConstSharedPtr v6_1 = + *Network::Utility::resolveUrl("tcp://[1::1]:1234"); + + auto host_or_error = Upstream::LogicalHost::create( + cluster_info, /*hostname=*/"", v4_1, /*address_list=*/{v4_1, v4_2, v6_1}, + envoy::config::endpoint::v3::LocalityLbEndpoints(), envoy::config::endpoint::v3::LbEndpoint(), + /*override_transport_socket_options=*/nullptr); + ASSERT_TRUE(host_or_error.ok()); + auto host = std::shared_ptr(std::move(*host_or_error)); + + // The raw list keeps the original order while the sorted list interleaves address families. + ASSERT_NE(host->addressListOrNull(), nullptr); + EXPECT_EQ(*host->addressListOrNull(), + (Upstream::HostDescription::AddressVector{v4_1, v4_2, v6_1})); + ASSERT_NE(host->sortedAddressListOrNull(), nullptr); + EXPECT_EQ(*host->sortedAddressListOrNull(), + (Upstream::HostDescription::AddressVector{v4_1, v6_1, v4_2})); + + // setNewAddresses recomputes both lists. + const Network::Address::InstanceConstSharedPtr v6_2 = + *Network::Utility::resolveUrl("tcp://[1::2]:1234"); + const Network::Address::InstanceConstSharedPtr v6_3 = + *Network::Utility::resolveUrl("tcp://[1::3]:1234"); + const Network::Address::InstanceConstSharedPtr v4_3 = + *Network::Utility::resolveUrl("tcp://10.0.0.3:1234"); + host->setNewAddresses(v6_2, {v6_2, v6_3, v4_3}, envoy::config::endpoint::v3::LbEndpoint()); + ASSERT_NE(host->addressListOrNull(), nullptr); + EXPECT_EQ(*host->addressListOrNull(), + (Upstream::HostDescription::AddressVector{v6_2, v6_3, v4_3})); + ASSERT_NE(host->sortedAddressListOrNull(), nullptr); + EXPECT_EQ(*host->sortedAddressListOrNull(), + (Upstream::HostDescription::AddressVector{v6_2, v4_3, v6_3})); + + // An update without an address list clears both lists. + host->setNewAddresses(v4_1, {}, envoy::config::endpoint::v3::LbEndpoint()); + EXPECT_EQ(host->addressListOrNull(), nullptr); + EXPECT_EQ(host->sortedAddressListOrNull(), nullptr); +} + // Test fixture for LogicalHost per-connection transport socket resolution. class LogicalHostTransportSocketResolutionTest : public testing::Test { public: