Merge pull request #3061 from TimeIncOSS/b-aws-ecs-iam-diff

Various ECS bugfixes (IAM, destroy timeout)

builtin/providers/aws/resource_aws_ecs_service.go

@@ -9,8 +9,8 @@ import (
 	"time"
 
 	"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/aws/aws-sdk-go/service/iam"
 	"github.com/hashicorp/terraform/helper/hashcode"
 	"github.com/hashicorp/terraform/helper/resource"
 	"github.com/hashicorp/terraform/helper/schema"
@@ -105,7 +105,30 @@ func resourceAwsEcsServiceCreate(d *schema.ResourceData, meta interface{}) error
 	}
 
 	log.Printf("[DEBUG] Creating ECS service: %s", input)
-	out, err := conn.CreateService(&input)
+
+	// Retry due to AWS IAM policy eventual consistency
+	// See https://github.com/hashicorp/terraform/issues/2869
+	var out *ecs.CreateServiceOutput
+	var err error
+	err = resource.Retry(2*time.Minute, func() error {
+		out, err = conn.CreateService(&input)
+
+		if err != nil {
+			ec2err, ok := err.(awserr.Error)
+			if !ok {
+				return &resource.RetryError{Err: err}
+			}
+			if ec2err.Code() == "InvalidParameterException" {
+				log.Printf("[DEBUG] Trying to create ECS service again: %q",
+					ec2err.Message())
+				return err
+			}
+
+			return &resource.RetryError{Err: err}
+		}
+
+		return nil
+	})
 	if err != nil {
 		return err
 	}
