diff --git a/source/extensions/clusters/redis/redis_cluster.cc b/source/extensions/clusters/redis/redis_cluster.cc index fb482bd7666cd..4caa7cbba16ec 100644 --- a/source/extensions/clusters/redis/redis_cluster.cc +++ b/source/extensions/clusters/redis/redis_cluster.cc @@ -340,8 +340,8 @@ void RedisCluster::RedisDiscoverySession::resolveClusterHostnames( parent_.dns_resolver_->resolve( slot.primary_hostname_, parent_.dns_lookup_family_, [this, slot_idx, slots, - &hostname_resolution_required_cnt](Network::DnsResolver::ResolutionStatus status, - std::list&& response) -> void { + hostname_resolution_required_cnt](Network::DnsResolver::ResolutionStatus status, + std::list&& response) -> void { auto& slot = (*slots)[slot_idx]; ENVOY_LOG(debug, "async DNS resolution complete for {}", slot.primary_hostname_); updateDnsStats(status, response.empty()); @@ -393,8 +393,8 @@ void RedisCluster::RedisDiscoverySession::resolveReplicas( parent_.dns_resolver_->resolve( replica.first, parent_.dns_lookup_family_, [this, index, slots, replica_idx, - &hostname_resolution_required_cnt](Network::DnsResolver::ResolutionStatus status, - std::list&& response) -> void { + hostname_resolution_required_cnt](Network::DnsResolver::ResolutionStatus status, + std::list&& response) -> void { auto& slot = (*slots)[index]; auto& replica = slot.replicas_to_resolve_[replica_idx]; ENVOY_LOG(debug, "async DNS resolution complete for {}", replica.first); diff --git a/test/extensions/clusters/redis/redis_cluster_test.cc b/test/extensions/clusters/redis/redis_cluster_test.cc index b9eda4278aa84..4019a7611484e 100644 --- a/test/extensions/clusters/redis/redis_cluster_test.cc +++ b/test/extensions/clusters/redis/redis_cluster_test.cc @@ -1,6 +1,7 @@ #include #include #include +#include #include #include "envoy/common/callback.h" @@ -36,6 +37,7 @@ using testing::Eq; using testing::NiceMock; using testing::Ref; using testing::Return; +using testing::SaveArg; namespace Envoy { namespace Extensions { @@ -400,6 +402,50 @@ class RedisClusterTest : public testing::Test, return response; } + NetworkFilters::Common::Redis::RespValuePtr + twoSlotsPrimariesHostnames(const std::string& primary1, const std::string& primary2, + int64_t port) const { + std::vector primary_1(2); + primary_1[0].type(NetworkFilters::Common::Redis::RespType::BulkString); + primary_1[0].asString() = primary1; + primary_1[1].type(NetworkFilters::Common::Redis::RespType::Integer); + primary_1[1].asInteger() = port; + + std::vector primary_2(2); + primary_2[0].type(NetworkFilters::Common::Redis::RespType::BulkString); + primary_2[0].asString() = primary2; + primary_2[1].type(NetworkFilters::Common::Redis::RespType::Integer); + primary_2[1].asInteger() = port; + + std::vector slot_1(3); + slot_1[0].type(NetworkFilters::Common::Redis::RespType::Integer); + slot_1[0].asInteger() = 0; + slot_1[1].type(NetworkFilters::Common::Redis::RespType::Integer); + slot_1[1].asInteger() = 9999; + slot_1[2].type(NetworkFilters::Common::Redis::RespType::Array); + slot_1[2].asArray().swap(primary_1); + + std::vector slot_2(3); + slot_2[0].type(NetworkFilters::Common::Redis::RespType::Integer); + slot_2[0].asInteger() = 10000; + slot_2[1].type(NetworkFilters::Common::Redis::RespType::Integer); + slot_2[1].asInteger() = 16383; + slot_2[2].type(NetworkFilters::Common::Redis::RespType::Array); + slot_2[2].asArray().swap(primary_2); + + std::vector slots(2); + slots[0].type(NetworkFilters::Common::Redis::RespType::Array); + slots[0].asArray().swap(slot_1); + slots[1].type(NetworkFilters::Common::Redis::RespType::Array); + slots[1].asArray().swap(slot_2); + + NetworkFilters::Common::Redis::RespValuePtr response( + new NetworkFilters::Common::Redis::RespValue()); + response->type(NetworkFilters::Common::Redis::RespType::Array); + response->asArray().swap(slots); + return response; + } + NetworkFilters::Common::Redis::RespValue createStringField(bool is_correct_type, const std::string& correct_value) const { NetworkFilters::Common::Redis::RespValue respValue; @@ -778,6 +824,38 @@ TEST_F(RedisClusterTest, AddressAsHostname) { EXPECT_EQ(0U, cluster_->info()->stats().update_failure_.value()); } +TEST_F(RedisClusterTest, AddressAsHostnameParallelResolution) { + // This test specifically ensures that DNS resolution of different hostnames running parallel + // works as expected. + setupFromV3Yaml(BasicConfig); + const std::list resolved_addresses{"127.0.0.1", "127.0.0.2"}; + expectResolveDiscovery(Network::DnsLookupFamily::V4Only, "foo.bar.com", resolved_addresses); + + Network::DnsResolver::ResolveCb primary1_resolve_cb; + Network::DnsResolver::ResolveCb primary2_resolve_cb; + EXPECT_CALL(*dns_resolver_, resolve("primary1.com", Network::DnsLookupFamily::V4Only, _)) + .WillOnce(DoAll(SaveArg<2>(&primary1_resolve_cb), Return(&dns_resolver_->active_query_))); + EXPECT_CALL(*dns_resolver_, resolve("primary2.com", Network::DnsLookupFamily::V4Only, _)) + .WillOnce(DoAll(SaveArg<2>(&primary2_resolve_cb), Return(&dns_resolver_->active_query_))); + + expectRedisResolve(true); + + EXPECT_CALL(membership_updated_, ready()); + EXPECT_CALL(initialized_, ready()); + EXPECT_CALL(*cluster_callback_, onClusterSlotUpdate(_, _)); + cluster_->initialize([&]() -> void { initialized_.ready(); }); + expectClusterSlotResponse(twoSlotsPrimariesHostnames("primary1.com", "primary2.com", 22120)); + primary1_resolve_cb(Network::DnsResolver::ResolutionStatus::Success, + TestUtility::makeDnsResponse(std::list{"127.0.1.1"})); + primary2_resolve_cb(Network::DnsResolver::ResolutionStatus::Success, + TestUtility::makeDnsResponse(std::list{"127.0.1.2"})); + expectHealthyHosts(std::list({ + "127.0.1.1:22120", + "127.0.1.2:22120", + })); + EXPECT_EQ(0U, cluster_->info()->stats().update_failure_.value()); +} + TEST_F(RedisClusterTest, AddressAsHostnameFailure) { setupFromV3Yaml(BasicConfig); const std::list resolved_addresses{"127.0.0.1", "127.0.0.2"};