diff --git a/api/envoy/api/v2/endpoint/endpoint.proto b/api/envoy/api/v2/endpoint/endpoint.proto index c458884c3a2bf..1d5d07f54d422 100644 --- a/api/envoy/api/v2/endpoint/endpoint.proto +++ b/api/envoy/api/v2/endpoint/endpoint.proto @@ -111,8 +111,8 @@ message LocalityLbEndpoints { // Locality weights are only considered when :ref:`locality weighted load // balancing ` is // configured. These weights are ignored otherwise. If no weights are - // specified when locality weighted load balancing is enabled, the cluster is - // assumed to have a weight of 1. + // specified when locality weighted load balancing is enabled, the locality is + // assigned no load. // // .. attention:: // diff --git a/source/common/upstream/maglev_lb.cc b/source/common/upstream/maglev_lb.cc index 292f901aed4c7..701e325b40190 100644 --- a/source/common/upstream/maglev_lb.cc +++ b/source/common/upstream/maglev_lb.cc @@ -26,7 +26,7 @@ MaglevTable::MaglevTable(const HostsPerLocality& hosts_per_locality, return host_weight; } else { auto locality_weight = (*locality_weights)[locality_index]; - ASSERT(locality_weight != 0); + // This might be zero, since locality weight might not be specified. return host_weight * locality_weight; } }; @@ -52,12 +52,22 @@ MaglevTable::MaglevTable(const HostsPerLocality& hosts_per_locality, for (uint32_t i = 0; i < hosts_per_locality.get().size(); ++i) { for (const auto& host : hosts_per_locality.get()[i]) { const std::string& address = host->address()->asString(); - table_build_entries.emplace_back(host, HashUtil::xxHash64(address) % table_size_, - (HashUtil::xxHash64(address, 1) % (table_size_ - 1)) + 1, - effective_weight(host->weight(), i)); + const uint32_t weight = effective_weight(host->weight(), i); + // If weight is zero, it should be totally excluded from table building + // below. + if (weight > 0) { + table_build_entries.emplace_back(host, HashUtil::xxHash64(address) % table_size_, + (HashUtil::xxHash64(address, 1) % (table_size_ - 1)) + 1, + weight); + } } } + // We can't do anything sensible with no table entries. + if (table_build_entries.empty()) { + return; + } + table_.resize(table_size_); uint64_t table_index = 0; uint32_t iteration = 1; diff --git a/test/common/upstream/maglev_lb_test.cc b/test/common/upstream/maglev_lb_test.cc index 34f196251da7f..895eb040e2e23 100644 --- a/test/common/upstream/maglev_lb_test.cc +++ b/test/common/upstream/maglev_lb_test.cc @@ -151,12 +151,13 @@ TEST_F(MaglevLoadBalancerTest, LocalityWeightedSameLocalityWeights) { // invert the Weighted effective weights). TEST_F(MaglevLoadBalancerTest, LocalityWeightedDifferentLocalityWeights) { host_set_.hosts_ = {makeTestHost(info_, "tcp://127.0.0.1:90", 1), - makeTestHost(info_, "tcp://127.0.0.1:91", 2)}; + makeTestHost(info_, "tcp://127.0.0.1:91", 2), + makeTestHost(info_, "tcp://127.0.0.1:92", 3)}; host_set_.healthy_hosts_ = host_set_.hosts_; host_set_.hosts_per_locality_ = - makeHostsPerLocality({{host_set_.hosts_[0]}, {host_set_.hosts_[1]}}); + makeHostsPerLocality({{host_set_.hosts_[0]}, {host_set_.hosts_[2]}, {host_set_.hosts_[1]}}); host_set_.healthy_hosts_per_locality_ = host_set_.hosts_per_locality_; - LocalityWeightsConstSharedPtr locality_weights{new LocalityWeights{8, 2}}; + LocalityWeightsConstSharedPtr locality_weights{new LocalityWeights{8, 0, 2}}; host_set_.locality_weights_ = locality_weights; host_set_.runCallbacks({}, {}); init(17); @@ -187,6 +188,21 @@ TEST_F(MaglevLoadBalancerTest, LocalityWeightedDifferentLocalityWeights) { } } +// Locality weighted with all localities zero weighted. +TEST_F(MaglevLoadBalancerTest, LocalityWeightedAllZeroLocalityWeights) { + host_set_.hosts_ = {makeTestHost(info_, "tcp://127.0.0.1:90", 1)}; + host_set_.healthy_hosts_ = host_set_.hosts_; + host_set_.hosts_per_locality_ = makeHostsPerLocality({{host_set_.hosts_[0]}}); + host_set_.healthy_hosts_per_locality_ = host_set_.hosts_per_locality_; + LocalityWeightsConstSharedPtr locality_weights{new LocalityWeights{0}}; + host_set_.locality_weights_ = locality_weights; + host_set_.runCallbacks({}, {}); + init(17); + LoadBalancerPtr lb = lb_->factory()->create(); + TestLoadBalancerContext context(0); + EXPECT_EQ(nullptr, lb->chooseHost(&context)); +} + // Validate that when we are in global panic and have localities, we get sane // results (fall back to non-healthy hosts). TEST_F(MaglevLoadBalancerTest, LocalityWeightedGlobalPanic) { diff --git a/test/server/server_corpus/clusterfuzz-testcase-config_fuzz_test-5067970991095808 b/test/server/server_corpus/clusterfuzz-testcase-config_fuzz_test-5067970991095808 new file mode 100644 index 0000000000000..ff46d4c233387 --- /dev/null +++ b/test/server/server_corpus/clusterfuzz-testcase-config_fuzz_test-5067970991095808 @@ -0,0 +1,58 @@ +static_resources { + clusters { + name: "6" + connect_timeout { + seconds: 2321 + } + lb_policy: MAGLEV + hosts { + pipe { + path: "=" + } + } + hosts { + pipe { + path: "=" + } + } + hosts { + pipe { + path: "t" + } + } + max_requests_per_connection { + value: 67108864 + } + dns_lookup_family: V4_ONLY + outlier_detection { + success_rate_minimum_hosts { + value: 4096 + } + } + common_lb_config { + locality_weighted_lb_config { + } + } + } +} +stats_sinks { + typed_config { + type_url: "type.googleapis.com/envoy.api.v2.route.Route" + value: "\022*J :2222222222222222222222222\022" + } +} +stats_sinks { + typed_config { + type_url: "type.googleapis.com/envoy.api.v2.route.Route" + value: "\022*J.:*static\'_resourc\022es {(\n cluster`s" + } +} +stats_sinks { + typed_config { + type_url: "type.googleapis.com/envoy.api.v2.route.Route" + value: "\022*J :2222222222222222222222221\022" + } +} +stats_flush_interval { + nanos: 2883584 +}