Skip to content

Commit

Permalink
provider/aws: fix ECS service CheckDestroy in tests
Browse files Browse the repository at this point in the history
  • Loading branch information
phinze committed Dec 22, 2015
1 parent f473c2a commit 47f8b0c
Showing 1 changed file with 5 additions and 0 deletions.
5 changes: 5 additions & 0 deletions builtin/providers/aws/resource_aws_ecs_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"testing"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/service/ecs"
"github.com/hashicorp/terraform/helper/resource"
"github.com/hashicorp/terraform/terraform"
Expand Down Expand Up @@ -210,6 +211,10 @@ func testAccCheckAWSEcsServiceDestroy(s *terraform.State) error {
Services: []*string{aws.String(rs.Primary.ID)},
})

if awserr, ok := err.(awserr.Error); ok && awserr.Code() == "ClusterNotFoundException" {
continue
}

if err == nil {
if len(out.Services) > 0 {
return fmt.Errorf("ECS service still exists:\n%#v", out.Services)
Expand Down

3 comments on commit 47f8b0c

@radeksimko
Copy link
Member

Choose a reason for hiding this comment

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

ClusterNotFoundException is actually a valid error which shouldn't be ignored, the real cause is that we're not passing Cluster attribute to DescribeServices few lines above - hence it uses the default cluster (named default I think), which may not exist.

Besides that, there's actually more to be fixed in ECS service deletion.

I'm fixing all the things I know about already in MeredithCorpOSS@86269da#diff-541cd13c0077eb59a7e6688908a5cc8eR217 ( #4366 )

@phinze
Copy link
Contributor Author

@phinze phinze commented on 47f8b0c Dec 22, 2015

Choose a reason for hiding this comment

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

Whoops good catch! I've been doing a big sweep getting the acctests green and definitely let this one slip. Will keep an eye on #4366 for your legitimate fixes. 👍

@radeksimko
Copy link
Member

Choose a reason for hiding this comment

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

FYI #4366 is ready for review

Please sign in to comment.