Skip to content

Commit

Permalink
Merge pull request #4366 from TimeIncOSS/b-ecs-svc-fixes
Browse files Browse the repository at this point in the history 
provider/aws: Fix bug w/ changing ECS svc/ELB association
  • Loading branch information
@radeksimko
radeksimko committed Jan 7, 2016
2 parents b65f9db + b7ed0e1 commit ddcee32
Show file tree
Hide file tree
Showing 2 changed files with 170 additions and 13 deletions.
40 changes: 33 additions & 7 deletions builtin/providers/aws/resource_aws_ecs_service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,27 +51,32 @@ func resourceAwsEcsService() *schema.Resource {

"iam_role": &schema.Schema{
Type: schema.TypeString,
ForceNew: true,
Optional: true,
},

"load_balancer": &schema.Schema{
Type: schema.TypeSet,
Optional: true,
ForceNew: true,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"elb_name": &schema.Schema{
Type: schema.TypeString,
Required: true,
ForceNew: true,
},

"container_name": &schema.Schema{
Type: schema.TypeString,
Required: true,
ForceNew: true,
},

"container_port": &schema.Schema{
Type: schema.TypeInt,
Required: true,
ForceNew: true,
},
},
},
Expand Down Expand Up @@ -274,13 +279,33 @@ func resourceAwsEcsServiceDelete(d *schema.ResourceData, meta interface{}) error
}
}

input := ecs.DeleteServiceInput{
Service: aws.String(d.Id()),
Cluster: aws.String(d.Get("cluster").(string)),
}
// Wait until the ECS service is drained
err = resource.Retry(5*time.Minute, func() error {
input := ecs.DeleteServiceInput{
Service: aws.String(d.Id()),
Cluster: aws.String(d.Get("cluster").(string)),
}

log.Printf("[DEBUG] Trying to delete ECS service %s", input)
_, err := conn.DeleteService(&input)
if err == nil {
return nil
}

log.Printf("[DEBUG] Deleting ECS service %s", input)
out, err := conn.DeleteService(&input)
ec2err, ok := err.(awserr.Error)
if !ok {
return &resource.RetryError{Err: err}
}
if ec2err.Code() == "InvalidParameterException" {
// Prevent "The service cannot be stopped while deployments are active."
log.Printf("[DEBUG] Trying to delete ECS service again: %q",
ec2err.Message())
return err
}

return &resource.RetryError{Err: err}

})
if err != nil {
return err
}
Expand All @@ -301,6 +326,7 @@ func resourceAwsEcsServiceDelete(d *schema.ResourceData, meta interface{}) error
return resp, "FAILED", err
}

log.Printf("[DEBUG] ECS service %s is currently %q", *resp.Services[0].Status)
return resp, *resp.Services[0].Status, nil
},
}
Expand All @@ -310,7 +336,7 @@ func resourceAwsEcsServiceDelete(d *schema.ResourceData, meta interface{}) error
return err
}

log.Printf("[DEBUG] ECS service %s deleted.", *out.Service.ServiceArn)
log.Printf("[DEBUG] ECS service %s deleted.", d.Id())
return nil
}

Expand Down
143 changes: 137 additions & 6 deletions builtin/providers/aws/resource_aws_ecs_service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ 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 @@ -179,6 +178,29 @@ func TestAccAWSEcsService_withIamRole(t *testing.T) {
})
}

// Regression for https://github.com/hashicorp/terraform/issues/3444
func TestAccAWSEcsService_withLbChanges(t *testing.T) {
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSEcsServiceDestroy,
Steps: []resource.TestStep{
resource.TestStep{
Config: testAccAWSEcsService_withLbChanges,
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEcsServiceExists("aws_ecs_service.with_lb_changes"),
),
},
resource.TestStep{
Config: testAccAWSEcsService_withLbChanges_modified,
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEcsServiceExists("aws_ecs_service.with_lb_changes"),
),
},
},
})
}

// Regression for https://github.com/hashicorp/terraform/issues/3361
func TestAccAWSEcsService_withEcsClusterName(t *testing.T) {
clusterName := regexp.MustCompile("^terraformecstestcluster$")
Expand Down Expand Up @@ -209,16 +231,24 @@ func testAccCheckAWSEcsServiceDestroy(s *terraform.State) error {

out, err := conn.DescribeServices(&ecs.DescribeServicesInput{
Services: []*string{aws.String(rs.Primary.ID)},
Cluster: aws.String(rs.Primary.Attributes["cluster"]),
})

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)
var activeServices []*ecs.Service
for _, svc := range out.Services {
if *svc.Status != "INACTIVE" {
activeServices = append(activeServices, svc)
}
}
if len(activeServices) == 0 {
return nil
}

return fmt.Errorf("ECS service still exists:\n%#v", activeServices)
}
return nil
}

return err
Expand Down Expand Up @@ -388,6 +418,107 @@ resource "aws_ecs_service" "ghost" {
}
`

var tpl_testAccAWSEcsService_withLbChanges = `
resource "aws_ecs_cluster" "main" {
name = "terraformecstest12"
}
resource "aws_ecs_task_definition" "with_lb_changes" {
family = "ghost_lbd"
container_definitions = <<DEFINITION
[
{
"cpu": 128,
"essential": true,
"image": "%s",
"memory": 128,
"name": "%s",
"portMappings": [
{
"containerPort": %d,
"hostPort": %d
}
]
}
]
DEFINITION
}
resource "aws_iam_role" "ecs_service" {
name = "EcsServiceLbd"
assume_role_policy = <<EOF
{
"Version": "2012-10-17",
"Statement": [
{
"Action": "sts:AssumeRole",
"Principal": {"AWS": "*"},
"Effect": "Allow",
"Sid": ""
}
]
}
EOF
}
resource "aws_iam_role_policy" "ecs_service" {
name = "EcsServiceLbd"
role = "${aws_iam_role.ecs_service.name}"
policy = <<EOF
{
"Version": "2012-10-17",
"Statement": [
{
"Effect": "Allow",
"Action": [
"elasticloadbalancing:*",
"ec2:*",
"ecs:*"
],
"Resource": [
"*"
]
}
]
}
EOF
}
resource "aws_elb" "main" {
availability_zones = ["us-west-2a"]
listener {
instance_port = %d
instance_protocol = "http"
lb_port = 80
lb_protocol = "http"
}
}
resource "aws_ecs_service" "with_lb_changes" {
name = "ghost"
cluster = "${aws_ecs_cluster.main.id}"
task_definition = "${aws_ecs_task_definition.with_lb_changes.arn}"
desired_count = 1
iam_role = "${aws_iam_role.ecs_service.name}"
load_balancer {
elb_name = "${aws_elb.main.id}"
container_name = "%s"
container_port = "%d"
}
depends_on = ["aws_iam_role_policy.ecs_service"]
}
`

var testAccAWSEcsService_withLbChanges = fmt.Sprintf(
tpl_testAccAWSEcsService_withLbChanges,
"ghost:latest", "ghost", 2368, 8080, 8080, "ghost", 2368)
var testAccAWSEcsService_withLbChanges_modified = fmt.Sprintf(
tpl_testAccAWSEcsService_withLbChanges,
"nginx:latest", "nginx", 80, 8080, 8080, "nginx", 80)

var testAccAWSEcsServiceWithFamilyAndRevision = `
resource "aws_ecs_cluster" "default" {
name = "terraformecstest2"
Expand Down

0 comments on commit ddcee32

Please sign in to comment.