Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
murgatroid99 committed Apr 24, 2023
1 parent 48ef1ed commit dfccd68
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 13 deletions.
8 changes: 3 additions & 5 deletions packages/grpc-js-xds/src/xds-stream-state/xds-stream-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,14 +172,13 @@ export abstract class BaseXdsStreamState<ResponseType> 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;
}
}
Expand All @@ -191,8 +190,7 @@ export abstract class BaseXdsStreamState<ResponseType> 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}`,
Expand Down
20 changes: 12 additions & 8 deletions packages/grpc-js-xds/test/test-nack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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') {
Expand All @@ -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());
Expand All @@ -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') {
Expand All @@ -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());
Expand All @@ -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') {
Expand All @@ -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());
Expand All @@ -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') {
Expand Down

0 comments on commit dfccd68

Please sign in to comment.