From 583c0a54c43ee29d7489786b71aadf88bab2a9e9 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Tue, 22 Dec 2015 21:10:23 +0100 Subject: [PATCH 1/5] aws: Fix bug w/ changing ECS service LB association - fixes #3444 - fixes #4227 --- builtin/providers/aws/resource_aws_ecs_service.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/builtin/providers/aws/resource_aws_ecs_service.go b/builtin/providers/aws/resource_aws_ecs_service.go index 805d968407bf..3af04dbda70f 100644 --- a/builtin/providers/aws/resource_aws_ecs_service.go +++ b/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, }, }, }, From f8bb48b28721a451d896d578f6db4a795be7f669 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Thu, 17 Dec 2015 15:21:05 +0100 Subject: [PATCH 2/5] aws: Wait for ECS service to be drained before deletion --- .../providers/aws/resource_aws_ecs_service.go | 35 +++++++++++++++---- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/builtin/providers/aws/resource_aws_ecs_service.go b/builtin/providers/aws/resource_aws_ecs_service.go index 3af04dbda70f..19225361c2da 100644 --- a/builtin/providers/aws/resource_aws_ecs_service.go +++ b/builtin/providers/aws/resource_aws_ecs_service.go @@ -279,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] Deleting ECS service %s", input) - out, err := conn.DeleteService(&input) + log.Printf("[DEBUG] Trying to delete ECS service %s", 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 } @@ -306,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 }, } @@ -315,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 } From 95367bc0fceb0a54abccdbf85ebea40ea7d562c6 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Wed, 23 Dec 2015 10:17:52 +0100 Subject: [PATCH 3/5] aws: Fix CheckDestroy for ecs service --- .../providers/aws/resource_aws_ecs_service_test.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/builtin/providers/aws/resource_aws_ecs_service_test.go b/builtin/providers/aws/resource_aws_ecs_service_test.go index 9eb9bce186f2..738741287bf8 100644 --- a/builtin/providers/aws/resource_aws_ecs_service_test.go +++ b/builtin/providers/aws/resource_aws_ecs_service_test.go @@ -209,6 +209,7 @@ 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" { @@ -217,8 +218,19 @@ func testAccCheckAWSEcsServiceDestroy(s *terraform.State) error { 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 From 9a625427ca31b4057c9c939dfd3bd3f0f77dc3fe Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Wed, 23 Dec 2015 10:43:37 +0100 Subject: [PATCH 4/5] aws: Add regression acc test for ecs svc lb changes --- .../aws/resource_aws_ecs_service_test.go | 124 ++++++++++++++++++ 1 file changed, 124 insertions(+) diff --git a/builtin/providers/aws/resource_aws_ecs_service_test.go b/builtin/providers/aws/resource_aws_ecs_service_test.go index 738741287bf8..40cb7975eeb1 100644 --- a/builtin/providers/aws/resource_aws_ecs_service_test.go +++ b/builtin/providers/aws/resource_aws_ecs_service_test.go @@ -179,6 +179,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$") @@ -400,6 +423,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 = < Date: Wed, 23 Dec 2015 11:35:30 +0100 Subject: [PATCH 5/5] Revert "provider/aws: fix ECS service CheckDestroy in tests" This reverts commit 47f8b0cd79b511805fa0cdbcf86c0ac2ae2eed1a. cc @phinze --- builtin/providers/aws/resource_aws_ecs_service_test.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/builtin/providers/aws/resource_aws_ecs_service_test.go b/builtin/providers/aws/resource_aws_ecs_service_test.go index 40cb7975eeb1..fcac09ba5158 100644 --- a/builtin/providers/aws/resource_aws_ecs_service_test.go +++ b/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" @@ -235,10 +234,6 @@ func testAccCheckAWSEcsServiceDestroy(s *terraform.State) error { 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 { var activeServices []*ecs.Service