Skip to content

Commit

Permalink
Retry ECS create/update when target group isn't yet attached to load …
Browse files Browse the repository at this point in the history 
…balancer

The ECS service can be attached to a load balancer's target group but at that point it may not yet be attached to a load balancer as the dependency graph Terraform generates normally will create the ECS service at the same time as the load balancer listener/listener rule.
This could be avoided by setting the ECS service 'depends_on' to include the load balancer listener/listener rule but this is not usable if the load balancer listener/listener rule is created in another module.

The target group in the acceptance test now gets the name from the load balancer, forcing a dependency between the load balancer and the target group.
Unfortunately it's not possible to use something from the listener to force the dependency as the listener already has a dependency on the target group.
  • Loading branch information
@tomelliff
tomelliff committed Jul 11, 2018
1 parent 716a390 commit 070dd7d
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 15 deletions.
6 changes: 6 additions & 0 deletions aws/resource_aws_ecs_service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,9 @@ func resourceAwsEcsServiceCreate(d *schema.ResourceData, meta interface{}) error
if isAWSErr(err, ecs.ErrCodeInvalidParameterException, "Please verify that the ECS service role being passed has the proper permissions.") {
return resource.RetryableError(err)
}
if isAWSErr(err, ecs.ErrCodeInvalidParameterException, "does not have an associated load balancer") {
return resource.RetryableError(err)
}
return resource.NonRetryableError(err)
}

Expand Down Expand Up @@ -780,6 +783,9 @@ func resourceAwsEcsServiceUpdate(d *schema.ResourceData, meta interface{}) error
if isAWSErr(err, ecs.ErrCodeInvalidParameterException, "Please verify that the ECS service role being passed has the proper permissions.") {
return resource.RetryableError(err)
}
if isAWSErr(err, ecs.ErrCodeInvalidParameterException, "does not have an associated load balancer") {
return resource.RetryableError(err)
}
return resource.NonRetryableError(err)
}

