-
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
removing unfound ecs service from state file #6039
Conversation
@barrytam20 do you have the error message and/or debug logs from this occurring? |
@bflad this is the error i was seeing when i tried to do a terraform refresh. the service existed in the statefile, but was deleted through the AWS console
|
hey @bflad is there anything i can do to help with this PR? I haven't really found any other resources with tests that remove unfound resources from the state file |
@barrytam20 check out
See also the testing added in #5967 |
@bflad - pushed up test
|
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.
Thanks so much for reporting and fixing this @barrytam20! Some minor adjustments post-merge to the testing noted below. ECS will keep INACTIVE
services around for awhile before reaping them asynchronously, which actually doesn't trigger the condition of them being missing in the out.Services
from DescribeServices()
. We don't want to sit around and guess how long to wait for that process to occur so keeping the nonexistent
import test in there actually is the easiest way to capture the same behavior you originally saw. That said, the addition of the _disappears
test is actually super helpful for verifying that INACTIVE
services are recreated, which previously wasn't being tested. 👍
At some point we should probably clean up that retry logic there, its very messy. 😅
22 tests passed (all tests)
--- PASS: TestAccAWSEcsService_withUnnormalizedPlacementStrategy (7.01s)
--- PASS: TestAccAWSEcsService_basicImport (7.92s)
--- PASS: TestAccAWSEcsService_disappears (8.71s)
--- PASS: TestAccAWSEcsService_withPlacementConstraints_emptyExpression (9.28s)
--- PASS: TestAccAWSEcsService_withEcsClusterName (9.28s)
--- PASS: TestAccAWSEcsService_withReplicaSchedulingStrategy (13.36s)
--- PASS: TestAccAWSEcsService_withRenamedCluster (20.26s)
--- PASS: TestAccAWSEcsService_withIamRole (24.88s)
--- PASS: TestAccAWSEcsService_withServiceRegistries (18.67s)
--- PASS: TestAccAWSEcsService_withDeploymentValues (30.90s)
--- PASS: TestAccAWSEcsService_withPlacementConstraints (31.47s)
--- PASS: TestAccAWSEcsServiceDataSource_basic (32.33s)
--- PASS: TestAccAWSEcsService_withFamilyAndRevision (33.50s)
--- PASS: TestAccAWSEcsService_withARN (33.71s)
--- PASS: TestAccAWSEcsService_withDaemonSchedulingStrategy (40.96s)
--- PASS: TestAccAWSEcsService_withServiceRegistries_container (46.65s)
--- PASS: TestAccAWSEcsService_withPlacementStrategy (62.98s)
--- PASS: TestAccAWSEcsService_withLaunchTypeEC2AndNetworkConfiguration (71.74s)
--- PASS: TestAccAWSEcsService_withLbChanges (214.00s)
--- PASS: TestAccAWSEcsService_healthCheckGracePeriodSeconds (228.75s)
--- PASS: TestAccAWSEcsService_withAlb (234.15s)
--- PASS: TestAccAWSEcsService_withLaunchTypeFargate (307.47s)
ImportStateId: fmt.Sprintf("%s/nonexistent", clusterName), | ||
ImportState: true, | ||
ImportStateVerify: false, | ||
ExpectError: regexp.MustCompile(`No ECS service found`), |
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.
We actually want to keep this test as its currently the only way we can verify the condition you were originally seeing. We just needed to grab something out of the new error message from import failing (properly!):
--- FAIL: TestAccAWSEcsService_basicImport (43.46s)
testing.go:520: Step 2, expected error:
1 error occurred:
* aws_ecs_service.jenkins (import id: tf-acc-cluster-svc-ye9z6h7j/nonexistent): 1 error occurred:
* import aws_ecs_service.jenkins result: nonexistent: import aws_ecs_service.jenkins (id: nonexistent): Terraform detected a resource with this ID doesn't
exist. Please verify the ID is correct. You cannot import non-existent
resources using Terraform import.
To match:
No ECS service found
I went with:
ExpectError: regexp.MustCompile(`Please verify the ID is correct`),
Force: aws.Bool(true), | ||
} | ||
|
||
_, err := conn.DeleteService(input) |
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.
It turns out the ECS service needs some additional checking to ensure its gone INACTIVE
looking at its Delete function:
func testAccCheckAWSEcsServiceDisappears(service *ecs.Service) resource.TestCheckFunc {
return func(s *terraform.State) error {
conn := testAccProvider.Meta().(*AWSClient).ecsconn
input := &ecs.DeleteServiceInput{
Cluster: service.ClusterArn,
Service: service.ServiceName,
Force: aws.Bool(true),
}
_, err := conn.DeleteService(input)
if err != nil {
return err
}
// Wait until it's deleted
wait := resource.StateChangeConf{
Pending: []string{"ACTIVE", "DRAINING"},
Target: []string{"INACTIVE"},
Timeout: 10 * time.Minute,
MinTimeout: 1 * time.Second,
Refresh: func() (interface{}, string, error) {
resp, err := conn.DescribeServices(&ecs.DescribeServicesInput{
Cluster: service.ClusterArn,
Services: []*string{service.ServiceName},
})
if err != nil {
return resp, "FAILED", err
}
return resp, aws.StringValue(resp.Services[0].Status), nil
},
}
_, err = wait.WaitForState()
return err
}
}
This has been released in version 1.40.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. |
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! |
Changes proposed in this pull request:
terraform refresh
to resolve driftnote about acceptance tests: my corporate aws account is not allowed to perform some actions required to run acceptance tests. below is the output, account id/profile have been scrubbed out
Output from acceptance testing: