diff --git a/api/envoy/api/v2/cds.proto b/api/envoy/api/v2/cds.proto index c8542387fc0d..647a5015063f 100644 --- a/api/envoy/api/v2/cds.proto +++ b/api/envoy/api/v2/cds.proto @@ -160,7 +160,12 @@ message Cluster { // Refer to the :ref:`original destination load balancing // policy` // for an explanation. - ORIGINAL_DST_LB = 4; + // + // .. attention:: + // + // **This load balancing policy is deprecated**. Use CLUSTER_PROVIDED instead. + // + ORIGINAL_DST_LB = 4 [deprecated = true]; // Refer to the :ref:`Maglev load balancing policy` // for an explanation. diff --git a/docs/root/intro/deprecated.rst b/docs/root/intro/deprecated.rst index 6fdd161ae631..c8399d9ab087 100644 --- a/docs/root/intro/deprecated.rst +++ b/docs/root/intro/deprecated.rst @@ -12,7 +12,7 @@ Deprecated items below are listed in chronological order. Version 1.12.0 (pending) ======================== - +* The ORIGINAL_DST_LB :ref:`load balancing policy ` is deprecated, use CLUSTER_PROVIDED policy instead when configuring an :ref:`original destination cluster `. Version 1.11.0 (July 11, 2019) ============================== diff --git a/source/common/upstream/cluster_manager_impl.cc b/source/common/upstream/cluster_manager_impl.cc index 4c29bf9a70a6..e2673034a675 100644 --- a/source/common/upstream/cluster_manager_impl.cc +++ b/source/common/upstream/cluster_manager_impl.cc @@ -1114,18 +1114,12 @@ ClusterManagerImpl::ThreadLocalClusterManagerImpl::ClusterEntry::ClusterEntry( } case LoadBalancerType::ClusterProvided: case LoadBalancerType::RingHash: - case LoadBalancerType::Maglev: { + case LoadBalancerType::Maglev: + case LoadBalancerType::OriginalDst: { ASSERT(lb_factory_ != nullptr); lb_ = lb_factory_->create(); break; } - case LoadBalancerType::OriginalDst: { - ASSERT(lb_factory_ == nullptr); - lb_ = std::make_unique( - priority_set_, parent.parent_.active_clusters_.at(cluster->name())->cluster_, - cluster->lbOriginalDstConfig()); - break; - } } } } diff --git a/source/common/upstream/original_dst_cluster.cc b/source/common/upstream/original_dst_cluster.cc index 9cb8414aa166..ec24c36e0e47 100644 --- a/source/common/upstream/original_dst_cluster.cc +++ b/source/common/upstream/original_dst_cluster.cc @@ -16,40 +16,11 @@ namespace Envoy { namespace Upstream { -// Static cast below is guaranteed to succeed, as code instantiating the cluster -// configuration, that is run prior to this code, checks that an OriginalDstCluster is -// always configured with an OriginalDstCluster::LoadBalancer, and that an -// OriginalDstCluster::LoadBalancer is never configured with any other type of cluster, -// and throws an exception otherwise. - -OriginalDstCluster::LoadBalancer::LoadBalancer( - PrioritySet& priority_set, ClusterSharedPtr& parent, - const absl::optional& config) - : priority_set_(priority_set), parent_(std::static_pointer_cast(parent)), - info_(parent->info()), use_http_header_(config ? config.value().use_http_header() : false) { - // priority_set_ is initially empty. - priority_set_.addMemberUpdateCb( - [this](const HostVector& hosts_added, const HostVector& hosts_removed) -> void { - // Update the hosts map - // TODO(ramaraochavali): use cluster stats and move the log lines to debug. - for (const HostSharedPtr& host : hosts_removed) { - ENVOY_LOG(debug, "Removing host {}.", host->address()->asString()); - host_map_.remove(host); - } - for (const HostSharedPtr& host : hosts_added) { - if (host_map_.insert(host)) { - ENVOY_LOG(debug, "Adding host {}.", host->address()->asString()); - } - } - }); -} - HostConstSharedPtr OriginalDstCluster::LoadBalancer::chooseHost(LoadBalancerContext* context) { if (context) { - // Check if override host header is present, if yes use it otherwise check local address. Network::Address::InstanceConstSharedPtr dst_host = nullptr; - if (use_http_header_) { + if (parent_->use_http_header_) { dst_host = requestOverrideHost(context); } if (dst_host == nullptr) { @@ -63,10 +34,10 @@ HostConstSharedPtr OriginalDstCluster::LoadBalancer::chooseHost(LoadBalancerCont if (dst_host) { const Network::Address::Instance& dst_addr = *dst_host.get(); - // Check if a host with the destination address is already in the host set. - HostSharedPtr host = host_map_.find(dst_addr); - if (host) { + auto it = host_map_->find(dst_addr.asString()); + if (it != host_map_->end()) { + HostSharedPtr host(it->second); // takes a reference ENVOY_LOG(debug, "Using existing host {}.", host->address()->asString()); host->used(true); // Mark as used. return host; @@ -77,29 +48,24 @@ HostConstSharedPtr OriginalDstCluster::LoadBalancer::chooseHost(LoadBalancerCont Network::Address::InstanceConstSharedPtr host_ip_port( Network::Utility::copyInternetAddressAndPort(*dst_ip)); // Create a host we can use immediately. - host.reset( - new HostImpl(info_, info_->name() + dst_addr.asString(), std::move(host_ip_port), - envoy::api::v2::core::Metadata::default_instance(), 1, - envoy::api::v2::core::Locality().default_instance(), - envoy::api::v2::endpoint::Endpoint::HealthCheckConfig().default_instance(), - 0, envoy::api::v2::core::HealthStatus::UNKNOWN)); - + auto info = parent_->info(); + HostSharedPtr host(std::make_shared( + info, info->name() + dst_addr.asString(), std::move(host_ip_port), + envoy::api::v2::core::Metadata::default_instance(), 1, + envoy::api::v2::core::Locality().default_instance(), + envoy::api::v2::endpoint::Endpoint::HealthCheckConfig().default_instance(), 0, + envoy::api::v2::core::HealthStatus::UNKNOWN)); ENVOY_LOG(debug, "Created host {}.", host->address()->asString()); - // Add the new host to the map. We just failed to find it in - // our local map above, so insert without checking (2nd arg == false). - host_map_.insert(host, false); - - if (std::shared_ptr parent = parent_.lock()) { - // lambda cannot capture a member by value. - std::weak_ptr post_parent = parent_; - parent->dispatcher_.post([post_parent, host]() mutable { - // The main cluster may have disappeared while this post was queued. - if (std::shared_ptr parent = post_parent.lock()) { - parent->addHost(host); - } - }); - } + // Tell the cluster about the new host + // lambda cannot capture a member by value. + std::weak_ptr post_parent = parent_; + parent_->dispatcher_.post([post_parent, host]() mutable { + // The main cluster may have disappeared while this post was queued. + if (std::shared_ptr parent = post_parent.lock()) { + parent->addHost(host); + } + }); return host; } else { ENVOY_LOG(debug, "Failed to create host for {}.", dst_addr.asString()); @@ -126,7 +92,7 @@ OriginalDstCluster::LoadBalancer::requestOverrideHost(LoadBalancerContext* conte ENVOY_LOG(debug, "Using request override host {}.", request_override_host); } catch (const Envoy::EnvoyException& e) { ENVOY_LOG(debug, "original_dst_load_balancer: invalid override header value. {}", e.what()); - info_->stats().original_dst_host_invalid_.inc(); + parent_->info()->stats().original_dst_host_invalid_.inc(); } } return request_host; @@ -140,7 +106,11 @@ OriginalDstCluster::OriginalDstCluster( dispatcher_(factory_context.dispatcher()), cleanup_interval_ms_( std::chrono::milliseconds(PROTOBUF_GET_MS_OR_DEFAULT(config, cleanup_interval, 5000))), - cleanup_timer_(dispatcher_.createTimer([this]() -> void { cleanup(); })) { + cleanup_timer_(dispatcher_.createTimer([this]() -> void { cleanup(); })), + use_http_header_(info_->lbOriginalDstConfig() + ? info_->lbOriginalDstConfig().value().use_http_header() + : false), + host_map_(std::make_shared()) { // TODO(dio): Remove hosts check once the hosts field is removed. if (config.has_load_assignment() || !config.hosts().empty()) { throw EnvoyException("ORIGINAL_DST clusters must have no load assignment or hosts configured"); @@ -149,41 +119,53 @@ OriginalDstCluster::OriginalDstCluster( } void OriginalDstCluster::addHost(HostSharedPtr& host) { - // Given the current config, only EDS clusters support multiple priorities. - ASSERT(priority_set_.hostSetsPerPriority().size() == 1); - const auto& first_host_set = priority_set_.getOrCreateHostSet(0); - HostVectorSharedPtr new_hosts(new HostVector(first_host_set.hosts())); - new_hosts->emplace_back(host); - priority_set_.updateHosts(0, - HostSetImpl::partitionHosts(new_hosts, HostsPerLocalityImpl::empty()), - {}, {std::move(host)}, {}, absl::nullopt); + HostMapSharedPtr new_host_map = std::make_shared(*getCurrentHostMap()); + auto pair = new_host_map->emplace(host->address()->asString(), host); + bool added = pair.second; + if (added) { + ENVOY_LOG(debug, "addHost() adding {}", host->address()->asString()); + setHostMap(new_host_map); + // Given the current config, only EDS clusters support multiple priorities. + ASSERT(priority_set_.hostSetsPerPriority().size() == 1); + const auto& first_host_set = priority_set_.getOrCreateHostSet(0); + HostVectorSharedPtr all_hosts(new HostVector(first_host_set.hosts())); + all_hosts->emplace_back(host); + priority_set_.updateHosts(0, + HostSetImpl::partitionHosts(all_hosts, HostsPerLocalityImpl::empty()), + {}, {std::move(host)}, {}, absl::nullopt); + } } void OriginalDstCluster::cleanup() { - HostVectorSharedPtr new_hosts(new HostVector); + HostVectorSharedPtr keeping_hosts(new HostVector); HostVector to_be_removed; - // Given the current config, only EDS clusters support multiple priorities. - ASSERT(priority_set_.hostSetsPerPriority().size() == 1); - const auto& host_set = priority_set_.getOrCreateHostSet(0); ENVOY_LOG(trace, "Stale original dst hosts cleanup triggered."); - if (!host_set.hosts().empty()) { - ENVOY_LOG(debug, "Cleaning up stale original dst hosts."); - for (const HostSharedPtr& host : host_set.hosts()) { + auto host_map = getCurrentHostMap(); + if (!host_map->empty()) { + ENVOY_LOG(trace, "Cleaning up stale original dst hosts."); + for (const auto& pair : *host_map) { + const std::string& addr = pair.first; + const HostSharedPtr& host = pair.second; if (host->used()) { - ENVOY_LOG(debug, "Keeping active host {}.", host->address()->asString()); - new_hosts->emplace_back(host); + ENVOY_LOG(trace, "Keeping active host {}.", addr); + keeping_hosts->emplace_back(host); host->used(false); // Mark to be removed during the next round. } else { - ENVOY_LOG(debug, "Removing stale host {}.", host->address()->asString()); + ENVOY_LOG(trace, "Removing stale host {}.", addr); to_be_removed.emplace_back(host); } } } if (!to_be_removed.empty()) { - priority_set_.updateHosts(0, - HostSetImpl::partitionHosts(new_hosts, HostsPerLocalityImpl::empty()), - {}, {}, to_be_removed, absl::nullopt); + HostMapSharedPtr new_host_map = std::make_shared(*host_map); + for (const HostSharedPtr& host : to_be_removed) { + new_host_map->erase(host->address()->asString()); + } + setHostMap(new_host_map); + priority_set_.updateHosts( + 0, HostSetImpl::partitionHosts(keeping_hosts, HostsPerLocalityImpl::empty()), {}, {}, + to_be_removed, absl::nullopt); } cleanup_timer_->enableTimer(cleanup_interval_ms_); @@ -194,10 +176,11 @@ OriginalDstClusterFactory::createClusterImpl( const envoy::api::v2::Cluster& cluster, ClusterFactoryContext& context, Server::Configuration::TransportSocketFactoryContext& socket_factory_context, Stats::ScopePtr&& stats_scope) { - if (cluster.lb_policy() != envoy::api::v2::Cluster::ORIGINAL_DST_LB) { + if (cluster.lb_policy() != envoy::api::v2::Cluster::ORIGINAL_DST_LB && + cluster.lb_policy() != envoy::api::v2::Cluster::CLUSTER_PROVIDED) { throw EnvoyException(fmt::format( - "cluster: LB policy {} is not valid for Cluster type {}. Only 'original_dst_lb' " - "is allowed with cluster type 'original_dst'", + "cluster: LB policy {} is not valid for Cluster type {}. Only 'CLUSTER_PROVIDED' or " + "'ORIGINAL_DST_LB' is allowed with cluster type 'ORIGINAL_DST'", envoy::api::v2::Cluster_LbPolicy_Name(cluster.lb_policy()), envoy::api::v2::Cluster_DiscoveryType_Name(cluster.type()))); } @@ -209,14 +192,15 @@ OriginalDstClusterFactory::createClusterImpl( // TODO(mattklein123): The original DST load balancer type should be deprecated and instead // the cluster should directly supply the load balancer. This will remove // a special case and allow this cluster to be compiled out as an extension. - return std::make_pair( + auto new_cluster = std::make_shared(cluster, context.runtime(), socket_factory_context, - std::move(stats_scope), context.addedViaApi()), - nullptr); + std::move(stats_scope), context.addedViaApi()); + auto lb = std::make_unique(new_cluster); + return std::make_pair(new_cluster, std::move(lb)); } /** - * Static registration for the strict dns cluster factory. @see RegisterFactory. + * Static registration for the original dst cluster factory. @see RegisterFactory. */ REGISTER_FACTORY(OriginalDstClusterFactory, ClusterFactory); diff --git a/source/common/upstream/original_dst_cluster.h b/source/common/upstream/original_dst_cluster.h index 64226fc71db4..1a88dfb1c61c 100644 --- a/source/common/upstream/original_dst_cluster.h +++ b/source/common/upstream/original_dst_cluster.h @@ -20,6 +20,9 @@ namespace Envoy { namespace Upstream { +using HostMapSharedPtr = std::shared_ptr; +using HostMapConstSharedPtr = std::shared_ptr; + /** * The OriginalDstCluster is a dynamic cluster that automatically adds hosts as needed based on the * original destination address of the downstream connection. These hosts are also automatically @@ -48,66 +51,52 @@ class OriginalDstCluster : public ClusterImplBase { */ class LoadBalancer : public Upstream::LoadBalancer { public: - LoadBalancer(PrioritySet& priority_set, ClusterSharedPtr& parent, - const absl::optional& config); + LoadBalancer(const std::shared_ptr& parent) + : parent_(parent), host_map_(parent->getCurrentHostMap()) {} // Upstream::LoadBalancer HostConstSharedPtr chooseHost(LoadBalancerContext* context) override; private: - /** - * Map from an host IP address/port to a HostSharedPtr. Due to races multiple distinct host - * objects with the same address can be created, so we need to use a multimap. - */ - class HostMap { - public: - bool insert(const HostSharedPtr& host, bool check = true) { - if (check) { - auto range = map_.equal_range(host->address()->asString()); - auto it = std::find_if( - range.first, range.second, - [&host](const decltype(map_)::value_type pair) { return pair.second == host; }); - if (it != range.second) { - return false; // 'host' already in the map, no need to insert. - } - } - map_.emplace(host->address()->asString(), host); - return true; - } - - void remove(const HostSharedPtr& host) { - auto range = map_.equal_range(host->address()->asString()); - auto it = - std::find_if(range.first, range.second, [&host](const decltype(map_)::value_type pair) { - return pair.second == host; - }); - ASSERT(it != range.second); - map_.erase(it); - } - - HostSharedPtr find(const Network::Address::Instance& address) { - auto it = map_.find(address.asString()); - - if (it != map_.end()) { - return it->second; - } - return nullptr; - } - - private: - std::unordered_multimap map_; - }; - Network::Address::InstanceConstSharedPtr requestOverrideHost(LoadBalancerContext* context); - PrioritySet& priority_set_; // Thread local priority set. - std::weak_ptr parent_; // Primary cluster managed by the main thread. - ClusterInfoConstSharedPtr info_; - const bool use_http_header_; - HostMap host_map_; + const std::shared_ptr parent_; + HostMapConstSharedPtr host_map_; }; private: + struct LoadBalancerFactory : public Upstream::LoadBalancerFactory { + LoadBalancerFactory(const std::shared_ptr& cluster) : cluster_(cluster) {} + + // Upstream::LoadBalancerFactory + Upstream::LoadBalancerPtr create() override { return std::make_unique(cluster_); } + + const std::shared_ptr cluster_; + }; + + struct ThreadAwareLoadBalancer : public Upstream::ThreadAwareLoadBalancer { + ThreadAwareLoadBalancer(const std::shared_ptr& cluster) + : cluster_(cluster) {} + + // Upstream::ThreadAwareLoadBalancer + Upstream::LoadBalancerFactorySharedPtr factory() override { + return std::make_shared(cluster_); + } + void initialize() override {} + + const std::shared_ptr cluster_; + }; + + HostMapConstSharedPtr getCurrentHostMap() { + absl::ReaderMutexLock lock(&host_map_lock_); + return host_map_; + } + + void setHostMap(const HostMapConstSharedPtr& new_host_map) { + absl::WriterMutexLock lock(&host_map_lock_); + host_map_ = new_host_map; + } + void addHost(HostSharedPtr&); void cleanup(); @@ -117,8 +106,16 @@ class OriginalDstCluster : public ClusterImplBase { Event::Dispatcher& dispatcher_; const std::chrono::milliseconds cleanup_interval_ms_; Event::TimerPtr cleanup_timer_; + const bool use_http_header_; + + absl::Mutex host_map_lock_; + HostMapConstSharedPtr host_map_ ABSL_GUARDED_BY(host_map_lock_); + + friend class OriginalDstClusterFactory; }; +using OriginalDstClusterSharedPtr = std::shared_ptr; + class OriginalDstClusterFactory : public ClusterFactoryImplBase { public: OriginalDstClusterFactory() diff --git a/source/common/upstream/upstream_impl.cc b/source/common/upstream/upstream_impl.cc index 12fc71e75290..f44e01f5a5de 100644 --- a/source/common/upstream/upstream_impl.cc +++ b/source/common/upstream/upstream_impl.cc @@ -637,13 +637,13 @@ ClusterInfoImpl::ClusterInfoImpl( break; case envoy::api::v2::Cluster::ORIGINAL_DST_LB: if (config.type() != envoy::api::v2::Cluster::ORIGINAL_DST) { - throw EnvoyException(fmt::format( - "cluster: LB policy {} is not valid for Cluster type {}. Only 'original_dst_lb' " - "is allowed with cluster type 'original_dst'", - envoy::api::v2::Cluster_LbPolicy_Name(config.lb_policy()), - envoy::api::v2::Cluster_DiscoveryType_Name(config.type()))); + throw EnvoyException( + fmt::format("cluster: LB policy {} is not valid for Cluster type {}. 'ORIGINAL_DST_LB' " + "is allowed only with cluster type 'ORIGINAL_DST'", + envoy::api::v2::Cluster_LbPolicy_Name(config.lb_policy()), + envoy::api::v2::Cluster_DiscoveryType_Name(config.type()))); } - lb_type_ = LoadBalancerType::OriginalDst; + lb_type_ = LoadBalancerType::ClusterProvided; break; case envoy::api::v2::Cluster::MAGLEV: lb_type_ = LoadBalancerType::Maglev; diff --git a/test/common/upstream/cluster_manager_impl_test.cc b/test/common/upstream/cluster_manager_impl_test.cc index 82ac958f3267..7e3fff828ad9 100644 --- a/test/common/upstream/cluster_manager_impl_test.cc +++ b/test/common/upstream/cluster_manager_impl_test.cc @@ -545,7 +545,7 @@ TEST_F(ClusterManagerImplTest, OriginalDstLbRestriction) { EXPECT_THROW_WITH_MESSAGE( create(parseBootstrapFromV2Yaml(yaml)), EnvoyException, "cluster: LB policy ROUND_ROBIN is not valid for Cluster type ORIGINAL_DST. Only " - "'original_dst_lb' is allowed with cluster type 'original_dst'"); + "'CLUSTER_PROVIDED' or 'ORIGINAL_DST_LB' is allowed with cluster type 'ORIGINAL_DST'"); } TEST_F(ClusterManagerImplTest, OriginalDstLbRestriction2) { @@ -568,8 +568,8 @@ TEST_F(ClusterManagerImplTest, OriginalDstLbRestriction2) { EXPECT_THROW_WITH_MESSAGE( create(parseBootstrapFromV2Yaml(yaml)), EnvoyException, - "cluster: LB policy ORIGINAL_DST_LB is not valid for Cluster type STATIC. Only " - "'original_dst_lb' is allowed with cluster type 'original_dst'"); + "cluster: LB policy ORIGINAL_DST_LB is not valid for Cluster type STATIC. " + "'ORIGINAL_DST_LB' is allowed only with cluster type 'ORIGINAL_DST'"); } TEST_F(ClusterManagerImplTest, SubsetLoadBalancerInitialization) { diff --git a/test/common/upstream/original_dst_cluster_test.cc b/test/common/upstream/original_dst_cluster_test.cc index ba5e955b6fa6..dd0a2c1babf3 100644 --- a/test/common/upstream/original_dst_cluster_test.cc +++ b/test/common/upstream/original_dst_cluster_test.cc @@ -37,7 +37,6 @@ using testing::SaveArg; namespace Envoy { namespace Upstream { -namespace OriginalDstClusterTest { namespace { class TestLoadBalancerContext : public LoadBalancerContextBase { @@ -89,7 +88,7 @@ class OriginalDstClusterTest : public testing::Test { Stats::IsolatedStoreImpl stats_store_; Ssl::MockContextManager ssl_context_manager_; - ClusterSharedPtr cluster_; + OriginalDstClusterSharedPtr cluster_; ReadyWatcher membership_updated_; ReadyWatcher initialized_; NiceMock runtime_; @@ -109,7 +108,7 @@ TEST(OriginalDstClusterConfigTest, GoodConfig) { name: name connect_timeout: 0.25s type: original_dst - lb_policy: original_dst_lb + lb_policy: cluster_provided cleanup_interval: 1s )EOF"; // Help Emacs balance quotation marks: " @@ -198,8 +197,7 @@ TEST_F(OriginalDstClusterTest, NoContext) { // No downstream connection => no host. { TestLoadBalancerContext lb_context(nullptr); - OriginalDstCluster::LoadBalancer lb(cluster_->prioritySet(), cluster_, - cluster_->info()->lbOriginalDstConfig()); + OriginalDstCluster::LoadBalancer lb(cluster_); EXPECT_CALL(dispatcher_, post(_)).Times(0); HostConstSharedPtr host = lb.chooseHost(&lb_context); EXPECT_EQ(host, nullptr); @@ -214,8 +212,7 @@ TEST_F(OriginalDstClusterTest, NoContext) { // First argument is normally the reference to the ThreadLocalCluster's HostSet, but in these // tests we do not have the thread local clusters, so we pass a reference to the HostSet of the // primary cluster. The implementation handles both cases the same. - OriginalDstCluster::LoadBalancer lb(cluster_->prioritySet(), cluster_, - cluster_->info()->lbOriginalDstConfig()); + OriginalDstCluster::LoadBalancer lb(cluster_); EXPECT_CALL(dispatcher_, post(_)).Times(0); HostConstSharedPtr host = lb.chooseHost(&lb_context); EXPECT_EQ(host, nullptr); @@ -228,8 +225,7 @@ TEST_F(OriginalDstClusterTest, NoContext) { connection.local_address_ = std::make_shared("unix://foo"); EXPECT_CALL(connection, localAddressRestored()).WillRepeatedly(Return(true)); - OriginalDstCluster::LoadBalancer lb(cluster_->prioritySet(), cluster_, - cluster_->info()->lbOriginalDstConfig()); + OriginalDstCluster::LoadBalancer lb(cluster_); EXPECT_CALL(dispatcher_, post(_)).Times(0); HostConstSharedPtr host = lb.chooseHost(&lb_context); EXPECT_EQ(host, nullptr); @@ -264,11 +260,10 @@ TEST_F(OriginalDstClusterTest, Membership) { connection.local_address_ = std::make_shared("10.10.11.11"); EXPECT_CALL(connection, localAddressRestored()).WillRepeatedly(Return(true)); - OriginalDstCluster::LoadBalancer lb(cluster_->prioritySet(), cluster_, - cluster_->info()->lbOriginalDstConfig()); Event::PostCb post_cb; EXPECT_CALL(dispatcher_, post(_)).WillOnce(SaveArg<0>(&post_cb)); - HostConstSharedPtr host = lb.chooseHost(&lb_context); + // Mock the cluster manager by recreating the load balancer each time to get a fresh host map + HostConstSharedPtr host = OriginalDstCluster::LoadBalancer(cluster_).chooseHost(&lb_context); post_cb(); auto cluster_hosts = cluster_->prioritySet().hostSetsPerPriority()[0]->hosts(); @@ -287,7 +282,8 @@ TEST_F(OriginalDstClusterTest, Membership) { *cluster_->prioritySet().hostSetsPerPriority()[0]->hosts()[0]->address()); // Same host is returned on the 2nd call - HostConstSharedPtr host2 = lb.chooseHost(&lb_context); + // Mock the cluster manager by recreating the load balancer with the new host map + HostConstSharedPtr host2 = OriginalDstCluster::LoadBalancer(cluster_).chooseHost(&lb_context); EXPECT_EQ(host2, host); // Make host time out, no membership changes happen on the first timeout. @@ -315,7 +311,8 @@ TEST_F(OriginalDstClusterTest, Membership) { // New host gets created EXPECT_CALL(membership_updated_, ready()); EXPECT_CALL(dispatcher_, post(_)).WillOnce(SaveArg<0>(&post_cb)); - HostConstSharedPtr host3 = lb.chooseHost(&lb_context); + // Mock the cluster manager by recreating the load balancer with the new host map + HostConstSharedPtr host3 = OriginalDstCluster::LoadBalancer(cluster_).chooseHost(&lb_context); post_cb(); EXPECT_NE(host3, nullptr); EXPECT_NE(host3, host); @@ -357,9 +354,7 @@ TEST_F(OriginalDstClusterTest, Membership2) { connection2.local_address_ = std::make_shared("10.10.11.12"); EXPECT_CALL(connection2, localAddressRestored()).WillRepeatedly(Return(true)); - OriginalDstCluster::LoadBalancer lb(cluster_->prioritySet(), cluster_, - cluster_->info()->lbOriginalDstConfig()); - + OriginalDstCluster::LoadBalancer lb(cluster_); EXPECT_CALL(membership_updated_, ready()); Event::PostCb post_cb; EXPECT_CALL(dispatcher_, post(_)).WillOnce(SaveArg<0>(&post_cb)); @@ -443,8 +438,7 @@ TEST_F(OriginalDstClusterTest, Connection) { connection.local_address_ = std::make_shared("FD00::1"); EXPECT_CALL(connection, localAddressRestored()).WillRepeatedly(Return(true)); - OriginalDstCluster::LoadBalancer lb(cluster_->prioritySet(), cluster_, - cluster_->info()->lbOriginalDstConfig()); + OriginalDstCluster::LoadBalancer lb(cluster_); Event::PostCb post_cb; EXPECT_CALL(dispatcher_, post(_)).WillOnce(SaveArg<0>(&post_cb)); HostConstSharedPtr host = lb.chooseHost(&lb_context); @@ -493,18 +487,16 @@ TEST_F(OriginalDstClusterTest, MultipleClusters) { connection.local_address_ = std::make_shared("FD00::1"); EXPECT_CALL(connection, localAddressRestored()).WillRepeatedly(Return(true)); - OriginalDstCluster::LoadBalancer lb1(cluster_->prioritySet(), cluster_, - cluster_->info()->lbOriginalDstConfig()); - OriginalDstCluster::LoadBalancer lb2(second, cluster_, cluster_->info()->lbOriginalDstConfig()); + OriginalDstCluster::LoadBalancer lb(cluster_); Event::PostCb post_cb; EXPECT_CALL(dispatcher_, post(_)).WillOnce(SaveArg<0>(&post_cb)); - HostConstSharedPtr host = lb1.chooseHost(&lb_context); + HostConstSharedPtr host = lb.chooseHost(&lb_context); post_cb(); ASSERT_NE(host, nullptr); EXPECT_EQ(*connection.local_address_, *host->address()); EXPECT_EQ(1UL, cluster_->prioritySet().hostSetsPerPriority()[0]->hosts().size()); - // Check that lb2 also gets updated + // Check that 'second' also gets updated EXPECT_EQ(1UL, second.hostSetsPerPriority()[0]->hosts().size()); EXPECT_EQ(host, cluster_->prioritySet().hostSetsPerPriority()[0]->hosts()[0]); @@ -532,8 +524,7 @@ TEST_F(OriginalDstClusterTest, UseHttpHeaderEnabled) { 0UL, cluster_->prioritySet().hostSetsPerPriority()[0]->healthyHostsPerLocality().get().size()); - OriginalDstCluster::LoadBalancer lb(cluster_->prioritySet(), cluster_, - cluster_->info()->lbOriginalDstConfig()); + OriginalDstCluster::LoadBalancer lb(cluster_); Event::PostCb post_cb; // HTTP header override. @@ -604,8 +595,7 @@ TEST_F(OriginalDstClusterTest, UseHttpHeaderDisabled) { 0UL, cluster_->prioritySet().hostSetsPerPriority()[0]->healthyHostsPerLocality().get().size()); - OriginalDstCluster::LoadBalancer lb(cluster_->prioritySet(), cluster_, - cluster_->info()->lbOriginalDstConfig()); + OriginalDstCluster::LoadBalancer lb(cluster_); Event::PostCb post_cb; // Downstream connection with original_dst filter, HTTP header override ignored. @@ -647,6 +637,5 @@ TEST_F(OriginalDstClusterTest, UseHttpHeaderDisabled) { } } // namespace -} // namespace OriginalDstClusterTest } // namespace Upstream } // namespace Envoy