xds: Put channel in TRANSIENT_FAILURE when CDS resource is removed.

src/core/ext/filters/client_channel/lb_policy/xds/cds.cc

@@ -215,22 +215,17 @@ void CdsLb::ClusterWatcher::OnError(grpc_error* error) {
 }
 
 void CdsLb::ClusterWatcher::OnResourceDoesNotExist() {
-  gpr_log(GPR_ERROR, "[cdslb %p] CDS resource for %s does not exist",
+  gpr_log(GPR_ERROR,
+          "[cdslb %p] CDS resource for %s does not exist -- reporting "
+          "TRANSIENT_FAILURE",
           parent_.get(), parent_->config_->cluster().c_str());
-  // Go into TRANSIENT_FAILURE if we have not yet created the child
+  parent_->channel_control_helper()->UpdateState(
-  // policy (i.e., we have not yet received data from xds).  Otherwise,
+      GRPC_CHANNEL_TRANSIENT_FAILURE,
-  // we keep running with the data we had previously.
+      absl::make_unique<TransientFailurePicker>(
-  // TODO(roth): Once traffic splitting is implemented, this should be
+          GRPC_ERROR_CREATE_FROM_COPIED_STRING(
-  // fixed to report TRANSIENT_FAILURE unconditionally.
+              absl::StrCat("CDS resource \"", parent_->config_->cluster(),
-  if (parent_->child_policy_ == nullptr) {
+                           "\" does not exist")
-    parent_->channel_control_helper()->UpdateState(
+                  .c_str())));
-        GRPC_CHANNEL_TRANSIENT_FAILURE,
-        absl::make_unique<TransientFailurePicker>(
-            GRPC_ERROR_CREATE_FROM_COPIED_STRING(
-                absl::StrCat("CDS resource \"", parent_->config_->cluster(),
-                             "\" does not exist")
-                    .c_str())));
-  }
 }
 
 //

src/core/ext/filters/client_channel/lb_policy/xds/eds.cc

@@ -342,20 +342,15 @@ class EdsLb::EndpointWatcher : public XdsClient::EndpointWatcherInterface {
   }
 
   void OnResourceDoesNotExist() override {
-    gpr_log(GPR_ERROR, "[edslb %p] EDS resource does not exist",
+    gpr_log(
-            eds_policy_.get());
+        GPR_ERROR,
-    // Go into TRANSIENT_FAILURE if we have not yet created the child
+        "[edslb %p] EDS resource does not exist -- reporting TRANSIENT_FAILURE",
-    // policy (i.e., we have not yet received data from xds).  Otherwise,
+        eds_policy_.get());
-    // we keep running with the data we had previously.
+    eds_policy_->channel_control_helper()->UpdateState(
-    // TODO(roth): Once traffic splitting is implemented, this should be
+        GRPC_CHANNEL_TRANSIENT_FAILURE,
-    // fixed to report TRANSIENT_FAILURE unconditionally.
+        absl::make_unique<TransientFailurePicker>(
-    if (eds_policy_->child_policy_ == nullptr) {
+            GRPC_ERROR_CREATE_FROM_STATIC_STRING(
-      eds_policy_->channel_control_helper()->UpdateState(
+                "EDS resource does not exist")));
-          GRPC_CHANNEL_TRANSIENT_FAILURE,
-          absl::make_unique<TransientFailurePicker>(
-              GRPC_ERROR_CREATE_FROM_STATIC_STRING(
-                  "EDS resource does not exist")));
-    }
   }
 
  private:

test/cpp/end2end/xds_end2end_test.cc

@@ -1908,7 +1908,7 @@ TEST_P(XdsResolverOnlyTest, ListenerRemoved) {
       AdsServiceImpl::BuildEdsResource(args));
   // We need to wait for all backends to come online.
   WaitForAllBackends();
-  // Unset CDS resource.
+  // Unset LDS resource.
   balancers_[0]->ads_service()->UnsetResource(kLdsTypeUrl,
                                               kDefaultResourceName);
   // Wait for RPCs to start failing.
@@ -1921,7 +1921,7 @@ TEST_P(XdsResolverOnlyTest, ListenerRemoved) {
             AdsServiceImpl::ResponseState::ACKED);
 }
 
-// Tests that things keep workng if the cluster resource disappears.
+// Tests that we go into TRANSIENT_FAILURE if the Cluster disappears.
 TEST_P(XdsResolverOnlyTest, ClusterRemoved) {
   SetNextResolution({});
   SetNextResolutionForLbChannelAllBalancers();
@@ -1935,8 +1935,11 @@ TEST_P(XdsResolverOnlyTest, ClusterRemoved) {
   // Unset CDS resource.
   balancers_[0]->ads_service()->UnsetResource(kCdsTypeUrl,
                                               kDefaultResourceName);
-  // Make sure RPCs are still succeeding.
+  // Wait for RPCs to start failing.
-  CheckRpcSendOk(100 * num_backends_);
+  do {
+  } while (SendRpc(RpcOptions(), nullptr).ok());
+  // Make sure RPCs are still failing.
+  CheckRpcSendFailure(1000);
   // Make sure we ACK'ed the update.
   EXPECT_EQ(balancers_[0]->ads_service()->cds_response_state().state,
             AdsServiceImpl::ResponseState::ACKED);