From fad019e95059e4895dd27459e114db2b8f662d6e Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Sun, 23 Aug 2015 15:13:02 +0100 Subject: [PATCH 1/4] ecs_service: Retry if IAM policy isn't ready yet - fixes #2869 --- .../providers/aws/resource_aws_ecs_service.go | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/builtin/providers/aws/resource_aws_ecs_service.go b/builtin/providers/aws/resource_aws_ecs_service.go index 874d8726a82a..cf8e47a53c64 100644 --- a/builtin/providers/aws/resource_aws_ecs_service.go +++ b/builtin/providers/aws/resource_aws_ecs_service.go @@ -9,6 +9,7 @@ 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" @@ -105,7 +106,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 } From 669d196a58718da919b74fdcf003b1177d302f61 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Sun, 23 Aug 2015 13:48:16 +0100 Subject: [PATCH 2/4] ecs_service: Role name can be used in iam_role (ARN was supported) - fixes #2722 --- .../providers/aws/resource_aws_ecs_service.go | 13 ++- .../aws/resource_aws_ecs_service_test.go | 109 ++++++++++++++++++ 2 files changed, 121 insertions(+), 1 deletion(-) diff --git a/builtin/providers/aws/resource_aws_ecs_service.go b/builtin/providers/aws/resource_aws_ecs_service.go index cf8e47a53c64..9651412a8a3b 100644 --- a/builtin/providers/aws/resource_aws_ecs_service.go +++ b/builtin/providers/aws/resource_aws_ecs_service.go @@ -178,8 +178,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 { @@ -333,6 +339,11 @@ func buildTaskDefinitionARN(taskDefinition string, meta interface{}) (string, er 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) { matches := taskDefinitionRE.FindAllStringSubmatch(taskDefinition, 2) diff --git a/builtin/providers/aws/resource_aws_ecs_service_test.go b/builtin/providers/aws/resource_aws_ecs_service_test.go index f7776b210274..53412ec028f8 100644 --- a/builtin/providers/aws/resource_aws_ecs_service_test.go +++ b/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,99 @@ 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 = < Date: Sun, 23 Aug 2015 17:32:12 +0100 Subject: [PATCH 3/4] ecs_service: Add note about race condition w/ IAM policy - fixes #2902 --- builtin/providers/aws/resource_aws_ecs_service_test.go | 2 ++ website/source/docs/providers/aws/r/ecs_service.html.markdown | 3 +++ 2 files changed, 5 insertions(+) diff --git a/builtin/providers/aws/resource_aws_ecs_service_test.go b/builtin/providers/aws/resource_aws_ecs_service_test.go index 53412ec028f8..2f9b8fedbf05 100644 --- a/builtin/providers/aws/resource_aws_ecs_service_test.go +++ b/builtin/providers/aws/resource_aws_ecs_service_test.go @@ -359,6 +359,8 @@ resource "aws_ecs_service" "ghost" { container_name = "ghost" container_port = "2368" } + + depends_on = ["aws_iam_role_policy.ecs_service"] } ` diff --git a/website/source/docs/providers/aws/r/ecs_service.html.markdown b/website/source/docs/providers/aws/r/ecs_service.html.markdown index 1a2d8fd9cb42..eb654b43f9a9 100644 --- a/website/source/docs/providers/aws/r/ecs_service.html.markdown +++ b/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}" From 00646b1d7b2e58f2c80668a52c7fa53891c13cb6 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Sun, 23 Aug 2015 17:32:27 +0100 Subject: [PATCH 4/4] ecs_service: Remove unused code --- .../providers/aws/resource_aws_ecs_service.go | 33 ------------------- 1 file changed, 33 deletions(-) diff --git a/builtin/providers/aws/resource_aws_ecs_service.go b/builtin/providers/aws/resource_aws_ecs_service.go index 9651412a8a3b..9d3a36ab2d77 100644 --- a/builtin/providers/aws/resource_aws_ecs_service.go +++ b/builtin/providers/aws/resource_aws_ecs_service.go @@ -11,7 +11,6 @@ import ( "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" @@ -307,38 +306,6 @@ 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]