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

[WIP] Issue #4633 retry lambda creation #5849

Merged
merged 1 commit into from
Sep 12, 2018
Merged

Conversation

macbutch
Copy link
Contributor

Treat the error 'Lambda was unable to configure access to your
environment variables because the KMS key is invalid for CreateGrant' as
retryable. The root cause seems to be (?) that the required role hasn't
fully been created when the lamdba creation is attmpted (but hard to
know). The failure is intermittent and does not occur on retry.

Fixes #4633

Changes proposed in this pull request:

  • Retry lambda creation after receiving error with text 'Lambda was unable to configure access to your environment variables because the KMS key is invalid for CreateGrant'

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSAvailabilityZones'

<not run yet>

Treat the error 'Lambda was unable to configure access to your
environment variables because the KMS key is invalid for CreateGrant' as
retryable. The root cause seems to be (?) that the required role hasn't
fully been created when the lamdba creation is attmpted (but hard to
know). The failure is intermittent and does not occur on retry.
@ghost ghost added the size/XS Managed by automation to categorize the size of a PR. label Sep 12, 2018
@macbutch
Copy link
Contributor Author

I'd love some feedback on what else is needed here? I don't think the docs need to be updated and my go knowledge is pretty limited so I've not attempted writing a test - anything else I should do?

@bflad bflad added bug Addresses a defect in current functionality. service/lambda Issues and PRs that pertain to the lambda service. labels Sep 12, 2018
@bflad bflad added this to the v1.36.0 milestone Sep 12, 2018
Copy link
Contributor

@bflad bflad left a comment

Choose a reason for hiding this comment

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

Eventual consistency issues are unfortunately unreliable to reproduce, so this is one of the few cases where there is not really acceptance testing to be added. 😅 We generally rely on the existing acceptance testing to ensure we are not creating any regressions. Luckily this change is fairly innocuous anyways (since worst case it just means the Terraform resource takes a minute longer to run).

Thanks @macbutch! 🚀 Please note I will follow this up to handle updates as well so we can get this released today.

32 tests passed (all tests)
--- PASS: TestAccAWSLambdaFunction_importS3 (17.05s)
--- PASS: TestAccAWSLambdaFunction_versioned (20.33s)
--- PASS: TestAccAWSLambdaFunction_nilDeadLetterConfig (24.65s)
--- PASS: TestAccAWSLambdaFunction_expectFilenameAndS3Attributes (27.97s)
--- PASS: TestAccAWSLambdaFunction_importLocalFile (38.63s)
--- PASS: TestAccAWSLambdaFunction_VPC (43.14s)
--- PASS: TestAccAWSLambdaFunction_updateRuntime (44.91s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_noRuntime (0.53s)
--- PASS: TestAccAWSLambdaFunction_importLocalFile_VPC (47.10s)
--- PASS: TestAccAWSLambdaFunction_versionedUpdate (47.45s)
--- PASS: TestAccAWSLambdaFunction_localUpdate_nameOnly (31.64s)
--- PASS: TestAccAWSLambdaFunction_VPCRemoval (59.84s)
--- PASS: TestAccAWSLambdaFunction_encryptedEnvVariables (63.30s)
--- PASS: TestAccAWSLambdaFunction_s3 (45.95s)
--- PASS: TestAccAWSLambdaFunction_EmptyVpcConfig (52.22s)
--- PASS: TestAccAWSLambdaFunction_s3Update_basic (35.02s)
--- PASS: TestAccAWSLambdaFunction_tracingConfig (74.28s)
--- PASS: TestAccAWSLambdaFunction_concurrency (74.57s)
--- PASS: TestAccAWSLambdaFunction_s3Update_unversioned (33.70s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_java8 (30.46s)
--- PASS: TestAccAWSLambdaFunction_concurrencyCycle (77.90s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_nodeJs43 (32.60s)
--- PASS: TestAccAWSLambdaFunction_VPCUpdate (78.81s)
--- PASS: TestAccAWSLambdaFunction_VPC_withInvocation (79.75s)
--- PASS: TestAccAWSLambdaFunction_basic (81.93s)
--- PASS: TestAccAWSLambdaFunction_DeadLetterConfig (81.91s)
--- PASS: TestAccAWSLambdaFunction_localUpdate (57.31s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_python36 (29.96s)
--- PASS: TestAccAWSLambdaFunction_envVariables (90.12s)
--- PASS: TestAccAWSLambdaFunction_runtimeValidation_python27 (43.35s)
--- PASS: TestAccAWSLambdaFunction_DeadLetterConfigUpdated (97.59s)
--- PASS: TestAccAWSLambdaFunction_tags (42.29s)

@@ -382,6 +382,10 @@ func resourceAwsLambdaFunctionCreate(d *schema.ResourceData, meta interface{}) e
log.Printf("[DEBUG] Received %s, retrying CreateFunction", err)
return resource.RetryableError(err)
}
if isAWSErr(err, "InvalidParameterValueException", "Lambda was unable to configure access to your environment variables because the KMS key is invalid for CreateGrant") {
Copy link
Contributor

Choose a reason for hiding this comment

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

In a followup commit (so we can release this today), I will also add this retry condition to the UpdateFunctionConfiguration call as well to prevent any odd cases with updating IAM roles or KMS backed environment variables.

@bflad bflad merged commit f4c6cd5 into hashicorp:master Sep 12, 2018
bflad added a commit that referenced this pull request Sep 12, 2018
…to UpdateFunctionConfiguration as well

Followup commit for: #5849
bflad added a commit that referenced this pull request Sep 12, 2018
@bflad
Copy link
Contributor

bflad commented Sep 13, 2018

This has been released in version 1.36.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

@macbutch
Copy link
Contributor Author

Thanks @bflad!

@ghost
Copy link

ghost commented Apr 3, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Apr 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. service/lambda Issues and PRs that pertain to the lambda service. size/XS Managed by automation to categorize the size of a PR.
Projects
None yet
2 participants