Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cli): support hotswapping Lambda function's description and environment variables #21305

Merged
merged 3 commits into from Aug 8, 2022
Merged

feat(cli): support hotswapping Lambda function's description and environment variables #21305

merged 3 commits into from Aug 8, 2022

Conversation

huyphan
Copy link
Contributor

@huyphan huyphan commented Jul 25, 2022

This change allows Lambda function to be hotswap'ed when there's change in the function's description and/or environment variables. These changes are categorized as configuration changes and are updated by calling updateFunctionConfiguration. Since the existing waiter "UpdateFunctionCodeToFinish" is now used to wait for both code update and configuration update, I renamed it to "UpdateFunctionPropertiesToFinish".

resolves #20787


All Submissions:

Adding new Unconventional Dependencies:

  • This PR adds new unconventional dependencies following the process described here

New Features

  • Have you added the new feature to an integration test?
    • Did you use yarn integ to deploy the infrastructure and generate the snapshot (i.e. yarn integ without --dry-run)?

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

@gitpod-io
Copy link

gitpod-io bot commented Jul 25, 2022

@github-actions github-actions bot added the p2 label Jul 25, 2022
@aws-cdk-automation aws-cdk-automation requested a review from a team July 25, 2022 02:31
@huyphan huyphan closed this Jul 25, 2022
@huyphan huyphan reopened this Jul 25, 2022
@huyphan huyphan marked this pull request as ready for review July 25, 2022 02:56
@github-actions github-actions bot added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. labels Jul 25, 2022
@rix0rrr
Copy link
Contributor

rix0rrr commented Jul 28, 2022

The goal of this feature is not to reimplement CloudFormation, but faster. It's to increase speed on common development tasks, such as iterating on runtime code.

What is the use case for changing these values so often that it becomes a productivity killer to have to wait 30s instead of 1s when you do?

@huyphan
Copy link
Contributor Author

huyphan commented Jul 28, 2022

The issue #20787 has more context there. But to summarize:

The goal of hotswap feature is not to reimplement CloudFormation, but faster.

The purpose behind this change is not to implement more CloudFormation's behaviour. This change is not even about improving the speed of updating Lambda function's description or environment variables. It's about unblocking hotswap when those fields are updated.

Why those fields need to be updated regularly? What is the use case?

It's one of the workarounds to get Lambda version to work with S3 code provider (see CDK's note about it).

Why can't we use the other workarounds, such as defining a dynamic Lambda::Version object?

This does allow hotswap to work, but will fail subsequent full deployments. See more in #20787.

--

Update: rephrase the summary to make the purpose of this change clearer.

…ronment variables

This change allows Lambda function to be hotswap'ed when there's change
in the function's description and/or environment variables. These changes
are categorized as configuration changes and are updated by calling
`updateFunctionConfiguration`. Since the existing waiter
"UpdateFunctionCodeToFinish" is now used to wait for both code update
and configuration update, I renamed it to "UpdateFunctionPropertiesToFinish".
This commit adds a basic integration test for the hotswap feature, for
the case when the Lambda function's description and environment variables
have changed.
@comcalvi
Copy link
Contributor

comcalvi commented Aug 2, 2022

In (3), the error you get is:

Modify the function to create a new version.

For subsequent deployments, can you make a trivial change to the function's description to get around this CFN error? I do not think that the complexity added to the lambda hotswap logic is worth it.

@huyphan
Copy link
Contributor Author

huyphan commented Aug 2, 2022

I can do it. But I don't think it's a right customer experience if we ask every one to do it:

  1. When the failure happens, the presented error message is about Lambda version conflict and does not explain exactly what users have to do to resolve it (source: I work for a team that helps other developers to build Lambda applications and many new developers did not know how to workaround this issue).
  2. Making changes to the Lambda function and rebuilding it every time you update your infrastructure package is not trivial. Large lambda code base takes time to build, and users can easily forget to do this step at times.

Copy link
Contributor

@rix0rrr rix0rrr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's one of the workarounds to get Lambda version to work with S3 code provider

Ahh, I see. Would have helped if you had made the motivation more clear in the PR body :).

@mergify
Copy link
Contributor

mergify bot commented Aug 8, 2022

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

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: e64df8e
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@mergify mergify bot merged commit fb92703 into aws:main Aug 8, 2022
@mergify
Copy link
Contributor

mergify bot commented Aug 8, 2022

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

vinayak-kukreja added a commit that referenced this pull request Aug 8, 2022
mergify bot pushed a commit that referenced this pull request Aug 8, 2022
…and environment variables" (#21509)

This reverts #21305 since it causes CLI integ test failures.
jmortlock pushed a commit to jmortlock/aws-cdk that referenced this pull request Aug 8, 2022
…ronment variables (aws#21305)

This change allows Lambda function to be hotswap'ed when there's change in the function's description and/or environment variables. These changes are categorized as configuration changes and are updated by calling `updateFunctionConfiguration`. Since the existing waiter "UpdateFunctionCodeToFinish" is now used to wait for both code update and configuration update, I renamed it to "UpdateFunctionPropertiesToFinish".

resolves aws#20787

----

### All Submissions:

* [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [X] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
jmortlock pushed a commit to jmortlock/aws-cdk that referenced this pull request Aug 8, 2022
…and environment variables" (aws#21509)

This reverts aws#21305 since it causes CLI integ test failures.
@mrgrain
Copy link
Contributor

mrgrain commented Aug 9, 2022

@huyphan This is failing the CLI integration tests in cli.integtest.ts
Unfortunately they are not run on the PR build, so you'll have to check them locally.

@huyphan
Copy link
Contributor Author

huyphan commented Aug 10, 2022

@mrgrain is there any chance I can have access to the full test log? The test case that you linked to (ci=true -> output to stdout) does not fail on my local environment, but another test failed instead (generating and loading assembly).

I will update the PR to address the generating and loading assembly test case, but it's a bit tricky to address the first one since I can't reproduce it yet.

@mrgrain
Copy link
Contributor

mrgrain commented Aug 10, 2022

@huyphan My bad! I had linked you to the wrong test case. generating and loading assembly is the only one failing. I will provide the log next time, that's a good call out. Unfortunately it's a manual thing to get and post it.

@huyphan
Copy link
Contributor Author

huyphan commented Aug 10, 2022

Thank you! Since this PR is closed, I made another PR with the same change and the fix for the failed integration test case: #21532

mergify bot pushed a commit that referenced this pull request Aug 10, 2022
…ronment variables (#21532)

CDK users who deploy their Lambda function with S3 code provider cannot use hotswap deployment today. It's because -- in order for the function version and alias to work -- they need to use a dynamic value in either function's description or environment variable. Since neither of them is supported by hotswap, CDK always performs a full deployment. 

This change allows Lambda function to be hotswappable when there's change in the function's description and/or environment variables. These changes are categorized as configuration changes and are updated by calling `updateFunctionConfiguration`. Since the existing waiter `UpdateFunctionCodeToFinish` is now used to wait for both code update and configuration update, I renamed it to `UpdateFunctionPropertiesToFinish`.

Functional wise, this PR is identical to the now-reverted changes in #21305. The only difference is the fix for integration test (commit [#82dbd4)](82dbd41)

resolves #20787

----


#21305


### All Submissions:

* [ X ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ X ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [  ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
josephedward pushed a commit to josephedward/aws-cdk that referenced this pull request Aug 30, 2022
…ronment variables (aws#21305)

This change allows Lambda function to be hotswap'ed when there's change in the function's description and/or environment variables. These changes are categorized as configuration changes and are updated by calling `updateFunctionConfiguration`. Since the existing waiter "UpdateFunctionCodeToFinish" is now used to wait for both code update and configuration update, I renamed it to "UpdateFunctionPropertiesToFinish".

resolves aws#20787

----

### All Submissions:

* [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [X] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
josephedward pushed a commit to josephedward/aws-cdk that referenced this pull request Aug 30, 2022
…and environment variables" (aws#21509)

This reverts aws#21305 since it causes CLI integ test failures.
josephedward pushed a commit to josephedward/aws-cdk that referenced this pull request Aug 30, 2022
…ronment variables (aws#21532)

CDK users who deploy their Lambda function with S3 code provider cannot use hotswap deployment today. It's because -- in order for the function version and alias to work -- they need to use a dynamic value in either function's description or environment variable. Since neither of them is supported by hotswap, CDK always performs a full deployment. 

This change allows Lambda function to be hotswappable when there's change in the function's description and/or environment variables. These changes are categorized as configuration changes and are updated by calling `updateFunctionConfiguration`. Since the existing waiter `UpdateFunctionCodeToFinish` is now used to wait for both code update and configuration update, I renamed it to `UpdateFunctionPropertiesToFinish`.

Functional wise, this PR is identical to the now-reverted changes in aws#21305. The only difference is the fix for integration test (commit [#82dbd4)](aws@82dbd41)

resolves aws#20787

----


aws#21305


### All Submissions:

* [ X ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ X ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [  ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(cli): Hotswapping support for Lambda function's description and or environment variable
5 participants