From 29711cd526133ab3b4e4d7e728113331e920bacf Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 2 Dec 2021 18:00:07 -0500 Subject: [PATCH] grpc-js-xds: CSDS: add tracing and fix bugs --- packages/grpc-js-xds/scripts/xds.sh | 2 +- packages/grpc-js-xds/src/csds.ts | 24 ++++++++++++++++++------ packages/grpc-js-xds/src/xds-client.ts | 12 ++++++++++++ 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/packages/grpc-js-xds/scripts/xds.sh b/packages/grpc-js-xds/scripts/xds.sh index 7fc09e2b..c7a9adb6 100755 --- a/packages/grpc-js-xds/scripts/xds.sh +++ b/packages/grpc-js-xds/scripts/xds.sh @@ -48,7 +48,7 @@ git clone -b master --single-branch --depth=1 https://github.com/grpc/grpc.git grpc/tools/run_tests/helper_scripts/prep_xds.sh -GRPC_NODE_TRACE=xds_client,xds_resolver,cds_balancer,eds_balancer,priority,weighted_target,round_robin,resolving_load_balancer,subchannel,keepalive,dns_resolver,fault_injection,http_filter \ +GRPC_NODE_TRACE=xds_client,xds_resolver,cds_balancer,eds_balancer,priority,weighted_target,round_robin,resolving_load_balancer,subchannel,keepalive,dns_resolver,fault_injection,http_filter,csds \ GRPC_NODE_VERBOSITY=DEBUG \ NODE_XDS_INTEROP_VERBOSITY=1 \ python3 grpc/tools/run_tests/run_xds_tests.py \ diff --git a/packages/grpc-js-xds/src/csds.ts b/packages/grpc-js-xds/src/csds.ts index 19f1e966..40ca67f1 100644 --- a/packages/grpc-js-xds/src/csds.ts +++ b/packages/grpc-js-xds/src/csds.ts @@ -23,12 +23,18 @@ import { ClientStatusResponse } from "./generated/envoy/service/status/v3/Client import { Timestamp } from "./generated/google/protobuf/Timestamp"; import { AdsTypeUrl, CDS_TYPE_URL_V2, CDS_TYPE_URL_V3, EDS_TYPE_URL_V2, EDS_TYPE_URL_V3, LDS_TYPE_URL_V2, LDS_TYPE_URL_V3, RDS_TYPE_URL_V2, RDS_TYPE_URL_V3 } from "./resources"; import { HandleResponseResult } from "./xds-stream-state/xds-stream-state"; -import { sendUnaryData, ServerDuplexStream, ServerUnaryCall, status, experimental, loadPackageDefinition } from '@grpc/grpc-js'; +import { sendUnaryData, ServerDuplexStream, ServerUnaryCall, status, experimental, loadPackageDefinition, logVerbosity } from '@grpc/grpc-js'; import { loadSync } from "@grpc/proto-loader"; import { ProtoGrpcType as CsdsProtoGrpcType } from "./generated/csds"; import registerAdminService = experimental.registerAdminService; +const TRACER_NAME = 'csds'; + +function trace(text: string): void { + experimental.trace(logVerbosity.DEBUG, TRACER_NAME, text); +} + function dateToProtoTimestamp(date?: Date | null): Timestamp | null { if (!date) { @@ -72,7 +78,8 @@ export function setCsdsClientNode(node: Node) { * @param typeUrl The resource type URL * @param names The list of resource names that are being requested */ -export function updateRequestedNameList(typeUrl: AdsTypeUrl, names: string[]) { +export function updateCsdsRequestedNameList(typeUrl: AdsTypeUrl, names: string[]) { + trace('Update type URL ' + typeUrl + ' with names [' + names + ']'); const currentTime = dateToProtoTimestamp(new Date()); const configMap = configStatus[typeUrl]; for (const name of names) { @@ -100,12 +107,13 @@ export function updateRequestedNameList(typeUrl: AdsTypeUrl, names: string[]) { * @param versionInfo The version info field from this response * @param updates The lists of resources that passed and failed validation */ -export function updateResourceResponse(typeUrl: AdsTypeUrl, versionInfo: string, updates: HandleResponseResult) { +export function updateCsdsResourceResponse(typeUrl: AdsTypeUrl, versionInfo: string, updates: HandleResponseResult) { const currentTime = dateToProtoTimestamp(new Date()); const configMap = configStatus[typeUrl]; for (const {name, raw} of updates.accepted) { const mapEntry = configMap.get(name); if (mapEntry) { + trace('Updated ' + typeUrl + ' resource ' + name + ' to state ACKED'); mapEntry.client_status = 'ACKED'; mapEntry.version_info = versionInfo; mapEntry.xds_config = raw; @@ -116,6 +124,7 @@ export function updateResourceResponse(typeUrl: AdsTypeUrl, versionInfo: string, for (const {name, error, raw} of updates.rejected) { const mapEntry = configMap.get(name); if (mapEntry) { + trace('Updated ' + typeUrl + ' resource ' + name + ' to state NACKED'); mapEntry.client_status = 'NACKED'; mapEntry.error_state = { failed_configuration: raw, @@ -128,6 +137,7 @@ export function updateResourceResponse(typeUrl: AdsTypeUrl, versionInfo: string, for (const name of updates.missing) { const mapEntry = configMap.get(name); if (mapEntry) { + trace('Updated ' + typeUrl + ' resource ' + name + ' to state DOES_NOT_EXIST'); mapEntry.client_status = 'DOES_NOT_EXIST'; mapEntry.version_info = versionInfo; mapEntry.xds_config = null; @@ -144,16 +154,18 @@ function getCurrentConfig(): ClientConfig { genericConfigList.push(configValue); } } - return { + const config = { node: clientNode, generic_xds_configs: genericConfigList }; + trace('Sending curent config ' + JSON.stringify(config, undefined, 2)); + return config; } const csdsImplementation: ClientStatusDiscoveryServiceHandlers = { FetchClientStatus(call: ServerUnaryCall, callback: sendUnaryData) { const request = call.request; - if (request.node_matchers !== null) { + if (request.node_matchers.length > 0) { callback({ code: status.INVALID_ARGUMENT, details: 'Node matchers not supported' @@ -166,7 +178,7 @@ const csdsImplementation: ClientStatusDiscoveryServiceHandlers = { }, StreamClientStatus(call: ServerDuplexStream) { call.on('data', (request: ClientStatusRequest__Output) => { - if (request.node_matchers !== null) { + if (request.node_matchers.length > 0) { call.emit('error', { code: status.INVALID_ARGUMENT, details: 'Node matchers not supported' diff --git a/packages/grpc-js-xds/src/xds-client.ts b/packages/grpc-js-xds/src/xds-client.ts index 6c8bcb67..75a567a5 100644 --- a/packages/grpc-js-xds/src/xds-client.ts +++ b/packages/grpc-js-xds/src/xds-client.ts @@ -51,6 +51,7 @@ import { Cluster__Output } from './generated/envoy/config/cluster/v3/Cluster'; import { RouteConfiguration__Output } from './generated/envoy/config/route/v3/RouteConfiguration'; import { Duration } from './generated/google/protobuf/Duration'; import { AdsOutputType, AdsTypeUrl, CDS_TYPE_URL_V2, CDS_TYPE_URL_V3, decodeSingleResource, EDS_TYPE_URL_V2, EDS_TYPE_URL_V3, LDS_TYPE_URL_V2, LDS_TYPE_URL_V3, RDS_TYPE_URL_V2, RDS_TYPE_URL_V3 } from './resources'; +import { setCsdsClientNode, updateCsdsRequestedNameList, updateCsdsResourceResponse } from './csds'; const TRACER_NAME = 'xds_client'; @@ -384,6 +385,7 @@ export class XdsClient { ...nodeV3, client_features: ['envoy.lrs.supports_send_all_clusters'], }; + setCsdsClientNode(this.adsNodeV3); if (this.apiVersion === XdsApiVersion.V2) { trace('ADS Node: ' + JSON.stringify(this.adsNodeV2, undefined, 2)); trace('LRS Node: ' + JSON.stringify(this.lrsNodeV2, undefined, 2)); @@ -514,12 +516,14 @@ export class XdsClient { } catch (e) { trace('Nacking message with protobuf parsing error: ' + e.message); this.nack(message.type_url, e.message); + return; } if (handleResponseResult === null) { // Null handleResponseResult means that the type_url was unrecognized trace('Nacking message with unknown type URL ' + message.type_url); this.nack(message.type_url, `Unknown type_url ${message.type_url}`); } else { + updateCsdsResourceResponse(message.type_url as AdsTypeUrl, message.version_info, handleResponseResult.result); if (handleResponseResult.result.rejected.length > 0) { // rejected.length > 0 means that at least one message validation failed const errorString = `${handleResponseResult.serviceKind.toUpperCase()} Error: ${handleResponseResult.result.rejected[0].error}`; @@ -754,8 +758,16 @@ export class XdsClient { } this.maybeStartAdsStream(); this.maybeStartLrsStream(); + if (!this.adsCallV2 && !this.adsCallV3) { + /* If the stream is not set up yet at this point, shortcut the rest + * becuase nothing will actually be sent. This would mainly happen if + * the bootstrap file has not been read yet. In that case, the output + * of getTypeUrl is garbage and everything after that is invalid. */ + return; + } trace('Sending update for ' + serviceKind + ' with names ' + this.adsState[serviceKind].getResourceNames()); const typeUrl = this.getTypeUrl(serviceKind); + updateCsdsRequestedNameList(typeUrl, this.adsState[serviceKind].getResourceNames()); this.maybeSendAdsMessage(typeUrl, this.adsState[serviceKind].getResourceNames(), this.adsState[serviceKind].nonce, this.adsState[serviceKind].versionInfo); }