XdsClient: change XdsApi to use absl::Status instead of grpc_error (#30426)

* change XdsApi to use absl::Status instead of grpc_error

* remove unused grpc_error_handle variable

* Automated change: Fix sanity tests

Co-authored-by: markdroth <markdroth@users.noreply.github.com>
pull/29598/head
Mark D. Roth 3 years ago committed by GitHub
parent 536b9351ec
commit 15c5a7dfb7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 22
      src/core/ext/xds/xds_api.cc
  2. 16
      src/core/ext/xds/xds_api.h
  3. 35
      src/core/ext/xds/xds_client.cc
  4. 2
      src/core/ext/xds/xds_cluster_specifier_plugin.cc

@ -52,7 +52,6 @@
#include "src/core/ext/xds/upb_utils.h"
#include "src/core/ext/xds/xds_client.h"
#include "src/core/lib/iomgr/error.h"
#include "src/core/lib/json/json.h"
// IWYU pragma: no_include "upb/msg_internal.h"
@ -271,7 +270,7 @@ std::string SerializeDiscoveryRequest(
std::string XdsApi::CreateAdsRequest(
const XdsBootstrap::XdsServer& server, absl::string_view type_url,
absl::string_view version, absl::string_view nonce,
const std::vector<std::string>& resource_names, grpc_error_handle error,
const std::vector<std::string>& resource_names, absl::Status status,
bool populate_node) {
upb::Arena arena;
const XdsEncodingContext context = {client_,
@ -300,7 +299,7 @@ std::string XdsApi::CreateAdsRequest(
}
// Set error_detail if it's a NACK.
std::string error_string_storage;
if (!GRPC_ERROR_IS_NONE(error)) {
if (!status.ok()) {
google_rpc_Status* error_detail =
envoy_service_discovery_v3_DiscoveryRequest_mutable_error_detail(
request, arena.ptr());
@ -309,12 +308,11 @@ std::string XdsApi::CreateAdsRequest(
// we could attach a status code to the individual errors where we
// generate them in the parsing code, and then use that here.
google_rpc_Status_set_code(error_detail, GRPC_STATUS_INVALID_ARGUMENT);
// Error description comes from the error that was passed in.
error_string_storage = grpc_error_std_string(error);
// Error description comes from the status that was passed in.
error_string_storage = std::string(status.message());
upb_StringView error_description =
StdStringToUpbString(error_string_storage);
google_rpc_Status_set_message(error_detail, error_description);
GRPC_ERROR_UNREF(error);
}
// Populate node.
if (populate_node) {
@ -595,10 +593,10 @@ std::string XdsApi::CreateLrsRequest(
return SerializeLrsRequest(context, request);
}
grpc_error_handle XdsApi::ParseLrsResponse(absl::string_view encoded_response,
bool* send_all_clusters,
std::set<std::string>* cluster_names,
Duration* load_reporting_interval) {
absl::Status XdsApi::ParseLrsResponse(absl::string_view encoded_response,
bool* send_all_clusters,
std::set<std::string>* cluster_names,
Duration* load_reporting_interval) {
upb::Arena arena;
// Decode the response.
const envoy_service_load_stats_v3_LoadStatsResponse* decoded_response =
@ -606,7 +604,7 @@ grpc_error_handle XdsApi::ParseLrsResponse(absl::string_view encoded_response,
encoded_response.data(), encoded_response.size(), arena.ptr());
// Parse the response.
if (decoded_response == nullptr) {
return GRPC_ERROR_CREATE_FROM_STATIC_STRING("Can't decode response.");
return absl::UnavailableError("Can't decode response.");
}
// Check send_all_clusters.
if (envoy_service_load_stats_v3_LoadStatsResponse_send_all_clusters(
@ -629,7 +627,7 @@ grpc_error_handle XdsApi::ParseLrsResponse(absl::string_view encoded_response,
*load_reporting_interval = Duration::FromSecondsAndNanoseconds(
google_protobuf_Duration_seconds(load_reporting_interval_duration),
google_protobuf_Duration_nanos(load_reporting_interval_duration));
return GRPC_ERROR_NONE;
return absl::OkStatus();
}
namespace {

@ -39,7 +39,6 @@
#include "src/core/lib/debug/trace.h"
#include "src/core/lib/gprpp/ref_counted_ptr.h"
#include "src/core/lib/gprpp/time.h"
#include "src/core/lib/iomgr/error.h"
namespace grpc_core {
@ -153,7 +152,7 @@ class XdsApi {
absl::string_view version,
absl::string_view nonce,
const std::vector<std::string>& resource_names,
grpc_error_handle error, bool populate_node);
absl::Status status, bool populate_node);
// Returns non-OK when failing to deserialize response message.
// Otherwise, all events are reported to the parser.
@ -167,13 +166,12 @@ class XdsApi {
// Creates an LRS request sending a client-side load report.
std::string CreateLrsRequest(ClusterLoadReportMap cluster_load_report_map);
// Parses the LRS response and returns \a
// load_reporting_interval for client-side load reporting. If there is any
// error, the output config is invalid.
grpc_error_handle ParseLrsResponse(absl::string_view encoded_response,
bool* send_all_clusters,
std::set<std::string>* cluster_names,
Duration* load_reporting_interval);
// Parses the LRS response and populates send_all_clusters,
// cluster_names, and load_reporting_interval.
absl::Status ParseLrsResponse(absl::string_view encoded_response,
bool* send_all_clusters,
std::set<std::string>* cluster_names,
Duration* load_reporting_interval);
// Assemble the client config proto message and return the serialized result.
std::string AssembleClientConfig(

@ -34,7 +34,6 @@
#include "absl/types/optional.h"
#include <grpc/event_engine/event_engine.h>
#include <grpc/status.h>
#include <grpc/support/log.h>
#include "src/core/ext/xds/upb_utils.h"
@ -47,7 +46,6 @@
#include "src/core/lib/gprpp/orphanable.h"
#include "src/core/lib/gprpp/ref_counted_ptr.h"
#include "src/core/lib/gprpp/sync.h"
#include "src/core/lib/iomgr/error.h"
#include "src/core/lib/uri/uri_parser.h"
#define GRPC_XDS_INITIAL_CONNECT_BACKOFF_SECONDS 1
@ -279,11 +277,9 @@ class XdsClient::ChannelState::AdsCallState
};
struct ResourceTypeState {
~ResourceTypeState() { GRPC_ERROR_UNREF(error); }
// Nonce and error for this resource type.
// Nonce and status for this resource type.
std::string nonce;
grpc_error_handle error = GRPC_ERROR_NONE;
absl::Status status;
// Subscribed resources of this type.
std::map<std::string /*authority*/,
@ -886,8 +882,7 @@ void XdsClient::ChannelState::AdsCallState::SendMessageLocked(
chand()->server_,
chand()->server_.ShouldUseV3() ? type->type_url() : type->v2_type_url(),
chand()->resource_type_version_map_[type], state.nonce,
ResourceNamesForRequest(type), GRPC_ERROR_REF(state.error),
!sent_initial_message_);
ResourceNamesForRequest(type), state.status, !sent_initial_message_);
sent_initial_message_ = true;
if (GRPC_TRACE_FLAG_ENABLED(grpc_xds_client_trace)) {
gpr_log(GPR_INFO,
@ -896,10 +891,9 @@ void XdsClient::ChannelState::AdsCallState::SendMessageLocked(
xds_client(), chand()->server_.server_uri.c_str(),
std::string(type->type_url()).c_str(),
chand()->resource_type_version_map_[type].c_str(),
state.nonce.c_str(), grpc_error_std_string(state.error).c_str());
state.nonce.c_str(), state.status.ToString().c_str());
}
GRPC_ERROR_UNREF(state.error);
state.error = GRPC_ERROR_NONE;
state.status = absl::OkStatus();
call_->SendMessage(std::move(serialized_message));
send_message_pending_ = true;
}
@ -977,18 +971,16 @@ void XdsClient::ChannelState::AdsCallState::OnRecvMessage(
state.nonce = result.nonce;
// If we got an error, set state.error so that we'll NACK the update.
if (!result.errors.empty()) {
std::string error = absl::StrJoin(result.errors, "; ");
state.status = absl::UnavailableError(
absl::StrCat("xDS response validation errors: [",
absl::StrJoin(result.errors, "; "), "]"));
gpr_log(GPR_ERROR,
"[xds_client %p] xds server %s: ADS response invalid for "
"resource "
"type %s version %s, will NACK: nonce=%s error=%s",
"type %s version %s, will NACK: nonce=%s status=%s",
xds_client(), chand()->server_.server_uri.c_str(),
result.type_url.c_str(), result.version.c_str(),
state.nonce.c_str(), error.c_str());
GRPC_ERROR_UNREF(state.error);
state.error = grpc_error_set_int(
GRPC_ERROR_CREATE_FROM_CPP_STRING(error),
GRPC_ERROR_INT_GRPC_STATUS, GRPC_STATUS_UNAVAILABLE);
state.nonce.c_str(), state.status.ToString().c_str());
}
// Delete resources not seen in update if needed.
if (result.type->AllResourcesRequiredInSotW()) {
@ -1293,15 +1285,14 @@ void XdsClient::ChannelState::LrsCallState::OnRecvMessage(
bool send_all_clusters = false;
std::set<std::string> new_cluster_names;
Duration new_load_reporting_interval;
grpc_error_handle parse_error = xds_client()->api_.ParseLrsResponse(
absl::Status status = xds_client()->api_.ParseLrsResponse(
payload, &send_all_clusters, &new_cluster_names,
&new_load_reporting_interval);
if (!GRPC_ERROR_IS_NONE(parse_error)) {
if (!status.ok()) {
gpr_log(GPR_ERROR,
"[xds_client %p] xds server %s: LRS response parsing failed: %s",
xds_client(), chand()->server_.server_uri.c_str(),
grpc_error_std_string(parse_error).c_str());
GRPC_ERROR_UNREF(parse_error);
status.ToString().c_str());
return;
}
seen_response_ = true;

@ -34,7 +34,6 @@
#include <grpc/support/log.h>
#include "src/core/ext/filters/client_channel/lb_policy_registry.h"
#include "src/core/lib/iomgr/error.h"
#include "src/core/lib/json/json.h"
#include "src/proto/grpc/lookup/v1/rls_config.upb.h"
#include "src/proto/grpc/lookup/v1/rls_config.upbdefs.h"
@ -93,7 +92,6 @@ XdsRouteLookupClusterSpecifierPlugin::GenerateLoadBalancingPolicyConfig(
Json::Array policies;
policies.emplace_back(std::move(policy));
Json lb_policy_config(std::move(policies));
grpc_error_handle parse_error = GRPC_ERROR_NONE;
// TODO(roth): If/when we ever add a second plugin, refactor this code
// somehow such that we automatically validate the resulting config against
// the gRPC LB policy registry instead of requiring each plugin to do that

Loading…
Cancel
Save