Expand Down
26 changes: 11 additions & 15 deletions aws/resource_aws_ecs_service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,6 @@ func TestAccAWSEcsService_healthCheckGracePeriodSeconds(t *testing.T) {
tdName := fmt.Sprintf("tf-acc-td-svc-w-hcgps-%s", rString)
roleName := fmt.Sprintf("tf-acc-role-svc-w-hcgps-%s", rString)
policyName := fmt.Sprintf("tf-acc-policy-svc-w-hcgps-%s", rString)
tgName := fmt.Sprintf("tf-acc-tg-svc-w-hcgps-%s", rString)
lbName := fmt.Sprintf("tf-acc-lb-svc-w-hcgps-%s", rString)
svcName := fmt.Sprintf("tf-acc-svc-w-hcgps-%s", rString)

Expand All @@ -276,25 +275,25 @@ func TestAccAWSEcsService_healthCheckGracePeriodSeconds(t *testing.T) {
Steps: []resource.TestStep{
{
Config: testAccAWSEcsService_healthCheckGracePeriodSeconds(vpcNameTag, clusterName, tdName,
roleName, policyName, tgName, lbName, svcName, -1),
roleName, policyName, lbName, svcName, -1),
ExpectError: regexp.MustCompile(`expected health_check_grace_period_seconds to be in the range`),
},
{
Config: testAccAWSEcsService_healthCheckGracePeriodSeconds(vpcNameTag, clusterName, tdName,
roleName, policyName, tgName, lbName, svcName, 7201),
roleName, policyName, lbName, svcName, 7201),
ExpectError: regexp.MustCompile(`expected health_check_grace_period_seconds to be in the range`),
},
{
Config: testAccAWSEcsService_healthCheckGracePeriodSeconds(vpcNameTag, clusterName, tdName,
roleName, policyName, tgName, lbName, svcName, 300),
roleName, policyName, lbName, svcName, 300),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEcsServiceExists(resourceName, &service),
resource.TestCheckResourceAttr(resourceName, "health_check_grace_period_seconds", "300"),
),
},
{
Config: testAccAWSEcsService_healthCheckGracePeriodSeconds(vpcNameTag, clusterName, tdName,
roleName, policyName, tgName, lbName, svcName, 600),
roleName, policyName, lbName, svcName, 600),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEcsServiceExists(resourceName, &service),
resource.TestCheckResourceAttr(resourceName, "health_check_grace_period_seconds", "600"),
Expand Down Expand Up @@ -422,7 +421,6 @@ func TestAccAWSEcsService_withAlb(t *testing.T) {
tdName := fmt.Sprintf("tf-acc-td-svc-w-alb-%s", rString)
roleName := fmt.Sprintf("tf-acc-role-svc-w-alb-%s", rString)
policyName := fmt.Sprintf("tf-acc-policy-svc-w-alb-%s", rString)
tgName := fmt.Sprintf("tf-acc-tg-svc-w-alb-%s", rString)
lbName := fmt.Sprintf("tf-acc-lb-svc-w-alb-%s", rString)
svcName := fmt.Sprintf("tf-acc-svc-w-alb-%s", rString)

Expand All @@ -432,7 +430,7 @@ func TestAccAWSEcsService_withAlb(t *testing.T) {
CheckDestroy: testAccCheckAWSEcsServiceDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSEcsServiceWithAlb(clusterName, tdName, roleName, policyName, tgName, lbName, svcName),
Config: testAccAWSEcsServiceWithAlb(clusterName, tdName, roleName, policyName, lbName, svcName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEcsServiceExists("aws_ecs_service.with_alb", &service),
resource.TestCheckResourceAttr("aws_ecs_service.with_alb", "load_balancer.#", "1"),
Expand Down Expand Up @@ -1127,7 +1125,7 @@ resource "aws_ecs_service" "main" {
}

func testAccAWSEcsService_healthCheckGracePeriodSeconds(vpcNameTag, clusterName, tdName, roleName, policyName,
tgName, lbName, svcName string, healthCheckGracePeriodSeconds int) string {
lbName, svcName string, healthCheckGracePeriodSeconds int) string {
return fmt.Sprintf(`
data "aws_availability_zones" "available" {}
Expand Down Expand Up @@ -1217,7 +1215,7 @@ EOF
}
resource "aws_lb_target_group" "test" {
name = "%s"
name = "${aws_lb.main.name}"
port = 80
protocol = "HTTP"
vpc_id = "${aws_vpc.main.id}"
Expand Down Expand Up @@ -1256,11 +1254,10 @@ resource "aws_ecs_service" "with_alb" {
depends_on = [
"aws_iam_role_policy.ecs_service",
"aws_lb_listener.front_end"
]
}
`, vpcNameTag, clusterName, tdName, roleName, policyName,
tgName, lbName, svcName, healthCheckGracePeriodSeconds)
lbName, svcName, healthCheckGracePeriodSeconds)
}

func testAccAWSEcsService_withIamRole(clusterName, tdName, roleName, policyName, svcName string) string {
Expand Down Expand Up @@ -1613,7 +1610,7 @@ resource "aws_ecs_service" "jenkins" {
`, clusterName, tdName, svcName)
}

func testAccAWSEcsServiceWithAlb(clusterName, tdName, roleName, policyName, tgName, lbName, svcName string) string {
func testAccAWSEcsServiceWithAlb(clusterName, tdName, roleName, policyName, lbName, svcName string) string {
return fmt.Sprintf(`
data "aws_availability_zones" "available" {}
Expand Down Expand Up @@ -1703,7 +1700,7 @@ EOF
}
resource "aws_lb_target_group" "test" {
name = "%s"
name = "${aws_lb.main.name}"
port = 80
protocol = "HTTP"
vpc_id = "${aws_vpc.main.id}"
Expand Down Expand Up @@ -1741,10 +1738,9 @@ resource "aws_ecs_service" "with_alb" {
depends_on = [
"aws_iam_role_policy.ecs_service",
"aws_lb_listener.front_end"
]
}
`, clusterName, tdName, roleName, policyName, tgName, lbName, svcName)
`, clusterName, tdName, roleName, policyName, lbName, svcName)
}

func testAccAWSEcsServiceWithNetworkConfiguration(sg1Name, sg2Name, clusterName, tdName, svcName string) string {
Expand Down

0 comments on commit 070dd7d

Please sign in to comment.