@@ -154,8 +177,14 @@ func resourceAwsEcsServiceRead(d *schema.ResourceData, meta interface{}) error {
 	d.Set("desired_count", *service.DesiredCount)
 	d.Set("cluster", *service.ClusterArn)
 
+	// Save IAM role in the same format
 	if service.RoleArn != nil {
-		d.Set("iam_role", *service.RoleArn)
+		if strings.HasPrefix(d.Get("iam_role").(string), "arn:aws:iam:") {
+			d.Set("iam_role", *service.RoleArn)
+		} else {
+			roleARN := buildIamRoleNameFromARN(*service.RoleArn)
+			d.Set("iam_role", roleARN)
+		}
 	}
 
 	if service.LoadBalancers != nil {
@@ -277,36 +306,9 @@ func buildFamilyAndRevisionFromARN(arn string) string {
 	return strings.Split(arn, "/")[1]
 }
 
-func buildTaskDefinitionARN(taskDefinition string, meta interface{}) (string, error) {
-	// If it's already an ARN, just return it
-	if strings.HasPrefix(taskDefinition, "arn:aws:ecs:") {
-		return taskDefinition, nil
-	}
-
-	// Parse out family & revision
-	family, revision, err := parseTaskDefinition(taskDefinition)
-	if err != nil {
-		return "", err
-	}
-
-	iamconn := meta.(*AWSClient).iamconn
-	region := meta.(*AWSClient).region
-
-	// An zero value GetUserInput{} defers to the currently logged in user
-	resp, err := iamconn.GetUser(&iam.GetUserInput{})
-	if err != nil {
-		return "", fmt.Errorf("GetUser ERROR: %#v", err)
-	}
-
-	// arn:aws:iam::0123456789:user/username
-	userARN := *resp.User.Arn
-	accountID := strings.Split(userARN, ":")[4]
-
-	// arn:aws:ecs:us-west-2:01234567890:task-definition/mongodb:3
-	arn := fmt.Sprintf("arn:aws:ecs:%s:%s:task-definition/%s:%s",
-		region, accountID, family, revision)
-	log.Printf("[DEBUG] Built task definition ARN: %s", arn)
-	return arn, nil
+func buildIamRoleNameFromARN(arn string) string {
+	// arn:aws:iam::0123456789:role/EcsService
+	return strings.Split(arn, "/")[1]
 }
 
 func parseTaskDefinition(taskDefinition string) (string, string, error) {

builtin/providers/aws/resource_aws_ecs_service_test.go

@@ -162,6 +162,22 @@ func TestAccAWSEcsServiceWithRenamedCluster(t *testing.T) {
 	})
 }
 
+func TestAccAWSEcsService_withIamRole(t *testing.T) {
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSEcsServiceDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccAWSEcsService_withIamRole,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSEcsServiceExists("aws_ecs_service.ghost"),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckAWSEcsServiceDestroy(s *terraform.State) error {
 	conn := testAccProvider.Meta().(*AWSClient).ecsconn
 
@@ -253,6 +269,101 @@ resource "aws_ecs_service" "mongo" {
 }
 `
 
+var testAccAWSEcsService_withIamRole = `
+resource "aws_ecs_cluster" "main" {
+	name = "terraformecstest11"
+}
+
+resource "aws_ecs_task_definition" "ghost" {
+  family = "ghost_service"
+  container_definitions = <<DEFINITION
+[
+  {
+    "cpu": 128,
+    "essential": true,
+    "image": "ghost:latest",
+    "memory": 128,
+    "name": "ghost",
+    "portMappings": [
+      {
+        "containerPort": 2368,
+        "hostPort": 8080
+      }
+    ]
+  }
+]
+DEFINITION
+}
+
+resource "aws_iam_role" "ecs_service" {
+    name = "EcsService"
+    assume_role_policy = <<EOF
+{
+    "Version": "2008-10-17",
+    "Statement": [
+        {
+            "Action": "sts:AssumeRole",
+            "Principal": {"AWS": "*"},
+            "Effect": "Allow",
+            "Sid": ""
+        }
+    ]
+}
+EOF
+}
+
+resource "aws_iam_role_policy" "ecs_service" {
+    name = "EcsService"
+    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" {
+  name = "foobar-terraform-test"
+  availability_zones = ["us-west-2a"]
+
+  listener {
+    instance_port = 8080
+    instance_protocol = "http"
+    lb_port = 80
+    lb_protocol = "http"
+  }
+}
+
+resource "aws_ecs_service" "ghost" {
+  name = "ghost"
+  cluster = "${aws_ecs_cluster.main.id}"
+  task_definition = "${aws_ecs_task_definition.ghost.arn}"
+  desired_count = 1
+  iam_role = "${aws_iam_role.ecs_service.name}"
+
+  load_balancer {
+    elb_name = "${aws_elb.main.id}"
+    container_name = "ghost"
+    container_port = "2368"
+  }
+
+  depends_on = ["aws_iam_role_policy.ecs_service"]
+}
+`
+
 var testAccAWSEcsServiceWithFamilyAndRevision = `
 resource "aws_ecs_cluster" "default" {
 	name = "terraformecstest2"

website/source/docs/providers/aws/r/ecs_service.html.markdown

@@ -8,6 +8,8 @@ description: |-
 
 # aws\_ecs\_service
 
+-> **Note:** To prevent race condition during service deletion, make sure to set `depends_on` to related `aws_iam_role_policy`, otherwise policy may be destroyed too soon and ECS service will then stuck in `DRAINING` state.
+
 Provides an ECS service - effectively a task that is expected to run until an error occures or user terminates it (typically a webserver or a database). 
 
 See [ECS Services section in AWS developer guide](http://docs.aws.amazon.com/AmazonECS/latest/developerguide/ecs_services.html).
@@ -21,6 +23,7 @@ resource "aws_ecs_service" "mongo" {
   task_definition = "${aws_ecs_task_definition.mongo.arn}"
   desired_count = 3
   iam_role = "${aws_iam_role.foo.arn}"
+  depends_on = ["aws_iam_role_policy.foo"]
 
   load_balancer {
     elb_name = "${aws_elb.foo.id}"