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

fix: make lambda function depend on the Cloudwatch log group #133

Conversation

Neki
Copy link
Contributor

@Neki Neki commented Mar 25, 2021

Description

Make the AWS Lambda function explicitly depend on the Cloudwatch log group. With this change, Terraform will create the log group before the lambda, and delete it after deleting the lambda.

This avoids a race condition where AWS creates the log group before Terraform can (see below).

Motivation and Context

If the lambda function writes logs when the log group doesn't exist, then AWS will create it, and Terraform will error with "log group already exists" on the following terraform apply.

It's possible to trigger this case by running terraform destroy when the lambda function is being invoked: the log group will be
deleted by Terraform and recreated by AWS before the lambda function is destroyed.

This change imposes an ordering between the lambda function and the log group creation/destruction, avoiding this race condition.

Breaking Changes

No breaking change, no module API change.

How Has This Been Tested?

  • I have tested and validated these changes using one or more of the provided examples/* projects

I ran terraform init && terraform apply && terraform destroy in the complete/ example.
Additionally, I've been using this change in production for one month now (with repeated terraform apply/terraform destroy), and it fixes the issue described above (that we observed fairly consistently).

@Neki
Copy link
Contributor Author

Neki commented Mar 25, 2021

The CI failure seems unrelated to this change, let me know if I need to add something to this PR 😃

@svenlito
Copy link
Contributor

@Neki Thanks for your PR. If you could merge in the latest changeset from the master branch, please. I'd help sort out the build issue.

Neki added 2 commits March 29, 2021 09:38
If the lambda function writes logs when the log group doesn't exist,
then AWS will create it, and Terraform will error with "log group
already exists" on the following `terraform apply`.

It's possible to trigger this case when running `terraform destroy`
when the lambda function is being invoked: the log group will be
deleted by Terraform and recreated by AWS before the lambda function
is destroyed.

This change imposes an ordering between the lambda function and the
log group creation/destruction, avoiding this race condition.
@Neki Neki force-pushed the fix-lambda-depends-on-cloudwatch-log branch from 413af30 to 973bfef Compare March 29, 2021 07:39
@Neki
Copy link
Contributor Author

Neki commented Mar 29, 2021

Done, the build issue is fixed, thanks.

@svenlito svenlito requested a review from antonbabenko March 29, 2021 09:04
@antonbabenko
Copy link
Member

@svenlito thanks for the mention. I will try to take a look at this and other PRs you assign to me at the beginning of the next, after Easter break.

@svenlito
Copy link
Contributor

@svenlito thanks for the mention. I will try to take a look at this and other PRs you assign to me at the beginning of the next, after Easter break.

SGTM, enjoy your break!

@antonbabenko antonbabenko merged commit 92c9b86 into terraform-aws-modules:master Apr 26, 2021
@antonbabenko
Copy link
Member

I missed this PR after I came back from my break. I apologize for that.

v1.48.0 has been just released.

@github-actions
Copy link

github-actions bot commented Nov 9, 2022

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants