diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/cds.cc b/src/core/ext/filters/client_channel/lb_policy/xds/cds.cc index 58ce4516619..3869f81a623 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/cds.cc +++ b/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 - // policy (i.e., we have not yet received data from xds). Otherwise, - // we keep running with the data we had previously. - // TODO(roth): Once traffic splitting is implemented, this should be - // fixed to report TRANSIENT_FAILURE unconditionally. - if (parent_->child_policy_ == nullptr) { - parent_->channel_control_helper()->UpdateState( - GRPC_CHANNEL_TRANSIENT_FAILURE, - absl::make_unique( - GRPC_ERROR_CREATE_FROM_COPIED_STRING( - absl::StrCat("CDS resource \"", parent_->config_->cluster(), - "\" does not exist") - .c_str()))); - } + parent_->channel_control_helper()->UpdateState( + GRPC_CHANNEL_TRANSIENT_FAILURE, + absl::make_unique( + GRPC_ERROR_CREATE_FROM_COPIED_STRING( + absl::StrCat("CDS resource \"", parent_->config_->cluster(), + "\" does not exist") + .c_str()))); } // diff --git a/src/core/ext/filters/client_channel/lb_policy/xds/eds.cc b/src/core/ext/filters/client_channel/lb_policy/xds/eds.cc index cd49192fc4c..4521d842818 100644 --- a/src/core/ext/filters/client_channel/lb_policy/xds/eds.cc +++ b/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", - eds_policy_.get()); - // Go into TRANSIENT_FAILURE if we have not yet created the child - // policy (i.e., we have not yet received data from xds). Otherwise, - // we keep running with the data we had previously. - // TODO(roth): Once traffic splitting is implemented, this should be - // fixed to report TRANSIENT_FAILURE unconditionally. - if (eds_policy_->child_policy_ == nullptr) { - eds_policy_->channel_control_helper()->UpdateState( - GRPC_CHANNEL_TRANSIENT_FAILURE, - absl::make_unique( - GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "EDS resource does not exist"))); - } + gpr_log( + GPR_ERROR, + "[edslb %p] EDS resource does not exist -- reporting TRANSIENT_FAILURE", + eds_policy_.get()); + eds_policy_->channel_control_helper()->UpdateState( + GRPC_CHANNEL_TRANSIENT_FAILURE, + absl::make_unique( + GRPC_ERROR_CREATE_FROM_STATIC_STRING( + "EDS resource does not exist"))); } private: diff --git a/test/cpp/end2end/xds_end2end_test.cc b/test/cpp/end2end/xds_end2end_test.cc index 49a975ca2f4..66973633af1 100644 --- a/test/cpp/end2end/xds_end2end_test.cc +++ b/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. - CheckRpcSendOk(100 * num_backends_); + // Wait for RPCs to start failing. + 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);