feat(cli): hotswap deployments by skinny85 · Pull Request #15748 · aws/aws-cdk

skinny85 · 2021-07-24T19:46:14Z

This is the first PR implementing the "Accelerated personal deployments" RFC.

It adds a (boolean) --hotswap flag to the deploy command that attempts to perform a short-circuit deployment, updating the resource directly, and skipping CloudFormation.
If we detect that the current change cannot be short-circuited
(because it contains an infrastructure change to the CDK code, most likely),
we fall back on performing a full CloudFormation deployment,
same as if cdk deploy was called without the --hotswap flag.

In this PR, the new switch supports only Lambda functions. Later PRs will add support for new resource types.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

gitpod-io · 2021-07-24T19:46:16Z

packages/aws-cdk/README.md

packages/aws-cdk/lib/api/accelerated-deployments.ts

skinny85 · 2021-07-27T01:25:22Z

@eladb thanks for the review! Incorporated all of your comments, would appreciate another pass!

packages/aws-cdk/README.md

packages/aws-cdk/lib/api/deploy-stack.ts

packages/aws-cdk/README.md

packages/aws-cdk/lib/api/hotswap/lambda-functions.ts

packages/aws-cdk/lib/api/hotswap-deployments.ts

skinny85 · 2021-07-30T00:07:07Z

@eladb thanks for the review! Incorporated all of your comments, would appreciate another pass.

packages/aws-cdk/README.md

packages/aws-cdk/bin/cdk.ts

packages/aws-cdk/lib/api/deploy-stack.ts

packages/aws-cdk/lib/api/hotswap/lambda-functions.ts

packages/aws-cdk/lib/api/hotswap-deployments.ts

packages/aws-cdk/lib/api/util/cloudformation/evaluate-cfn.ts

skinny85 · 2021-08-28T00:21:23Z

@BenChaimberg thanks a lot for the review! I've pushed a new revision addressing your comments, would appreciate another pass!

packages/aws-cdk/lib/api/hotswap/lambda-functions.ts

packages/aws-cdk/lib/api/util/cloudformation/evaluate-cfn.ts

packages/aws-cdk/lib/api/hotswap-deployments.ts

rix0rrr · 2021-08-30T07:50:57Z

packages/aws-cdk/lib/api/deploy-stack.ts

+      return hotswapDeploymentResult;
+    }
+    // could not short-circuit the deployment, perform a full CFN deploy instead
+    print('Could not perform a hotswap deployment, as the stack %s contains non-Asset changes', stackArtifact.displayName);


💅 Putting this here is not extensible. The tryHotswapDeployment function should probably return a reason saying why it couldn't do it.

Can you elaborate what would you like to see here? Should the Lambda "hotswap detector" return something like "Change is to property 'X', while only changes to the 'Code' property are supported", or something like that? Or did you mean something else?

Yes, I meant something like that.

packages/aws-cdk/lib/api/deploy-stack.ts

rix0rrr · 2021-08-30T07:52:17Z

packages/aws-cdk/lib/api/hotswap-deployments.ts

