Merge pull request #4366 from TimeIncOSS/b-ecs-svc-fixes

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
 		}
 	}
 
+	// 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] Deleting ECS service %s", input)
+		log.Printf("[DEBUG] Trying to delete ECS service %s", input)
-	out, err := conn.DeleteService(&input)
+		_, err := conn.DeleteService(&input)
+		if err == nil {
+			return nil
+		}
+
+		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,17 +231,25 @@ 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"