Merge pull request #1707 from murgatroid99/grpc-js-xds_lrs_server_nam…

…e_fix grpc-js-xds: Fix handling of empty LRS server names
grpc · Mar 5, 2021 · 5b362be · 5b362be
2 parents 6f08692 + 3107419
commit 5b362be
Show file tree

Hide file tree

Showing 7 changed files with 9 additions and 21 deletions.
diff --git a/packages/grpc-js-xds/package.json b/packages/grpc-js-xds/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@grpc/grpc-js-xds",
-  "version": "1.2.2",
+  "version": "1.2.3",
   "description": "Plugin for @grpc/grpc-js. Adds the xds:// URL scheme and associated features.",
   "main": "build/src/index.js",
   "scripts": {
@@ -45,7 +45,7 @@
     "@grpc/proto-loader": "^0.6.0-pre14"
   },
   "peerDependencies": {
-    "@grpc/grpc-js": "~1.2.7"
+    "@grpc/grpc-js": "~1.2.10"
   },
   "engines": {
     "node": ">=10.10.0"

diff --git a/packages/grpc-js-xds/scripts/xds.sh b/packages/grpc-js-xds/scripts/xds.sh
@@ -52,7 +52,7 @@ GRPC_NODE_TRACE=xds_client,xds_resolver,cds_balancer,eds_balancer,priority,weigh
   GRPC_NODE_VERBOSITY=DEBUG \
   NODE_XDS_INTEROP_VERBOSITY=1 \
   python3 grpc/tools/run_tests/run_xds_tests.py \
-    --test_case="backends_restart,change_backend_service,gentle_failover,ping_pong,remove_instance_group,round_robin,secondary_locality_gets_no_requests_on_partial_primary_failure,secondary_locality_gets_requests_on_primary_failure" \
+    --test_case="backends_restart,change_backend_service,gentle_failover,ping_pong,remove_instance_group,round_robin,secondary_locality_gets_no_requests_on_partial_primary_failure,secondary_locality_gets_requests_on_primary_failure,load_report_based_failover" \
     --project_id=grpc-testing \
     --source_image=projects/grpc-testing/global/images/xds-test-server-2 \
     --path_to_server_binary=/java_server/grpc-java/interop-testing/build/install/grpc-interop-testing/bin/xds-test-server \

diff --git a/packages/grpc-js-xds/src/load-balancer-eds.ts b/packages/grpc-js-xds/src/load-balancer-eds.ts
@@ -377,7 +377,7 @@ export class EdsLoadBalancer implements LoadBalancer {
           validateLoadBalancingConfig({ round_robin: {} }),
         ];
         let childPolicy: LoadBalancingConfig[];
-        if (this.lastestConfig.getLrsLoadReportingServerName()) {
+        if (this.lastestConfig.getLrsLoadReportingServerName() !== undefined) {
           childPolicy = [new LrsLoadBalancingConfig(this.lastestConfig.getCluster(), this.lastestConfig.getEdsServiceName() ?? '', this.lastestConfig.getLrsLoadReportingServerName()!, localityObj.locality, endpointPickingPolicy)];
         } else {
           childPolicy = endpointPickingPolicy;

diff --git a/packages/grpc-js-xds/src/load-balancer-lrs.ts b/packages/grpc-js-xds/src/load-balancer-lrs.ts
@@ -16,7 +16,6 @@
  */
 
 import { connectivityState as ConnectivityState, StatusObject, status as Status, experimental } from '@grpc/grpc-js';
-import { type } from 'os';
 import { Locality__Output } from './generated/envoy/api/v2/core/Locality';
 import { XdsClusterLocalityStats, XdsClient } from './xds-client';
 import LoadBalancer = experimental.LoadBalancer;

diff --git a/packages/grpc-js-xds/src/xds-client.ts b/packages/grpc-js-xds/src/xds-client.ts
@@ -1068,7 +1068,6 @@ export class XdsClient {
     if (!this.lrsCall) {
       return;
     }
-    trace('Sending LRS stats');
     const clusterStats: ClusterStats[] = [];
     for (const [
       { clusterName, edsServiceName },
@@ -1129,6 +1128,7 @@ export class XdsClient {
         }
       }
     }
+    trace('Sending LRS stats ' + JSON.stringify(clusterStats, undefined, 2));
     this.lrsCall.write({
       node: this.lrsNode!,
       cluster_stats: clusterStats,
@@ -1174,6 +1174,7 @@ export class XdsClient {
     clusterName: string,
     edsServiceName: string
   ): XdsClusterDropStats {
+    trace('addClusterDropStats(lrsServer=' + lrsServer + ', clusterName=' + clusterName + ', edsServiceName=' + edsServiceName + ')');
     if (lrsServer !== '') {
       return {
         addCallDropped: (category) => {},
@@ -1197,6 +1198,7 @@ export class XdsClient {
     edsServiceName: string,
     locality: Locality__Output
   ): XdsClusterLocalityStats {
+    trace('addClusterLocalityStats(lrsServer=' + lrsServer + ', clusterName=' + clusterName + ', edsServiceName=' + edsServiceName + ', locality=' + JSON.stringify(locality) + ')');
     if (lrsServer !== '') {
       return {
         addCallStarted: () => {},

diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@grpc/grpc-js",
-  "version": "1.2.9",
+  "version": "1.2.10",
   "description": "gRPC Library for Node - pure JS implementation",
   "homepage": "https://grpc.io/",
   "repository": "https://github.com/grpc/grpc-node/tree/master/packages/grpc-js",

diff --git a/packages/grpc-js/src/call-stream.ts b/packages/grpc-js/src/call-stream.ts
@@ -442,21 +442,8 @@ export class Http2CallStream implements Call {
       );
     }
     const status: StatusObject = { code, details, metadata };
-    let finalStatus;
-    try {
-      // Attempt to assign final status.
-      finalStatus = this.filterStack.receiveTrailers(status);
-    } catch (error) {
-      // This is a no-op if the call was already ended when handling headers.
-      this.endCall({
-        code: Status.INTERNAL,
-        details: 'Failed to process received status',
-        metadata: new Metadata(),
-      });
-      return;
-    }
     // This is a no-op if the call was already ended when handling headers.
-    this.endCall(finalStatus);
+    this.endCall(status);
   }
 
   attachHttp2Stream(