+  const resolvedEnv = await sdkProvider.resolveEnvironment(stackArtifact.environment);
+  const hotswappableChanges = findAllHotswappableChanges(stackChanges, {
+    ...assetParams,
+    'AWS::Region': resolvedEnv.region,


AWS::Partition as well probably

And I'd think AWS::URLSuffix

Sure, but where can we get those from? I'd rather avoid doing more service calls here (this is supposed to be as fast as possible).

cdk-assets has code to get these values.

packages/aws-cdk/lib/api/hotswap-deployments.ts

packages/aws-cdk/lib/api/hotswap/lambda-functions.ts

skinny85 · 2021-08-30T19:33:53Z

@rix0rrr @BenChaimberg thanks for the review, the PR should be ready for another pass!

BenChaimberg

lgtm

skinny85

@calvin-cc In general, looks great! 2 questions.

packages/aws-cdk/bin/cdk.ts

skinny85 · 2021-09-01T21:36:35Z

packages/aws-cdk/lib/api/deploy-stack.ts

+    let disableRollback;
+
+    if (options.rollback === undefined && options.hotswap === true) {
+      disableRollback = { DisableRollback: true };
+    } else {
+      disableRollback = options.rollback === false ? { DisableRollback: true } : undefined;
+    }


We're repeating the { DisableRollback: true } expression for no reason here.

Suggested change

let disableRollback;

if (options.rollback === undefined && options.hotswap === true) {

disableRollback = { DisableRollback: true };

} else {

disableRollback = options.rollback === false ? { DisableRollback: true } : undefined;

}

const shouldDisableRollback = (options.rollback === undefined && options.hotswap === true) || options.rollback === false;

const disableRollback = shouldDisableRollback ? { DisableRollback: true } : undefined;

skinny85

Looks great! One more small simplification.

skinny85 · 2021-09-02T00:36:52Z

packages/aws-cdk/lib/cdk-toolkit.ts

          progress: options.progress,
          ci: options.ci,
-          rollback: options.rollback,
+          rollback: shouldDisableRollback ? false : true,


Since you moved this here, we can simplify further:

Suggested change

rollback: shouldDisableRollback ? false : true,

rollback: options.rollback === undefined && options.hotswap ? false : options.rollback,

And get rid of the shouldDisableRollback variable.

skinny85

We're very close! Just revert the readonly change, and move the logic down a little bit.

skinny85 · 2021-09-02T17:12:02Z

packages/aws-cdk/lib/api/deploy-stack.ts

   * @default true
   */
-  readonly rollback?: boolean;
+  rollback?: boolean;


I don't like changing this to not be readonly. See my solution below on how to handle this in a more idiomatic way. This change should be reverted.

skinny85 · 2021-09-02T17:13:50Z

packages/aws-cdk/lib/api/deploy-stack.ts

+  const shouldDisableRollback = (options.rollback === undefined && options.hotswap === true) || options.rollback === false;
+  options.rollback = shouldDisableRollback ? false : true;


Not a fan of doing this here. tryHotswapDeployment() doesn't use this option, so dealing with it here is a little confusing to me. Let's move this down to prepareAndExecuteChangeSet(), where it's actually used.

skinny85 · 2021-09-02T17:17:23Z

packages/aws-cdk/lib/api/deploy-stack.ts

+  const executionId = uuid.v4();
  const changeSet = await cfn.createChangeSet({
    StackName: deployName,
    ChangeSetName: changeSetName,


The defaulting of rollback should be done here. I can't comment on the line, because it wasn't changed, but it should be done on line 332.

skinny85

@comcalvi looks great! Thanks for the change. I will merge this PR in sec.

Mergify dismissed it accidentally

mergify · 2021-09-02T21:52:14Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2021-09-02T22:32:25Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
Commit ID: 3bbe048
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2021-09-02T22:32:50Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

This is the first PR implementing the ["Accelerated personal deployments" RFC](https://github.com/aws/aws-cdk-rfcs/blob/master/text/0001-cdk-update.md). It adds a (boolean) `--hotswap` flag to the `deploy` command that attempts to perform a short-circuit deployment, updating the resource directly, and skipping CloudFormation. If we detect that the current change cannot be short-circuited (because it contains an infrastructure change to the CDK code, most likely), we fall back on performing a full CloudFormation deployment, same as if `cdk deploy` was called without the `--hotswap` flag. In this PR, the new switch supports only Lambda functions. Later PRs will add support for new resource types. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

revmischa · 2021-09-16T06:53:43Z

Thanks so much for the work on this! I'm eager to try it out but it doesn't seem to be functional yet for nested stacks. Are there plans to support that?

skinny85 · 2021-09-16T15:57:51Z

@revmischa has opened #16508 to track this, so let's move the conversation there 🙂.

skinny85 requested a review from eladb July 24, 2021 19:46

skinny85 self-assigned this Jul 24, 2021

mergify bot added the contribution/core This is a PR that came from AWS. label Jul 24, 2021

eladb suggested changes Jul 25, 2021

View reviewed changes

skinny85 force-pushed the cdk-watch branch from 80d5e4a to 7b50f65 Compare July 27, 2021 01:20

skinny85 requested a review from eladb July 27, 2021 01:25

eladb changed the title ~~feat(cli): add the --accelerated option to the deploy command~~ feat(cli): hotswap deployments Jul 27, 2021

eladb previously requested changes Jul 29, 2021

View reviewed changes

skinny85 force-pushed the cdk-watch branch from 281c096 to 579cb08 Compare July 30, 2021 00:05

skinny85 requested a review from eladb July 30, 2021 00:06

BenChaimberg previously requested changes Aug 27, 2021

View reviewed changes

skinny85 force-pushed the cdk-watch branch from 579cb08 to f5d0a41 Compare August 28, 2021 00:18

skinny85 requested a review from BenChaimberg August 28, 2021 00:21

BenChaimberg reviewed Aug 28, 2021

View reviewed changes

packages/aws-cdk/lib/api/hotswap/lambda-functions.ts Show resolved Hide resolved

packages/aws-cdk/lib/api/util/cloudformation/evaluate-cfn.ts Show resolved Hide resolved

packages/aws-cdk/lib/api/hotswap-deployments.ts Show resolved Hide resolved

rix0rrr reviewed Aug 30, 2021

View reviewed changes

skinny85 requested review from BenChaimberg and rix0rrr August 30, 2021 19:33

BenChaimberg reviewed Aug 30, 2021

View reviewed changes

skinny85 added the pr/do-not-merge This PR should not be merged at this time. label Aug 30, 2021

rix0rrr approved these changes Aug 31, 2021

View reviewed changes

comcalvi and others added 3 commits August 31, 2021 11:16

resolved merge conflicts

8e3bea9

Ben's comments.

d36627d

Rico's comments.

d16813e

comcalvi force-pushed the cdk-watch branch from 416b287 to d16813e Compare August 31, 2021 18:27

skinny85 mentioned this pull request Aug 31, 2021

CDK Watch aws/aws-cdk-rfcs#1

Closed

11 tasks

comcalvi added 2 commits August 31, 2021 17:37

made rollback default to false if hotswap is true

f2d61e5

hotswap deployments have rollback disabled by default

5be50b1

skinny85 commented Sep 1, 2021

View reviewed changes

packages/aws-cdk/bin/cdk.ts Outdated Show resolved Hide resolved

packages/aws-cdk/bin/cdk.ts Outdated Show resolved Hide resolved

skinny85 commented Sep 1, 2021

View reviewed changes

skinny85 commented Sep 2, 2021

View reviewed changes

comcalvi force-pushed the cdk-watch branch from e9e15d6 to bad64f4 Compare September 2, 2021 17:06

skinny85 commented Sep 2, 2021

View reviewed changes

moved defaulting logic to prepareAndExecuteChangeSet()

7ea427f

comcalvi force-pushed the cdk-watch branch from bad64f4 to 7ea427f Compare September 2, 2021 17:42

removed unneeded default

65d2c95

skinny85 commented Sep 2, 2021

View reviewed changes

Move shouldDisableRollback above the existing comment

d5b4a7f

skinny85 removed the pr/do-not-merge This PR should not be merged at this time. label Sep 2, 2021

Merge branch 'master' into cdk-watch

3bbe048

mergify bot merged commit 6e55c95 into aws:master Sep 2, 2021

skinny85 deleted the cdk-watch branch September 2, 2021 22:37

lambdasawa mentioned this pull request Nov 3, 2021

function hotswap for development aws-amplify/amplify-cli#8642

Open

2 tasks

skinny85 mentioned this pull request Jan 8, 2022

(cli): Support hotswap for Docker Image Lambda function #18302

Closed

2 tasks

skinny85 mentioned this pull request Nov 15, 2024

(hotswap): hotswap support for CloudWatch Dashboards aws/aws-cdk-cli#877

Open

2 tasks

	rollback: shouldDisableRollback ? false : true,
	rollback: options.rollback === undefined && options.hotswap ? false : options.rollback,

		const shouldDisableRollback = (options.rollback === undefined && options.hotswap === true) \|\| options.rollback === false;
		options.rollback = shouldDisableRollback ? false : true;

Conversation

skinny85 commented Jul 24, 2021 • edited by eladb Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gitpod-io bot commented Jul 24, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

skinny85 commented Jul 27, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

skinny85 commented Jul 30, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

skinny85 commented Aug 28, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skinny85 Aug 30, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

skinny85 commented Aug 30, 2021

Uh oh!

BenChaimberg left a comment

Choose a reason for hiding this comment

Uh oh!

skinny85 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

skinny85 Sep 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skinny85 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skinny85 left a comment

Choose a reason for hiding this comment

Uh oh!

skinny85 commented Jul 24, 2021 •

edited by eladb

Loading

skinny85 Aug 30, 2021 •

edited

Loading

skinny85 Sep 1, 2021 •

edited

Loading

skinny85 Sep 2, 2021 •

edited

Loading