From c80b25e2a52f383a7f6eaf2f09548069ab29623a Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Wed, 6 Apr 2022 17:15:22 -0700 Subject: [PATCH 01/56] grpc-js: Use real channelz IDs when channelz is disabled --- packages/grpc-js/src/channel.ts | 9 +-- packages/grpc-js/src/channelz.ts | 24 ++++-- packages/grpc-js/src/server.ts | 116 +++++++++++------------------ packages/grpc-js/src/subchannel.ts | 11 +-- 4 files changed, 64 insertions(+), 96 deletions(-) diff --git a/packages/grpc-js/src/channel.ts b/packages/grpc-js/src/channel.ts index 635b52d6..3d014607 100644 --- a/packages/grpc-js/src/channel.ts +++ b/packages/grpc-js/src/channel.ts @@ -219,16 +219,9 @@ export class ChannelImplementation implements Channel { } this.channelzTrace = new ChannelzTrace(); + this.channelzRef = registerChannelzChannel(target, () => this.getChannelzInfo(), this.channelzEnabled); if (this.channelzEnabled) { - this.channelzRef = registerChannelzChannel(target, () => this.getChannelzInfo()); this.channelzTrace.addTrace('CT_INFO', 'Channel created'); - } else { - // Dummy channelz ref that will never be used - this.channelzRef = { - kind: 'channel', - id: -1, - name: '' - }; } if (this.options['grpc.default_authority']) { diff --git a/packages/grpc-js/src/channelz.ts b/packages/grpc-js/src/channelz.ts index 14c94fd0..5a7a5476 100644 --- a/packages/grpc-js/src/channelz.ts +++ b/packages/grpc-js/src/channelz.ts @@ -347,31 +347,39 @@ const subchannels: (SubchannelEntry | undefined)[] = []; const servers: (ServerEntry | undefined)[] = []; const sockets: (SocketEntry | undefined)[] = []; -export function registerChannelzChannel(name: string, getInfo: () => ChannelInfo): ChannelRef { +export function registerChannelzChannel(name: string, getInfo: () => ChannelInfo, channelzEnabled: boolean): ChannelRef { const id = getNextId(); const ref: ChannelRef = {id, name, kind: 'channel'}; - channels[id] = { ref, getInfo }; + if (channelzEnabled) { + channels[id] = { ref, getInfo }; + } return ref; } -export function registerChannelzSubchannel(name: string, getInfo:() => SubchannelInfo): SubchannelRef { +export function registerChannelzSubchannel(name: string, getInfo:() => SubchannelInfo, channelzEnabled: boolean): SubchannelRef { const id = getNextId(); const ref: SubchannelRef = {id, name, kind: 'subchannel'}; - subchannels[id] = { ref, getInfo }; + if (channelzEnabled) { + subchannels[id] = { ref, getInfo }; + } return ref; } -export function registerChannelzServer(getInfo: () => ServerInfo): ServerRef { +export function registerChannelzServer(getInfo: () => ServerInfo, channelzEnabled: boolean): ServerRef { const id = getNextId(); const ref: ServerRef = {id, kind: 'server'}; - servers[id] = { ref, getInfo }; + if (channelzEnabled) { + servers[id] = { ref, getInfo }; + } return ref; } -export function registerChannelzSocket(name: string, getInfo: () => SocketInfo): SocketRef { +export function registerChannelzSocket(name: string, getInfo: () => SocketInfo, channelzEnabled: boolean): SocketRef { const id = getNextId(); const ref: SocketRef = {id, name, kind: 'socket'}; - sockets[id] = { ref, getInfo}; + if (channelzEnabled) { + sockets[id] = { ref, getInfo}; + } return ref; } diff --git a/packages/grpc-js/src/server.ts b/packages/grpc-js/src/server.ts index 829941fa..974c3dfc 100644 --- a/packages/grpc-js/src/server.ts +++ b/packages/grpc-js/src/server.ts @@ -161,17 +161,11 @@ export class Server { if (this.options['grpc.enable_channelz'] === 0) { this.channelzEnabled = false; } + this.channelzRef = registerChannelzServer(() => this.getChannelzInfo(), this.channelzEnabled); if (this.channelzEnabled) { - this.channelzRef = registerChannelzServer(() => this.getChannelzInfo()); this.channelzTrace.addTrace('CT_INFO', 'Server created'); - this.trace('Server constructed'); - } else { - // Dummy channelz ref that will never be used - this.channelzRef = { - kind: 'server', - id: -1 - }; } + this.trace('Server constructed'); } private getChannelzInfo(): ServerInfo { @@ -431,34 +425,28 @@ export class Server { } } let channelzRef: SocketRef; - if (this.channelzEnabled) { - channelzRef = registerChannelzSocket(subchannelAddressToString(boundSubchannelAddress), () => { - return { - localAddress: boundSubchannelAddress, - remoteAddress: null, - security: null, - remoteName: null, - streamsStarted: 0, - streamsSucceeded: 0, - streamsFailed: 0, - messagesSent: 0, - messagesReceived: 0, - keepAlivesSent: 0, - lastLocalStreamCreatedTimestamp: null, - lastRemoteStreamCreatedTimestamp: null, - lastMessageSentTimestamp: null, - lastMessageReceivedTimestamp: null, - localFlowControlWindow: null, - remoteFlowControlWindow: null - }; - }); - this.listenerChildrenTracker.refChild(channelzRef); - } else { - channelzRef = { - kind: 'socket', - id: -1, - name: '' + channelzRef = registerChannelzSocket(subchannelAddressToString(boundSubchannelAddress), () => { + return { + localAddress: boundSubchannelAddress, + remoteAddress: null, + security: null, + remoteName: null, + streamsStarted: 0, + streamsSucceeded: 0, + streamsFailed: 0, + messagesSent: 0, + messagesReceived: 0, + keepAlivesSent: 0, + lastLocalStreamCreatedTimestamp: null, + lastRemoteStreamCreatedTimestamp: null, + lastMessageSentTimestamp: null, + lastMessageReceivedTimestamp: null, + localFlowControlWindow: null, + remoteFlowControlWindow: null }; + }, this.channelzEnabled); + if (this.channelzEnabled) { + this.listenerChildrenTracker.refChild(channelzRef); } this.http2ServerList.push({server: http2Server, channelzRef: channelzRef}); this.trace('Successfully bound ' + subchannelAddressToString(boundSubchannelAddress)); @@ -509,34 +497,28 @@ export class Server { port: boundAddress.port }; let channelzRef: SocketRef; - if (this.channelzEnabled) { - channelzRef = registerChannelzSocket(subchannelAddressToString(boundSubchannelAddress), () => { - return { - localAddress: boundSubchannelAddress, - remoteAddress: null, - security: null, - remoteName: null, - streamsStarted: 0, - streamsSucceeded: 0, - streamsFailed: 0, - messagesSent: 0, - messagesReceived: 0, - keepAlivesSent: 0, - lastLocalStreamCreatedTimestamp: null, - lastRemoteStreamCreatedTimestamp: null, - lastMessageSentTimestamp: null, - lastMessageReceivedTimestamp: null, - localFlowControlWindow: null, - remoteFlowControlWindow: null - }; - }); - this.listenerChildrenTracker.refChild(channelzRef); - } else { - channelzRef = { - kind: 'socket', - id: -1, - name: '' + channelzRef = registerChannelzSocket(subchannelAddressToString(boundSubchannelAddress), () => { + return { + localAddress: boundSubchannelAddress, + remoteAddress: null, + security: null, + remoteName: null, + streamsStarted: 0, + streamsSucceeded: 0, + streamsFailed: 0, + messagesSent: 0, + messagesReceived: 0, + keepAlivesSent: 0, + lastLocalStreamCreatedTimestamp: null, + lastRemoteStreamCreatedTimestamp: null, + lastMessageSentTimestamp: null, + lastMessageReceivedTimestamp: null, + localFlowControlWindow: null, + remoteFlowControlWindow: null }; + }, this.channelzEnabled); + if (this.channelzEnabled) { + this.listenerChildrenTracker.refChild(channelzRef); } this.http2ServerList.push({server: http2Server, channelzRef: channelzRef}); this.trace('Successfully bound ' + subchannelAddressToString(boundSubchannelAddress)); @@ -893,15 +875,7 @@ export class Server { } let channelzRef: SocketRef; - if (this.channelzEnabled) { - channelzRef = registerChannelzSocket(session.socket.remoteAddress ?? 'unknown', this.getChannelzSessionInfoGetter(session)); - } else { - channelzRef = { - kind: 'socket', - id: -1, - name: '' - } - } + channelzRef = registerChannelzSocket(session.socket.remoteAddress ?? 'unknown', this.getChannelzSessionInfoGetter(session), this.channelzEnabled); const channelzSessionInfo: ChannelzSessionInfo = { ref: channelzRef, diff --git a/packages/grpc-js/src/subchannel.ts b/packages/grpc-js/src/subchannel.ts index 800274f9..5ef06b7b 100644 --- a/packages/grpc-js/src/subchannel.ts +++ b/packages/grpc-js/src/subchannel.ts @@ -227,16 +227,9 @@ export class Subchannel { this.channelzEnabled = false; } this.channelzTrace = new ChannelzTrace(); + this.channelzRef = registerChannelzSubchannel(this.subchannelAddressString, () => this.getChannelzInfo(), this.channelzEnabled); if (this.channelzEnabled) { - this.channelzRef = registerChannelzSubchannel(this.subchannelAddressString, () => this.getChannelzInfo()); this.channelzTrace.addTrace('CT_INFO', 'Subchannel created'); - } else { - // Dummy channelz ref that will never be used - this.channelzRef = { - kind: 'subchannel', - id: -1, - name: '' - }; } this.trace('Subchannel constructed with options ' + JSON.stringify(options, undefined, 2)); } @@ -484,8 +477,8 @@ export class Subchannel { connectionOptions ); this.session = session; + this.channelzSocketRef = registerChannelzSocket(this.subchannelAddressString, () => this.getChannelzSocketInfo()!, this.channelzEnabled); if (this.channelzEnabled) { - this.channelzSocketRef = registerChannelzSocket(this.subchannelAddressString, () => this.getChannelzSocketInfo()!); this.childrenTracker.refChild(this.channelzSocketRef); } session.unref(); From 12c58c29237ea48f9278fa3d01f4889a1f558bc1 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 7 Apr 2022 17:57:18 -0700 Subject: [PATCH 02/56] grpc-js: Disable per-session memory limit by default --- packages/grpc-js/package.json | 2 +- packages/grpc-js/src/subchannel.ts | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index bb8adff8..9fab8d83 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js", - "version": "1.6.2", + "version": "1.6.3", "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/subchannel.ts b/packages/grpc-js/src/subchannel.ts index 800274f9..474a0556 100644 --- a/packages/grpc-js/src/subchannel.ts +++ b/packages/grpc-js/src/subchannel.ts @@ -407,6 +407,12 @@ export class Subchannel { connectionOptions.maxSessionMemory = this.options[ 'grpc-node.max_session_memory' ]; + } else { + /* By default, set a very large max session memory limit, to effectively + * disable enforcement of the limit. Some testing indicates that Node's + * behavior degrades badly when this limit is reached, so we solve that + * by disabling the check entirely. */ + connectionOptions.maxSessionMemory = Number.MAX_SAFE_INTEGER; } let addressScheme = 'http://'; if ('secureContext' in connectionOptions) { From 7ac345e4dc7c5122b9a4707613f8a9aa506f800b Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Fri, 8 Apr 2022 10:13:46 -0700 Subject: [PATCH 03/56] grpc-js: Add more details to keepalive ping tracing --- packages/grpc-js/src/subchannel.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/grpc-js/src/subchannel.ts b/packages/grpc-js/src/subchannel.ts index 800274f9..58f995d7 100644 --- a/packages/grpc-js/src/subchannel.ts +++ b/packages/grpc-js/src/subchannel.ts @@ -328,6 +328,10 @@ export class Subchannel { logging.trace(LogVerbosity.DEBUG, 'subchannel_internals', '(' + this.channelzRef.id + ') ' + this.subchannelAddressString + ' ' + text); } + private keepaliveTrace(text: string): void { + logging.trace(LogVerbosity.DEBUG, 'keepalive', '(' + this.channelzRef.id + ') ' + this.subchannelAddressString + ' ' + text); + } + private handleBackoffTimer() { if (this.continueConnecting) { this.transitionToState( @@ -358,18 +362,15 @@ export class Subchannel { if (this.channelzEnabled) { this.keepalivesSent += 1; } - logging.trace( - LogVerbosity.DEBUG, - 'keepalive', - '(' + this.channelzRef.id + ') ' + this.subchannelAddressString + ' ' + - 'Sending ping' - ); + this.keepaliveTrace('Sending ping with timeout ' + this.keepaliveTimeoutMs + 'ms'); this.keepaliveTimeoutId = setTimeout(() => { + this.keepaliveTrace('Ping timeout passed without response'); this.transitionToState([ConnectivityState.READY], ConnectivityState.IDLE); }, this.keepaliveTimeoutMs); this.keepaliveTimeoutId.unref?.(); this.session!.ping( (err: Error | null, duration: number, payload: Buffer) => { + this.keepaliveTrace('Received ping response'); clearTimeout(this.keepaliveTimeoutId); } ); From abbaf13c62fa02da001a5d5598bf3cdf1a86563a Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Fri, 8 Apr 2022 10:25:15 -0700 Subject: [PATCH 04/56] grpc-js-xds: Don't stop backoff timers for LRS streams --- packages/grpc-js-xds/src/xds-client.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/grpc-js-xds/src/xds-client.ts b/packages/grpc-js-xds/src/xds-client.ts index 7a12af1f..bdc12395 100644 --- a/packages/grpc-js-xds/src/xds-client.ts +++ b/packages/grpc-js-xds/src/xds-client.ts @@ -781,7 +781,6 @@ export class XdsClient { trace('Received LRS response'); /* Once we get any response from the server, we assume that the stream is * in a good state, so we can reset the backoff timer. */ - this.lrsBackoff.stop(); this.lrsBackoff.reset(); if ( !this.receivedLrsSettingsForCurrentStream || From ae93d556ec44cd372fff78e69c674904ba63b223 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Fri, 8 Apr 2022 11:23:00 -0700 Subject: [PATCH 05/56] grpc-js: Don't clear ping timeout when still connected --- packages/grpc-js/src/subchannel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grpc-js/src/subchannel.ts b/packages/grpc-js/src/subchannel.ts index 800274f9..22c3363c 100644 --- a/packages/grpc-js/src/subchannel.ts +++ b/packages/grpc-js/src/subchannel.ts @@ -773,7 +773,7 @@ export class Subchannel { } this.backoffTimeout.unref(); if (!this.keepaliveWithoutCalls) { - this.stopKeepalivePings(); + clearInterval(this.keepaliveIntervalId); } this.checkBothRefcounts(); } From 57d7827ab8341ca90c4c512a371a212760a41aaf Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Mon, 11 Apr 2022 10:12:24 -0700 Subject: [PATCH 06/56] grpc-js-xds: Include Node ID in XdsClient status errors --- packages/grpc-js-xds/src/xds-client.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/grpc-js-xds/src/xds-client.ts b/packages/grpc-js-xds/src/xds-client.ts index 7a12af1f..2eabc586 100644 --- a/packages/grpc-js-xds/src/xds-client.ts +++ b/packages/grpc-js-xds/src/xds-client.ts @@ -726,7 +726,7 @@ export class XdsClient { if (serviceKind) { this.adsState[serviceKind].reportStreamError({ code: status.UNAVAILABLE, - details: message, + details: message + ' Node ID=' + this.adsNodeV3!.id, metadata: new Metadata() }); resourceNames = this.adsState[serviceKind].getResourceNames(); @@ -771,6 +771,7 @@ export class XdsClient { } private reportStreamError(status: StatusObject) { + status = {...status, details: status.details + ' Node ID=' + this.adsNodeV3!.id}; this.adsState.eds.reportStreamError(status); this.adsState.cds.reportStreamError(status); this.adsState.rds.reportStreamError(status); From 1e1f73236387bbce8dd216317643b739ad464fa8 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Mon, 11 Apr 2022 10:42:42 -0700 Subject: [PATCH 07/56] grpc-js-xds: Reject EDS updates with duplicate locality/priority pairs --- .../grpc-js-xds/src/xds-stream-state/eds-state.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/packages/grpc-js-xds/src/xds-stream-state/eds-state.ts b/packages/grpc-js-xds/src/xds-stream-state/eds-state.ts index fe9f3c62..a050b44c 100644 --- a/packages/grpc-js-xds/src/xds-stream-state/eds-state.ts +++ b/packages/grpc-js-xds/src/xds-stream-state/eds-state.ts @@ -17,6 +17,7 @@ import { experimental, logVerbosity, StatusObject } from "@grpc/grpc-js"; import { isIPv4, isIPv6 } from "net"; +import { Locality__Output } from "../generated/envoy/config/core/v3/Locality"; import { ClusterLoadAssignment__Output } from "../generated/envoy/config/endpoint/v3/ClusterLoadAssignment"; import { Any__Output } from "../generated/google/protobuf/Any"; import { HandleResponseResult, RejectedResourceEntry, ResourcePair, Watcher, XdsStreamState } from "./xds-stream-state"; @@ -27,6 +28,10 @@ function trace(text: string): void { experimental.trace(logVerbosity.DEBUG, TRACER_NAME, text); } +function localitiesEqual(a: Locality__Output, b: Locality__Output) { + return a.region === b.region && a.sub_zone === b.sub_zone && a.zone === b.zone; +} + export class EdsState implements XdsStreamState { public versionInfo = ''; public nonce = ''; @@ -112,7 +117,17 @@ export class EdsState implements XdsStreamState { * @param message */ private validateResponse(message: ClusterLoadAssignment__Output) { + const seenLocalities: {locality: Locality__Output, priority: number}[] = []; for (const endpoint of message.endpoints) { + if (!endpoint.locality) { + return false; + } + for (const {locality, priority} of seenLocalities) { + if (localitiesEqual(endpoint.locality, locality) && endpoint.priority === priority) { + return false; + } + } + seenLocalities.push({locality: endpoint.locality, priority: endpoint.priority}); for (const lb of endpoint.lb_endpoints) { const socketAddress = lb.endpoint?.address?.socket_address; if (!socketAddress) { From 553fb7a819cdb555d03b44c24ca4750a0d85ac31 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Mon, 11 Apr 2022 14:43:18 -0700 Subject: [PATCH 08/56] grpc-js: Add comment about stopKeepalivePings usage --- packages/grpc-js/src/subchannel.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/grpc-js/src/subchannel.ts b/packages/grpc-js/src/subchannel.ts index 22c3363c..39ccc01b 100644 --- a/packages/grpc-js/src/subchannel.ts +++ b/packages/grpc-js/src/subchannel.ts @@ -384,6 +384,11 @@ export class Subchannel { * sending pings should also involve some network activity. */ } + /** + * Stop keepalive pings when terminating a connection. This discards the + * outstanding ping timeout, so it should not be called if the same + * connection will still be used. + */ private stopKeepalivePings() { clearInterval(this.keepaliveIntervalId); clearTimeout(this.keepaliveTimeoutId); From 6c686772cbee628651910dbbd1a8da5b488deeda Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Tue, 12 Apr 2022 16:16:57 -0700 Subject: [PATCH 09/56] grpc-js: Fix handling of calls after resolution failure --- packages/grpc-js/src/channel.ts | 30 +++++++++++++++---- .../grpc-js/src/resolving-load-balancer.ts | 5 ++-- packages/grpc-js/test/test-client.ts | 24 +++++++++++++++ 3 files changed, 51 insertions(+), 8 deletions(-) diff --git a/packages/grpc-js/src/channel.ts b/packages/grpc-js/src/channel.ts index 3d014607..88bf3a7e 100644 --- a/packages/grpc-js/src/channel.ts +++ b/packages/grpc-js/src/channel.ts @@ -20,6 +20,7 @@ import { Call, Http2CallStream, CallStreamOptions, + StatusObject, } from './call-stream'; import { ChannelCredentials } from './channel-credentials'; import { ChannelOptions } from './channel-options'; @@ -170,6 +171,14 @@ export class ChannelImplementation implements Channel { */ private callRefTimer: NodeJS.Timer; private configSelector: ConfigSelector | null = null; + /** + * This is the error from the name resolver if it failed most recently. It + * is only used to end calls that start while there is no config selector + * and the name resolver is in backoff, so it should be nulled if + * configSelector becomes set or the channel state becomes anything other + * than TRANSIENT_FAILURE. + */ + private currentResolutionError: StatusObject | null = null; // Channelz info private readonly channelzEnabled: boolean = true; @@ -290,6 +299,7 @@ export class ChannelImplementation implements Channel { this.channelzTrace.addTrace('CT_INFO', 'Address resolution succeeded'); } this.configSelector = configSelector; + this.currentResolutionError = null; /* We process the queue asynchronously to ensure that the corresponding * load balancer update has completed. */ process.nextTick(() => { @@ -309,6 +319,9 @@ export class ChannelImplementation implements Channel { if (this.configSelectionQueue.length > 0) { this.trace('Name resolution failed with calls queued for config selection'); } + if (this.configSelector === null) { + this.currentResolutionError = status; + } const localQueue = this.configSelectionQueue; this.configSelectionQueue = []; this.callRefTimerUnref(); @@ -591,6 +604,9 @@ export class ChannelImplementation implements Channel { watcherObject.callback(); } } + if (newState !== ConnectivityState.TRANSIENT_FAILURE) { + this.currentResolutionError = null; + } } private tryGetConfig(stream: Http2CallStream, metadata: Metadata) { @@ -605,11 +621,15 @@ export class ChannelImplementation implements Channel { * ResolvingLoadBalancer may be idle and if so it needs to be kicked * because it now has a pending request. */ this.resolvingLoadBalancer.exitIdle(); - this.configSelectionQueue.push({ - callStream: stream, - callMetadata: metadata, - }); - this.callRefTimerRef(); + if (this.currentResolutionError && !metadata.getOptions().waitForReady) { + stream.cancelWithStatus(this.currentResolutionError.code, this.currentResolutionError.details); + } else { + this.configSelectionQueue.push({ + callStream: stream, + callMetadata: metadata, + }); + this.callRefTimerRef(); + } } else { const callConfig = this.configSelector(stream.getMethod(), metadata); if (callConfig.status === Status.OK) { diff --git a/packages/grpc-js/src/resolving-load-balancer.ts b/packages/grpc-js/src/resolving-load-balancer.ts index 907067df..7b9f92cb 100644 --- a/packages/grpc-js/src/resolving-load-balancer.ts +++ b/packages/grpc-js/src/resolving-load-balancer.ts @@ -268,6 +268,7 @@ export class ResolvingLoadBalancer implements LoadBalancer { if (this.currentState === ConnectivityState.IDLE) { this.updateState(ConnectivityState.CONNECTING, new QueuePicker(this)); } + this.backoffTimeout.runOnce(); } private updateState(connectivityState: ConnectivityState, picker: Picker) { @@ -294,18 +295,16 @@ export class ResolvingLoadBalancer implements LoadBalancer { ); this.onFailedResolution(error); } - this.backoffTimeout.runOnce(); } exitIdle() { this.childLoadBalancer.exitIdle(); - if (this.currentState === ConnectivityState.IDLE) { + if (this.currentState === ConnectivityState.IDLE || this.currentState === ConnectivityState.TRANSIENT_FAILURE) { if (this.backoffTimeout.isRunning()) { this.continueResolving = true; } else { this.updateResolution(); } - this.updateState(ConnectivityState.CONNECTING, new QueuePicker(this)); } } diff --git a/packages/grpc-js/test/test-client.ts b/packages/grpc-js/test/test-client.ts index 4a02ff3d..21dad99f 100644 --- a/packages/grpc-js/test/test-client.ts +++ b/packages/grpc-js/test/test-client.ts @@ -92,4 +92,28 @@ describe('Client without a server', () => { }); }); }); +}); + +describe('Client with a nonexistent target domain', () => { + let client: Client; + before(() => { + // DNS name that does not exist per RFC 6761 section 6.4 + client = new Client('host.invalid', clientInsecureCreds); + }); + after(() => { + client.close(); + }); + it('should fail multiple calls', function(done) { + this.timeout(5000); + // Regression test for https://github.com/grpc/grpc-node/issues/1411 + client.makeUnaryRequest('/service/method', x => x, x => x, Buffer.from([]), (error, value) => { + assert(error); + assert.strictEqual(error?.code, grpc.status.UNAVAILABLE); + client.makeUnaryRequest('/service/method', x => x, x => x, Buffer.from([]), (error, value) => { + assert(error); + assert.strictEqual(error?.code, grpc.status.UNAVAILABLE); + done(); + }); + }); + }); }); \ No newline at end of file From c112d167bb0af504c4a1121d2f6f28c9454d3e4d Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Wed, 13 Apr 2022 11:27:31 -0700 Subject: [PATCH 10/56] grpc-js: Update version to 1.6.4 --- packages/grpc-js/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index 9fab8d83..408168e9 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js", - "version": "1.6.3", + "version": "1.6.4", "description": "gRPC Library for Node - pure JS implementation", "homepage": "https://grpc.io/", "repository": "https://github.com/grpc/grpc-node/tree/master/packages/grpc-js", From 478900d191d8f4e6c2e57e9ff732ad974253ceef Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 14 Apr 2022 16:51:16 -0700 Subject: [PATCH 11/56] grpc-js: Consistently re-resolve when idle --- packages/grpc-js/package.json | 2 +- packages/grpc-js/src/resolving-load-balancer.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index 408168e9..09a88b5a 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js", - "version": "1.6.4", + "version": "1.6.5", "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/resolving-load-balancer.ts b/packages/grpc-js/src/resolving-load-balancer.ts index 7b9f92cb..985b6e31 100644 --- a/packages/grpc-js/src/resolving-load-balancer.ts +++ b/packages/grpc-js/src/resolving-load-balancer.ts @@ -298,7 +298,6 @@ export class ResolvingLoadBalancer implements LoadBalancer { } exitIdle() { - this.childLoadBalancer.exitIdle(); if (this.currentState === ConnectivityState.IDLE || this.currentState === ConnectivityState.TRANSIENT_FAILURE) { if (this.backoffTimeout.isRunning()) { this.continueResolving = true; @@ -306,6 +305,7 @@ export class ResolvingLoadBalancer implements LoadBalancer { this.updateResolution(); } } + this.childLoadBalancer.exitIdle(); } updateAddressList( From 8cbc3dc8258fa31d29a15f59ef230abb69651bb0 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 14 Apr 2022 17:28:22 -0700 Subject: [PATCH 12/56] grpc-js: Make a reachable code path for requestReresolution in pick_first --- .../grpc-js/src/load-balancer-pick-first.ts | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/packages/grpc-js/src/load-balancer-pick-first.ts b/packages/grpc-js/src/load-balancer-pick-first.ts index 884af50b..240f0e9f 100644 --- a/packages/grpc-js/src/load-balancer-pick-first.ts +++ b/packages/grpc-js/src/load-balancer-pick-first.ts @@ -184,8 +184,10 @@ export class PickFirstLoadBalancer implements LoadBalancer { ) { /* If all of the subchannels are IDLE we should go back to a * basic IDLE state where there is no subchannel list to avoid - * holding unused resources */ - this.resetSubchannelList(); + * holding unused resources. We do not reset triedAllSubchannels + * because that is a reminder to request reresolution the next time + * this LB policy needs to connect. */ + this.resetSubchannelList(false); this.updateState(ConnectivityState.IDLE, new QueuePicker(this)); return; } @@ -337,7 +339,7 @@ export class PickFirstLoadBalancer implements LoadBalancer { this.channelControlHelper.updateState(newState, picker); } - private resetSubchannelList() { + private resetSubchannelList(resetTriedAllSubchannels = true) { for (const subchannel of this.subchannels) { subchannel.removeConnectivityStateListener(this.subchannelStateListener); subchannel.unref(); @@ -352,7 +354,9 @@ export class PickFirstLoadBalancer implements LoadBalancer { [ConnectivityState.TRANSIENT_FAILURE]: 0, }; this.subchannels = []; - this.triedAllSubchannels = false; + if (resetTriedAllSubchannels) { + this.triedAllSubchannels = false; + } } /** @@ -425,6 +429,12 @@ export class PickFirstLoadBalancer implements LoadBalancer { } exitIdle() { + if ( + this.currentState === ConnectivityState.IDLE || + this.triedAllSubchannels + ) { + this.channelControlHelper.requestReresolution(); + } for (const subchannel of this.subchannels) { subchannel.startConnecting(); } @@ -433,12 +443,6 @@ export class PickFirstLoadBalancer implements LoadBalancer { this.connectToAddressList(); } } - if ( - this.currentState === ConnectivityState.IDLE || - this.triedAllSubchannels - ) { - this.channelControlHelper.requestReresolution(); - } } resetBackoff() { From cf11b60ce20123394dc6cb21488c665c7787d84e Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Mon, 18 Apr 2022 09:36:57 -0700 Subject: [PATCH 13/56] grpc-js: End calls when keepalive pings time out --- packages/grpc-js/package.json | 2 +- packages/grpc-js/src/subchannel.ts | 18 +++++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index 09a88b5a..a0df5573 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js", - "version": "1.6.5", + "version": "1.6.6", "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/subchannel.ts b/packages/grpc-js/src/subchannel.ts index 5880f3f9..5d479a73 100644 --- a/packages/grpc-js/src/subchannel.ts +++ b/packages/grpc-js/src/subchannel.ts @@ -358,7 +358,7 @@ export class Subchannel { this.keepaliveTrace('Sending ping with timeout ' + this.keepaliveTimeoutMs + 'ms'); this.keepaliveTimeoutId = setTimeout(() => { this.keepaliveTrace('Ping timeout passed without response'); - this.transitionToState([ConnectivityState.READY], ConnectivityState.IDLE); + this.handleDisconnect(); }, this.keepaliveTimeoutMs); this.keepaliveTimeoutId.unref?.(); this.session!.ping( @@ -642,6 +642,15 @@ export class Subchannel { ); } + private handleDisconnect() { + this.transitionToState( + [ConnectivityState.READY], + ConnectivityState.TRANSIENT_FAILURE); + for (const listener of this.disconnectListeners) { + listener(); + } + } + /** * Initiate a state transition from any element of oldStates to the new * state. If the current connectivityState is not in oldStates, do nothing. @@ -672,12 +681,7 @@ export class Subchannel { const session = this.session!; session.socket.once('close', () => { if (this.session === session) { - this.transitionToState( - [ConnectivityState.READY], - ConnectivityState.TRANSIENT_FAILURE); - for (const listener of this.disconnectListeners) { - listener(); - } + this.handleDisconnect(); } }); if (this.keepaliveWithoutCalls) { From c9b7d4d285176a6be437b724c118c85f3f08c66d Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Mon, 18 Apr 2022 09:56:06 -0700 Subject: [PATCH 14/56] grpc-js: DNS: unset continueResolving when starting a resolution attempt --- packages/grpc-js/src/resolver-dns.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/grpc-js/src/resolver-dns.ts b/packages/grpc-js/src/resolver-dns.ts index c4cb64a7..47de8dbc 100644 --- a/packages/grpc-js/src/resolver-dns.ts +++ b/packages/grpc-js/src/resolver-dns.ts @@ -314,6 +314,7 @@ class DnsResolver implements Resolver { } private startResolutionWithBackoff() { + this.continueResolving = false; this.startResolution(); this.backoff.runOnce(); this.startNextResolutionTimer(); From 964c7a68aabd3e45f87b0d0045bb604e1bd67ae7 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Tue, 19 Apr 2022 10:21:49 -0700 Subject: [PATCH 15/56] grpc-js: Fix double resolver calls in DNS resolver --- packages/grpc-js/src/backoff-timeout.ts | 1 + packages/grpc-js/src/resolver-dns.ts | 10 +++++++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/grpc-js/src/backoff-timeout.ts b/packages/grpc-js/src/backoff-timeout.ts index dc7be277..f523e259 100644 --- a/packages/grpc-js/src/backoff-timeout.ts +++ b/packages/grpc-js/src/backoff-timeout.ts @@ -100,6 +100,7 @@ export class BackoffTimeout { } private runTimer(delay: number) { + clearTimeout(this.timerId); this.timerId = setTimeout(() => { this.callback(); this.running = false; diff --git a/packages/grpc-js/src/resolver-dns.ts b/packages/grpc-js/src/resolver-dns.ts index 47de8dbc..14de2656 100644 --- a/packages/grpc-js/src/resolver-dns.ts +++ b/packages/grpc-js/src/resolver-dns.ts @@ -158,7 +158,6 @@ class DnsResolver implements Resolver { if (this.ipResult !== null) { trace('Returning IP address for target ' + uriToString(this.target)); setImmediate(() => { - this.backoff.reset(); this.listener.onSuccessfulResolution( this.ipResult!, null, @@ -167,6 +166,8 @@ class DnsResolver implements Resolver { {} ); }); + this.backoff.stop(); + this.backoff.reset(); return; } if (this.dnsHostname === null) { @@ -178,7 +179,11 @@ class DnsResolver implements Resolver { metadata: new Metadata(), }); }); + this.stopNextResolutionTimer(); } else { + if (this.pendingLookupPromise !== null) { + return; + } trace('Looking up DNS hostname ' + this.dnsHostname); /* We clear out latestLookupResult here to ensure that it contains the * latest result since the last time we started resolving. That way, the @@ -299,6 +304,7 @@ class DnsResolver implements Resolver { } private startNextResolutionTimer() { + clearTimeout(this.nextResolutionTimer); this.nextResolutionTimer = setTimeout(() => { this.stopNextResolutionTimer(); if (this.continueResolving) { @@ -314,10 +320,12 @@ class DnsResolver implements Resolver { } private startResolutionWithBackoff() { + if (this.pendingLookupPromise === null) { this.continueResolving = false; this.startResolution(); this.backoff.runOnce(); this.startNextResolutionTimer(); + } } updateResolution() { From 5311c03867c7e3bdb1c167fcd76d44d6b5d2e63a Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Tue, 19 Apr 2022 13:18:59 -0700 Subject: [PATCH 16/56] grpc-js: Report error when no message received for unary response --- packages/grpc-js/src/client.ts | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/packages/grpc-js/src/client.ts b/packages/grpc-js/src/client.ts index ed9407cd..20f56bcb 100644 --- a/packages/grpc-js/src/client.ts +++ b/packages/grpc-js/src/client.ts @@ -338,7 +338,15 @@ export class Client { } receivedStatus = true; if (status.code === Status.OK) { - callProperties.callback!(null, responseMessage!); + if (responseMessage === null) { + callProperties.callback!(callErrorFromStatus({ + code: Status.INTERNAL, + details: 'No message received', + metadata: status.metadata + })); + } else { + callProperties.callback!(null, responseMessage); + } } else { callProperties.callback!(callErrorFromStatus(status)); } @@ -455,7 +463,15 @@ export class Client { } receivedStatus = true; if (status.code === Status.OK) { - callProperties.callback!(null, responseMessage!); + if (responseMessage === null) { + callProperties.callback!(callErrorFromStatus({ + code: Status.INTERNAL, + details: 'No message received', + metadata: status.metadata + })); + } else { + callProperties.callback!(null, responseMessage); + } } else { callProperties.callback!(callErrorFromStatus(status)); } From 32514224ce1c85cd6e3c0c2e4a8c197acc90e0b9 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Wed, 20 Apr 2022 13:06:43 -0700 Subject: [PATCH 17/56] grpc-js: Fix shutting down subchannels in separate pools --- packages/grpc-js/package.json | 2 +- packages/grpc-js/src/subchannel-pool.ts | 14 +++++--------- packages/grpc-js/src/subchannel.ts | 2 +- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index a0df5573..723b108f 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js", - "version": "1.6.6", + "version": "1.6.7", "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/subchannel-pool.ts b/packages/grpc-js/src/subchannel-pool.ts index cd74cad8..b7ef362c 100644 --- a/packages/grpc-js/src/subchannel-pool.ts +++ b/packages/grpc-js/src/subchannel-pool.ts @@ -49,10 +49,8 @@ export class SubchannelPool { /** * A pool of subchannels use for making connections. Subchannels with the * exact same parameters will be reused. - * @param global If true, this is the global subchannel pool. Otherwise, it - * is the pool for a single channel. */ - constructor(private global: boolean) {} + constructor() {} /** * Unrefs all unused subchannels and cancels the cleanup task if all @@ -95,7 +93,7 @@ export class SubchannelPool { * Ensures that the cleanup task is spawned. */ ensureCleanupTask(): void { - if (this.global && this.cleanupTimer === null) { + if (this.cleanupTimer === null) { this.cleanupTimer = setInterval(() => { this.unrefUnusedSubchannels(); }, REF_CHECK_INTERVAL); @@ -156,14 +154,12 @@ export class SubchannelPool { channelCredentials, subchannel, }); - if (this.global) { - subchannel.ref(); - } + subchannel.ref(); return subchannel; } } -const globalSubchannelPool = new SubchannelPool(true); +const globalSubchannelPool = new SubchannelPool(); /** * Get either the global subchannel pool, or a new subchannel pool. @@ -173,6 +169,6 @@ export function getSubchannelPool(global: boolean): SubchannelPool { if (global) { return globalSubchannelPool; } else { - return new SubchannelPool(false); + return new SubchannelPool(); } } diff --git a/packages/grpc-js/src/subchannel.ts b/packages/grpc-js/src/subchannel.ts index 5d479a73..372ab516 100644 --- a/packages/grpc-js/src/subchannel.ts +++ b/packages/grpc-js/src/subchannel.ts @@ -741,7 +741,7 @@ export class Subchannel { } this.transitionToState( [ConnectivityState.CONNECTING, ConnectivityState.READY], - ConnectivityState.TRANSIENT_FAILURE + ConnectivityState.IDLE ); if (this.channelzEnabled) { unregisterChannelzRef(this.channelzRef); From b07ea8b35418ecfd88bccf856bffe8b75eef3727 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 21 Apr 2022 14:42:04 -0700 Subject: [PATCH 18/56] grpc-js: Update outlier detection to address recent spec changes --- .../src/load-balancer-outlier-detection.ts | 51 +++++++++++++++---- 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/packages/grpc-js/src/load-balancer-outlier-detection.ts b/packages/grpc-js/src/load-balancer-outlier-detection.ts index e69e2ef9..3bd06211 100644 --- a/packages/grpc-js/src/load-balancer-outlier-detection.ts +++ b/packages/grpc-js/src/load-balancer-outlier-detection.ts @@ -334,17 +334,21 @@ class OutlierDetectionCounterFilterFactory implements FilterFactory = new Map(); private latestConfig: OutlierDetectionLoadBalancingConfig | null = null; private ejectionTimer: NodeJS.Timer; + private timerStartTime: Date | null = null; constructor(channelControlHelper: ChannelControlHelper) { this.childBalancer = new ChildLoadBalancerHandler(createChildChannelControlHelper(channelControlHelper, { @@ -373,7 +378,7 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer { }, updateState: (connectivityState: ConnectivityState, picker: Picker) => { if (connectivityState === ConnectivityState.READY) { - channelControlHelper.updateState(connectivityState, new OutlierDetectionPicker(picker)); + channelControlHelper.updateState(connectivityState, new OutlierDetectionPicker(picker, this.isCountingEnabled())); } else { channelControlHelper.updateState(connectivityState, picker); } @@ -383,6 +388,12 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer { clearInterval(this.ejectionTimer); } + private isCountingEnabled(): boolean { + return this.latestConfig !== null && + (this.latestConfig.getSuccessRateEjectionConfig() !== null || + this.latestConfig.getFailurePercentageEjectionConfig() !== null); + } + private getCurrentEjectionPercent() { let ejectionCount = 0; for (const mapEntry of this.addressMap.values()) { @@ -501,16 +512,26 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer { } } - private runChecks() { - const ejectionTimestamp = new Date(); - + private switchAllBuckets() { for (const mapEntry of this.addressMap.values()) { mapEntry.counter.switchBuckets(); } + } + + private startTimer(delayMs: number) { + this.ejectionTimer = setTimeout(() => this.runChecks(), delayMs); + } + + private runChecks() { + const ejectionTimestamp = new Date(); + + this.switchAllBuckets(); if (!this.latestConfig) { return; } + this.timerStartTime = ejectionTimestamp; + this.startTimer(this.latestConfig.getIntervalMs()); this.runSuccessRateCheck(ejectionTimestamp); this.runFailurePercentageCheck(ejectionTimestamp); @@ -561,10 +582,21 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer { ); this.childBalancer.updateAddressList(addressList, childPolicy, attributes); - if (this.latestConfig === null || this.latestConfig.getIntervalMs() !== lbConfig.getIntervalMs()) { - clearInterval(this.ejectionTimer); - this.ejectionTimer = setInterval(() => this.runChecks(), lbConfig.getIntervalMs()); + if (lbConfig.getSuccessRateEjectionConfig() || lbConfig.getFailurePercentageEjectionConfig()) { + if (this.timerStartTime) { + clearTimeout(this.ejectionTimer); + const remainingDelay = lbConfig.getIntervalMs() - ((new Date()).getTime() - this.timerStartTime.getTime()); + this.startTimer(remainingDelay); + } else { + this.timerStartTime = new Date(); + this.startTimer(lbConfig.getIntervalMs()); + this.switchAllBuckets(); + } + } else { + this.timerStartTime = null; + clearTimeout(this.ejectionTimer); } + this.latestConfig = lbConfig; } exitIdle(): void { @@ -574,6 +606,7 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer { this.childBalancer.resetBackoff(); } destroy(): void { + clearTimeout(this.ejectionTimer); this.childBalancer.destroy(); } getTypeName(): string { From cd58695674679bdf070fea3390b86d51dddf0996 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 21 Apr 2022 16:09:24 -0700 Subject: [PATCH 19/56] grpc-js: Add regression tests for repeated DNS requests --- packages/grpc-js/test/test-resolver.ts | 64 ++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/packages/grpc-js/test/test-resolver.ts b/packages/grpc-js/test/test-resolver.ts index 354413ea..512740ca 100644 --- a/packages/grpc-js/test/test-resolver.ts +++ b/packages/grpc-js/test/test-resolver.ts @@ -356,6 +356,70 @@ describe('Name Resolver', () => { const resolver2 = resolverManager.createResolver(target2, listener, {}); resolver2.updateResolution(); }); + it('should not keep repeating successful resolutions', done => { + const target = resolverManager.mapUriDefaultScheme(parseUri('localhost')!)!; + let resultCount = 0; + const resolver = resolverManager.createResolver(target, { + onSuccessfulResolution: ( + addressList: SubchannelAddress[], + serviceConfig: ServiceConfig | null, + serviceConfigError: StatusObject | null + ) => { + assert( + addressList.some( + addr => + isTcpSubchannelAddress(addr) && + addr.host === '127.0.0.1' && + addr.port === 443 + ) + ); + assert( + addressList.some( + addr => + isTcpSubchannelAddress(addr) && + addr.host === '::1' && + addr.port === 443 + ) + ); + resultCount += 1; + if (resultCount === 1) { + process.nextTick(() => resolver.updateResolution()); + } + }, + onError: (error: StatusObject) => { + assert.ifError(error); + }, + }, {'grpc.dns_min_time_between_resolutions_ms': 2000}); + resolver.updateResolution(); + setTimeout(() => { + assert.strictEqual(resultCount, 2, `resultCount ${resultCount} !== 2`); + done(); + }, 10_000); + }).timeout(15_000); + it('should not keep repeating failed resolutions', done => { + const target = resolverManager.mapUriDefaultScheme(parseUri('host.invalid')!)!; + let resultCount = 0; + const resolver = resolverManager.createResolver(target, { + onSuccessfulResolution: ( + addressList: SubchannelAddress[], + serviceConfig: ServiceConfig | null, + serviceConfigError: StatusObject | null + ) => { + assert.fail('Resolution succeeded unexpectedly'); + }, + onError: (error: StatusObject) => { + resultCount += 1; + if (resultCount === 1) { + process.nextTick(() => resolver.updateResolution()); + } + }, + }, {}); + resolver.updateResolution(); + setTimeout(() => { + assert.strictEqual(resultCount, 2, `resultCount ${resultCount} !== 2`); + done(); + }, 10_000); + }).timeout(15_000); }); describe('UDS Names', () => { it('Should handle a relative Unix Domain Socket name', done => { From db65d566e651c96cdba909f22e28bbf7be312778 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Tue, 26 Apr 2022 10:09:22 -0700 Subject: [PATCH 20/56] grpc-js: Fix mean calculation in outlier detection LB policy --- packages/grpc-js/src/load-balancer-outlier-detection.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grpc-js/src/load-balancer-outlier-detection.ts b/packages/grpc-js/src/load-balancer-outlier-detection.ts index 3bd06211..2c231c40 100644 --- a/packages/grpc-js/src/load-balancer-outlier-detection.ts +++ b/packages/grpc-js/src/load-balancer-outlier-detection.ts @@ -429,7 +429,7 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer { } // Step 2 - const successRateMean = successRates.reduce((a, b) => a + b); + const successRateMean = successRates.reduce((a, b) => a + b) / successRates.length; let successRateVariance = 0; for (const rate of successRates) { const deviation = rate - successRateMean; From 01823377be02b78869c92d8c147944a1b789139b Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 28 Apr 2022 10:34:30 -0700 Subject: [PATCH 21/56] grpc-js: Add calling context to call errors --- packages/grpc-js/src/call.ts | 6 ++++-- packages/grpc-js/src/client.ts | 16 ++++++++++------ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/grpc-js/src/call.ts b/packages/grpc-js/src/call.ts index fcc3159d..10b606a4 100644 --- a/packages/grpc-js/src/call.ts +++ b/packages/grpc-js/src/call.ts @@ -76,9 +76,11 @@ export type ClientDuplexStream< * error is not necessarily a problem in gRPC itself. * @param status */ -export function callErrorFromStatus(status: StatusObject): ServiceError { +export function callErrorFromStatus(status: StatusObject, callerStack: string): ServiceError { const message = `${status.code} ${Status[status.code]}: ${status.details}`; - return Object.assign(new Error(message), status); + const error = new Error(message); + const stack = `${error.stack}\nfor call at\n${callerStack}`; + return Object.assign(new Error(message), status, {stack}); } export class ClientUnaryCallImpl diff --git a/packages/grpc-js/src/client.ts b/packages/grpc-js/src/client.ts index 20f56bcb..747c5c87 100644 --- a/packages/grpc-js/src/client.ts +++ b/packages/grpc-js/src/client.ts @@ -321,6 +321,7 @@ export class Client { } let responseMessage: ResponseType | null = null; let receivedStatus = false; + const callerStack = (new Error().stack!).split('\n').slice(1).join('\n'); call.start(callProperties.metadata, { onReceiveMetadata: (metadata) => { emitter.emit('metadata', metadata); @@ -343,12 +344,12 @@ export class Client { code: Status.INTERNAL, details: 'No message received', metadata: status.metadata - })); + }, callerStack)); } else { callProperties.callback!(null, responseMessage); } } else { - callProperties.callback!(callErrorFromStatus(status)); + callProperties.callback!(callErrorFromStatus(status, callerStack)); } emitter.emit('status', status); }, @@ -446,6 +447,7 @@ export class Client { } let responseMessage: ResponseType | null = null; let receivedStatus = false; + const callerStack = (new Error().stack!).split('\n').slice(1).join('\n'); call.start(callProperties.metadata, { onReceiveMetadata: (metadata) => { emitter.emit('metadata', metadata); @@ -468,12 +470,12 @@ export class Client { code: Status.INTERNAL, details: 'No message received', metadata: status.metadata - })); + }, callerStack)); } else { callProperties.callback!(null, responseMessage); } } else { - callProperties.callback!(callErrorFromStatus(status)); + callProperties.callback!(callErrorFromStatus(status, callerStack)); } emitter.emit('status', status); }, @@ -575,6 +577,7 @@ export class Client { call.setCredentials(callProperties.callOptions.credentials); } let receivedStatus = false; + const callerStack = (new Error().stack!).split('\n').slice(1).join('\n'); call.start(callProperties.metadata, { onReceiveMetadata(metadata: Metadata) { stream.emit('metadata', metadata); @@ -590,7 +593,7 @@ export class Client { receivedStatus = true; stream.push(null); if (status.code !== Status.OK) { - stream.emit('error', callErrorFromStatus(status)); + stream.emit('error', callErrorFromStatus(status, callerStack)); } stream.emit('status', status); }, @@ -672,6 +675,7 @@ export class Client { call.setCredentials(callProperties.callOptions.credentials); } let receivedStatus = false; + const callerStack = (new Error().stack!).split('\n').slice(1).join('\n'); call.start(callProperties.metadata, { onReceiveMetadata(metadata: Metadata) { stream.emit('metadata', metadata); @@ -686,7 +690,7 @@ export class Client { receivedStatus = true; stream.push(null); if (status.code !== Status.OK) { - stream.emit('error', callErrorFromStatus(status)); + stream.emit('error', callErrorFromStatus(status, callerStack)); } stream.emit('status', status); }, From 0a0e13eedecedb31e91ad2d250c5fca6f31e46b6 Mon Sep 17 00:00:00 2001 From: Bart Slinger Date: Thu, 19 May 2022 23:45:11 +0200 Subject: [PATCH 22/56] handle disconnectListeners in reverse to allow listener removal in loop --- packages/grpc-js/src/subchannel.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/grpc-js/src/subchannel.ts b/packages/grpc-js/src/subchannel.ts index 5d479a73..eb7d0840 100644 --- a/packages/grpc-js/src/subchannel.ts +++ b/packages/grpc-js/src/subchannel.ts @@ -646,7 +646,8 @@ export class Subchannel { this.transitionToState( [ConnectivityState.READY], ConnectivityState.TRANSIENT_FAILURE); - for (const listener of this.disconnectListeners) { + for (let i = this.disconnectListeners.length - 1; i >= 0; i--) { + const listener = this.disconnectListeners[i]; listener(); } } From 97717003f4dce64c64748517cff4a2077484ba04 Mon Sep 17 00:00:00 2001 From: Bart Slinger Date: Fri, 20 May 2022 08:27:01 +0200 Subject: [PATCH 23/56] grpc-js: Use Set instead of Array for disconnectListeners --- packages/grpc-js/src/subchannel.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/packages/grpc-js/src/subchannel.ts b/packages/grpc-js/src/subchannel.ts index eb7d0840..4d18ca9c 100644 --- a/packages/grpc-js/src/subchannel.ts +++ b/packages/grpc-js/src/subchannel.ts @@ -108,7 +108,7 @@ export class Subchannel { * socket disconnects. Used for ending active calls with an UNAVAILABLE * status. */ - private disconnectListeners: Array<() => void> = []; + private disconnectListeners: Set<() => void> = new Set(); private backoffTimeout: BackoffTimeout; @@ -646,8 +646,7 @@ export class Subchannel { this.transitionToState( [ConnectivityState.READY], ConnectivityState.TRANSIENT_FAILURE); - for (let i = this.disconnectListeners.length - 1; i >= 0; i--) { - const listener = this.disconnectListeners[i]; + for (const listener of this.disconnectListeners.values()) { listener(); } } @@ -972,14 +971,11 @@ export class Subchannel { } addDisconnectListener(listener: () => void) { - this.disconnectListeners.push(listener); + this.disconnectListeners.add(listener); } removeDisconnectListener(listener: () => void) { - const listenerIndex = this.disconnectListeners.indexOf(listener); - if (listenerIndex > -1) { - this.disconnectListeners.splice(listenerIndex, 1); - } + this.disconnectListeners.delete(listener); } /** From aa97aa8a1c3e936be6fa4938f6467e044bc66ec6 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Mon, 23 May 2022 11:28:39 -0700 Subject: [PATCH 24/56] grpc-js-xds: Add support for k8s interop test framework --- packages/grpc-js-xds/interop/Dockerfile | 30 ++++ packages/grpc-js-xds/scripts/xds_k8s_lb.sh | 166 +++++++++++++++++++++ test/kokoro/xds_k8s_lb.cfg | 26 ++++ 3 files changed, 222 insertions(+) create mode 100644 packages/grpc-js-xds/interop/Dockerfile create mode 100755 packages/grpc-js-xds/scripts/xds_k8s_lb.sh create mode 100644 test/kokoro/xds_k8s_lb.cfg diff --git a/packages/grpc-js-xds/interop/Dockerfile b/packages/grpc-js-xds/interop/Dockerfile new file mode 100644 index 00000000..dbb4433b --- /dev/null +++ b/packages/grpc-js-xds/interop/Dockerfile @@ -0,0 +1,30 @@ +# Copyright 2022 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Dockerfile for building the xDS interop client. To build the image, run the +# following command from grpc-node directory: +# docker build -t -f packages/grpc-js-xds/interop/Dockerfile . + +FROM node:16-alpine + +# Make a grpc-node directory and copy the repo into it. +WORKDIR /node/src/grpc-node +COPY . . + +WORKDIR /node/src/grpc-node/packages/grpc-js +RUN npm install +WORKDIR /node/src/grpc-node/packages/grpc-js-xds +RUN npm install + +ENTRYPOINT [ "node", "/node/src/grpc-node/packages/grpc-js-xds/build/interop/xds-interop-client" ] diff --git a/packages/grpc-js-xds/scripts/xds_k8s_lb.sh b/packages/grpc-js-xds/scripts/xds_k8s_lb.sh new file mode 100755 index 00000000..7672251d --- /dev/null +++ b/packages/grpc-js-xds/scripts/xds_k8s_lb.sh @@ -0,0 +1,166 @@ +#!/usr/bin/env bash +# Copyright 2022 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +set -eo pipefail + +# Constants +readonly GITHUB_REPOSITORY_NAME="grpc-node" +readonly TEST_DRIVER_INSTALL_SCRIPT_URL="https://raw.githubusercontent.com/${TEST_DRIVER_REPO_OWNER:-grpc}/grpc/${TEST_DRIVER_BRANCH:-master}/tools/internal_ci/linux/grpc_xds_k8s_install_test_driver.sh" +## xDS test client Docker images +readonly SERVER_IMAGE_NAME="gcr.io/grpc-testing/xds-interop/java-server:v1.46.x" +readonly CLIENT_IMAGE_NAME="gcr.io/grpc-testing/xds-interop/node-client" +readonly FORCE_IMAGE_BUILD="${FORCE_IMAGE_BUILD:-0}" +readonly BUILD_APP_PATH="packages/grpc-js-xds/interop/Dockerfile" +readonly LANGUAGE_NAME="Node" + +####################################### +# Builds test app Docker images and pushes them to GCR +# Globals: +# BUILD_APP_PATH +# CLIENT_IMAGE_NAME: Test client Docker image name +# GIT_COMMIT: SHA-1 of git commit being built +# Arguments: +# None +# Outputs: +# Writes the output of `gcloud builds submit` to stdout, stderr +####################################### +build_test_app_docker_images() { + echo "Building ${LANGUAGE_NAME} xDS interop test app Docker images" + + pushd "${SRC_DIR}" + docker build \ + -f "${BUILD_APP_PATH}" \ + -t "${CLIENT_IMAGE_NAME}:${GIT_COMMIT}" \ + . + + popd + + gcloud -q auth configure-docker + + docker push "${CLIENT_IMAGE_NAME}:${GIT_COMMIT}" +} + +####################################### +# Builds test app and its docker images unless they already exist +# Globals: +# CLIENT_IMAGE_NAME: Test client Docker image name +# GIT_COMMIT: SHA-1 of git commit being built +# FORCE_IMAGE_BUILD +# Arguments: +# None +# Outputs: +# Writes the output to stdout, stderr +####################################### +build_docker_images_if_needed() { + # Check if images already exist + client_tags="$(gcloud_gcr_list_image_tags "${CLIENT_IMAGE_NAME}" "${GIT_COMMIT}")" + printf "Client image: %s:%s\n" "${CLIENT_IMAGE_NAME}" "${GIT_COMMIT}" + echo "${client_tags:-Client image not found}" + + # Build if any of the images are missing, or FORCE_IMAGE_BUILD=1 + if [[ "${FORCE_IMAGE_BUILD}" == "1" || -z "${client_tags}" ]]; then + build_test_app_docker_images + else + echo "Skipping ${LANGUAGE_NAME} test app build" + fi +} + +####################################### +# Executes the test case +# Globals: +# TEST_DRIVER_FLAGFILE: Relative path to test driver flagfile +# KUBE_CONTEXT: The name of kubectl context with GKE cluster access +# SECONDARY_KUBE_CONTEXT: The name of kubectl context with secondary GKE cluster access, if any +# TEST_XML_OUTPUT_DIR: Output directory for the test xUnit XML report +# CLIENT_IMAGE_NAME: Test client Docker image name +# GIT_COMMIT: SHA-1 of git commit being built +# Arguments: +# Test case name +# Outputs: +# Writes the output of test execution to stdout, stderr +# Test xUnit report to ${TEST_XML_OUTPUT_DIR}/${test_name}/sponge_log.xml +####################################### +run_test() { + # Test driver usage: + # https://github.com/grpc/grpc/tree/master/tools/run_tests/xds_k8s_test_driver#basic-usage + local test_name="${1:?Usage: run_test test_name}" + # testing_version is used by the framework to determine the supported PSM + # features. It's captured from Kokoro job name of the Node repo, which takes + # the form: + # grpc/node// + python3 -m "tests.${test_name}" \ + --flagfile="${TEST_DRIVER_FLAGFILE}" \ + --kube_context="${KUBE_CONTEXT}" \ + --secondary_kube_context="${SECONDARY_KUBE_CONTEXT}" \ + --client_image="${CLIENT_IMAGE_NAME}:${GIT_COMMIT}" \ + --server_image="${SERVER_IMAGE_NAME}" \ + --testing_version=$(echo "$KOKORO_JOB_NAME" | sed -E 's|^grpc/node/([^/]+)/.*|\1|') \ + --xml_output_file="${TEST_XML_OUTPUT_DIR}/${test_name}/sponge_log.xml" +} + +####################################### +# Main function: provision software necessary to execute tests, and run them +# Globals: +# KOKORO_ARTIFACTS_DIR +# GITHUB_REPOSITORY_NAME +# SRC_DIR: Populated with absolute path to the source repo +# TEST_DRIVER_REPO_DIR: Populated with the path to the repo containing +# the test driver +# TEST_DRIVER_FULL_DIR: Populated with the path to the test driver source code +# TEST_DRIVER_FLAGFILE: Populated with relative path to test driver flagfile +# TEST_XML_OUTPUT_DIR: Populated with the path to test xUnit XML report +# GIT_ORIGIN_URL: Populated with the origin URL of git repo used for the build +# GIT_COMMIT: Populated with the SHA-1 of git commit being built +# GIT_COMMIT_SHORT: Populated with the short SHA-1 of git commit being built +# KUBE_CONTEXT: Populated with name of kubectl context with GKE cluster access +# SECONDARY_KUBE_CONTEXT: Populated with name of kubectl context with secondary GKE cluster access, if any +# Arguments: +# None +# Outputs: +# Writes the output of test execution to stdout, stderr +####################################### +main() { + local script_dir + script_dir="$(dirname "$0")" + + # Source the test driver from the master branch. + echo "Sourcing test driver install script from: ${TEST_DRIVER_INSTALL_SCRIPT_URL}" + source /dev/stdin <<< "$(curl -s "${TEST_DRIVER_INSTALL_SCRIPT_URL}")" + + activate_gke_cluster GKE_CLUSTER_PSM_SECURITY + activate_secondary_gke_cluster GKE_CLUSTER_PSM_SECURITY + + set -x + if [[ -n "${KOKORO_ARTIFACTS_DIR}" ]]; then + kokoro_setup_test_driver "${GITHUB_REPOSITORY_NAME}" + else + local_setup_test_driver "${script_dir}" + fi + build_docker_images_if_needed + + # Run tests + cd "${TEST_DRIVER_FULL_DIR}" + local failed_tests=0 + test_suites=("api_listener_test" "change_backend_service_test" "failover_test" "remove_neg_test" "round_robin_test") + for test in "${test_suites[@]}"; do + run_test $test || (( failed_tests++ )) + done + echo "Failed test suites: ${failed_tests}" + if (( failed_tests > 0 )); then + exit 1 + fi +} + +main "$@" \ No newline at end of file diff --git a/test/kokoro/xds_k8s_lb.cfg b/test/kokoro/xds_k8s_lb.cfg new file mode 100644 index 00000000..17a3f3b3 --- /dev/null +++ b/test/kokoro/xds_k8s_lb.cfg @@ -0,0 +1,26 @@ +# Copyright 2022 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Config file for Kokoro (in protobuf text format) + +# Location of the continuous shell script in repository. +build_file: "grpc-node/packages/grpc-js-xds/scripts/xds_k8s_lb.sh" +timeout_mins: 180 +action { + define_artifacts { + regex: "artifacts/**/*sponge_log.xml" + regex: "artifacts/**/*sponge_log.log" + strip_prefix: "artifacts" + } +} \ No newline at end of file From ce169c22b483a27e776e003d7630b158bd0cc7f7 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Mon, 23 May 2022 15:59:01 -0700 Subject: [PATCH 25/56] Reduce docker image size with extra build step --- packages/grpc-js-xds/interop/Dockerfile | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/grpc-js-xds/interop/Dockerfile b/packages/grpc-js-xds/interop/Dockerfile index dbb4433b..bc85d3b2 100644 --- a/packages/grpc-js-xds/interop/Dockerfile +++ b/packages/grpc-js-xds/interop/Dockerfile @@ -16,7 +16,7 @@ # following command from grpc-node directory: # docker build -t -f packages/grpc-js-xds/interop/Dockerfile . -FROM node:16-alpine +FROM node:16-alpine as build # Make a grpc-node directory and copy the repo into it. WORKDIR /node/src/grpc-node @@ -27,4 +27,9 @@ RUN npm install WORKDIR /node/src/grpc-node/packages/grpc-js-xds RUN npm install +FROM node:16-alpine +WORKDIR /node/src/grpc-node +COPY --from=build /node/src/grpc-node/packages/grpc-js ./packages/grpc-js/ +COPY --from=build /node/src/grpc-node/packages/grpc-js-xds ./packages/grpc-js-xds/ + ENTRYPOINT [ "node", "/node/src/grpc-node/packages/grpc-js-xds/build/interop/xds-interop-client" ] From 30bf5a1bac096d4eebeb0b848ede389bff27e226 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Mon, 23 May 2022 16:19:07 -0700 Subject: [PATCH 26/56] Fix cluster references Co-authored-by: Sergii Tkachenko --- packages/grpc-js-xds/scripts/xds_k8s_lb.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/grpc-js-xds/scripts/xds_k8s_lb.sh b/packages/grpc-js-xds/scripts/xds_k8s_lb.sh index 7672251d..8ec1760a 100755 --- a/packages/grpc-js-xds/scripts/xds_k8s_lb.sh +++ b/packages/grpc-js-xds/scripts/xds_k8s_lb.sh @@ -139,8 +139,8 @@ main() { echo "Sourcing test driver install script from: ${TEST_DRIVER_INSTALL_SCRIPT_URL}" source /dev/stdin <<< "$(curl -s "${TEST_DRIVER_INSTALL_SCRIPT_URL}")" - activate_gke_cluster GKE_CLUSTER_PSM_SECURITY - activate_secondary_gke_cluster GKE_CLUSTER_PSM_SECURITY + activate_gke_cluster GKE_CLUSTER_PSM_LB + activate_secondary_gke_cluster GKE_CLUSTER_PSM_LB set -x if [[ -n "${KOKORO_ARTIFACTS_DIR}" ]]; then From 55175ae8c281307e05f147987822aedf5b4d85ee Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Mon, 23 May 2022 16:21:27 -0700 Subject: [PATCH 27/56] Add missing newlines at EOF --- packages/grpc-js-xds/scripts/xds_k8s_lb.sh | 2 +- test/kokoro/xds_k8s_lb.cfg | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/grpc-js-xds/scripts/xds_k8s_lb.sh b/packages/grpc-js-xds/scripts/xds_k8s_lb.sh index 8ec1760a..9cf98d2f 100755 --- a/packages/grpc-js-xds/scripts/xds_k8s_lb.sh +++ b/packages/grpc-js-xds/scripts/xds_k8s_lb.sh @@ -163,4 +163,4 @@ main() { fi } -main "$@" \ No newline at end of file +main "$@" diff --git a/test/kokoro/xds_k8s_lb.cfg b/test/kokoro/xds_k8s_lb.cfg index 17a3f3b3..b9940cfd 100644 --- a/test/kokoro/xds_k8s_lb.cfg +++ b/test/kokoro/xds_k8s_lb.cfg @@ -23,4 +23,4 @@ action { regex: "artifacts/**/*sponge_log.log" strip_prefix: "artifacts" } -} \ No newline at end of file +} From 67cfbae7aac90c3e60320f72c14dc4ec2985cd14 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Mon, 23 May 2022 16:46:51 -0700 Subject: [PATCH 28/56] Use cd instead of WORKDIR in Dockerfile Co-authored-by: Sergii Tkachenko --- packages/grpc-js-xds/interop/Dockerfile | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/grpc-js-xds/interop/Dockerfile b/packages/grpc-js-xds/interop/Dockerfile index bc85d3b2..b18d02a4 100644 --- a/packages/grpc-js-xds/interop/Dockerfile +++ b/packages/grpc-js-xds/interop/Dockerfile @@ -22,10 +22,8 @@ FROM node:16-alpine as build WORKDIR /node/src/grpc-node COPY . . -WORKDIR /node/src/grpc-node/packages/grpc-js -RUN npm install -WORKDIR /node/src/grpc-node/packages/grpc-js-xds -RUN npm install +RUN cd packages/grpc-js && npm install +RUN cd packages/grpc-js-xds && npm install FROM node:16-alpine WORKDIR /node/src/grpc-node From ad6d650408f4650c6e7c897c02ae4bf7bc3fddb1 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Tue, 24 May 2022 09:41:05 -0700 Subject: [PATCH 29/56] Revert "Use cd instead of WORKDIR in Dockerfile" --- packages/grpc-js-xds/interop/Dockerfile | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/grpc-js-xds/interop/Dockerfile b/packages/grpc-js-xds/interop/Dockerfile index b18d02a4..bc85d3b2 100644 --- a/packages/grpc-js-xds/interop/Dockerfile +++ b/packages/grpc-js-xds/interop/Dockerfile @@ -22,8 +22,10 @@ FROM node:16-alpine as build WORKDIR /node/src/grpc-node COPY . . -RUN cd packages/grpc-js && npm install -RUN cd packages/grpc-js-xds && npm install +WORKDIR /node/src/grpc-node/packages/grpc-js +RUN npm install +WORKDIR /node/src/grpc-node/packages/grpc-js-xds +RUN npm install FROM node:16-alpine WORKDIR /node/src/grpc-node From ee0c6bc9eaa80e43c82f44e40566b038cb49fd24 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Tue, 24 May 2022 10:35:55 -0700 Subject: [PATCH 30/56] Add baseline_test to test list --- packages/grpc-js-xds/scripts/xds_k8s_lb.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grpc-js-xds/scripts/xds_k8s_lb.sh b/packages/grpc-js-xds/scripts/xds_k8s_lb.sh index 9cf98d2f..efdde91e 100755 --- a/packages/grpc-js-xds/scripts/xds_k8s_lb.sh +++ b/packages/grpc-js-xds/scripts/xds_k8s_lb.sh @@ -153,7 +153,7 @@ main() { # Run tests cd "${TEST_DRIVER_FULL_DIR}" local failed_tests=0 - test_suites=("api_listener_test" "change_backend_service_test" "failover_test" "remove_neg_test" "round_robin_test") + test_suites=("baseline_test" "api_listener_test" "change_backend_service_test" "failover_test" "remove_neg_test" "round_robin_test") for test in "${test_suites[@]}"; do run_test $test || (( failed_tests++ )) done From be749bf94f96a03ec5e3bf0aabed2566f5c79f31 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Tue, 24 May 2022 11:14:26 -0700 Subject: [PATCH 31/56] Update submodules in test script --- packages/grpc-js-xds/scripts/xds_k8s_lb.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/grpc-js-xds/scripts/xds_k8s_lb.sh b/packages/grpc-js-xds/scripts/xds_k8s_lb.sh index efdde91e..0ff34ac4 100755 --- a/packages/grpc-js-xds/scripts/xds_k8s_lb.sh +++ b/packages/grpc-js-xds/scripts/xds_k8s_lb.sh @@ -135,6 +135,8 @@ main() { local script_dir script_dir="$(dirname "$0")" + git submodule update --init --recursive + # Source the test driver from the master branch. echo "Sourcing test driver install script from: ${TEST_DRIVER_INSTALL_SCRIPT_URL}" source /dev/stdin <<< "$(curl -s "${TEST_DRIVER_INSTALL_SCRIPT_URL}")" From e61c8f9ecd2c20a1baaeaf284aa813bc8e66473c Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Tue, 24 May 2022 13:11:41 -0700 Subject: [PATCH 32/56] Change directory before submodules update --- packages/grpc-js-xds/scripts/xds_k8s_lb.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/grpc-js-xds/scripts/xds_k8s_lb.sh b/packages/grpc-js-xds/scripts/xds_k8s_lb.sh index 0ff34ac4..b4c9f81a 100755 --- a/packages/grpc-js-xds/scripts/xds_k8s_lb.sh +++ b/packages/grpc-js-xds/scripts/xds_k8s_lb.sh @@ -135,6 +135,8 @@ main() { local script_dir script_dir="$(dirname "$0")" + cd "${script_dir}" + git submodule update --init --recursive # Source the test driver from the master branch. From e94bd36bf15d75fb3e49f52eeddadc371702b90e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Conall=20=C3=93=20Cofaigh?= Date: Mon, 30 May 2022 13:54:05 +0100 Subject: [PATCH 33/56] bump protobufjs to "^6.11.3" --- packages/proto-loader/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/proto-loader/package.json b/packages/proto-loader/package.json index 759e5471..53b6e829 100644 --- a/packages/proto-loader/package.json +++ b/packages/proto-loader/package.json @@ -48,7 +48,7 @@ "@types/long": "^4.0.1", "lodash.camelcase": "^4.3.0", "long": "^4.0.0", - "protobufjs": "^6.10.0", + "protobufjs": "^6.11.3", "yargs": "^16.2.0" }, "devDependencies": { From 475559f976088deae1c3d8a512fd9c57425ddba3 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Mon, 6 Jun 2022 14:08:13 -0700 Subject: [PATCH 34/56] proto-loader: Increment version to 0.6.13 --- packages/proto-loader/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/proto-loader/package.json b/packages/proto-loader/package.json index 53b6e829..65639c50 100644 --- a/packages/proto-loader/package.json +++ b/packages/proto-loader/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/proto-loader", - "version": "0.6.12", + "version": "0.6.13", "author": "Google Inc.", "contributors": [ { From 07b73ad129e0368dfa95fab531d45ab754b9844b Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Tue, 7 Jun 2022 11:07:47 -0700 Subject: [PATCH 35/56] grpc-js: Add a test for compressing large messages --- .../grpc-js/test/fixtures/test_service.proto | 1 + packages/grpc-js/test/generated/Response.ts | 2 ++ .../grpc-js/test/generated/TestService.ts | 32 +++++++++---------- packages/grpc-js/test/test-server.ts | 16 +++++++++- 4 files changed, 34 insertions(+), 17 deletions(-) diff --git a/packages/grpc-js/test/fixtures/test_service.proto b/packages/grpc-js/test/fixtures/test_service.proto index f99393d1..64ce0d37 100644 --- a/packages/grpc-js/test/fixtures/test_service.proto +++ b/packages/grpc-js/test/fixtures/test_service.proto @@ -25,6 +25,7 @@ message Request { message Response { int32 count = 1; + string message = 2; } service TestService { diff --git a/packages/grpc-js/test/generated/Response.ts b/packages/grpc-js/test/generated/Response.ts index 217fc75e..465ab720 100644 --- a/packages/grpc-js/test/generated/Response.ts +++ b/packages/grpc-js/test/generated/Response.ts @@ -3,8 +3,10 @@ export interface Response { 'count'?: (number); + 'message'?: (string); } export interface Response__Output { 'count': (number); + 'message': (string); } diff --git a/packages/grpc-js/test/generated/TestService.ts b/packages/grpc-js/test/generated/TestService.ts index 75bff33e..e477c99b 100644 --- a/packages/grpc-js/test/generated/TestService.ts +++ b/packages/grpc-js/test/generated/TestService.ts @@ -11,28 +11,28 @@ export interface TestServiceClient extends grpc.Client { bidiStream(metadata: grpc.Metadata, options?: grpc.CallOptions): grpc.ClientDuplexStream<_Request, _Response__Output>; bidiStream(options?: grpc.CallOptions): grpc.ClientDuplexStream<_Request, _Response__Output>; - ClientStream(metadata: grpc.Metadata, options: grpc.CallOptions, callback: (error?: grpc.ServiceError, result?: _Response__Output) => void): grpc.ClientWritableStream<_Request>; - ClientStream(metadata: grpc.Metadata, callback: (error?: grpc.ServiceError, result?: _Response__Output) => void): grpc.ClientWritableStream<_Request>; - ClientStream(options: grpc.CallOptions, callback: (error?: grpc.ServiceError, result?: _Response__Output) => void): grpc.ClientWritableStream<_Request>; - ClientStream(callback: (error?: grpc.ServiceError, result?: _Response__Output) => void): grpc.ClientWritableStream<_Request>; - clientStream(metadata: grpc.Metadata, options: grpc.CallOptions, callback: (error?: grpc.ServiceError, result?: _Response__Output) => void): grpc.ClientWritableStream<_Request>; - clientStream(metadata: grpc.Metadata, callback: (error?: grpc.ServiceError, result?: _Response__Output) => void): grpc.ClientWritableStream<_Request>; - clientStream(options: grpc.CallOptions, callback: (error?: grpc.ServiceError, result?: _Response__Output) => void): grpc.ClientWritableStream<_Request>; - clientStream(callback: (error?: grpc.ServiceError, result?: _Response__Output) => void): grpc.ClientWritableStream<_Request>; + ClientStream(metadata: grpc.Metadata, options: grpc.CallOptions, callback: grpc.requestCallback<_Response__Output>): grpc.ClientWritableStream<_Request>; + ClientStream(metadata: grpc.Metadata, callback: grpc.requestCallback<_Response__Output>): grpc.ClientWritableStream<_Request>; + ClientStream(options: grpc.CallOptions, callback: grpc.requestCallback<_Response__Output>): grpc.ClientWritableStream<_Request>; + ClientStream(callback: grpc.requestCallback<_Response__Output>): grpc.ClientWritableStream<_Request>; + clientStream(metadata: grpc.Metadata, options: grpc.CallOptions, callback: grpc.requestCallback<_Response__Output>): grpc.ClientWritableStream<_Request>; + clientStream(metadata: grpc.Metadata, callback: grpc.requestCallback<_Response__Output>): grpc.ClientWritableStream<_Request>; + clientStream(options: grpc.CallOptions, callback: grpc.requestCallback<_Response__Output>): grpc.ClientWritableStream<_Request>; + clientStream(callback: grpc.requestCallback<_Response__Output>): grpc.ClientWritableStream<_Request>; ServerStream(argument: _Request, metadata: grpc.Metadata, options?: grpc.CallOptions): grpc.ClientReadableStream<_Response__Output>; ServerStream(argument: _Request, options?: grpc.CallOptions): grpc.ClientReadableStream<_Response__Output>; serverStream(argument: _Request, metadata: grpc.Metadata, options?: grpc.CallOptions): grpc.ClientReadableStream<_Response__Output>; serverStream(argument: _Request, options?: grpc.CallOptions): grpc.ClientReadableStream<_Response__Output>; - Unary(argument: _Request, metadata: grpc.Metadata, options: grpc.CallOptions, callback: (error?: grpc.ServiceError, result?: _Response__Output) => void): grpc.ClientUnaryCall; - Unary(argument: _Request, metadata: grpc.Metadata, callback: (error?: grpc.ServiceError, result?: _Response__Output) => void): grpc.ClientUnaryCall; - Unary(argument: _Request, options: grpc.CallOptions, callback: (error?: grpc.ServiceError, result?: _Response__Output) => void): grpc.ClientUnaryCall; - Unary(argument: _Request, callback: (error?: grpc.ServiceError, result?: _Response__Output) => void): grpc.ClientUnaryCall; - unary(argument: _Request, metadata: grpc.Metadata, options: grpc.CallOptions, callback: (error?: grpc.ServiceError, result?: _Response__Output) => void): grpc.ClientUnaryCall; - unary(argument: _Request, metadata: grpc.Metadata, callback: (error?: grpc.ServiceError, result?: _Response__Output) => void): grpc.ClientUnaryCall; - unary(argument: _Request, options: grpc.CallOptions, callback: (error?: grpc.ServiceError, result?: _Response__Output) => void): grpc.ClientUnaryCall; - unary(argument: _Request, callback: (error?: grpc.ServiceError, result?: _Response__Output) => void): grpc.ClientUnaryCall; + Unary(argument: _Request, metadata: grpc.Metadata, options: grpc.CallOptions, callback: grpc.requestCallback<_Response__Output>): grpc.ClientUnaryCall; + Unary(argument: _Request, metadata: grpc.Metadata, callback: grpc.requestCallback<_Response__Output>): grpc.ClientUnaryCall; + Unary(argument: _Request, options: grpc.CallOptions, callback: grpc.requestCallback<_Response__Output>): grpc.ClientUnaryCall; + Unary(argument: _Request, callback: grpc.requestCallback<_Response__Output>): grpc.ClientUnaryCall; + unary(argument: _Request, metadata: grpc.Metadata, options: grpc.CallOptions, callback: grpc.requestCallback<_Response__Output>): grpc.ClientUnaryCall; + unary(argument: _Request, metadata: grpc.Metadata, callback: grpc.requestCallback<_Response__Output>): grpc.ClientUnaryCall; + unary(argument: _Request, options: grpc.CallOptions, callback: grpc.requestCallback<_Response__Output>): grpc.ClientUnaryCall; + unary(argument: _Request, callback: grpc.requestCallback<_Response__Output>): grpc.ClientUnaryCall; } diff --git a/packages/grpc-js/test/test-server.ts b/packages/grpc-js/test/test-server.ts index 2513b1fe..0c0ba168 100644 --- a/packages/grpc-js/test/test-server.ts +++ b/packages/grpc-js/test/test-server.ts @@ -623,7 +623,7 @@ describe('Generic client and server', () => { describe('Compressed requests', () => { const testServiceHandlers: TestServiceHandlers = { Unary(call, callback) { - callback(null, { count: 500000 }); + callback(null, { count: 500000, message: call.request.message }); }, ClientStream(call, callback) { @@ -847,6 +847,20 @@ describe('Compressed requests', () => { }) }); }); + + it('Should handle large messages', done => { + let longMessage = ''; + for (let i = 0; i < 400000; i++) { + const letter = 'abcdefghijklmnopqrstuvwxyz'[Math.floor(Math.random() * 26)]; + longMessage = longMessage + letter.repeat(10); + } + + client.unary({message: longMessage}, (err, response) => { + assert.ifError(err); + assert.strictEqual(response?.message, longMessage); + done(); + }) + }) /* As of Node 16, Writable and Duplex streams validate the encoding * argument to write, and the flags values we are passing there are not From 0c543aa2b27f9ea2890bbe3d7cb88c696412225d Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Wed, 15 Jun 2022 10:09:53 -0700 Subject: [PATCH 36/56] xDS k8s interop script: publish version-tagged docker images --- packages/grpc-js-xds/scripts/xds_k8s_lb.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/grpc-js-xds/scripts/xds_k8s_lb.sh b/packages/grpc-js-xds/scripts/xds_k8s_lb.sh index b4c9f81a..0d333b7e 100755 --- a/packages/grpc-js-xds/scripts/xds_k8s_lb.sh +++ b/packages/grpc-js-xds/scripts/xds_k8s_lb.sh @@ -45,6 +45,11 @@ build_test_app_docker_images() { -t "${CLIENT_IMAGE_NAME}:${GIT_COMMIT}" \ . + if [[ -n $KOKORO_JOB_NAME ]]; then + branch_name=$(echo "$KOKORO_JOB_NAME" | sed -E 's|^grpc/node/([^/]+)/.*|\1|') + tag_and_push_docker_image "${CLIENT_IMAGE_NAME}" "${GIT_COMMIT}" "${branch_name}" + fi + popd gcloud -q auth configure-docker From e4435a50f6c857dfd8edbade4856c172b8856617 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Tue, 21 Jun 2022 15:51:03 -0700 Subject: [PATCH 37/56] Use new TESTING_VERSION variable --- packages/grpc-js-xds/scripts/xds_k8s_lb.sh | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/grpc-js-xds/scripts/xds_k8s_lb.sh b/packages/grpc-js-xds/scripts/xds_k8s_lb.sh index 0d333b7e..58a95137 100755 --- a/packages/grpc-js-xds/scripts/xds_k8s_lb.sh +++ b/packages/grpc-js-xds/scripts/xds_k8s_lb.sh @@ -45,9 +45,8 @@ build_test_app_docker_images() { -t "${CLIENT_IMAGE_NAME}:${GIT_COMMIT}" \ . - if [[ -n $KOKORO_JOB_NAME ]]; then - branch_name=$(echo "$KOKORO_JOB_NAME" | sed -E 's|^grpc/node/([^/]+)/.*|\1|') - tag_and_push_docker_image "${CLIENT_IMAGE_NAME}" "${GIT_COMMIT}" "${branch_name}" + if is_version_branch "${TESTING_VERSION}"; then + tag_and_push_docker_image "${CLIENT_IMAGE_NAME}" "${GIT_COMMIT}" "${TESTING_VERSION}" fi popd @@ -111,7 +110,7 @@ run_test() { --secondary_kube_context="${SECONDARY_KUBE_CONTEXT}" \ --client_image="${CLIENT_IMAGE_NAME}:${GIT_COMMIT}" \ --server_image="${SERVER_IMAGE_NAME}" \ - --testing_version=$(echo "$KOKORO_JOB_NAME" | sed -E 's|^grpc/node/([^/]+)/.*|\1|') \ + --testing_version="${TESTING_VERSION}" \ --xml_output_file="${TEST_XML_OUTPUT_DIR}/${test_name}/sponge_log.xml" } From 92cf5df8d57431a6eca94a6bec057363b29a8182 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 23 Jun 2022 14:33:15 -0700 Subject: [PATCH 38/56] Add xDS k8s url_map test script --- .../grpc-js-xds/scripts/xds_k8s_url_map.sh | 152 ++++++++++++++++++ test/kokoro/xds_k8s_url_map.cfg | 26 +++ 2 files changed, 178 insertions(+) create mode 100644 packages/grpc-js-xds/scripts/xds_k8s_url_map.sh create mode 100644 test/kokoro/xds_k8s_url_map.cfg diff --git a/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh b/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh new file mode 100644 index 00000000..ca33cf0d --- /dev/null +++ b/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh @@ -0,0 +1,152 @@ +#!/usr/bin/env bash +# Copyright 2021 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +set -eo pipefail + +# Constants +readonly GITHUB_REPOSITORY_NAME="grpc-node" +readonly TEST_DRIVER_INSTALL_SCRIPT_URL="https://raw.githubusercontent.com/${TEST_DRIVER_REPO_OWNER:-grpc}/grpc/${TEST_DRIVER_BRANCH:-master}/tools/internal_ci/linux/grpc_xds_k8s_install_test_driver.sh" +## xDS test client Docker images +readonly CLIENT_IMAGE_NAME="gcr.io/grpc-testing/xds-interop/node-client" +readonly FORCE_IMAGE_BUILD="${FORCE_IMAGE_BUILD:-0}" +readonly BUILD_APP_PATH="packages/grpc-js-xds/interop/Dockerfile" +readonly LANGUAGE_NAME="Node" + +####################################### +# Builds test app Docker images and pushes them to GCR +# Globals: +# BUILD_APP_PATH +# CLIENT_IMAGE_NAME: Test client Docker image name +# GIT_COMMIT: SHA-1 of git commit being built +# Arguments: +# None +# Outputs: +# Writes the output of `gcloud builds submit` to stdout, stderr +####################################### +build_test_app_docker_images() { + echo "Building ${LANGUAGE_NAME} xDS interop test app Docker images" + + pushd "${SRC_DIR}" + docker build \ + -f src/python/grpcio_tests/tests_py3_only/interop/Dockerfile.client \ + -t "${CLIENT_IMAGE_NAME}:${GIT_COMMIT}" \ + . + + popd + + gcloud -q auth configure-docker + + docker push "${CLIENT_IMAGE_NAME}:${GIT_COMMIT}" +} + +####################################### +# Builds test app and its docker images unless they already exist +# Globals: +# CLIENT_IMAGE_NAME: Test client Docker image name +# GIT_COMMIT: SHA-1 of git commit being built +# FORCE_IMAGE_BUILD +# Arguments: +# None +# Outputs: +# Writes the output to stdout, stderr +####################################### +build_docker_images_if_needed() { + # Check if images already exist + client_tags="$(gcloud_gcr_list_image_tags "${CLIENT_IMAGE_NAME}" "${GIT_COMMIT}")" + printf "Client image: %s:%s\n" "${CLIENT_IMAGE_NAME}" "${GIT_COMMIT}" + echo "${client_tags:-Client image not found}" + + # Build if any of the images are missing, or FORCE_IMAGE_BUILD=1 + if [[ "${FORCE_IMAGE_BUILD}" == "1" || -z "${client_tags}" ]]; then + build_test_app_docker_images + else + echo "Skipping ${LANGUAGE_NAME} test app build" + fi +} + +####################################### +# Executes the test case +# Globals: +# TEST_DRIVER_FLAGFILE: Relative path to test driver flagfile +# KUBE_CONTEXT: The name of kubectl context with GKE cluster access +# TEST_XML_OUTPUT_DIR: Output directory for the test xUnit XML report +# CLIENT_IMAGE_NAME: Test client Docker image name +# GIT_COMMIT: SHA-1 of git commit being built +# TESTING_VERSION: version branch under test: used by the framework to determine the supported PSM +# features. +# Arguments: +# Test case name +# Outputs: +# Writes the output of test execution to stdout, stderr +# Test xUnit report to ${TEST_XML_OUTPUT_DIR}/${test_name}/sponge_log.xml +####################################### +run_test() { + # Test driver usage: + # https://github.com/grpc/grpc/tree/master/tools/run_tests/xds_k8s_test_driver#basic-usage + local test_name="${1:?Usage: run_test test_name}" + set -x + python3 -m "tests.${test_name}" \ + --flagfile="${TEST_DRIVER_FLAGFILE}" \ + --kube_context="${KUBE_CONTEXT}" \ + --client_image="${CLIENT_IMAGE_NAME}:${GIT_COMMIT}" \ + --testing_version="${TESTING_VERSION}" \ + --xml_output_file="${TEST_XML_OUTPUT_DIR}/${test_name}/sponge_log.xml" \ + --flagfile="config/url-map.cfg" + set +x +} + +####################################### +# Main function: provision software necessary to execute tests, and run them +# Globals: +# KOKORO_ARTIFACTS_DIR +# GITHUB_REPOSITORY_NAME +# SRC_DIR: Populated with absolute path to the source repo +# TEST_DRIVER_REPO_DIR: Populated with the path to the repo containing +# the test driver +# TEST_DRIVER_FULL_DIR: Populated with the path to the test driver source code +# TEST_DRIVER_FLAGFILE: Populated with relative path to test driver flagfile +# TEST_XML_OUTPUT_DIR: Populated with the path to test xUnit XML report +# GIT_ORIGIN_URL: Populated with the origin URL of git repo used for the build +# GIT_COMMIT: Populated with the SHA-1 of git commit being built +# GIT_COMMIT_SHORT: Populated with the short SHA-1 of git commit being built +# KUBE_CONTEXT: Populated with name of kubectl context with GKE cluster access +# Arguments: +# None +# Outputs: +# Writes the output of test execution to stdout, stderr +####################################### +main() { + local script_dir + script_dir="$(dirname "$0")" + + # Source the test driver from the master branch. + echo "Sourcing test driver install script from: ${TEST_DRIVER_INSTALL_SCRIPT_URL}" + source /dev/stdin <<< "$(curl -s "${TEST_DRIVER_INSTALL_SCRIPT_URL}")" + + activate_gke_cluster GKE_CLUSTER_PSM_BASIC + + set -x + if [[ -n "${KOKORO_ARTIFACTS_DIR}" ]]; then + kokoro_setup_test_driver "${GITHUB_REPOSITORY_NAME}" + else + local_setup_test_driver "${script_dir}" + fi + build_docker_images_if_needed + # Run tests + cd "${TEST_DRIVER_FULL_DIR}" + run_test url_map +} + +main "$@" diff --git a/test/kokoro/xds_k8s_url_map.cfg b/test/kokoro/xds_k8s_url_map.cfg new file mode 100644 index 00000000..dd4cce76 --- /dev/null +++ b/test/kokoro/xds_k8s_url_map.cfg @@ -0,0 +1,26 @@ +# Copyright 2022 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Config file for Kokoro (in protobuf text format) + +# Location of the continuous shell script in repository. +build_file: "grpc-node/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh" +timeout_mins: 180 +action { + define_artifacts { + regex: "artifacts/**/*sponge_log.xml" + regex: "artifacts/**/*sponge_log.log" + strip_prefix: "artifacts" + } +} From 47461134fb1a282517e8c2717f3e98c26488f229 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 23 Jun 2022 15:28:22 -0700 Subject: [PATCH 39/56] Update copyright date --- packages/grpc-js-xds/scripts/xds_k8s_url_map.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh b/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh index ca33cf0d..901938ad 100644 --- a/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh +++ b/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh @@ -1,5 +1,5 @@ #!/usr/bin/env bash -# Copyright 2021 gRPC authors. +# Copyright 2022 gRPC authors. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. From 1ad6a58059b6637a408b836d8ad591135e859d3d Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Fri, 24 Jun 2022 10:20:01 -0700 Subject: [PATCH 40/56] xDS k8s tests: fix a line in the url_map test script The line copied from the Python script was hardcoded and not handled by a variable, so I did not change it on the first pass. --- packages/grpc-js-xds/scripts/xds_k8s_url_map.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh b/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh index 901938ad..bf2d4df6 100644 --- a/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh +++ b/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh @@ -40,7 +40,7 @@ build_test_app_docker_images() { pushd "${SRC_DIR}" docker build \ - -f src/python/grpcio_tests/tests_py3_only/interop/Dockerfile.client \ + -f ${BUILD_APP_PATH} \ -t "${CLIENT_IMAGE_NAME}:${GIT_COMMIT}" \ . From 5a039aa90b743919782cd64d349830ce46162f82 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Fri, 24 Jun 2022 15:29:52 -0700 Subject: [PATCH 41/56] Use quotation marks for variable substitution Co-authored-by: Sergii Tkachenko --- packages/grpc-js-xds/scripts/xds_k8s_url_map.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh b/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh index bf2d4df6..8b07b655 100644 --- a/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh +++ b/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh @@ -40,7 +40,7 @@ build_test_app_docker_images() { pushd "${SRC_DIR}" docker build \ - -f ${BUILD_APP_PATH} \ + -f "${BUILD_APP_PATH}" \ -t "${CLIENT_IMAGE_NAME}:${GIT_COMMIT}" \ . From 4db0cd2ac995efe08185f6b825b2aa641441dc57 Mon Sep 17 00:00:00 2001 From: Sergii Tkachenko Date: Fri, 24 Jun 2022 16:08:59 -0700 Subject: [PATCH 42/56] xDS k8s tests: fix docker auth error in the build scripts Docker auth needed to be done before pushing thats to the GCR. Also add missing versioned image tagging to xds_k8s_url_map.sh. --- packages/grpc-js-xds/scripts/xds_k8s_lb.sh | 8 +++----- packages/grpc-js-xds/scripts/xds_k8s_url_map.sh | 8 +++++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/grpc-js-xds/scripts/xds_k8s_lb.sh b/packages/grpc-js-xds/scripts/xds_k8s_lb.sh index 58a95137..ca300f0c 100755 --- a/packages/grpc-js-xds/scripts/xds_k8s_lb.sh +++ b/packages/grpc-js-xds/scripts/xds_k8s_lb.sh @@ -31,6 +31,7 @@ readonly LANGUAGE_NAME="Node" # BUILD_APP_PATH # CLIENT_IMAGE_NAME: Test client Docker image name # GIT_COMMIT: SHA-1 of git commit being built +# TESTING_VERSION: version branch under test, f.e. v1.42.x, master # Arguments: # None # Outputs: @@ -45,15 +46,12 @@ build_test_app_docker_images() { -t "${CLIENT_IMAGE_NAME}:${GIT_COMMIT}" \ . + gcloud -q auth configure-docker + docker push "${CLIENT_IMAGE_NAME}:${GIT_COMMIT}" if is_version_branch "${TESTING_VERSION}"; then tag_and_push_docker_image "${CLIENT_IMAGE_NAME}" "${GIT_COMMIT}" "${TESTING_VERSION}" fi - popd - - gcloud -q auth configure-docker - - docker push "${CLIENT_IMAGE_NAME}:${GIT_COMMIT}" } ####################################### diff --git a/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh b/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh index 901938ad..a865e4e5 100644 --- a/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh +++ b/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh @@ -30,6 +30,7 @@ readonly LANGUAGE_NAME="Node" # BUILD_APP_PATH # CLIENT_IMAGE_NAME: Test client Docker image name # GIT_COMMIT: SHA-1 of git commit being built +# TESTING_VERSION: version branch under test, f.e. v1.42.x, master # Arguments: # None # Outputs: @@ -44,11 +45,12 @@ build_test_app_docker_images() { -t "${CLIENT_IMAGE_NAME}:${GIT_COMMIT}" \ . - popd - gcloud -q auth configure-docker - docker push "${CLIENT_IMAGE_NAME}:${GIT_COMMIT}" + if is_version_branch "${TESTING_VERSION}"; then + tag_and_push_docker_image "${CLIENT_IMAGE_NAME}" "${GIT_COMMIT}" "${TESTING_VERSION}" + fi + popd } ####################################### From 1463ffa42ed2d5de7c23c772684b944a60c5c90c Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Tue, 28 Jun 2022 14:05:29 -0700 Subject: [PATCH 43/56] Backport xDS k8s test scripts to the v1.6.x branch --- packages/grpc-js-xds/scripts/xds_k8s_lb.sh | 172 ++++++++++++++++++ .../grpc-js-xds/scripts/xds_k8s_url_map.sh | 154 ++++++++++++++++ test/kokoro/xds_k8s_lb.cfg | 26 +++ test/kokoro/xds_k8s_url_map.cfg | 26 +++ 4 files changed, 378 insertions(+) create mode 100755 packages/grpc-js-xds/scripts/xds_k8s_lb.sh create mode 100644 packages/grpc-js-xds/scripts/xds_k8s_url_map.sh create mode 100644 test/kokoro/xds_k8s_lb.cfg create mode 100644 test/kokoro/xds_k8s_url_map.cfg diff --git a/packages/grpc-js-xds/scripts/xds_k8s_lb.sh b/packages/grpc-js-xds/scripts/xds_k8s_lb.sh new file mode 100755 index 00000000..ca300f0c --- /dev/null +++ b/packages/grpc-js-xds/scripts/xds_k8s_lb.sh @@ -0,0 +1,172 @@ +#!/usr/bin/env bash +# Copyright 2022 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +set -eo pipefail + +# Constants +readonly GITHUB_REPOSITORY_NAME="grpc-node" +readonly TEST_DRIVER_INSTALL_SCRIPT_URL="https://raw.githubusercontent.com/${TEST_DRIVER_REPO_OWNER:-grpc}/grpc/${TEST_DRIVER_BRANCH:-master}/tools/internal_ci/linux/grpc_xds_k8s_install_test_driver.sh" +## xDS test client Docker images +readonly SERVER_IMAGE_NAME="gcr.io/grpc-testing/xds-interop/java-server:v1.46.x" +readonly CLIENT_IMAGE_NAME="gcr.io/grpc-testing/xds-interop/node-client" +readonly FORCE_IMAGE_BUILD="${FORCE_IMAGE_BUILD:-0}" +readonly BUILD_APP_PATH="packages/grpc-js-xds/interop/Dockerfile" +readonly LANGUAGE_NAME="Node" + +####################################### +# Builds test app Docker images and pushes them to GCR +# Globals: +# BUILD_APP_PATH +# CLIENT_IMAGE_NAME: Test client Docker image name +# GIT_COMMIT: SHA-1 of git commit being built +# TESTING_VERSION: version branch under test, f.e. v1.42.x, master +# Arguments: +# None +# Outputs: +# Writes the output of `gcloud builds submit` to stdout, stderr +####################################### +build_test_app_docker_images() { + echo "Building ${LANGUAGE_NAME} xDS interop test app Docker images" + + pushd "${SRC_DIR}" + docker build \ + -f "${BUILD_APP_PATH}" \ + -t "${CLIENT_IMAGE_NAME}:${GIT_COMMIT}" \ + . + + gcloud -q auth configure-docker + docker push "${CLIENT_IMAGE_NAME}:${GIT_COMMIT}" + if is_version_branch "${TESTING_VERSION}"; then + tag_and_push_docker_image "${CLIENT_IMAGE_NAME}" "${GIT_COMMIT}" "${TESTING_VERSION}" + fi + popd +} + +####################################### +# Builds test app and its docker images unless they already exist +# Globals: +# CLIENT_IMAGE_NAME: Test client Docker image name +# GIT_COMMIT: SHA-1 of git commit being built +# FORCE_IMAGE_BUILD +# Arguments: +# None +# Outputs: +# Writes the output to stdout, stderr +####################################### +build_docker_images_if_needed() { + # Check if images already exist + client_tags="$(gcloud_gcr_list_image_tags "${CLIENT_IMAGE_NAME}" "${GIT_COMMIT}")" + printf "Client image: %s:%s\n" "${CLIENT_IMAGE_NAME}" "${GIT_COMMIT}" + echo "${client_tags:-Client image not found}" + + # Build if any of the images are missing, or FORCE_IMAGE_BUILD=1 + if [[ "${FORCE_IMAGE_BUILD}" == "1" || -z "${client_tags}" ]]; then + build_test_app_docker_images + else + echo "Skipping ${LANGUAGE_NAME} test app build" + fi +} + +####################################### +# Executes the test case +# Globals: +# TEST_DRIVER_FLAGFILE: Relative path to test driver flagfile +# KUBE_CONTEXT: The name of kubectl context with GKE cluster access +# SECONDARY_KUBE_CONTEXT: The name of kubectl context with secondary GKE cluster access, if any +# TEST_XML_OUTPUT_DIR: Output directory for the test xUnit XML report +# CLIENT_IMAGE_NAME: Test client Docker image name +# GIT_COMMIT: SHA-1 of git commit being built +# Arguments: +# Test case name +# Outputs: +# Writes the output of test execution to stdout, stderr +# Test xUnit report to ${TEST_XML_OUTPUT_DIR}/${test_name}/sponge_log.xml +####################################### +run_test() { + # Test driver usage: + # https://github.com/grpc/grpc/tree/master/tools/run_tests/xds_k8s_test_driver#basic-usage + local test_name="${1:?Usage: run_test test_name}" + # testing_version is used by the framework to determine the supported PSM + # features. It's captured from Kokoro job name of the Node repo, which takes + # the form: + # grpc/node// + python3 -m "tests.${test_name}" \ + --flagfile="${TEST_DRIVER_FLAGFILE}" \ + --kube_context="${KUBE_CONTEXT}" \ + --secondary_kube_context="${SECONDARY_KUBE_CONTEXT}" \ + --client_image="${CLIENT_IMAGE_NAME}:${GIT_COMMIT}" \ + --server_image="${SERVER_IMAGE_NAME}" \ + --testing_version="${TESTING_VERSION}" \ + --xml_output_file="${TEST_XML_OUTPUT_DIR}/${test_name}/sponge_log.xml" +} + +####################################### +# Main function: provision software necessary to execute tests, and run them +# Globals: +# KOKORO_ARTIFACTS_DIR +# GITHUB_REPOSITORY_NAME +# SRC_DIR: Populated with absolute path to the source repo +# TEST_DRIVER_REPO_DIR: Populated with the path to the repo containing +# the test driver +# TEST_DRIVER_FULL_DIR: Populated with the path to the test driver source code +# TEST_DRIVER_FLAGFILE: Populated with relative path to test driver flagfile +# TEST_XML_OUTPUT_DIR: Populated with the path to test xUnit XML report +# GIT_ORIGIN_URL: Populated with the origin URL of git repo used for the build +# GIT_COMMIT: Populated with the SHA-1 of git commit being built +# GIT_COMMIT_SHORT: Populated with the short SHA-1 of git commit being built +# KUBE_CONTEXT: Populated with name of kubectl context with GKE cluster access +# SECONDARY_KUBE_CONTEXT: Populated with name of kubectl context with secondary GKE cluster access, if any +# Arguments: +# None +# Outputs: +# Writes the output of test execution to stdout, stderr +####################################### +main() { + local script_dir + script_dir="$(dirname "$0")" + + cd "${script_dir}" + + git submodule update --init --recursive + + # Source the test driver from the master branch. + echo "Sourcing test driver install script from: ${TEST_DRIVER_INSTALL_SCRIPT_URL}" + source /dev/stdin <<< "$(curl -s "${TEST_DRIVER_INSTALL_SCRIPT_URL}")" + + activate_gke_cluster GKE_CLUSTER_PSM_LB + activate_secondary_gke_cluster GKE_CLUSTER_PSM_LB + + set -x + if [[ -n "${KOKORO_ARTIFACTS_DIR}" ]]; then + kokoro_setup_test_driver "${GITHUB_REPOSITORY_NAME}" + else + local_setup_test_driver "${script_dir}" + fi + build_docker_images_if_needed + + # Run tests + cd "${TEST_DRIVER_FULL_DIR}" + local failed_tests=0 + test_suites=("baseline_test" "api_listener_test" "change_backend_service_test" "failover_test" "remove_neg_test" "round_robin_test") + for test in "${test_suites[@]}"; do + run_test $test || (( failed_tests++ )) + done + echo "Failed test suites: ${failed_tests}" + if (( failed_tests > 0 )); then + exit 1 + fi +} + +main "$@" diff --git a/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh b/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh new file mode 100644 index 00000000..4371f235 --- /dev/null +++ b/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh @@ -0,0 +1,154 @@ +#!/usr/bin/env bash +# Copyright 2022 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +set -eo pipefail + +# Constants +readonly GITHUB_REPOSITORY_NAME="grpc-node" +readonly TEST_DRIVER_INSTALL_SCRIPT_URL="https://raw.githubusercontent.com/${TEST_DRIVER_REPO_OWNER:-grpc}/grpc/${TEST_DRIVER_BRANCH:-master}/tools/internal_ci/linux/grpc_xds_k8s_install_test_driver.sh" +## xDS test client Docker images +readonly CLIENT_IMAGE_NAME="gcr.io/grpc-testing/xds-interop/node-client" +readonly FORCE_IMAGE_BUILD="${FORCE_IMAGE_BUILD:-0}" +readonly BUILD_APP_PATH="packages/grpc-js-xds/interop/Dockerfile" +readonly LANGUAGE_NAME="Node" + +####################################### +# Builds test app Docker images and pushes them to GCR +# Globals: +# BUILD_APP_PATH +# CLIENT_IMAGE_NAME: Test client Docker image name +# GIT_COMMIT: SHA-1 of git commit being built +# TESTING_VERSION: version branch under test, f.e. v1.42.x, master +# Arguments: +# None +# Outputs: +# Writes the output of `gcloud builds submit` to stdout, stderr +####################################### +build_test_app_docker_images() { + echo "Building ${LANGUAGE_NAME} xDS interop test app Docker images" + + pushd "${SRC_DIR}" + docker build \ + -f "${BUILD_APP_PATH}" \ + -t "${CLIENT_IMAGE_NAME}:${GIT_COMMIT}" \ + . + + gcloud -q auth configure-docker + docker push "${CLIENT_IMAGE_NAME}:${GIT_COMMIT}" + if is_version_branch "${TESTING_VERSION}"; then + tag_and_push_docker_image "${CLIENT_IMAGE_NAME}" "${GIT_COMMIT}" "${TESTING_VERSION}" + fi + popd +} + +####################################### +# Builds test app and its docker images unless they already exist +# Globals: +# CLIENT_IMAGE_NAME: Test client Docker image name +# GIT_COMMIT: SHA-1 of git commit being built +# FORCE_IMAGE_BUILD +# Arguments: +# None +# Outputs: +# Writes the output to stdout, stderr +####################################### +build_docker_images_if_needed() { + # Check if images already exist + client_tags="$(gcloud_gcr_list_image_tags "${CLIENT_IMAGE_NAME}" "${GIT_COMMIT}")" + printf "Client image: %s:%s\n" "${CLIENT_IMAGE_NAME}" "${GIT_COMMIT}" + echo "${client_tags:-Client image not found}" + + # Build if any of the images are missing, or FORCE_IMAGE_BUILD=1 + if [[ "${FORCE_IMAGE_BUILD}" == "1" || -z "${client_tags}" ]]; then + build_test_app_docker_images + else + echo "Skipping ${LANGUAGE_NAME} test app build" + fi +} + +####################################### +# Executes the test case +# Globals: +# TEST_DRIVER_FLAGFILE: Relative path to test driver flagfile +# KUBE_CONTEXT: The name of kubectl context with GKE cluster access +# TEST_XML_OUTPUT_DIR: Output directory for the test xUnit XML report +# CLIENT_IMAGE_NAME: Test client Docker image name +# GIT_COMMIT: SHA-1 of git commit being built +# TESTING_VERSION: version branch under test: used by the framework to determine the supported PSM +# features. +# Arguments: +# Test case name +# Outputs: +# Writes the output of test execution to stdout, stderr +# Test xUnit report to ${TEST_XML_OUTPUT_DIR}/${test_name}/sponge_log.xml +####################################### +run_test() { + # Test driver usage: + # https://github.com/grpc/grpc/tree/master/tools/run_tests/xds_k8s_test_driver#basic-usage + local test_name="${1:?Usage: run_test test_name}" + set -x + python3 -m "tests.${test_name}" \ + --flagfile="${TEST_DRIVER_FLAGFILE}" \ + --kube_context="${KUBE_CONTEXT}" \ + --client_image="${CLIENT_IMAGE_NAME}:${GIT_COMMIT}" \ + --testing_version="${TESTING_VERSION}" \ + --xml_output_file="${TEST_XML_OUTPUT_DIR}/${test_name}/sponge_log.xml" \ + --flagfile="config/url-map.cfg" + set +x +} + +####################################### +# Main function: provision software necessary to execute tests, and run them +# Globals: +# KOKORO_ARTIFACTS_DIR +# GITHUB_REPOSITORY_NAME +# SRC_DIR: Populated with absolute path to the source repo +# TEST_DRIVER_REPO_DIR: Populated with the path to the repo containing +# the test driver +# TEST_DRIVER_FULL_DIR: Populated with the path to the test driver source code +# TEST_DRIVER_FLAGFILE: Populated with relative path to test driver flagfile +# TEST_XML_OUTPUT_DIR: Populated with the path to test xUnit XML report +# GIT_ORIGIN_URL: Populated with the origin URL of git repo used for the build +# GIT_COMMIT: Populated with the SHA-1 of git commit being built +# GIT_COMMIT_SHORT: Populated with the short SHA-1 of git commit being built +# KUBE_CONTEXT: Populated with name of kubectl context with GKE cluster access +# Arguments: +# None +# Outputs: +# Writes the output of test execution to stdout, stderr +####################################### +main() { + local script_dir + script_dir="$(dirname "$0")" + + # Source the test driver from the master branch. + echo "Sourcing test driver install script from: ${TEST_DRIVER_INSTALL_SCRIPT_URL}" + source /dev/stdin <<< "$(curl -s "${TEST_DRIVER_INSTALL_SCRIPT_URL}")" + + activate_gke_cluster GKE_CLUSTER_PSM_BASIC + + set -x + if [[ -n "${KOKORO_ARTIFACTS_DIR}" ]]; then + kokoro_setup_test_driver "${GITHUB_REPOSITORY_NAME}" + else + local_setup_test_driver "${script_dir}" + fi + build_docker_images_if_needed + # Run tests + cd "${TEST_DRIVER_FULL_DIR}" + run_test url_map +} + +main "$@" diff --git a/test/kokoro/xds_k8s_lb.cfg b/test/kokoro/xds_k8s_lb.cfg new file mode 100644 index 00000000..b9940cfd --- /dev/null +++ b/test/kokoro/xds_k8s_lb.cfg @@ -0,0 +1,26 @@ +# Copyright 2022 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Config file for Kokoro (in protobuf text format) + +# Location of the continuous shell script in repository. +build_file: "grpc-node/packages/grpc-js-xds/scripts/xds_k8s_lb.sh" +timeout_mins: 180 +action { + define_artifacts { + regex: "artifacts/**/*sponge_log.xml" + regex: "artifacts/**/*sponge_log.log" + strip_prefix: "artifacts" + } +} diff --git a/test/kokoro/xds_k8s_url_map.cfg b/test/kokoro/xds_k8s_url_map.cfg new file mode 100644 index 00000000..dd4cce76 --- /dev/null +++ b/test/kokoro/xds_k8s_url_map.cfg @@ -0,0 +1,26 @@ +# Copyright 2022 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Config file for Kokoro (in protobuf text format) + +# Location of the continuous shell script in repository. +build_file: "grpc-node/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh" +timeout_mins: 180 +action { + define_artifacts { + regex: "artifacts/**/*sponge_log.xml" + regex: "artifacts/**/*sponge_log.log" + strip_prefix: "artifacts" + } +} From 15a962aab629bf7c3c001407f4244279fc72c6e3 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 7 Jul 2022 10:37:15 -0700 Subject: [PATCH 44/56] Enable xDS tracing in k8s framework tests --- packages/grpc-js-xds/interop/Dockerfile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/grpc-js-xds/interop/Dockerfile b/packages/grpc-js-xds/interop/Dockerfile index bc85d3b2..e8a0a98c 100644 --- a/packages/grpc-js-xds/interop/Dockerfile +++ b/packages/grpc-js-xds/interop/Dockerfile @@ -32,4 +32,7 @@ WORKDIR /node/src/grpc-node COPY --from=build /node/src/grpc-node/packages/grpc-js ./packages/grpc-js/ COPY --from=build /node/src/grpc-node/packages/grpc-js-xds ./packages/grpc-js-xds/ +ENV GRPC_VERBOSITY="DEBUG" +ENV GRPC_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 + ENTRYPOINT [ "node", "/node/src/grpc-node/packages/grpc-js-xds/build/interop/xds-interop-client" ] From c6d7d2aa033b2ebd88bd010552124c47e9af79e8 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 7 Jul 2022 14:34:20 -0700 Subject: [PATCH 45/56] Backport xDS k8s interop docker image to version branch --- packages/grpc-js-xds/interop/Dockerfile | 38 +++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 packages/grpc-js-xds/interop/Dockerfile diff --git a/packages/grpc-js-xds/interop/Dockerfile b/packages/grpc-js-xds/interop/Dockerfile new file mode 100644 index 00000000..e8a0a98c --- /dev/null +++ b/packages/grpc-js-xds/interop/Dockerfile @@ -0,0 +1,38 @@ +# Copyright 2022 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Dockerfile for building the xDS interop client. To build the image, run the +# following command from grpc-node directory: +# docker build -t -f packages/grpc-js-xds/interop/Dockerfile . + +FROM node:16-alpine as build + +# Make a grpc-node directory and copy the repo into it. +WORKDIR /node/src/grpc-node +COPY . . + +WORKDIR /node/src/grpc-node/packages/grpc-js +RUN npm install +WORKDIR /node/src/grpc-node/packages/grpc-js-xds +RUN npm install + +FROM node:16-alpine +WORKDIR /node/src/grpc-node +COPY --from=build /node/src/grpc-node/packages/grpc-js ./packages/grpc-js/ +COPY --from=build /node/src/grpc-node/packages/grpc-js-xds ./packages/grpc-js-xds/ + +ENV GRPC_VERBOSITY="DEBUG" +ENV GRPC_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 + +ENTRYPOINT [ "node", "/node/src/grpc-node/packages/grpc-js-xds/build/interop/xds-interop-client" ] From 914ccdc19d52b22e7ea5fc5e8250984a68165201 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Fri, 8 Jul 2022 10:05:07 -0700 Subject: [PATCH 46/56] Clone submodules in xds k8s url map script --- packages/grpc-js-xds/scripts/xds_k8s_url_map.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh b/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh index 4371f235..c29d0c0a 100644 --- a/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh +++ b/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh @@ -133,6 +133,10 @@ main() { local script_dir script_dir="$(dirname "$0")" + cd "${script_dir}" + + git submodule update --init --recursive + # Source the test driver from the master branch. echo "Sourcing test driver install script from: ${TEST_DRIVER_INSTALL_SCRIPT_URL}" source /dev/stdin <<< "$(curl -s "${TEST_DRIVER_INSTALL_SCRIPT_URL}")" From 6641494e029e648206b42e218a4a8d9536955f8b Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Fri, 8 Jul 2022 10:05:07 -0700 Subject: [PATCH 47/56] Clone submodules in xds k8s url map script --- packages/grpc-js-xds/scripts/xds_k8s_url_map.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh b/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh index 4371f235..c29d0c0a 100644 --- a/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh +++ b/packages/grpc-js-xds/scripts/xds_k8s_url_map.sh @@ -133,6 +133,10 @@ main() { local script_dir script_dir="$(dirname "$0")" + cd "${script_dir}" + + git submodule update --init --recursive + # Source the test driver from the master branch. echo "Sourcing test driver install script from: ${TEST_DRIVER_INSTALL_SCRIPT_URL}" source /dev/stdin <<< "$(curl -s "${TEST_DRIVER_INSTALL_SCRIPT_URL}")" From 7b4704cc921cddf7c6084c69704ac940c4450b98 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Mon, 11 Jul 2022 11:32:04 -0700 Subject: [PATCH 48/56] proto-loader: Update protobufjs dependency to 7.x --- packages/proto-loader/package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/proto-loader/package.json b/packages/proto-loader/package.json index 65639c50..07e66c9f 100644 --- a/packages/proto-loader/package.json +++ b/packages/proto-loader/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/proto-loader", - "version": "0.6.13", + "version": "0.7.0", "author": "Google Inc.", "contributors": [ { @@ -48,7 +48,7 @@ "@types/long": "^4.0.1", "lodash.camelcase": "^4.3.0", "long": "^4.0.0", - "protobufjs": "^6.11.3", + "protobufjs": "^7.0.0", "yargs": "^16.2.0" }, "devDependencies": { From 27b7bb8928118292892dd5727089be32b325fed3 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Tue, 19 Jul 2022 10:52:02 -0700 Subject: [PATCH 49/56] grpc-js: Update proto-loader dependency to ^0.7.0 --- packages/grpc-js/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index 723b108f..5fae717a 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -58,7 +58,7 @@ "generate-test-types": "proto-loader-gen-types --keepCase --longs String --enums String --defaults --oneofs --includeComments --include-dirs test/fixtures/ -O test/generated/ --grpcLib ../../src/index test_service.proto" }, "dependencies": { - "@grpc/proto-loader": "^0.6.4", + "@grpc/proto-loader": "^0.7.0", "@types/node": ">=12.12.47" }, "files": [ From 50c58238ff62b5e8d94fd4e795ff42ebc76d08a4 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Wed, 20 Jul 2022 15:22:53 -0700 Subject: [PATCH 50/56] grpc-js: Update version to 1.6.8 --- packages/grpc-js/package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/grpc-js/package.json b/packages/grpc-js/package.json index 5fae717a..3ceaefeb 100644 --- a/packages/grpc-js/package.json +++ b/packages/grpc-js/package.json @@ -1,6 +1,6 @@ { "name": "@grpc/grpc-js", - "version": "1.6.7", + "version": "1.6.8", "description": "gRPC Library for Node - pure JS implementation", "homepage": "https://grpc.io/", "repository": "https://github.com/grpc/grpc-node/tree/master/packages/grpc-js", From fbf7944646b4286b97a93c15274180ab5ac29b6c Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Fri, 22 Jul 2022 12:58:22 -0700 Subject: [PATCH 51/56] grpc-js: Outlier detection: Fix standard deviation calculation --- packages/grpc-js/src/load-balancer-outlier-detection.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/grpc-js/src/load-balancer-outlier-detection.ts b/packages/grpc-js/src/load-balancer-outlier-detection.ts index 2c231c40..dde618b7 100644 --- a/packages/grpc-js/src/load-balancer-outlier-detection.ts +++ b/packages/grpc-js/src/load-balancer-outlier-detection.ts @@ -430,11 +430,12 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer { // Step 2 const successRateMean = successRates.reduce((a, b) => a + b) / successRates.length; - let successRateVariance = 0; + let successRateDeviationSum = 0; for (const rate of successRates) { const deviation = rate - successRateMean; - successRateVariance += deviation * deviation; + successRateDeviationSum += deviation * deviation; } + const successRateVariance = successRateDeviationSum / successRates.length; const successRateStdev = Math.sqrt(successRateVariance); const ejectionThreshold = successRateMean - successRateStdev * (successRateConfig.stdev_factor / 1000); From 90e8886d988bd5348c994bfb5d29a88df6a84782 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Tue, 2 Aug 2022 11:02:02 -0700 Subject: [PATCH 52/56] grpc-js: Add outlier detection tracing and enable it in interop tests --- packages/grpc-js-xds/interop/Dockerfile | 2 +- .../src/load-balancer-outlier-detection.ts | 31 ++++++++++++++++--- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/packages/grpc-js-xds/interop/Dockerfile b/packages/grpc-js-xds/interop/Dockerfile index e8a0a98c..b93e309d 100644 --- a/packages/grpc-js-xds/interop/Dockerfile +++ b/packages/grpc-js-xds/interop/Dockerfile @@ -33,6 +33,6 @@ COPY --from=build /node/src/grpc-node/packages/grpc-js ./packages/grpc-js/ COPY --from=build /node/src/grpc-node/packages/grpc-js-xds ./packages/grpc-js-xds/ ENV GRPC_VERBOSITY="DEBUG" -ENV GRPC_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 +ENV GRPC_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,outlier_detection ENTRYPOINT [ "node", "/node/src/grpc-node/packages/grpc-js-xds/build/interop/xds-interop-client" ] diff --git a/packages/grpc-js/src/load-balancer-outlier-detection.ts b/packages/grpc-js/src/load-balancer-outlier-detection.ts index dde618b7..e9793bea 100644 --- a/packages/grpc-js/src/load-balancer-outlier-detection.ts +++ b/packages/grpc-js/src/load-balancer-outlier-detection.ts @@ -18,7 +18,7 @@ import { ChannelOptions, connectivityState, StatusObject } from "."; import { Call } from "./call-stream"; import { ConnectivityState } from "./connectivity-state"; -import { Status } from "./constants"; +import { LogVerbosity, Status } from "./constants"; import { durationToMs, isDuration, msToDuration } from "./duration"; import { ChannelControlHelper, createChildChannelControlHelper, registerLoadBalancerType } from "./experimental"; import { BaseFilter, Filter, FilterFactory } from "./filter"; @@ -28,7 +28,13 @@ import { PickArgs, Picker, PickResult, PickResultType, QueuePicker, UnavailableP import { Subchannel } from "./subchannel"; import { SubchannelAddress, subchannelAddressToString } from "./subchannel-address"; import { BaseSubchannelWrapper, ConnectivityStateListener, SubchannelInterface } from "./subchannel-interface"; +import * as logging from './logging'; +const TRACER_NAME = 'outlier_detection'; + +function trace(text: string): void { + logging.trace(LogVerbosity.DEBUG, TRACER_NAME, text); +} const TYPE_NAME = 'outlier_detection'; @@ -412,6 +418,7 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer { if (!successRateConfig) { return; } + trace('Running success rate check'); // Step 1 const targetRequestVolume = successRateConfig.request_volume; let addresesWithTargetVolume = 0; @@ -424,6 +431,7 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer { successRates.push(successes/(successes + failures)); } } + trace('Found ' + addresesWithTargetVolume + ' success rate candidates; currentEjectionPercent=' + this.getCurrentEjectionPercent() + ' successRates=[' + successRates + ']'); if (addresesWithTargetVolume < successRateConfig.minimum_hosts) { return; } @@ -438,9 +446,10 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer { const successRateVariance = successRateDeviationSum / successRates.length; const successRateStdev = Math.sqrt(successRateVariance); const ejectionThreshold = successRateMean - successRateStdev * (successRateConfig.stdev_factor / 1000); + trace('stdev=' + successRateStdev + ' ejectionThreshold=' + ejectionThreshold); // Step 3 - for (const mapEntry of this.addressMap.values()) { + for (const [address, mapEntry] of this.addressMap.entries()) { // Step 3.i if (this.getCurrentEjectionPercent() > this.latestConfig.getMaxEjectionPercent()) { break; @@ -453,9 +462,12 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer { } // Step 3.iii const successRate = successes / (successes + failures); + trace('Checking candidate ' + address + ' successRate=' + successRate); if (successRate < ejectionThreshold) { const randomNumber = Math.random() * 100; + trace('Candidate ' + address + ' randomNumber=' + randomNumber + ' enforcement_percentage=' + successRateConfig.enforcement_percentage); if (randomNumber < successRateConfig.enforcement_percentage) { + trace('Ejecting candidate ' + address); this.eject(mapEntry, ejectionTimestamp); } } @@ -470,13 +482,14 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer { if (!failurePercentageConfig) { return; } + trace('Running failure percentage check. threshold=' + failurePercentageConfig.threshold + ' request volume threshold=' + failurePercentageConfig.request_volume); // Step 1 if (this.addressMap.size < failurePercentageConfig.minimum_hosts) { return; } // Step 2 - for (const mapEntry of this.addressMap.values()) { + for (const [address, mapEntry] of this.addressMap.entries()) { // Step 2.i if (this.getCurrentEjectionPercent() > this.latestConfig.getMaxEjectionPercent()) { break; @@ -484,6 +497,7 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer { // Step 2.ii const successes = mapEntry.counter.getLastSuccesses(); const failures = mapEntry.counter.getLastFailures(); + trace('Candidate successes=' + successes + ' failures=' + failures); if (successes + failures < failurePercentageConfig.request_volume) { continue; } @@ -491,7 +505,9 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer { const failurePercentage = (failures * 100) / (failures + successes); if (failurePercentage > failurePercentageConfig.threshold) { const randomNumber = Math.random() * 100; + trace('Candidate ' + address + ' randomNumber=' + randomNumber + ' enforcement_percentage=' + failurePercentageConfig.enforcement_percentage); if (randomNumber < failurePercentageConfig.enforcement_percentage) { + trace('Ejecting candidate ' + address); this.eject(mapEntry, ejectionTimestamp); } } @@ -525,6 +541,7 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer { private runChecks() { const ejectionTimestamp = new Date(); + trace('Ejection timer running'); this.switchAllBuckets(); @@ -537,7 +554,7 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer { this.runSuccessRateCheck(ejectionTimestamp); this.runFailurePercentageCheck(ejectionTimestamp); - for (const mapEntry of this.addressMap.values()) { + for (const [address, mapEntry] of this.addressMap.entries()) { if (mapEntry.currentEjectionTimestamp === null) { if (mapEntry.ejectionTimeMultiplier > 0) { mapEntry.ejectionTimeMultiplier -= 1; @@ -548,6 +565,7 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer { const returnTime = new Date(mapEntry.currentEjectionTimestamp.getTime()); returnTime.setMilliseconds(returnTime.getMilliseconds() + Math.min(baseEjectionTimeMs * mapEntry.ejectionTimeMultiplier, Math.max(baseEjectionTimeMs, maxEjectionTimeMs))); if (returnTime < new Date()) { + trace('Unejecting ' + address); this.uneject(mapEntry); } } @@ -564,6 +582,7 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer { } for (const address of subchannelAddresses) { if (!this.addressMap.has(address)) { + trace('Adding map entry for ' + address); this.addressMap.set(address, { counter: new CallCounter(), currentEjectionTimestamp: null, @@ -574,6 +593,7 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer { } for (const key of this.addressMap.keys()) { if (!subchannelAddresses.has(key)) { + trace('Removing map entry for ' + key); this.addressMap.delete(key); } } @@ -585,15 +605,18 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer { if (lbConfig.getSuccessRateEjectionConfig() || lbConfig.getFailurePercentageEjectionConfig()) { if (this.timerStartTime) { + trace('Previous timer existed. Replacing timer'); clearTimeout(this.ejectionTimer); const remainingDelay = lbConfig.getIntervalMs() - ((new Date()).getTime() - this.timerStartTime.getTime()); this.startTimer(remainingDelay); } else { + trace('Starting new timer'); this.timerStartTime = new Date(); this.startTimer(lbConfig.getIntervalMs()); this.switchAllBuckets(); } } else { + trace('Counting disabled. Cancelling timer.'); this.timerStartTime = null; clearTimeout(this.ejectionTimer); } From 1e531501550d989c7260370261ecf9423b3cf599 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Tue, 2 Aug 2022 13:48:16 -0700 Subject: [PATCH 53/56] Update outlier detection behavior for gRFC updates --- packages/grpc-js-xds/src/load-balancer-cds.ts | 5 ++--- packages/grpc-js/src/load-balancer-outlier-detection.ts | 9 ++++++++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/grpc-js-xds/src/load-balancer-cds.ts b/packages/grpc-js-xds/src/load-balancer-cds.ts index a6dbf42f..4d47b254 100644 --- a/packages/grpc-js-xds/src/load-balancer-cds.ts +++ b/packages/grpc-js-xds/src/load-balancer-cds.ts @@ -79,9 +79,8 @@ function translateOutlierDetectionConfig(outlierDetection: OutlierDetection__Out return undefined; } if (!outlierDetection) { - /* No-op outlier detection config, with max possible interval and no - * ejection criteria configured. */ - return new OutlierDetectionLoadBalancingConfig(~(1<<31), null, null, null, null, null, []); + /* No-op outlier detection config, with all fields unset. */ + return new OutlierDetectionLoadBalancingConfig(null, null, null, null, null, null, []); } let successRateConfig: Partial | null = null; /* Success rate ejection is enabled by default, so we only disable it if diff --git a/packages/grpc-js/src/load-balancer-outlier-detection.ts b/packages/grpc-js/src/load-balancer-outlier-detection.ts index e9793bea..4219e293 100644 --- a/packages/grpc-js/src/load-balancer-outlier-detection.ts +++ b/packages/grpc-js/src/load-balancer-outlier-detection.ts @@ -357,7 +357,10 @@ class OutlierDetectionPicker implements Picker { extraFilterFactories: extraFilterFactories }; } else { - return wrappedPick; + return { + ...wrappedPick, + subchannel: subchannelWrapper.getWrappedSubchannel() + } } } else { return wrappedPick; @@ -619,6 +622,10 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer { trace('Counting disabled. Cancelling timer.'); this.timerStartTime = null; clearTimeout(this.ejectionTimer); + for (const mapEntry of this.addressMap.values()) { + this.uneject(mapEntry); + mapEntry.ejectionTimeMultiplier = 0; + } } this.latestConfig = lbConfig; From b3f23d805efd1f30d44375c62de82845e78d5fd2 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 4 Aug 2022 12:54:15 -0700 Subject: [PATCH 54/56] grpc-js: Implement getConnectivityState in subchannel wrapper --- packages/grpc-js/src/load-balancer-outlier-detection.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/grpc-js/src/load-balancer-outlier-detection.ts b/packages/grpc-js/src/load-balancer-outlier-detection.ts index 4219e293..afbeb345 100644 --- a/packages/grpc-js/src/load-balancer-outlier-detection.ts +++ b/packages/grpc-js/src/load-balancer-outlier-detection.ts @@ -215,6 +215,14 @@ class OutlierDetectionSubchannelWrapper extends BaseSubchannelWrapper implements }); } + getConnectivityState(): connectivityState { + if (this.ejected) { + return ConnectivityState.TRANSIENT_FAILURE; + } else { + return this.childSubchannelState; + } + } + /** * Add a listener function to be called whenever the wrapper's * connectivity state changes. From 78fe8c6d05f82eddc270dcbd3995868e3a49c038 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 4 Aug 2022 15:56:28 -0700 Subject: [PATCH 55/56] grpc-js: Initialize connectivity state from subchannel in outlier detection subchannel wrapper --- packages/grpc-js/src/load-balancer-outlier-detection.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/grpc-js/src/load-balancer-outlier-detection.ts b/packages/grpc-js/src/load-balancer-outlier-detection.ts index afbeb345..0ef401c9 100644 --- a/packages/grpc-js/src/load-balancer-outlier-detection.ts +++ b/packages/grpc-js/src/load-balancer-outlier-detection.ts @@ -199,12 +199,13 @@ export class OutlierDetectionLoadBalancingConfig implements LoadBalancingConfig } class OutlierDetectionSubchannelWrapper extends BaseSubchannelWrapper implements SubchannelInterface { - private childSubchannelState: ConnectivityState = ConnectivityState.IDLE; + private childSubchannelState: ConnectivityState; private stateListeners: ConnectivityStateListener[] = []; private ejected: boolean = false; private refCount: number = 0; constructor(childSubchannel: SubchannelInterface, private mapEntry?: MapEntry) { super(childSubchannel); + this.childSubchannelState = childSubchannel.getConnectivityState(); childSubchannel.addConnectivityStateListener((subchannel, previousState, newState) => { this.childSubchannelState = newState; if (!this.ejected) { From 001cce7db07ee06c648e9f364003e5fb7879d052 Mon Sep 17 00:00:00 2001 From: Michael Lumish Date: Thu, 4 Aug 2022 16:02:05 -0700 Subject: [PATCH 56/56] grpc-js: Propagate ejection when recreating outlier detection subchannel wrapper --- packages/grpc-js/src/load-balancer-outlier-detection.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/grpc-js/src/load-balancer-outlier-detection.ts b/packages/grpc-js/src/load-balancer-outlier-detection.ts index 0ef401c9..ee400d45 100644 --- a/packages/grpc-js/src/load-balancer-outlier-detection.ts +++ b/packages/grpc-js/src/load-balancer-outlier-detection.ts @@ -391,6 +391,10 @@ export class OutlierDetectionLoadBalancer implements LoadBalancer { const originalSubchannel = channelControlHelper.createSubchannel(subchannelAddress, subchannelArgs); const mapEntry = this.addressMap.get(subchannelAddressToString(subchannelAddress)); const subchannelWrapper = new OutlierDetectionSubchannelWrapper(originalSubchannel, mapEntry); + if (mapEntry?.currentEjectionTimestamp !== null) { + // If the address is ejected, propagate that to the new subchannel wrapper + subchannelWrapper.eject(); + } mapEntry?.subchannelWrappers.push(subchannelWrapper); return subchannelWrapper; },