resource/aws_cloudfront_distribution: Add wait_for_deployment argument #8116

bflad · 2019-03-28T16:38:24Z

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Since we are adding a virtual attribute with a Default, we must include a schema state migration to prevent the "" => "true" difference on upgrade.

Output from acceptance testing:

--- PASS: TestAccAWSCloudFrontDistribution_Origin_EmptyDomainName (1.25s)
--- PASS: TestAccAWSCloudFrontDistribution_Origin_EmptyOriginID (1.30s)
--- PASS: TestAccAWSCloudFrontDistribution_disappears (571.01s)
--- PASS: TestAccAWSCloudFrontDistribution_noOptionalItemsConfig (1263.05s)
--- PASS: TestAccAWSCloudFrontDistribution_noCustomErrorResponseConfig (1265.06s)
--- PASS: TestAccAWSCloudFrontDistribution_HTTP11Config (1265.34s)
--- PASS: TestAccAWSCloudFrontDistribution_customOrigin (1267.09s)
--- PASS: TestAccAWSCloudFrontDistribution_OriginGroups (1267.68s)
--- PASS: TestAccAWSCloudFrontDistribution_RetainOnDelete (1268.95s)
--- PASS: TestAccAWSCloudFrontDistribution_orderedCacheBehavior (1271.12s)
--- PASS: TestAccAWSCloudFrontDistribution_S3Origin (1271.30s)
--- PASS: TestAccAWSCloudFrontDistribution_multiOrigin (1271.50s)
--- PASS: TestAccAWSCloudFrontDistribution_IsIPV6EnabledConfig (1272.82s)
--- PASS: TestAccAWSCloudFrontDistribution_ViewerCertificate_AcmCertificateArn (1275.16s)
--- PASS: TestAccAWSCloudFrontDistribution_WaitForDeployment (1275.81s)
--- PASS: TestAccAWSCloudFrontDistribution_ViewerCertificate_AcmCertificateArn_ConflictsWithCloudFrontDefaultCertificate (1372.94s)
--- PASS: TestAccAWSCloudFrontDistribution_S3OriginWithTags (1782.07s)
--- PASS: TestAccAWSCloudFrontDistribution_Enabled (1782.38s)

Reference: #8073 Since we are adding a virtual attribute with a `Default`, we must use include a schema state migration to prevent the `"" => "true"` difference on upgrade. Output from acceptance testing: ``` --- PASS: TestAccAWSCloudFrontDistribution_Origin_EmptyDomainName (1.25s) --- PASS: TestAccAWSCloudFrontDistribution_Origin_EmptyOriginID (1.30s) --- PASS: TestAccAWSCloudFrontDistribution_disappears (571.01s) --- PASS: TestAccAWSCloudFrontDistribution_noOptionalItemsConfig (1263.05s) --- PASS: TestAccAWSCloudFrontDistribution_noCustomErrorResponseConfig (1265.06s) --- PASS: TestAccAWSCloudFrontDistribution_HTTP11Config (1265.34s) --- PASS: TestAccAWSCloudFrontDistribution_customOrigin (1267.09s) --- PASS: TestAccAWSCloudFrontDistribution_OriginGroups (1267.68s) --- PASS: TestAccAWSCloudFrontDistribution_RetainOnDelete (1268.95s) --- PASS: TestAccAWSCloudFrontDistribution_orderedCacheBehavior (1271.12s) --- PASS: TestAccAWSCloudFrontDistribution_S3Origin (1271.30s) --- PASS: TestAccAWSCloudFrontDistribution_multiOrigin (1271.50s) --- PASS: TestAccAWSCloudFrontDistribution_IsIPV6EnabledConfig (1272.82s) --- PASS: TestAccAWSCloudFrontDistribution_ViewerCertificate_AcmCertificateArn (1275.16s) --- PASS: TestAccAWSCloudFrontDistribution_WaitForDeployment (1275.81s) --- PASS: TestAccAWSCloudFrontDistribution_ViewerCertificate_AcmCertificateArn_ConflictsWithCloudFrontDefaultCertificate (1372.94s) --- PASS: TestAccAWSCloudFrontDistribution_S3OriginWithTags (1782.07s) --- PASS: TestAccAWSCloudFrontDistribution_Enabled (1782.38s) ```

…gument documentation

nywilken · 2019-03-28T20:05:12Z

aws/resource_aws_cloudfront_distribution_migrate_test.go

