From 0e08eebd2f13ab0da6cac7b91288845cad530192 Mon Sep 17 00:00:00 2001 From: Cory Hall <43035978+corymhall@users.noreply.github.com> Date: Fri, 21 Jan 2022 12:36:51 -0500 Subject: [PATCH] fix(cli): hotswap should wait for lambda's `updateFunctionCode` to complete (#18536) There are [upcoming changes](https://aws.amazon.com/blogs/compute/coming-soon-expansion-of-aws-lambda-states-to-all-functions/) that will rollout Lambda states to all Lambda Functions. Prior to this update (current functionality) when you made an `updateFunctionCode` request the function was immediately available for both invocation and future updates. Once this change is rolled out this will no longer be the case. With Lambda states, when you make an update to a Lambda Function, it will not be available for future updates until the `LastUpdateStatus` returns `Successful`. This PR introduces a custom waiter that will wait for the update to complete before proceeding. The waiter will wait until the `State=Active` and the `LastUpdateStatus=Successful`. The `State` controls whether or not the function can be invoked, and the `LastUpdateStatus` controls whether the function can be updated. Based on this, I am not considering a deployment complete until both are successful. To see a more in depth analysis of the different values see #18386. In my testing I found that the time it took for a function to go from `LastUpdateStatus=InProgress` to `LastUpdateStatus=Successful` was: - ~1 second for a zip Function not in a VPC - ~25 seconds for a container Function or a Function in a VPC - ~2 minutes to deploy a VPC function (best proxy for StateReasonCode=Restoring) There are a couple of built in waiters that could have been used for this, namely [functionUpdated](https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/Lambda.html#functionUpdated-waiter). This waiter uses `getFunctionConfiguration` which has a quota of 15 requests/second. In addition the waiter polls every 5 seconds and this cannot be configured. Because hotswapping is sensitive to any latency that is introduced, I created a custom waiter that uses `getFunction`. `getFunction` has a quota of 100 requests/second and the custom waiter can be configured to poll every 1 second or every 5 seconds depending on what type of function is being updated. fixes #18386 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../lib/api/hotswap/lambda-functions.ts | 63 ++++-- .../api/hotswap/hotswap-deployments.test.ts | 2 +- .../test/api/hotswap/hotswap-test-setup.ts | 25 ++- ...nctions-docker-hotswap-deployments.test.ts | 62 +++++- ...mbda-functions-hotswap-deployments.test.ts | 184 +++++++++++++++++- ...nctions-inline-hotswap-deployments.test.ts | 2 +- ...rsions-aliases-hotswap-deployments.test.ts | 2 +- packages/aws-cdk/test/util/mock-sdk.ts | 4 +- 8 files changed, 323 insertions(+), 21 deletions(-) diff --git a/packages/aws-cdk/lib/api/hotswap/lambda-functions.ts b/packages/aws-cdk/lib/api/hotswap/lambda-functions.ts index d2966e756b69d..ac87e25c29655 100644 --- a/packages/aws-cdk/lib/api/hotswap/lambda-functions.ts +++ b/packages/aws-cdk/lib/api/hotswap/lambda-functions.ts @@ -1,5 +1,6 @@ import { Writable } from 'stream'; import * as archiver from 'archiver'; +import * as AWS from 'aws-sdk'; import { flatMap } from '../../util'; import { ISDK } from '../aws-auth'; import { CfnEvaluationException, EvaluateCloudFormationTemplate } from '../evaluate-cloudformation-template'; @@ -232,7 +233,7 @@ class LambdaFunctionHotswapOperation implements HotswapOperation { const operations: Promise[] = []; if (resource.code !== undefined) { - const updateFunctionCodePromise = lambda.updateFunctionCode({ + const updateFunctionCodeResponse = await lambda.updateFunctionCode({ FunctionName: this.lambdaFunctionResource.physicalName, S3Bucket: resource.code.s3Bucket, S3Key: resource.code.s3Key, @@ -240,17 +241,10 @@ class LambdaFunctionHotswapOperation implements HotswapOperation { ZipFile: resource.code.functionCodeZip, }).promise(); + await this.waitForLambdasCodeUpdateToFinish(updateFunctionCodeResponse, lambda); + // only if the code changed is there any point in publishing a new Version if (this.lambdaFunctionResource.publishVersion) { - // we need to wait for the code update to be done before publishing a new Version - await updateFunctionCodePromise; - // if we don't wait for the Function to finish updating, - // we can get a "The operation cannot be performed at this time. An update is in progress for resource:" - // error when publishing a new Version - await lambda.waitFor('functionUpdated', { - FunctionName: this.lambdaFunctionResource.physicalName, - }).promise(); - const publishVersionPromise = lambda.publishVersion({ FunctionName: this.lambdaFunctionResource.physicalName, }).promise(); @@ -269,8 +263,6 @@ class LambdaFunctionHotswapOperation implements HotswapOperation { } else { operations.push(publishVersionPromise); } - } else { - operations.push(updateFunctionCodePromise); } } @@ -304,6 +296,53 @@ class LambdaFunctionHotswapOperation implements HotswapOperation { // run all of our updates in parallel return Promise.all(operations); } + + /** + * After a Lambda Function is updated, it cannot be updated again until the + * `State=Active` and the `LastUpdateStatus=Successful`. + * + * Depending on the configuration of the Lambda Function this could happen relatively quickly + * or very slowly. For example, Zip based functions _not_ in a VPC can take ~1 second whereas VPC + * or Container functions can take ~25 seconds (and 'idle' VPC functions can take minutes). + */ + private async waitForLambdasCodeUpdateToFinish(currentFunctionConfiguration: AWS.Lambda.FunctionConfiguration, lambda: AWS.Lambda): Promise { + const functionIsInVpcOrUsesDockerForCode = currentFunctionConfiguration.VpcConfig?.VpcId || + currentFunctionConfiguration.PackageType === 'Image'; + + // if the function is deployed in a VPC or if it is a container image function + // then the update will take much longer and we can wait longer between checks + // otherwise, the update will be quick, so a 1-second delay is fine + const delaySeconds = functionIsInVpcOrUsesDockerForCode ? 5 : 1; + + // configure a custom waiter to wait for the function update to complete + (lambda as any).api.waiters.updateFunctionCodeToFinish = { + name: 'UpdateFunctionCodeToFinish', + operation: 'getFunction', + // equates to 1 minute for zip function not in a VPC and + // 5 minutes for container functions or function in a VPC + maxAttempts: 60, + delay: delaySeconds, + acceptors: [ + { + matcher: 'path', + argument: "Configuration.LastUpdateStatus == 'Successful' && Configuration.State == 'Active'", + expected: true, + state: 'success', + }, + { + matcher: 'path', + argument: 'Configuration.LastUpdateStatus', + expected: 'Failed', + state: 'failure', + }, + ], + }; + + const updateFunctionCodeWaiter = new (AWS as any).ResourceWaiter(lambda, 'updateFunctionCodeToFinish'); + await updateFunctionCodeWaiter.wait({ + FunctionName: this.lambdaFunctionResource.physicalName, + }).promise(); + } } /** diff --git a/packages/aws-cdk/test/api/hotswap/hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/hotswap-deployments.test.ts index 00a6b2095a569..67b0117a8d7ae 100644 --- a/packages/aws-cdk/test/api/hotswap/hotswap-deployments.test.ts +++ b/packages/aws-cdk/test/api/hotswap/hotswap-deployments.test.ts @@ -8,7 +8,7 @@ let mockGetEndpointSuffix: () => string; beforeEach(() => { hotswapMockSdkProvider = setup.setupHotswapTests(); - mockUpdateLambdaCode = jest.fn(); + mockUpdateLambdaCode = jest.fn().mockReturnValue({}); mockUpdateMachineDefinition = jest.fn(); mockGetEndpointSuffix = jest.fn(() => 'amazonaws.com'); hotswapMockSdkProvider.stubLambda({ diff --git a/packages/aws-cdk/test/api/hotswap/hotswap-test-setup.ts b/packages/aws-cdk/test/api/hotswap/hotswap-test-setup.ts index e47b127e68766..7b6aebb9a81ee 100644 --- a/packages/aws-cdk/test/api/hotswap/hotswap-test-setup.ts +++ b/packages/aws-cdk/test/api/hotswap/hotswap-test-setup.ts @@ -83,8 +83,29 @@ export class HotswapMockSdkProvider { }); } - public stubLambda(stubs: SyncHandlerSubsetOf) { - this.mockSdkProvider.stubLambda(stubs); + public stubLambda( + stubs: SyncHandlerSubsetOf, + serviceStubs?: SyncHandlerSubsetOf, + additionalProperties: { [key: string]: any } = {}, + ): void { + this.mockSdkProvider.stubLambda(stubs, { + api: { + waiters: {}, + }, + makeRequest() { + return { + promise: () => Promise.resolve({}), + response: {}, + addListeners: () => {}, + }; + }, + ...serviceStubs, + ...additionalProperties, + }); + } + + public getLambdaApiWaiters(): { [key: string]: any } { + return (this.mockSdkProvider.sdk.lambda() as any).api.waiters; } public setUpdateProjectMock(mockUpdateProject: (input: codebuild.UpdateProjectInput) => codebuild.UpdateProjectOutput) { diff --git a/packages/aws-cdk/test/api/hotswap/lambda-functions-docker-hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/lambda-functions-docker-hotswap-deployments.test.ts index 9aef8b8778cf0..3ed53e5fd908b 100644 --- a/packages/aws-cdk/test/api/hotswap/lambda-functions-docker-hotswap-deployments.test.ts +++ b/packages/aws-cdk/test/api/hotswap/lambda-functions-docker-hotswap-deployments.test.ts @@ -5,16 +5,26 @@ let mockUpdateLambdaCode: (params: Lambda.Types.UpdateFunctionCodeRequest) => La let mockTagResource: (params: Lambda.Types.TagResourceRequest) => {}; let mockUntagResource: (params: Lambda.Types.UntagResourceRequest) => {}; let hotswapMockSdkProvider: setup.HotswapMockSdkProvider; +let mockMakeRequest: (operation: string, params: any) => AWS.Request; beforeEach(() => { hotswapMockSdkProvider = setup.setupHotswapTests(); - mockUpdateLambdaCode = jest.fn(); + mockUpdateLambdaCode = jest.fn().mockReturnValue({ + PackageType: 'Image', + }); mockTagResource = jest.fn(); mockUntagResource = jest.fn(); + mockMakeRequest = jest.fn().mockReturnValue({ + promise: () => Promise.resolve({}), + response: {}, + addListeners: () => {}, + }); hotswapMockSdkProvider.stubLambda({ updateFunctionCode: mockUpdateLambdaCode, tagResource: mockTagResource, untagResource: mockUntagResource, + }, { + makeRequest: mockMakeRequest, }); }); @@ -65,3 +75,53 @@ test('calls the updateLambdaCode() API when it receives only a code difference i ImageUri: 'new-image', }); }); + +test('calls the getFunction() API with a delay of 5', async () => { + // GIVEN + setup.setCurrentCfnStackTemplate({ + Resources: { + Func: { + Type: 'AWS::Lambda::Function', + Properties: { + Code: { + ImageUri: 'current-image', + }, + FunctionName: 'my-function', + }, + Metadata: { + 'aws:asset:path': 'old-path', + }, + }, + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + Func: { + Type: 'AWS::Lambda::Function', + Properties: { + Code: { + ImageUri: 'new-image', + }, + FunctionName: 'my-function', + }, + Metadata: { + 'aws:asset:path': 'new-path', + }, + }, + }, + }, + }); + + // WHEN + await hotswapMockSdkProvider.tryHotswapDeployment(cdkStackArtifact); + + // THEN + expect(mockMakeRequest).toHaveBeenCalledWith('getFunction', { FunctionName: 'my-function' }); + expect(hotswapMockSdkProvider.getLambdaApiWaiters()).toEqual(expect.objectContaining({ + updateFunctionCodeToFinish: expect.objectContaining({ + name: 'UpdateFunctionCodeToFinish', + delay: 5, + }), + })); +}); diff --git a/packages/aws-cdk/test/api/hotswap/lambda-functions-hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/lambda-functions-hotswap-deployments.test.ts index 0b43563f233d9..d96b8530bce88 100644 --- a/packages/aws-cdk/test/api/hotswap/lambda-functions-hotswap-deployments.test.ts +++ b/packages/aws-cdk/test/api/hotswap/lambda-functions-hotswap-deployments.test.ts @@ -4,17 +4,25 @@ import * as setup from './hotswap-test-setup'; let mockUpdateLambdaCode: (params: Lambda.Types.UpdateFunctionCodeRequest) => Lambda.Types.FunctionConfiguration; let mockTagResource: (params: Lambda.Types.TagResourceRequest) => {}; let mockUntagResource: (params: Lambda.Types.UntagResourceRequest) => {}; +let mockMakeRequest: (operation: string, params: any) => AWS.Request; let hotswapMockSdkProvider: setup.HotswapMockSdkProvider; beforeEach(() => { hotswapMockSdkProvider = setup.setupHotswapTests(); - mockUpdateLambdaCode = jest.fn(); + mockUpdateLambdaCode = jest.fn().mockReturnValue({}); mockTagResource = jest.fn(); mockUntagResource = jest.fn(); + mockMakeRequest = jest.fn().mockReturnValue({ + promise: () => Promise.resolve({}), + response: {}, + addListeners: () => {}, + }); hotswapMockSdkProvider.stubLambda({ updateFunctionCode: mockUpdateLambdaCode, tagResource: mockTagResource, untagResource: mockUntagResource, + }, { + makeRequest: mockMakeRequest, }); }); @@ -539,3 +547,177 @@ test('does not call the updateLambdaCode() API when a resource with type that is expect(deployStackResult).toBeUndefined(); expect(mockUpdateLambdaCode).not.toHaveBeenCalled(); }); + +test('calls getFunction() after function code is updated with delay 1', async () => { + // GIVEN + setup.setCurrentCfnStackTemplate({ + Resources: { + Func: { + Type: 'AWS::Lambda::Function', + Properties: { + Code: { + S3Bucket: 'current-bucket', + S3Key: 'current-key', + }, + FunctionName: 'my-function', + }, + Metadata: { + 'aws:asset:path': 'old-path', + }, + }, + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + Func: { + Type: 'AWS::Lambda::Function', + Properties: { + Code: { + S3Bucket: 'current-bucket', + S3Key: 'new-key', + }, + FunctionName: 'my-function', + }, + Metadata: { + 'aws:asset:path': 'new-path', + }, + }, + }, + }, + }); + + // WHEN + await hotswapMockSdkProvider.tryHotswapDeployment(cdkStackArtifact); + + // THEN + expect(mockMakeRequest).toHaveBeenCalledWith('getFunction', { FunctionName: 'my-function' }); + expect(hotswapMockSdkProvider.getLambdaApiWaiters()).toEqual(expect.objectContaining({ + updateFunctionCodeToFinish: expect.objectContaining({ + name: 'UpdateFunctionCodeToFinish', + delay: 1, + }), + })); +}); + +test('calls getFunction() after function code is updated and VpcId is empty string with delay 1', async () => { + // GIVEN + mockUpdateLambdaCode = jest.fn().mockReturnValue({ + VpcConfig: { + VpcId: '', + }, + }); + hotswapMockSdkProvider.stubLambda({ + updateFunctionCode: mockUpdateLambdaCode, + tagResource: mockTagResource, + untagResource: mockUntagResource, + }); + setup.setCurrentCfnStackTemplate({ + Resources: { + Func: { + Type: 'AWS::Lambda::Function', + Properties: { + Code: { + S3Bucket: 'current-bucket', + S3Key: 'current-key', + }, + FunctionName: 'my-function', + }, + Metadata: { + 'aws:asset:path': 'old-path', + }, + }, + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + Func: { + Type: 'AWS::Lambda::Function', + Properties: { + Code: { + S3Bucket: 'current-bucket', + S3Key: 'new-key', + }, + FunctionName: 'my-function', + }, + Metadata: { + 'aws:asset:path': 'new-path', + }, + }, + }, + }, + }); + + // WHEN + await hotswapMockSdkProvider.tryHotswapDeployment(cdkStackArtifact); + + // THEN + expect(hotswapMockSdkProvider.getLambdaApiWaiters()).toEqual(expect.objectContaining({ + updateFunctionCodeToFinish: expect.objectContaining({ + name: 'UpdateFunctionCodeToFinish', + delay: 1, + }), + })); +}); + +test('calls getFunction() after function code is updated on a VPC function with delay 5', async () => { + // GIVEN + mockUpdateLambdaCode = jest.fn().mockReturnValue({ + VpcConfig: { + VpcId: 'abc', + }, + }); + hotswapMockSdkProvider.stubLambda({ + updateFunctionCode: mockUpdateLambdaCode, + tagResource: mockTagResource, + untagResource: mockUntagResource, + }); + setup.setCurrentCfnStackTemplate({ + Resources: { + Func: { + Type: 'AWS::Lambda::Function', + Properties: { + Code: { + S3Bucket: 'current-bucket', + S3Key: 'current-key', + }, + FunctionName: 'my-function', + }, + Metadata: { + 'aws:asset:path': 'old-path', + }, + }, + }, + }); + const cdkStackArtifact = setup.cdkStackArtifactOf({ + template: { + Resources: { + Func: { + Type: 'AWS::Lambda::Function', + Properties: { + Code: { + S3Bucket: 'current-bucket', + S3Key: 'new-key', + }, + FunctionName: 'my-function', + }, + Metadata: { + 'aws:asset:path': 'new-path', + }, + }, + }, + }, + }); + + // WHEN + await hotswapMockSdkProvider.tryHotswapDeployment(cdkStackArtifact); + + // THEN + expect(hotswapMockSdkProvider.getLambdaApiWaiters()).toEqual(expect.objectContaining({ + updateFunctionCodeToFinish: expect.objectContaining({ + name: 'UpdateFunctionCodeToFinish', + delay: 5, + }), + })); +}); diff --git a/packages/aws-cdk/test/api/hotswap/lambda-functions-inline-hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/lambda-functions-inline-hotswap-deployments.test.ts index 13554cc655dcb..478fd80b538bf 100644 --- a/packages/aws-cdk/test/api/hotswap/lambda-functions-inline-hotswap-deployments.test.ts +++ b/packages/aws-cdk/test/api/hotswap/lambda-functions-inline-hotswap-deployments.test.ts @@ -8,7 +8,7 @@ let hotswapMockSdkProvider: setup.HotswapMockSdkProvider; beforeEach(() => { hotswapMockSdkProvider = setup.setupHotswapTests(); - mockUpdateLambdaCode = jest.fn(); + mockUpdateLambdaCode = jest.fn().mockReturnValue({}); mockTagResource = jest.fn(); mockUntagResource = jest.fn(); hotswapMockSdkProvider.stubLambda({ diff --git a/packages/aws-cdk/test/api/hotswap/lambda-versions-aliases-hotswap-deployments.test.ts b/packages/aws-cdk/test/api/hotswap/lambda-versions-aliases-hotswap-deployments.test.ts index 4596bb2c96ac0..f937be7c5a5a0 100644 --- a/packages/aws-cdk/test/api/hotswap/lambda-versions-aliases-hotswap-deployments.test.ts +++ b/packages/aws-cdk/test/api/hotswap/lambda-versions-aliases-hotswap-deployments.test.ts @@ -8,7 +8,7 @@ let hotswapMockSdkProvider: setup.HotswapMockSdkProvider; beforeEach(() => { hotswapMockSdkProvider = setup.setupHotswapTests(); - mockUpdateLambdaCode = jest.fn(); + mockUpdateLambdaCode = jest.fn().mockReturnValue({}); mockPublishVersion = jest.fn(); mockUpdateAlias = jest.fn(); hotswapMockSdkProvider.stubLambda({ diff --git a/packages/aws-cdk/test/util/mock-sdk.ts b/packages/aws-cdk/test/util/mock-sdk.ts index a1ed61a431366..bb8eaae251c47 100644 --- a/packages/aws-cdk/test/util/mock-sdk.ts +++ b/packages/aws-cdk/test/util/mock-sdk.ts @@ -102,8 +102,8 @@ export class MockSdkProvider extends SdkProvider { (this.sdk as any).ssm = jest.fn().mockReturnValue(partialAwsService(stubs)); } - public stubLambda(stubs: SyncHandlerSubsetOf) { - (this.sdk as any).lambda = jest.fn().mockReturnValue(partialAwsService(stubs)); + public stubLambda(stubs: SyncHandlerSubsetOf, additionalProperties: { [key: string]: any } = {}) { + (this.sdk as any).lambda = jest.fn().mockReturnValue(partialAwsService(stubs, additionalProperties)); } public stubStepFunctions(stubs: SyncHandlerSubsetOf) {