-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
resource/aws_ecs_capacity_provider: Add support for ECS capacity provider update #16942
resource/aws_ecs_capacity_provider: Add support for ECS capacity provider update #16942
Conversation
da33041
to
f5ecb77
Compare
This PR conflicts with #16941 |
f5ecb77
to
2035385
Compare
Hey folks, My team is really keen to see this PR make its way through to a release 🙏 . I thought it might be beneficial to the review process if I were to build the AWS Provider from this branch and do some manual testing. It took me a bit of twoing and froing to figure out how to get Terraform to use the locally built provider binary but I have completed some basic testing and can happily confirm that I was able to update the following properties in-place, without requiring resource replacement:
Demonstrative screenshots:Capacity Provider: Before stateTerraform apply: Proposed changesTerraform outputCapacity Provider: After StateHope this is helpful, happy to do any more testing if it'll help get this through to inclusion in a release. |
Ready for rebase now that #16941 has been merged. 👍 |
Thank you! I'll update the PR! |
2035385
to
448b7df
Compare
@bflad I've updated the PR so would you review it? |
Is there any other testing or review that needs to be done on this PR? It would make workflows with capacity providers significantly more viable. |
@shuheiktgw Thank you very much for implementing this! Could you please rebase you branch once again and fix conflicts? Hopefully, @bflad can review it soon ;) |
448b7df
to
8e0a760
Compare
Thank you for pinging me, @legal90! I've rebased it again 👍 |
Anything anyone can do to help move this along? I (and it appears plenty of others) would love to see this merged... |
8e0a760
to
13d6a44
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀.
Commercial
% make testacc TEST=./aws TESTARGS='-run=TestAccAWSEcsCapacityProvider_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSEcsCapacityProvider_ -timeout 180m
=== RUN TestAccAWSEcsCapacityProvider_basic
=== PAUSE TestAccAWSEcsCapacityProvider_basic
=== RUN TestAccAWSEcsCapacityProvider_disappears
=== PAUSE TestAccAWSEcsCapacityProvider_disappears
=== RUN TestAccAWSEcsCapacityProvider_ManagedScaling
=== PAUSE TestAccAWSEcsCapacityProvider_ManagedScaling
=== RUN TestAccAWSEcsCapacityProvider_ManagedScalingPartial
=== PAUSE TestAccAWSEcsCapacityProvider_ManagedScalingPartial
=== RUN TestAccAWSEcsCapacityProvider_Tags
=== PAUSE TestAccAWSEcsCapacityProvider_Tags
=== CONT TestAccAWSEcsCapacityProvider_basic
=== CONT TestAccAWSEcsCapacityProvider_ManagedScalingPartial
=== CONT TestAccAWSEcsCapacityProvider_Tags
=== CONT TestAccAWSEcsCapacityProvider_disappears
=== CONT TestAccAWSEcsCapacityProvider_ManagedScaling
--- PASS: TestAccAWSEcsCapacityProvider_disappears (59.40s)
--- PASS: TestAccAWSEcsCapacityProvider_ManagedScalingPartial (60.01s)
--- PASS: TestAccAWSEcsCapacityProvider_basic (70.51s)
--- PASS: TestAccAWSEcsCapacityProvider_Tags (99.70s)
--- PASS: TestAccAWSEcsCapacityProvider_ManagedScaling (122.22s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 126.208s
GovCloud
% make testacc TEST=./aws TESTARGS='-run=TestAccAWSEcsCapacityProvider_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSEcsCapacityProvider_ -timeout 180m
=== RUN TestAccAWSEcsCapacityProvider_basic
=== PAUSE TestAccAWSEcsCapacityProvider_basic
=== RUN TestAccAWSEcsCapacityProvider_disappears
=== PAUSE TestAccAWSEcsCapacityProvider_disappears
=== RUN TestAccAWSEcsCapacityProvider_ManagedScaling
=== PAUSE TestAccAWSEcsCapacityProvider_ManagedScaling
=== RUN TestAccAWSEcsCapacityProvider_ManagedScalingPartial
=== PAUSE TestAccAWSEcsCapacityProvider_ManagedScalingPartial
=== RUN TestAccAWSEcsCapacityProvider_Tags
=== PAUSE TestAccAWSEcsCapacityProvider_Tags
=== CONT TestAccAWSEcsCapacityProvider_basic
=== CONT TestAccAWSEcsCapacityProvider_ManagedScalingPartial
=== CONT TestAccAWSEcsCapacityProvider_ManagedScaling
=== CONT TestAccAWSEcsCapacityProvider_Tags
=== CONT TestAccAWSEcsCapacityProvider_disappears
--- PASS: TestAccAWSEcsCapacityProvider_basic (62.11s)
--- PASS: TestAccAWSEcsCapacityProvider_disappears (62.52s)
--- PASS: TestAccAWSEcsCapacityProvider_Tags (98.56s)
--- PASS: TestAccAWSEcsCapacityProvider_ManagedScalingPartial (105.01s)
--- PASS: TestAccAWSEcsCapacityProvider_ManagedScaling (145.98s)
PASS
ok github.com/terraform-providers/terraform-provider-aws/aws 149.215s
@shuheiktgw Thanks for the contribution 🎉 👏. |
This functionality has been released in v3.47.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
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. |
Community Note
Closes #16943
Release note for CHANGELOG:
Output from acceptance testing:
Thank you for your review! 👍