Repository navigation
Fixed ECS TestAccECSClusterCapacityProviders_disappears acc test - #50374
Open
garutilorenzo wants to merge 7 commits into
Open
garutilorenzo wants to merge 7 commits into
garutilorenzo wants to merge 7 commits into
Conversation
…e correctly the Post-apply refresh plan check(s)
Contributor
Community GuidelinesThis comment is added to every new Pull Request to provide quick reference to how the Terraform AWS Provider is maintained. Please review the information below, and thank you for contributing to the community that keeps the provider thriving! 🚀 Voting for Prioritization
Pull Request Authors
|
github-actions
Bot
added
needs-triage
Moved comment explaining the behavior of the ECS cluster capacity providers resource during deletion and state management.
…e correctly the Post-apply refresh plan check(s)
…e correctly the Post-apply refresh plan check(s)
jar-b
removed
the
needs-triage
…e/deleted clusters Previously attempts to delete capacity providers linked to a deleted cluster would result in the resource hanging for the full 10 minute retry window before failing. Errors indicating the cluster is not ACTIVE now return immediately for delete operations. Added a new `_disappears_Cluster` test case to verify the behavior. ```console % make t K=ecs T=TestAccECSClusterCapacityProviders_disappears make: Verifying source code with gofmt... ==> Checking that code complies with gofmt requirements... make: Running acceptance tests on branch: 🌿 fix/ecs-cluster-capacity-providers-disappears-test 🌿... TF_ACC=1 go1.26.8 test ./internal/service/ecs/... -v -count 1 -parallel 20 -run='TestAccECSClusterCapacityProviders_disappears' -timeout 360m -vet=off -buildvcs=false 2026/10/09 11:20:26 Creating Terraform AWS Provider (SDKv2-style)... 2026/10/09 11:20:26 Initializing Terraform AWS Provider (SDKv2-style)... --- PASS: TestAccECSClusterCapacityProviders_disappears_Cluster (38.12s) --- PASS: TestAccECSClusterCapacityProviders_disappears (58.21s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/ecs 66.409s ```
Contributor
|
✅ Thank you for correcting the previously detected issues! The maintainers appreciate your efforts to make the review process as smooth as possible. |
github-actions
Bot
added
size/S
Member
|
/test PATTERN=TestAccECSClusterCapacityProviders_ |
Contributor
|
Automated tests have been triggered for package |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new acceptance test lacks the standard ECS partition-availability precheck.
1 open finding
What changed in this PR
Fixes ECS cluster capacity-provider disappearance handling and inactive-cluster deletion.
Changes:
- Expects updates after capacity providers disappear.
- Handles deleted parent clusters without retrying inactive-cluster errors.
- Adds parent-cluster disappearance coverage.
| File | Description |
|---|---|
.changelog/50374.txt |
Documents the bug fix. |
internal/service/ecs/cluster_capacity_providers.go |
Adjusts deletion retry/error handling. |
internal/service/ecs/cluster_capacity_providers_test.go |
Updates and expands disappearance tests. |
🧠 Review effort: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Member
|
Thanks for correcting the assertions in the While reviewing I added a related test which deletes the parent cluster out-of-band to verify the behavior in that scenario. The new test uncovered a bug which caused the capacity provider resource to hang during deletion when the parent cluster was no longer present. I've fixed that as part of this change and will merge once CI completes and the full test suite results are posted. |
Collaborator
Latest automated test results:% TF_ACC=1 go test './internal/service/ecs/...' -count=1 -json -v -run='TestAccECSClusterCapacityProviders_' -parallel '20' -timeout=0 -vet=off -buildvcs=false
TestAccECSClusterCapacityProviders_basic: [PASS] 70.78s
TestAccECSClusterCapacityProviders_disappears: [PASS] 74.14s
TestAccECSClusterCapacityProviders_disappears_Cluster: [PASS] 56.43s
TestAccECSClusterCapacityProviders_defaults: [PASS] 71.11s
TestAccECSClusterCapacityProviders_destroy: [FAIL] 161.06s
------- Stdout: -------
cluster_capacity_providers_test.go:166: Step 1/2 error: Check failed: Check 2/2 error: RegisteredContainerInstancesCount = 0, want 2
--- FAIL: TestAccECSClusterCapacityProviders_destroy (161.06s)
TestAccECSClusterCapacityProviders_Update_capacityProviders: [PASS] 144s
TestAccECSClusterCapacityProviders_Update_defaultStrategy: [PASS] 143.47s |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rollback Plan
If a change needs to be reverted, we will publish an updated version of the library.
Changes to Security Controls
No
Description
Fixed TestAccECSClusterCapacityProviders_disappears function to handle correctly the Post-apply refresh plan check(s)
== NAME TestAccECSClusterCapacityProviders_disappears cluster_capacity_providers_test.go:61: Step 1/1 error: Post-apply refresh plan check(s) failed: 'aws_ecs_cluster_capacity_providers.test' - expected Create, got action(s): [update]Relations
Relates PR #50360 fix acc tests
Output from Acceptance Testing
acc tests not fun for this PR