[xDS] for logical_dns clusters, use a single endpoint instead of hard…

…-coding pick_first
markdroth · Dec 6, 2024 · 32c4512 · 32c4512
1 parent 32f5136
commit 32c4512
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 16 deletions.
diff --git a/src/core/load_balancing/xds/cds.cc b/src/core/load_balancing/xds/cds.cc
@@ -546,15 +546,8 @@ Json CdsLb::CreateChildPolicyConfigForLeafCluster(
           cluster_resource.type);
   // Determine what xDS LB policy to use.
   Json xds_lb_policy;
-  if (is_logical_dns) {
-    xds_lb_policy = Json::FromArray({
-        Json::FromObject({
-            {"pick_first", Json::FromObject({})},
-        }),
-    });
-  }
-  // TODO(roth): Remove this "else if" block after the 1.63 release.
-  else if (XdsAggregateClusterBackwardCompatibilityEnabled() &&
+  // TODO(roth): Remove this "if" condition after the 1.63 release.
+  if (XdsAggregateClusterBackwardCompatibilityEnabled() &&
            aggregate_cluster_resource != nullptr) {
     xds_lb_policy =
         Json::FromArray(aggregate_cluster_resource->lb_policy_config);

diff --git a/src/core/resolver/xds/xds_dependency_manager.cc b/src/core/resolver/xds/xds_dependency_manager.cc
@@ -631,11 +631,16 @@ void XdsDependencyManager::PopulateDnsUpdate(const std::string& dns_name,
   locality.name = MakeRefCounted<XdsLocalityName>("", "", "");
   locality.lb_weight = 1;
   if (result.addresses.ok()) {
+    std::vector<grpc_resolved_address> addresses;
+    ChannelArgs address_args = result.addresses->front().args();
     for (const auto& address : *result.addresses) {
-      locality.endpoints.emplace_back(
-          address.addresses(),
-          address.args().Set(GRPC_ARG_ADDRESS_NAME, dns_name));
+      CHECK_EQ(address.addresses().size(), 1);
+      addresses.emplace_back(address.addresses().front());
+      CHECK_EQ(address_args, address.args());
     }
+    locality.endpoints.emplace_back(
+        std::move(addresses),
+        address_args.Set(GRPC_ARG_ADDRESS_NAME, dns_name));
     dns_state->update.resolution_note = std::move(result.resolution_note);
   } else if (result.resolution_note.empty()) {
     dns_state->update.resolution_note =

diff --git a/test/cpp/end2end/xds/xds_cluster_type_end2end_test.cc b/test/cpp/end2end/xds/xds_cluster_type_end2end_test.cc
@@ -166,8 +166,8 @@ TEST_P(LogicalDNSClusterTest, FailedBackendConnectionCausesReresolution) {
     if (!result.status.ok()) {
       EXPECT_EQ(StatusCode::UNAVAILABLE, result.status.error_code());
       EXPECT_THAT(result.status.error_message(),
-                  MakeConnectionFailureRegex(
-                      "connections to all backends failing; last error: "));
+                  ::testing::MatchesRegex(MakeConnectionFailureRegex(
+                      "connections to all backends failing; last error: ")));
     }
   });
 }
@@ -798,8 +798,8 @@ TEST_P(AggregateClusterTest, ReconfigEdsWhileLogicalDnsChildFails) {
   }
   // When an RPC fails, we know the channel has seen the update.
   constexpr char kErrorMessage[] =
-      "empty address list: DNS resolution failed for server.example.com:443: "
-      "UNAVAILABLE: injected error";
+      "no children in weighted_target policy: DNS resolution failed for "
+      "server.example.com:443: UNAVAILABLE: injected error";
   CheckRpcSendFailure(DEBUG_LOCATION, StatusCode::UNAVAILABLE, kErrorMessage);
   // Send an EDS update that moves locality1 to priority 0.
   args1 = EdsResourceArgs({