provider/aws: Fix bug w/ changing ECS svc/ELB association

builtin/providers/aws/resource_aws_ecs_service.go

@@ -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,
  },
  },
  },
@@ -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
  }
@@ -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
  },
  }
@@ -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
 }
 

builtin/providers/aws/resource_aws_ecs_service_test.go

@@ -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"
@@ -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$")
@@ -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
@@ -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"