From dfccd687f0bc1df7d8d7dd599249b098a772cf53 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Mon, 24 Apr 2023 16:21:12 -0700 Subject: [PATCH] Address review comments --- .../src/xds-stream-state/xds-stream-state.ts | 8 +++----- packages/grpc-js-xds/test/test-nack.ts | 20 +++++++++++-------- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/packages/grpc-js-xds/src/xds-stream-state/xds-stream-state.ts b/packages/grpc-js-xds/src/xds-stream-state/xds-stream-state.ts index b2d7b2279..b04adb79a 100644 --- a/packages/grpc-js-xds/src/xds-stream-state/xds-stream-state.ts +++ b/packages/grpc-js-xds/src/xds-stream-state/xds-stream-state.ts @@ -172,14 +172,13 @@ export abstract class BaseXdsStreamState implements XdsStreamState name: resourceName, raw: raw}); if (subscriptionEntry) { - const watchers = subscriptionEntry.watchers; - for (const watcher of watchers) { + for (const watcher of subscriptionEntry.watchers) { watcher.onValidUpdate(resource); } clearTimeout(subscriptionEntry.resourceTimer); subscriptionEntry.cachedResponse = resource; if (subscriptionEntry.deletionIgnored) { - experimental.log(logVerbosity.INFO, 'Received resource with previously ignored deletion: ' + resourceName); + experimental.log(logVerbosity.INFO, `Received resource with previously ignored deletion: ${resourceName}`); subscriptionEntry.deletionIgnored = false; } } @@ -191,8 +190,7 @@ export abstract class BaseXdsStreamState implements XdsStreamState error: `Validation failed for resource ${resourceName}` }); if (subscriptionEntry) { - const watchers = subscriptionEntry.watchers; - for (const watcher of watchers) { + for (const watcher of subscriptionEntry.watchers) { watcher.onTransientError({ code: status.UNAVAILABLE, details: `Validation failed for resource ${resourceName}`, diff --git a/packages/grpc-js-xds/test/test-nack.ts b/packages/grpc-js-xds/test/test-nack.ts index ad1aad448..b5bfb773e 100644 --- a/packages/grpc-js-xds/test/test-nack.ts +++ b/packages/grpc-js-xds/test/test-nack.ts @@ -38,7 +38,7 @@ describe('Validation errors', () => { xdsServer?.shutdownServer(); }); it('Should continue to use a valid resource after receiving an invalid EDS update', done => { - const cluster = new FakeCluster('cluster1', [{backends: [new Backend()], locality:{region: 'region1'}}]); + const cluster = new FakeCluster('cluster1', [{backends: [new Backend()], locality: {region: 'region1'}}]); const routeGroup = new FakeRouteGroup('route1', [{cluster: cluster}]); routeGroup.startAllBackends().then(() => { xdsServer.setEdsResource(cluster.getEndpointConfig()); @@ -49,7 +49,8 @@ describe('Validation errors', () => { client.startCalls(100); routeGroup.waitForAllBackendsToReceiveTraffic().then(() => { // After backends receive calls, set invalid EDS resource - xdsServer.setEdsResource({cluster_name: cluster.getEndpointConfig().cluster_name, endpoints: [{}]}); + const invalidEdsResource = {cluster_name: cluster.getEndpointConfig().cluster_name, endpoints: [{}]}; + xdsServer.setEdsResource(invalidEdsResource); let seenNack = false; xdsServer.addResponseListener((typeUrl, responseState) => { if (responseState.state === 'NACKED') { @@ -67,7 +68,7 @@ describe('Validation errors', () => { }, reason => done(reason)); }); it('Should continue to use a valid resource after receiving an invalid CDS update', done => { - const cluster = new FakeCluster('cluster1', [{backends: [new Backend()], locality:{region: 'region1'}}]); + const cluster = new FakeCluster('cluster1', [{backends: [new Backend()], locality: {region: 'region1'}}]); const routeGroup = new FakeRouteGroup('route1', [{cluster: cluster}]); routeGroup.startAllBackends().then(() => { xdsServer.setEdsResource(cluster.getEndpointConfig()); @@ -78,7 +79,8 @@ describe('Validation errors', () => { client.startCalls(100); routeGroup.waitForAllBackendsToReceiveTraffic().then(() => { // After backends receive calls, set invalid CDS resource - xdsServer.setCdsResource({name: cluster.getClusterConfig().name}); + const invalidCdsResource = {name: cluster.getClusterConfig().name}; + xdsServer.setCdsResource(invalidCdsResource); let seenNack = false; xdsServer.addResponseListener((typeUrl, responseState) => { if (responseState.state === 'NACKED') { @@ -96,7 +98,7 @@ describe('Validation errors', () => { }, reason => done(reason)); }); it('Should continue to use a valid resource after receiving an invalid RDS update', done => { - const cluster = new FakeCluster('cluster1', [{backends: [new Backend()], locality:{region: 'region1'}}]); + const cluster = new FakeCluster('cluster1', [{backends: [new Backend()], locality: {region: 'region1'}}]); const routeGroup = new FakeRouteGroup('route1', [{cluster: cluster}]); routeGroup.startAllBackends().then(() => { xdsServer.setEdsResource(cluster.getEndpointConfig()); @@ -107,7 +109,8 @@ describe('Validation errors', () => { client.startCalls(100); routeGroup.waitForAllBackendsToReceiveTraffic().then(() => { // After backends receive calls, set invalid RDS resource - xdsServer.setRdsResource({name: routeGroup.getRouteConfiguration().name, virtual_hosts: [{domains: ['**']}]}); + const invalidRdsResource = {name: routeGroup.getRouteConfiguration().name, virtual_hosts: [{domains: ['**']}]}; + xdsServer.setRdsResource(invalidRdsResource); let seenNack = false; xdsServer.addResponseListener((typeUrl, responseState) => { if (responseState.state === 'NACKED') { @@ -125,7 +128,7 @@ describe('Validation errors', () => { }, reason => done(reason)); }); it('Should continue to use a valid resource after receiving an invalid LDS update', done => { - const cluster = new FakeCluster('cluster1', [{backends: [new Backend()], locality:{region: 'region1'}}]); + const cluster = new FakeCluster('cluster1', [{backends: [new Backend()], locality: {region: 'region1'}}]); const routeGroup = new FakeRouteGroup('route1', [{cluster: cluster}]); routeGroup.startAllBackends().then(() => { xdsServer.setEdsResource(cluster.getEndpointConfig()); @@ -136,7 +139,8 @@ describe('Validation errors', () => { client.startCalls(100); routeGroup.waitForAllBackendsToReceiveTraffic().then(() => { // After backends receive calls, set invalid LDS resource - xdsServer.setLdsResource({name: routeGroup.getListener().name}); + const invalidLdsResource = {name: routeGroup.getListener().name}; + xdsServer.setLdsResource(invalidLdsResource); let seenNack = false; xdsServer.addResponseListener((typeUrl, responseState) => { if (responseState.state === 'NACKED') {