+		},
+	}
+
+	for tn, tc := range cases {


The variable names are a bit confusing here. I would stick with the std. i for index so that we don't have to think what it means sometime in the future. I would apply this thinking to any other variable that makes you think for more than two seconds.

👍 will update these. We should consider updating the rest of the codebase too as this was mostly copied from elsewhere.

Nowaker · 2019-03-28T20:42:15Z

website/docs/r/cloudfront_distribution.html.markdown

@@ -255,6 +255,10 @@ of several sub-resources - these resources are laid out below.
    deleting it when destroying the resource through Terraform. If this is set,
    the distribution needs to be deleted manually afterwards. Default: `false`.

+  * `wait_for_deployment` (Optional) - By default, the resource will wait
+    for creation and updates to fully deploy. Setting this to `false`
+    will skip waiting for deployments to complete. Default: `true`.


@bflad I would suggest this to read instead so it's clear what's happening here:

If enabled, the resource will wait for the distribution status to change from InProgress to Deployed.
Setting this to false will skip the process. Default: true.

👍 great suggestion, will update.

…e naming and improve wait_for_deployment documentation Reference: #8116

nathanielks · 2019-04-01T15:27:31Z

Thanks for giving us the option to skip! This is great 👍

nywilken

LGTM 👍

DJAlPee · 2019-04-03T15:00:59Z

@bflad Why is default behavior true and not false?

Before introducing this new feature, the default behavior was not waiting for the complete deployment to be finished. To stick with this behavior, I now have to change every configuration, where I use cloudfront... :(

bflad · 2019-04-03T18:21:16Z

Hi @DJAlPee 👋 Sorry for the hassle as CloudFront deployments are particularly long as compared with many other AWS resources.

A design philosophy with most Terraform resources is that they should be designed to wait until they have completed any creation/update/deletion action. This is so any downstream Terraform configuration or post-Terraform actions that are intending to interact with the infrastructure does so when the infrastructure is in a stable state. Where this waiting behavior does not happen we typically consider that to be a bug in the Terraform resource and do not consider it a breaking change as we were not previously performing an expected Terraform resource behavior.

If you have suggestions where we might be able to document these concepts better, we would gladly take a look, just please submit a new GitHub issue so it doesn't get lost in a previously closed issue/pull request.

bflad · 2019-04-05T10:52:09Z

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

ghost · 2020-03-30T17:51:00Z

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!

bflad added enhancement Requests to existing resources that expand the functionality or scope. service/cloudfront Issues and PRs that pertain to the cloudfront service. labels Mar 28, 2019

bflad requested a review from a team March 28, 2019 16:38

ghost added tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. size/XL Managed by automation to categorize the size of a PR. labels Mar 28, 2019

docs/resource/aws_cloudfront_distribution: Add wait_for_deployment ar…

0a0ac22

…gument documentation

ghost added the documentation Introduces or discusses updates to documentation. label Mar 28, 2019

bflad mentioned this pull request Mar 28, 2019

Release 2.1.0 (commit #7794) introduces serious and problematic performance degradation for CloudFront Distribution updates #8073

Closed

nywilken reviewed Mar 28, 2019

View reviewed changes

Nowaker reviewed Mar 28, 2019

View reviewed changes

resource/aws_cloudfront_distribution: Clarify state migration variabl…

6bec133

…e naming and improve wait_for_deployment documentation Reference: #8116

This was referenced Apr 1, 2019

CloudFront get redeployed every time if forward custom headers #8140

Closed

Cloudfront Distribution changes ForwardedValues Headers for Behaviours #7867

Closed

nywilken approved these changes Apr 1, 2019

View reviewed changes

bflad added this to the v2.5.0 milestone Apr 1, 2019

bflad merged commit 15b99f2 into master Apr 1, 2019

bflad deleted the f-aws_cloudfront_distribution-skip_wait_for_deployment branch April 1, 2019 16:10

bflad added a commit that referenced this pull request Apr 1, 2019

Update CHANGELOG for #8116

23fc472

bflad mentioned this pull request Apr 3, 2019

Cloudfront parameter to NOT wait for distribution to finish deploying #8175

Closed

richardgavel mentioned this pull request Sep 18, 2019

Add ecs_service wait_for_stable property #10149

Closed

richardgavel mentioned this pull request Dec 29, 2019

Wait until ECS deployment is complete? #3107

Closed

ghost locked and limited conversation to collaborators Mar 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

resource/aws_cloudfront_distribution: Add wait_for_deployment argument #8116

resource/aws_cloudfront_distribution: Add wait_for_deployment argument #8116

bflad commented Mar 28, 2019 •

edited by nywilken

Loading

nywilken Mar 28, 2019

bflad Mar 28, 2019

Nowaker Mar 28, 2019

bflad Mar 28, 2019

nathanielks commented Apr 1, 2019

nywilken left a comment

DJAlPee commented Apr 3, 2019 •

edited

Loading

bflad commented Apr 3, 2019

bflad commented Apr 5, 2019

ghost commented Mar 30, 2020

resource/aws_cloudfront_distribution: Add wait_for_deployment argument #8116

resource/aws_cloudfront_distribution: Add wait_for_deployment argument #8116

Conversation

bflad commented Mar 28, 2019 • edited by nywilken Loading

Community Note

nywilken Mar 28, 2019

Choose a reason for hiding this comment

bflad Mar 28, 2019

Choose a reason for hiding this comment

Nowaker Mar 28, 2019

Choose a reason for hiding this comment

bflad Mar 28, 2019

Choose a reason for hiding this comment

nathanielks commented Apr 1, 2019

nywilken left a comment

Choose a reason for hiding this comment

DJAlPee commented Apr 3, 2019 • edited Loading

bflad commented Apr 3, 2019

bflad commented Apr 5, 2019

ghost commented Mar 30, 2020

bflad commented Mar 28, 2019 •

edited by nywilken

Loading

DJAlPee commented Apr 3, 2019 •

edited

Loading