From 1ee4ecdc3bb93a584df874a7effa0bacb2f18d04 Mon Sep 17 00:00:00 2001 From: John Norton Date: Tue, 5 Dec 2017 15:23:42 -0700 Subject: [PATCH 01/19] add assign public ip property to network configuration --- aws/resource_aws_ecs_service.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/aws/resource_aws_ecs_service.go b/aws/resource_aws_ecs_service.go index e1942508400..d79b5c2bef4 100644 --- a/aws/resource_aws_ecs_service.go +++ b/aws/resource_aws_ecs_service.go @@ -112,21 +112,30 @@ func resourceAwsEcsService() *schema.Resource { "network_configuration": { Type: schema.TypeList, Optional: true, + ForceNew: true, MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "security_groups": { Type: schema.TypeSet, Optional: true, + ForceNew: true, Elem: &schema.Schema{Type: schema.TypeString}, Set: schema.HashString, }, "subnets": { Type: schema.TypeSet, Required: true, + ForceNew: true, Elem: &schema.Schema{Type: schema.TypeString}, Set: schema.HashString, }, + "assign_public_ip": &schema.Schema{ + Type: schema.TypeString, + Optional: true, + ForceNew: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, }, }, }, @@ -403,6 +412,9 @@ func flattenEcsNetworkConfigration(nc *ecs.NetworkConfiguration) []interface{} { result := make(map[string]interface{}) result["security_groups"] = schema.NewSet(schema.HashString, flattenStringList(nc.AwsvpcConfiguration.SecurityGroups)) result["subnets"] = schema.NewSet(schema.HashString, flattenStringList(nc.AwsvpcConfiguration.Subnets)) + if val, ok := result["assign_public_ip"].(string); ok { + result["assign_public_ip"] = val + } return []interface{}{result} } @@ -416,6 +428,11 @@ func expandEcsNetworkConfigration(nc []interface{}) *ecs.NetworkConfiguration { awsVpcConfig.SecurityGroups = expandStringSet(val.(*schema.Set)) } awsVpcConfig.Subnets = expandStringSet(raw["subnets"].(*schema.Set)) + + //if val, ok := raw["assign_public_ip"].(string); ok { + // awsVpcConfig.AssignPublicIp = aws.String(val) + // } + return &ecs.NetworkConfiguration{AwsvpcConfiguration: awsVpcConfig} } From 927d68c187ef9f11f89242a0d840c4d38d33d9a8 Mon Sep 17 00:00:00 2001 From: John Norton Date: Tue, 5 Dec 2017 16:19:03 -0700 Subject: [PATCH 02/19] force new resource on network config change, update docs --- aws/resource_aws_ecs_service.go | 12 ++++++------ website/docs/r/ecs_service.html.markdown | 1 + 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/aws/resource_aws_ecs_service.go b/aws/resource_aws_ecs_service.go index d79b5c2bef4..371719bf9c6 100644 --- a/aws/resource_aws_ecs_service.go +++ b/aws/resource_aws_ecs_service.go @@ -409,12 +409,12 @@ func flattenEcsNetworkConfigration(nc *ecs.NetworkConfiguration) []interface{} { if nc == nil { return nil } + result := make(map[string]interface{}) result["security_groups"] = schema.NewSet(schema.HashString, flattenStringList(nc.AwsvpcConfiguration.SecurityGroups)) result["subnets"] = schema.NewSet(schema.HashString, flattenStringList(nc.AwsvpcConfiguration.Subnets)) - if val, ok := result["assign_public_ip"].(string); ok { - result["assign_public_ip"] = val - } + result["assign_public_ip"] = *nc.AwsvpcConfiguration.AssignPublicIp + return []interface{}{result} } @@ -429,9 +429,9 @@ func expandEcsNetworkConfigration(nc []interface{}) *ecs.NetworkConfiguration { } awsVpcConfig.Subnets = expandStringSet(raw["subnets"].(*schema.Set)) - //if val, ok := raw["assign_public_ip"].(string); ok { - // awsVpcConfig.AssignPublicIp = aws.String(val) - // } + if val, ok := raw["assign_public_ip"].(string); ok { + awsVpcConfig.AssignPublicIp = aws.String(val) + } return &ecs.NetworkConfiguration{AwsvpcConfiguration: awsVpcConfig} } diff --git a/website/docs/r/ecs_service.html.markdown b/website/docs/r/ecs_service.html.markdown index fe90a84088f..76082fa3fd2 100644 --- a/website/docs/r/ecs_service.html.markdown +++ b/website/docs/r/ecs_service.html.markdown @@ -100,6 +100,7 @@ Guide](http://docs.aws.amazon.com/AmazonECS/latest/developerguide/cluster-query- `network_configuration` support the following: * `subnets` - (Required) The subnets associated with the task or service. * `security_groups` - (Optional) The security groups associated with the task or service. If you do not specify a security group, the default security group for the VPC is used. +* `assign_public_ip` - (Optional) Valid values are "ENABLED" or "DISABLED". Will assign a public IP address to the ENI. For more information, see [Task Networking](http://docs.aws.amazon.com/AmazonECS/latest/developerguidetask-networking.html) ## Attributes Reference From 36934737ab354734b6b7dee841d3a310df09907c Mon Sep 17 00:00:00 2001 From: John Norton Date: Tue, 5 Dec 2017 21:28:20 -0700 Subject: [PATCH 03/19] set default value for assign_public_ip --- aws/resource_aws_ecs_service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws/resource_aws_ecs_service.go b/aws/resource_aws_ecs_service.go index 371719bf9c6..0c255f9ab20 100644 --- a/aws/resource_aws_ecs_service.go +++ b/aws/resource_aws_ecs_service.go @@ -134,6 +134,7 @@ func resourceAwsEcsService() *schema.Resource { Type: schema.TypeString, Optional: true, ForceNew: true, + Default: "DISABLED", Elem: &schema.Schema{Type: schema.TypeString}, }, }, @@ -432,7 +433,6 @@ func expandEcsNetworkConfigration(nc []interface{}) *ecs.NetworkConfiguration { if val, ok := raw["assign_public_ip"].(string); ok { awsVpcConfig.AssignPublicIp = aws.String(val) } - return &ecs.NetworkConfiguration{AwsvpcConfiguration: awsVpcConfig} } From 8d71878089fb03b83b37d994780cc767153073fb Mon Sep 17 00:00:00 2001 From: John Norton Date: Thu, 7 Dec 2017 11:18:18 -0700 Subject: [PATCH 04/19] fix networkcofnig force new resource...add test --- aws/resource_aws_ecs_service.go | 13 ++++--------- aws/resource_aws_ecs_service_test.go | 1 + 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/aws/resource_aws_ecs_service.go b/aws/resource_aws_ecs_service.go index 0c255f9ab20..38a4ed5286f 100644 --- a/aws/resource_aws_ecs_service.go +++ b/aws/resource_aws_ecs_service.go @@ -112,30 +112,25 @@ func resourceAwsEcsService() *schema.Resource { "network_configuration": { Type: schema.TypeList, Optional: true, - ForceNew: true, MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "security_groups": { Type: schema.TypeSet, Optional: true, - ForceNew: true, Elem: &schema.Schema{Type: schema.TypeString}, Set: schema.HashString, }, "subnets": { Type: schema.TypeSet, Required: true, - ForceNew: true, Elem: &schema.Schema{Type: schema.TypeString}, Set: schema.HashString, }, "assign_public_ip": &schema.Schema{ Type: schema.TypeString, Optional: true, - ForceNew: true, Default: "DISABLED", - Elem: &schema.Schema{Type: schema.TypeString}, }, }, }, @@ -429,9 +424,10 @@ func expandEcsNetworkConfigration(nc []interface{}) *ecs.NetworkConfiguration { awsVpcConfig.SecurityGroups = expandStringSet(val.(*schema.Set)) } awsVpcConfig.Subnets = expandStringSet(raw["subnets"].(*schema.Set)) - + log.Printf("[DEBUG] assign_public_ip %s", raw["assign_public_ip"]) if val, ok := raw["assign_public_ip"].(string); ok { awsVpcConfig.AssignPublicIp = aws.String(val) + log.Printf("[DEBUG] AssingPublicIp %s", awsVpcConfig.AssignPublicIp) } return &ecs.NetworkConfiguration{AwsvpcConfiguration: awsVpcConfig} } @@ -498,9 +494,8 @@ func resourceAwsEcsServiceUpdate(d *schema.ResourceData, meta interface{}) error } } - if d.HasChange("network_configration") { - input.NetworkConfiguration = expandEcsNetworkConfigration(d.Get("network_configuration").([]interface{})) - } + //d.HasChange("network_configration") is not working, so explicity calling method. + input.NetworkConfiguration = expandEcsNetworkConfigration(d.Get("network_configuration").([]interface{})) // Retry due to IAM & ECS eventual consistency err := resource.Retry(2*time.Minute, func() *resource.RetryError { diff --git a/aws/resource_aws_ecs_service_test.go b/aws/resource_aws_ecs_service_test.go index 2b10eaeaac4..fd77da3f0e2 100644 --- a/aws/resource_aws_ecs_service_test.go +++ b/aws/resource_aws_ecs_service_test.go @@ -1261,6 +1261,7 @@ resource "aws_ecs_service" "main" { network_configuration { security_groups = ["${aws_security_group.allow_all_a.id}", "${aws_security_group.allow_all_b.id}"] subnets = ["${aws_subnet.main.*.id}"] + assign_public_ip = "ENABLED" } } `, rName, rName) From b48ae4a0067627ec4d0d61b3fe1846520dcefb8f Mon Sep 17 00:00:00 2001 From: John Norton Date: Thu, 7 Dec 2017 11:24:58 -0700 Subject: [PATCH 05/19] remove debug output --- aws/resource_aws_ecs_service.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/aws/resource_aws_ecs_service.go b/aws/resource_aws_ecs_service.go index 38a4ed5286f..071f744881c 100644 --- a/aws/resource_aws_ecs_service.go +++ b/aws/resource_aws_ecs_service.go @@ -424,10 +424,8 @@ func expandEcsNetworkConfigration(nc []interface{}) *ecs.NetworkConfiguration { awsVpcConfig.SecurityGroups = expandStringSet(val.(*schema.Set)) } awsVpcConfig.Subnets = expandStringSet(raw["subnets"].(*schema.Set)) - log.Printf("[DEBUG] assign_public_ip %s", raw["assign_public_ip"]) if val, ok := raw["assign_public_ip"].(string); ok { awsVpcConfig.AssignPublicIp = aws.String(val) - log.Printf("[DEBUG] AssingPublicIp %s", awsVpcConfig.AssignPublicIp) } return &ecs.NetworkConfiguration{AwsvpcConfiguration: awsVpcConfig} } From 83d84067d63856eece61472a155f09e512c8e08a Mon Sep 17 00:00:00 2001 From: John Norton Date: Tue, 16 Jan 2018 11:41:48 -0700 Subject: [PATCH 06/19] Change assign_public_ip to boolean --- aws/resource_aws_ecs_service.go | 17 +++++++++++------ aws/resource_aws_ecs_service_test.go | 2 +- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/aws/resource_aws_ecs_service.go b/aws/resource_aws_ecs_service.go index 934e2dcabbf..f68edb990bc 100644 --- a/aws/resource_aws_ecs_service.go +++ b/aws/resource_aws_ecs_service.go @@ -134,9 +134,9 @@ func resourceAwsEcsService() *schema.Resource { Set: schema.HashString, }, "assign_public_ip": &schema.Schema{ - Type: schema.TypeString, + Type: schema.TypeBool, Optional: true, - Default: "DISABLED", + Default: false, }, }, }, @@ -419,8 +419,10 @@ func flattenEcsNetworkConfigration(nc *ecs.NetworkConfiguration) []interface{} { result := make(map[string]interface{}) result["security_groups"] = schema.NewSet(schema.HashString, flattenStringList(nc.AwsvpcConfiguration.SecurityGroups)) result["subnets"] = schema.NewSet(schema.HashString, flattenStringList(nc.AwsvpcConfiguration.Subnets)) - result["assign_public_ip"] = *nc.AwsvpcConfiguration.AssignPublicIp - + result["assign_public_ip"] = "true" + if *nc.AwsvpcConfiguration.AssignPublicIp == "DISABLED" { + result["assign_public_ip"] = "false" + } return []interface{}{result} } @@ -434,8 +436,11 @@ func expandEcsNetworkConfigration(nc []interface{}) *ecs.NetworkConfiguration { awsVpcConfig.SecurityGroups = expandStringSet(val.(*schema.Set)) } awsVpcConfig.Subnets = expandStringSet(raw["subnets"].(*schema.Set)) - if val, ok := raw["assign_public_ip"].(string); ok { - awsVpcConfig.AssignPublicIp = aws.String(val) + if val, ok := raw["assign_public_ip"].(bool); ok { + awsVpcConfig.AssignPublicIp = aws.String("DISABLED") + if val { + awsVpcConfig.AssignPublicIp = aws.String("ENABLED") + } } return &ecs.NetworkConfiguration{AwsvpcConfiguration: awsVpcConfig} } diff --git a/aws/resource_aws_ecs_service_test.go b/aws/resource_aws_ecs_service_test.go index 1a78244e283..6dc999199f2 100644 --- a/aws/resource_aws_ecs_service_test.go +++ b/aws/resource_aws_ecs_service_test.go @@ -1517,7 +1517,7 @@ resource "aws_ecs_service" "main" { network_configuration { security_groups = ["${aws_security_group.allow_all_a.id}", "${aws_security_group.allow_all_b.id}"] subnets = ["${aws_subnet.main.*.id}"] - assign_public_ip = "ENABLED" + assign_public_ip = true } } `, sg1Name, sg2Name, clusterName, tdName, svcName) From 5c56c41678d754f015d796895173956a87e9a6e5 Mon Sep 17 00:00:00 2001 From: John Norton Date: Wed, 24 Jan 2018 23:25:47 -0700 Subject: [PATCH 07/19] Update ecs_service.html.markdown --- website/docs/r/ecs_service.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/r/ecs_service.html.markdown b/website/docs/r/ecs_service.html.markdown index 17232ea4851..481ec7376ed 100644 --- a/website/docs/r/ecs_service.html.markdown +++ b/website/docs/r/ecs_service.html.markdown @@ -102,7 +102,7 @@ Guide](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/cluster-query * `subnets` - (Required) The subnets associated with the task or service. * `security_groups` - (Optional) The security groups associated with the task or service. If you do not specify a security group, the default security group for the VPC is used. -* `assign_public_ip` - (Optional) Valid values are "ENABLED" or "DISABLED". Will assign a public IP address to the ENI. +* `assign_public_ip` - (Optional) Valid values are "true" or "false". Will assign a public IP address to the ENI. For more information, see [Task Networking](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task-networking.html) ## Attributes Reference From 1da6118bc70ccad26974a58c68a3aa8294fe4427 Mon Sep 17 00:00:00 2001 From: John Norton Date: Thu, 1 Feb 2018 22:37:39 -0700 Subject: [PATCH 08/19] change to true or false, update docs. --- aws/resource_aws_ecs_service_test.go | 34 ++++++++++++++++++++---- aws/resource_aws_lb_listener.go | 1 - examples/networking/region/numbering.tf | 1 - examples/networking/subnet/numbering.tf | 1 - website/docs/r/ecs_service.html.markdown | 2 +- 5 files changed, 30 insertions(+), 9 deletions(-) delete mode 120000 examples/networking/region/numbering.tf delete mode 120000 examples/networking/subnet/numbering.tf diff --git a/aws/resource_aws_ecs_service_test.go b/aws/resource_aws_ecs_service_test.go index 6dc999199f2..89496b4368e 100644 --- a/aws/resource_aws_ecs_service_test.go +++ b/aws/resource_aws_ecs_service_test.go @@ -486,7 +486,7 @@ func TestAccAWSEcsService_withLaunchTypeFargate(t *testing.T) { }) } -func TestAccAWSEcsService_withNetworkConfiguration(t *testing.T) { +func TestAccAWSEcsService_withNetworkConfigurationAssignPublicIp(t *testing.T) { rString := acctest.RandString(8) sg1Name := fmt.Sprintf("tf-acc-sg-1-svc-w-nc-%s", rString) @@ -501,7 +501,31 @@ func TestAccAWSEcsService_withNetworkConfiguration(t *testing.T) { CheckDestroy: testAccCheckAWSEcsServiceDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSEcsServiceWithNetworkConfigration(sg1Name, sg2Name, clusterName, tdName, svcName), + Config: testAccAWSEcsServiceWithNetworkConfigration(sg1Name, sg2Name, clusterName, tdName, svcName, "true"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSEcsServiceExists("aws_ecs_service.main"), + ), + }, + }, + }) +} + +func TestAccAWSEcsService_withNetworkConfigurationDoNotAssignPublicIp(t *testing.T) { + rString := acctest.RandString(8) + + sg1Name := fmt.Sprintf("tf-acc-sg-1-svc-w-nc-%s", rString) + sg2Name := fmt.Sprintf("tf-acc-sg-2-svc-w-nc-%s", rString) + clusterName := fmt.Sprintf("tf-acc-cluster-svc-w-nc-%s", rString) + tdName := fmt.Sprintf("tf-acc-td-svc-w-nc-%s", rString) + svcName := fmt.Sprintf("tf-acc-svc-w-nc-%s", rString) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSEcsServiceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSEcsServiceWithNetworkConfigration(sg1Name, sg2Name, clusterName, tdName, svcName, "false"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSEcsServiceExists("aws_ecs_service.main"), ), @@ -1448,7 +1472,7 @@ resource "aws_ecs_service" "with_alb" { `, clusterName, tdName, roleName, policyName, tgName, lbName, svcName) } -func testAccAWSEcsServiceWithNetworkConfigration(sg1Name, sg2Name, clusterName, tdName, svcName string) string { +func testAccAWSEcsServiceWithNetworkConfigration(sg1Name, sg2Name, clusterName, tdName, svcName, assignPublicIp string) string { return fmt.Sprintf(` data "aws_availability_zones" "available" {} @@ -1517,8 +1541,8 @@ resource "aws_ecs_service" "main" { network_configuration { security_groups = ["${aws_security_group.allow_all_a.id}", "${aws_security_group.allow_all_b.id}"] subnets = ["${aws_subnet.main.*.id}"] - assign_public_ip = true + assign_public_ip = %s } } -`, sg1Name, sg2Name, clusterName, tdName, svcName) +`, sg1Name, sg2Name, clusterName, tdName, svcName, assignPublicIp) } diff --git a/aws/resource_aws_lb_listener.go b/aws/resource_aws_lb_listener.go index ec99fa366f4..f96e6e6aa00 100644 --- a/aws/resource_aws_lb_listener.go +++ b/aws/resource_aws_lb_listener.go @@ -34,7 +34,6 @@ func resourceAwsLbListener() *schema.Resource { "load_balancer_arn": { Type: schema.TypeString, Required: true, - ForceNew: true, }, "port": { diff --git a/examples/networking/region/numbering.tf b/examples/networking/region/numbering.tf deleted file mode 120000 index 49f7617b054..00000000000 --- a/examples/networking/region/numbering.tf +++ /dev/null @@ -1 +0,0 @@ -../numbering/variables.tf \ No newline at end of file diff --git a/examples/networking/subnet/numbering.tf b/examples/networking/subnet/numbering.tf deleted file mode 120000 index 49f7617b054..00000000000 --- a/examples/networking/subnet/numbering.tf +++ /dev/null @@ -1 +0,0 @@ -../numbering/variables.tf \ No newline at end of file diff --git a/website/docs/r/ecs_service.html.markdown b/website/docs/r/ecs_service.html.markdown index 481ec7376ed..a427f15e2e7 100644 --- a/website/docs/r/ecs_service.html.markdown +++ b/website/docs/r/ecs_service.html.markdown @@ -102,7 +102,7 @@ Guide](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/cluster-query * `subnets` - (Required) The subnets associated with the task or service. * `security_groups` - (Optional) The security groups associated with the task or service. If you do not specify a security group, the default security group for the VPC is used. -* `assign_public_ip` - (Optional) Valid values are "true" or "false". Will assign a public IP address to the ENI. +* `assign_public_ip` - (Optional) Valid values are "true" or "false". Will assign a public IP address to the ENI. Default value is "false". For more information, see [Task Networking](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task-networking.html) ## Attributes Reference From 642d87085b973a3f09b3c6eecf0739c7100f00a0 Mon Sep 17 00:00:00 2001 From: John Norton Date: Thu, 1 Feb 2018 23:01:20 -0700 Subject: [PATCH 09/19] Use SDK Enum --- aws/resource_aws_ecs_service.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/aws/resource_aws_ecs_service.go b/aws/resource_aws_ecs_service.go index f68edb990bc..adeb7e6176b 100644 --- a/aws/resource_aws_ecs_service.go +++ b/aws/resource_aws_ecs_service.go @@ -420,7 +420,7 @@ func flattenEcsNetworkConfigration(nc *ecs.NetworkConfiguration) []interface{} { result["security_groups"] = schema.NewSet(schema.HashString, flattenStringList(nc.AwsvpcConfiguration.SecurityGroups)) result["subnets"] = schema.NewSet(schema.HashString, flattenStringList(nc.AwsvpcConfiguration.Subnets)) result["assign_public_ip"] = "true" - if *nc.AwsvpcConfiguration.AssignPublicIp == "DISABLED" { + if *nc.AwsvpcConfiguration.AssignPublicIp == ecs.AssignPublicIpDisabled { result["assign_public_ip"] = "false" } return []interface{}{result} @@ -437,11 +437,12 @@ func expandEcsNetworkConfigration(nc []interface{}) *ecs.NetworkConfiguration { } awsVpcConfig.Subnets = expandStringSet(raw["subnets"].(*schema.Set)) if val, ok := raw["assign_public_ip"].(bool); ok { - awsVpcConfig.AssignPublicIp = aws.String("DISABLED") + awsVpcConfig.AssignPublicIp = aws.String(ecs.AssignPublicIpDisabled) if val { - awsVpcConfig.AssignPublicIp = aws.String("ENABLED") + awsVpcConfig.AssignPublicIp = aws.String(ecs.AssignPublicIpEnabled) } } + return &ecs.NetworkConfiguration{AwsvpcConfiguration: awsVpcConfig} } From 554f63dc8e903c81775da3f961243745b6d97730 Mon Sep 17 00:00:00 2001 From: John Norton Date: Thu, 1 Feb 2018 23:03:13 -0700 Subject: [PATCH 10/19] remove &schema.Schema{ --- aws/resource_aws_ecs_service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws/resource_aws_ecs_service.go b/aws/resource_aws_ecs_service.go index adeb7e6176b..3c151589d95 100644 --- a/aws/resource_aws_ecs_service.go +++ b/aws/resource_aws_ecs_service.go @@ -133,7 +133,7 @@ func resourceAwsEcsService() *schema.Resource { Elem: &schema.Schema{Type: schema.TypeString}, Set: schema.HashString, }, - "assign_public_ip": &schema.Schema{ + "assign_public_ip": { Type: schema.TypeBool, Optional: true, Default: false, From 82f4d9b073a8027f7839170f909d4ffd52a371bf Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Fri, 2 Feb 2018 10:08:11 -0500 Subject: [PATCH 11/19] resource/aws_ecs_service: Prevent crash on missing AssignPublicIp and acceptance test assign_public_ip state and updates --- aws/resource_aws_ecs_service.go | 12 ++++++----- aws/resource_aws_ecs_service_test.go | 30 +++++++++++++++++----------- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/aws/resource_aws_ecs_service.go b/aws/resource_aws_ecs_service.go index 3c151589d95..ea8651321ef 100644 --- a/aws/resource_aws_ecs_service.go +++ b/aws/resource_aws_ecs_service.go @@ -419,10 +419,11 @@ func flattenEcsNetworkConfigration(nc *ecs.NetworkConfiguration) []interface{} { result := make(map[string]interface{}) result["security_groups"] = schema.NewSet(schema.HashString, flattenStringList(nc.AwsvpcConfiguration.SecurityGroups)) result["subnets"] = schema.NewSet(schema.HashString, flattenStringList(nc.AwsvpcConfiguration.Subnets)) - result["assign_public_ip"] = "true" - if *nc.AwsvpcConfiguration.AssignPublicIp == ecs.AssignPublicIpDisabled { - result["assign_public_ip"] = "false" + + if nc.AwsvpcConfiguration.AssignPublicIp != nil { + result["assign_public_ip"] = *nc.AwsvpcConfiguration.AssignPublicIp == ecs.AssignPublicIpEnabled } + return []interface{}{result} } @@ -512,8 +513,9 @@ func resourceAwsEcsServiceUpdate(d *schema.ResourceData, meta interface{}) error } } - //d.HasChange("network_configration") is not working, so explicity calling method. - input.NetworkConfiguration = expandEcsNetworkConfigration(d.Get("network_configuration").([]interface{})) + if d.HasChange("network_configuration") { + input.NetworkConfiguration = expandEcsNetworkConfigration(d.Get("network_configuration").([]interface{})) + } // Retry due to IAM & ECS eventual consistency err := resource.Retry(2*time.Minute, func() *resource.RetryError { diff --git a/aws/resource_aws_ecs_service_test.go b/aws/resource_aws_ecs_service_test.go index 89496b4368e..87076fe6a79 100644 --- a/aws/resource_aws_ecs_service_test.go +++ b/aws/resource_aws_ecs_service_test.go @@ -480,6 +480,7 @@ func TestAccAWSEcsService_withLaunchTypeFargate(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckAWSEcsServiceExists("aws_ecs_service.main"), resource.TestCheckResourceAttr("aws_ecs_service.main", "launch_type", "FARGATE"), + resource.TestCheckResourceAttr("aws_ecs_service.main", "network_configuration.0.assign_public_ip", "false"), ), }, }, @@ -495,6 +496,20 @@ func TestAccAWSEcsService_withNetworkConfigurationAssignPublicIp(t *testing.T) { tdName := fmt.Sprintf("tf-acc-td-svc-w-nc-%s", rString) svcName := fmt.Sprintf("tf-acc-svc-w-nc-%s", rString) + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAWSEcsServiceDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSEcsServiceWithNetworkConfigration(sg1Name, sg2Name, clusterName, tdName, svcName, "false"), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSEcsServiceExists("aws_ecs_service.main"), + resource.TestCheckResourceAttr("aws_ecs_service.main", "network_configuration.0.assign_public_ip", "false"), + ), + }, + }, + }) resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -504,21 +519,11 @@ func TestAccAWSEcsService_withNetworkConfigurationAssignPublicIp(t *testing.T) { Config: testAccAWSEcsServiceWithNetworkConfigration(sg1Name, sg2Name, clusterName, tdName, svcName, "true"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSEcsServiceExists("aws_ecs_service.main"), + resource.TestCheckResourceAttr("aws_ecs_service.main", "network_configuration.0.assign_public_ip", "true"), ), }, }, }) -} - -func TestAccAWSEcsService_withNetworkConfigurationDoNotAssignPublicIp(t *testing.T) { - rString := acctest.RandString(8) - - sg1Name := fmt.Sprintf("tf-acc-sg-1-svc-w-nc-%s", rString) - sg2Name := fmt.Sprintf("tf-acc-sg-2-svc-w-nc-%s", rString) - clusterName := fmt.Sprintf("tf-acc-cluster-svc-w-nc-%s", rString) - tdName := fmt.Sprintf("tf-acc-td-svc-w-nc-%s", rString) - svcName := fmt.Sprintf("tf-acc-svc-w-nc-%s", rString) - resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, @@ -528,6 +533,7 @@ func TestAccAWSEcsService_withNetworkConfigurationDoNotAssignPublicIp(t *testing Config: testAccAWSEcsServiceWithNetworkConfigration(sg1Name, sg2Name, clusterName, tdName, svcName, "false"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSEcsServiceExists("aws_ecs_service.main"), + resource.TestCheckResourceAttr("aws_ecs_service.main", "network_configuration.0.assign_public_ip", "false"), ), }, }, @@ -1537,7 +1543,7 @@ resource "aws_ecs_service" "main" { name = "%s" cluster = "${aws_ecs_cluster.main.id}" task_definition = "${aws_ecs_task_definition.mongo.arn}" - desired_count = 1 + desired_count = 0 network_configuration { security_groups = ["${aws_security_group.allow_all_a.id}", "${aws_security_group.allow_all_b.id}"] subnets = ["${aws_subnet.main.*.id}"] From e3036223cfee65333adc57e558dc161d2be88803 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Fri, 2 Feb 2018 10:08:35 -0500 Subject: [PATCH 12/19] resource/aws_lb_listener: Revert extraneous ForceNew change --- aws/resource_aws_lb_listener.go | 1 + 1 file changed, 1 insertion(+) diff --git a/aws/resource_aws_lb_listener.go b/aws/resource_aws_lb_listener.go index f96e6e6aa00..ec99fa366f4 100644 --- a/aws/resource_aws_lb_listener.go +++ b/aws/resource_aws_lb_listener.go @@ -34,6 +34,7 @@ func resourceAwsLbListener() *schema.Resource { "load_balancer_arn": { Type: schema.TypeString, Required: true, + ForceNew: true, }, "port": { From b55f369c71cec103d79d8a5c1cc08967f874075b Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Fri, 2 Feb 2018 10:34:41 -0500 Subject: [PATCH 13/19] resource/aws_ecs_service: Refactor error handling, especially to fix catching incorrect InvalidParameterException --- aws/resource_aws_ecs_service.go | 63 +++++++++++---------------------- 1 file changed, 21 insertions(+), 42 deletions(-) diff --git a/aws/resource_aws_ecs_service.go b/aws/resource_aws_ecs_service.go index ea8651321ef..51ec8dfd251 100644 --- a/aws/resource_aws_ecs_service.go +++ b/aws/resource_aws_ecs_service.go @@ -9,7 +9,6 @@ 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/hashicorp/terraform/helper/hashcode" "github.com/hashicorp/terraform/helper/resource" @@ -292,21 +291,12 @@ func resourceAwsEcsServiceCreate(d *schema.ResourceData, meta interface{}) error out, err = conn.CreateService(&input) if err != nil { - awsErr, ok := err.(awserr.Error) - if !ok { - return resource.NonRetryableError(err) - } - if awsErr.Code() == "InvalidParameterException" { - log.Printf("[DEBUG] Trying to create ECS service again: %q", - awsErr.Message()) + if isAWSErr(err, ecs.ErrCodeClusterNotFoundException, "") { return resource.RetryableError(err) } - if awsErr.Code() == "ClusterNotFoundException" { - log.Printf("[DEBUG] Trying to create ECS service again: %q", - awsErr.Message()) + if isAWSErr(err, ecs.ErrCodeInvalidParameterException, "Please verify that the ECS service role being passed has the proper permissions.") { return resource.RetryableError(err) } - return resource.NonRetryableError(err) } @@ -517,20 +507,13 @@ func resourceAwsEcsServiceUpdate(d *schema.ResourceData, meta interface{}) error input.NetworkConfiguration = expandEcsNetworkConfigration(d.Get("network_configuration").([]interface{})) } - // Retry due to IAM & ECS eventual consistency + // Retry due to IAM eventual consistency err := resource.Retry(2*time.Minute, func() *resource.RetryError { out, err := conn.UpdateService(&input) if err != nil { - awsErr, ok := err.(awserr.Error) - if ok && awsErr.Code() == "InvalidParameterException" { - log.Printf("[DEBUG] Trying to update ECS service again: %#v", err) - return resource.RetryableError(err) - } - if ok && awsErr.Code() == "ServiceNotFoundException" { - log.Printf("[DEBUG] Trying to update ECS service again: %#v", err) + if isAWSErr(err, ecs.ErrCodeInvalidParameterException, "Please verify that the ECS service role being passed has the proper permissions.") { return resource.RetryableError(err) } - return resource.NonRetryableError(err) } @@ -553,11 +536,17 @@ func resourceAwsEcsServiceDelete(d *schema.ResourceData, meta interface{}) error Cluster: aws.String(d.Get("cluster").(string)), }) if err != nil { + if isAWSErr(err, ecs.ErrCodeServiceNotFoundException, "") { + log.Printf("[DEBUG] Removing ECS Service from state, %q is already gone", d.Id()) + d.SetId("") + return nil + } return err } if len(resp.Services) == 0 { - log.Printf("[DEBUG] ECS Service %q is already gone", d.Id()) + log.Printf("[DEBUG] Removing ECS Service from state, %q is already gone", d.Id()) + d.SetId("") return nil } @@ -580,33 +569,23 @@ 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() *resource.RetryError { - input := ecs.DeleteServiceInput{ - Service: aws.String(d.Id()), - Cluster: aws.String(d.Get("cluster").(string)), - } - 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 { + if err != nil { + if isAWSErr(err, ecs.ErrCodeInvalidParameterException, "The service cannot be stopped while deployments are active.") { + return resource.RetryableError(err) + } return resource.NonRetryableError(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 resource.RetryableError(err) - } - - return resource.NonRetryableError(err) - + return nil }) + if err != nil { return err } From 7043e4f001ddba2e6c3c347292d6f274376d41fe Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Fri, 2 Feb 2018 11:12:41 -0500 Subject: [PATCH 14/19] test/resource/aws_ecs_service: Move assign_public_ip testing to Fargate only for now --- aws/resource_aws_ecs_service_test.go | 50 +++++++++++++++------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/aws/resource_aws_ecs_service_test.go b/aws/resource_aws_ecs_service_test.go index 87076fe6a79..25673b6480f 100644 --- a/aws/resource_aws_ecs_service_test.go +++ b/aws/resource_aws_ecs_service_test.go @@ -476,36 +476,27 @@ func TestAccAWSEcsService_withLaunchTypeFargate(t *testing.T) { CheckDestroy: testAccCheckAWSEcsServiceDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSEcsServiceWithLaunchTypeFargate(sg1Name, sg2Name, clusterName, tdName, svcName), + Config: testAccAWSEcsServiceWithLaunchTypeFargate(sg1Name, sg2Name, clusterName, tdName, svcName, "false"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSEcsServiceExists("aws_ecs_service.main"), resource.TestCheckResourceAttr("aws_ecs_service.main", "launch_type", "FARGATE"), resource.TestCheckResourceAttr("aws_ecs_service.main", "network_configuration.0.assign_public_ip", "false"), + resource.TestCheckResourceAttr("aws_ecs_service.main", "network_configuration.0.security_groups.#", "2"), + resource.TestCheckResourceAttr("aws_ecs_service.main", "network_configuration.0.subnets.#", "2"), ), }, }, }) -} - -func TestAccAWSEcsService_withNetworkConfigurationAssignPublicIp(t *testing.T) { - rString := acctest.RandString(8) - - sg1Name := fmt.Sprintf("tf-acc-sg-1-svc-w-nc-%s", rString) - sg2Name := fmt.Sprintf("tf-acc-sg-2-svc-w-nc-%s", rString) - clusterName := fmt.Sprintf("tf-acc-cluster-svc-w-nc-%s", rString) - tdName := fmt.Sprintf("tf-acc-td-svc-w-nc-%s", rString) - svcName := fmt.Sprintf("tf-acc-svc-w-nc-%s", rString) - resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckAWSEcsServiceDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSEcsServiceWithNetworkConfigration(sg1Name, sg2Name, clusterName, tdName, svcName, "false"), + Config: testAccAWSEcsServiceWithLaunchTypeFargate(sg1Name, sg2Name, clusterName, tdName, svcName, "true"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSEcsServiceExists("aws_ecs_service.main"), - resource.TestCheckResourceAttr("aws_ecs_service.main", "network_configuration.0.assign_public_ip", "false"), + resource.TestCheckResourceAttr("aws_ecs_service.main", "network_configuration.0.assign_public_ip", "true"), ), }, }, @@ -516,24 +507,37 @@ func TestAccAWSEcsService_withNetworkConfigurationAssignPublicIp(t *testing.T) { CheckDestroy: testAccCheckAWSEcsServiceDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSEcsServiceWithNetworkConfigration(sg1Name, sg2Name, clusterName, tdName, svcName, "true"), + Config: testAccAWSEcsServiceWithLaunchTypeFargate(sg1Name, sg2Name, clusterName, tdName, svcName, "false"), Check: resource.ComposeTestCheckFunc( testAccCheckAWSEcsServiceExists("aws_ecs_service.main"), - resource.TestCheckResourceAttr("aws_ecs_service.main", "network_configuration.0.assign_public_ip", "true"), + resource.TestCheckResourceAttr("aws_ecs_service.main", "network_configuration.0.assign_public_ip", "false"), ), }, }, }) +} + +func TestAccAWSEcsService_withLaunchTypeEC2AndNetworkConfiguration(t *testing.T) { + rString := acctest.RandString(8) + + sg1Name := fmt.Sprintf("tf-acc-sg-1-svc-w-nc-%s", rString) + sg2Name := fmt.Sprintf("tf-acc-sg-2-svc-w-nc-%s", rString) + clusterName := fmt.Sprintf("tf-acc-cluster-svc-w-nc-%s", rString) + tdName := fmt.Sprintf("tf-acc-td-svc-w-nc-%s", rString) + svcName := fmt.Sprintf("tf-acc-svc-w-nc-%s", rString) + resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckAWSEcsServiceDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSEcsServiceWithNetworkConfigration(sg1Name, sg2Name, clusterName, tdName, svcName, "false"), + Config: testAccAWSEcsServiceWithNetworkConfigration(sg1Name, sg2Name, clusterName, tdName, svcName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSEcsServiceExists("aws_ecs_service.main"), resource.TestCheckResourceAttr("aws_ecs_service.main", "network_configuration.0.assign_public_ip", "false"), + resource.TestCheckResourceAttr("aws_ecs_service.main", "network_configuration.0.security_groups.#", "2"), + resource.TestCheckResourceAttr("aws_ecs_service.main", "network_configuration.0.subnets.#", "2"), ), }, }, @@ -780,7 +784,7 @@ resource "aws_ecs_service" "mongo" { `, clusterName, tdName, svcName) } -func testAccAWSEcsServiceWithLaunchTypeFargate(sg1Name, sg2Name, clusterName, tdName, svcName string) string { +func testAccAWSEcsServiceWithLaunchTypeFargate(sg1Name, sg2Name, clusterName, tdName, svcName, assignPublicIP string) string { return fmt.Sprintf(` provider "aws" { region = "us-east-1" @@ -858,9 +862,10 @@ resource "aws_ecs_service" "main" { network_configuration { security_groups = ["${aws_security_group.allow_all_a.id}", "${aws_security_group.allow_all_b.id}"] subnets = ["${aws_subnet.main.*.id}"] + assign_public_ip = %s } } -`, sg1Name, sg2Name, clusterName, tdName, svcName) +`, sg1Name, sg2Name, clusterName, tdName, svcName, assignPublicIP) } func testAccAWSEcsService_healthCheckGracePeriodSeconds(vpcNameTag, clusterName, tdName, roleName, policyName, @@ -1478,7 +1483,7 @@ resource "aws_ecs_service" "with_alb" { `, clusterName, tdName, roleName, policyName, tgName, lbName, svcName) } -func testAccAWSEcsServiceWithNetworkConfigration(sg1Name, sg2Name, clusterName, tdName, svcName, assignPublicIp string) string { +func testAccAWSEcsServiceWithNetworkConfigration(sg1Name, sg2Name, clusterName, tdName, svcName string) string { return fmt.Sprintf(` data "aws_availability_zones" "available" {} @@ -1543,12 +1548,11 @@ resource "aws_ecs_service" "main" { name = "%s" cluster = "${aws_ecs_cluster.main.id}" task_definition = "${aws_ecs_task_definition.mongo.arn}" - desired_count = 0 + desired_count = 1 network_configuration { security_groups = ["${aws_security_group.allow_all_a.id}", "${aws_security_group.allow_all_b.id}"] subnets = ["${aws_subnet.main.*.id}"] - assign_public_ip = %s } } -`, sg1Name, sg2Name, clusterName, tdName, svcName, assignPublicIp) +`, sg1Name, sg2Name, clusterName, tdName, svcName) } From fa68bf0f957f36c79a279eee45a3ee3af6d82233 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Fri, 2 Feb 2018 11:15:21 -0500 Subject: [PATCH 15/19] docs/resource/aws_ecs_service: Note assign_public_ip limitation for Fargate --- website/docs/r/ecs_service.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/r/ecs_service.html.markdown b/website/docs/r/ecs_service.html.markdown index a427f15e2e7..e98f50e1bea 100644 --- a/website/docs/r/ecs_service.html.markdown +++ b/website/docs/r/ecs_service.html.markdown @@ -102,7 +102,7 @@ Guide](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/cluster-query * `subnets` - (Required) The subnets associated with the task or service. * `security_groups` - (Optional) The security groups associated with the task or service. If you do not specify a security group, the default security group for the VPC is used. -* `assign_public_ip` - (Optional) Valid values are "true" or "false". Will assign a public IP address to the ENI. Default value is "false". +* `assign_public_ip` - (Optional) Only valid for `FARGATE` launch type and valid values are `true` or `false`. Will assign a public IP address to the ENI. Default value is `false`. For more information, see [Task Networking](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task-networking.html) ## Attributes Reference From 8d990bc9c16e6b8eb1ec9c260bbb1220f4240436 Mon Sep 17 00:00:00 2001 From: "Dominic R. May" Date: Sat, 3 Feb 2018 18:01:37 +0800 Subject: [PATCH 16/19] resource/aws_ecs_service: Merge in typo fixes of network_configuration --- aws/resource_aws_ecs_service.go | 10 +++++----- aws/resource_aws_ecs_service_test.go | 4 ++-- aws/resource_aws_security_group_test.go | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/aws/resource_aws_ecs_service.go b/aws/resource_aws_ecs_service.go index 51ec8dfd251..0bc7b2e3c21 100644 --- a/aws/resource_aws_ecs_service.go +++ b/aws/resource_aws_ecs_service.go @@ -240,7 +240,7 @@ func resourceAwsEcsServiceCreate(d *schema.ResourceData, meta interface{}) error input.Role = aws.String(v.(string)) } - input.NetworkConfiguration = expandEcsNetworkConfigration(d.Get("network_configuration").([]interface{})) + input.NetworkConfiguration = expandEcsNetworkConfiguration(d.Get("network_configuration").([]interface{})) strategies := d.Get("placement_strategy").(*schema.Set).List() if len(strategies) > 0 { @@ -394,14 +394,14 @@ func resourceAwsEcsServiceRead(d *schema.ResourceData, meta interface{}) error { log.Printf("[ERR] Error setting placement_constraints for (%s): %s", d.Id(), err) } - if err := d.Set("network_configuration", flattenEcsNetworkConfigration(service.NetworkConfiguration)); err != nil { + if err := d.Set("network_configuration", flattenEcsNetworkConfiguration(service.NetworkConfiguration)); err != nil { return fmt.Errorf("[ERR] Error setting network_configuration for (%s): %s", d.Id(), err) } return nil } -func flattenEcsNetworkConfigration(nc *ecs.NetworkConfiguration) []interface{} { +func flattenEcsNetworkConfiguration(nc *ecs.NetworkConfiguration) []interface{} { if nc == nil { return nil } @@ -417,7 +417,7 @@ func flattenEcsNetworkConfigration(nc *ecs.NetworkConfiguration) []interface{} { return []interface{}{result} } -func expandEcsNetworkConfigration(nc []interface{}) *ecs.NetworkConfiguration { +func expandEcsNetworkConfiguration(nc []interface{}) *ecs.NetworkConfiguration { if len(nc) == 0 { return nil } @@ -504,7 +504,7 @@ func resourceAwsEcsServiceUpdate(d *schema.ResourceData, meta interface{}) error } if d.HasChange("network_configuration") { - input.NetworkConfiguration = expandEcsNetworkConfigration(d.Get("network_configuration").([]interface{})) + input.NetworkConfiguration = expandEcsNetworkConfiguration(d.Get("network_configuration").([]interface{})) } // Retry due to IAM eventual consistency diff --git a/aws/resource_aws_ecs_service_test.go b/aws/resource_aws_ecs_service_test.go index 25673b6480f..ea63c7e6535 100644 --- a/aws/resource_aws_ecs_service_test.go +++ b/aws/resource_aws_ecs_service_test.go @@ -532,7 +532,7 @@ func TestAccAWSEcsService_withLaunchTypeEC2AndNetworkConfiguration(t *testing.T) CheckDestroy: testAccCheckAWSEcsServiceDestroy, Steps: []resource.TestStep{ { - Config: testAccAWSEcsServiceWithNetworkConfigration(sg1Name, sg2Name, clusterName, tdName, svcName), + Config: testAccAWSEcsServiceWithNetworkConfiguration(sg1Name, sg2Name, clusterName, tdName, svcName), Check: resource.ComposeTestCheckFunc( testAccCheckAWSEcsServiceExists("aws_ecs_service.main"), resource.TestCheckResourceAttr("aws_ecs_service.main", "network_configuration.0.assign_public_ip", "false"), @@ -1483,7 +1483,7 @@ resource "aws_ecs_service" "with_alb" { `, clusterName, tdName, roleName, policyName, tgName, lbName, svcName) } -func testAccAWSEcsServiceWithNetworkConfigration(sg1Name, sg2Name, clusterName, tdName, svcName string) string { +func testAccAWSEcsServiceWithNetworkConfiguration(sg1Name, sg2Name, clusterName, tdName, svcName string) string { return fmt.Sprintf(` data "aws_availability_zones" "available" {} diff --git a/aws/resource_aws_security_group_test.go b/aws/resource_aws_security_group_test.go index 5de843f5812..9c640edba6f 100644 --- a/aws/resource_aws_security_group_test.go +++ b/aws/resource_aws_security_group_test.go @@ -506,7 +506,7 @@ func TestAccAWSSecurityGroup_forceRevokeRules_true(t *testing.T) { testAddCycle, ), }, - // Verify the DependencyViolation error by using a configration with the + // Verify the DependencyViolation error by using a configuration with the // groups removed. Terraform tries to destroy them but cannot. Expect a // DependencyViolation error { @@ -579,7 +579,7 @@ func TestAccAWSSecurityGroup_forceRevokeRules_false(t *testing.T) { testAddCycle, ), }, - // Verify the DependencyViolation error by using a configration with the + // Verify the DependencyViolation error by using a configuration with the // groups removed, and the Groups not configured to revoke their ruls. // Terraform tries to destroy them but cannot. Expect a // DependencyViolation error From 542d130b99a03f46da8cf2e4ef8be8c3685d9d11 Mon Sep 17 00:00:00 2001 From: Dominic May Date: Sat, 3 Feb 2018 19:33:21 +0800 Subject: [PATCH 17/19] test/resource/aws_ecs_service: Merge LaunchTypeEc2AndNetworkConfiguration update testing --- aws/resource_aws_ecs_service_test.go | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/aws/resource_aws_ecs_service_test.go b/aws/resource_aws_ecs_service_test.go index ea63c7e6535..9536c5e1260 100644 --- a/aws/resource_aws_ecs_service_test.go +++ b/aws/resource_aws_ecs_service_test.go @@ -540,6 +540,15 @@ func TestAccAWSEcsService_withLaunchTypeEC2AndNetworkConfiguration(t *testing.T) resource.TestCheckResourceAttr("aws_ecs_service.main", "network_configuration.0.subnets.#", "2"), ), }, + { + Config: testAccAWSEcsServiceWithNetworkConfiguration_modified(sg1Name, sg2Name, clusterName, tdName, svcName), + Check: resource.ComposeTestCheckFunc( + testAccCheckAWSEcsServiceExists("aws_ecs_service.main"), + resource.TestCheckResourceAttr("aws_ecs_service.main", "network_configuration.0.assign_public_ip", "false"), + resource.TestCheckResourceAttr("aws_ecs_service.main", "network_configuration.0.security_groups.#", "1"), + resource.TestCheckResourceAttr("aws_ecs_service.main", "network_configuration.0.subnets.#", "2"), + ), + }, }, }) } @@ -1484,6 +1493,19 @@ resource "aws_ecs_service" "with_alb" { } func testAccAWSEcsServiceWithNetworkConfiguration(sg1Name, sg2Name, clusterName, tdName, svcName string) string { + return tpl_testAccAWSEcsServiceWithNetworkConfiguration( + sg1Name, sg2Name, clusterName, tdName, svcName, + `"${aws_security_group.allow_all_a.id}", "${aws_security_group.allow_all_b.id}"`, + ) +} +func testAccAWSEcsServiceWithNetworkConfiguration_modified(sg1Name, sg2Name, clusterName, tdName, svcName string) string { + return tpl_testAccAWSEcsServiceWithNetworkConfiguration( + sg1Name, sg2Name, clusterName, tdName, svcName, + `"${aws_security_group.allow_all_a.id}"`, + ) +} + +func tpl_testAccAWSEcsServiceWithNetworkConfiguration(sg1Name, sg2Name, clusterName, tdName, svcName string, securityGroups string) string { return fmt.Sprintf(` data "aws_availability_zones" "available" {} @@ -1550,9 +1572,9 @@ resource "aws_ecs_service" "main" { task_definition = "${aws_ecs_task_definition.mongo.arn}" desired_count = 1 network_configuration { - security_groups = ["${aws_security_group.allow_all_a.id}", "${aws_security_group.allow_all_b.id}"] + security_groups = [%s] subnets = ["${aws_subnet.main.*.id}"] } } -`, sg1Name, sg2Name, clusterName, tdName, svcName) +`, sg1Name, sg2Name, clusterName, tdName, svcName, securityGroups) } From e63a0143ad588c8326da1783007d24ee3382ddb4 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Mon, 5 Feb 2018 17:05:29 -0500 Subject: [PATCH 18/19] docs/resource/aws_ecs_service: Updated wording for assign_public_ip based on #3240 feedback --- website/docs/r/ecs_service.html.markdown | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/website/docs/r/ecs_service.html.markdown b/website/docs/r/ecs_service.html.markdown index e98f50e1bea..ec7e3331287 100644 --- a/website/docs/r/ecs_service.html.markdown +++ b/website/docs/r/ecs_service.html.markdown @@ -102,7 +102,8 @@ Guide](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/cluster-query * `subnets` - (Required) The subnets associated with the task or service. * `security_groups` - (Optional) The security groups associated with the task or service. If you do not specify a security group, the default security group for the VPC is used. -* `assign_public_ip` - (Optional) Only valid for `FARGATE` launch type and valid values are `true` or `false`. Will assign a public IP address to the ENI. Default value is `false`. +* `assign_public_ip` - (Optional) Assign a public IP address to the ENI (Fargate launch type only). Valid values are `true` or `false`. Default `false`. + For more information, see [Task Networking](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task-networking.html) ## Attributes Reference From 7551f9293f5f92b57e506cefe7ee8968347feb72 Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Thu, 8 Feb 2018 14:57:11 -0500 Subject: [PATCH 19/19] resource/aws_ecs_service: Use Read after Create and retry on ServiceNotFound for new resources --- aws/resource_aws_ecs_service.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/aws/resource_aws_ecs_service.go b/aws/resource_aws_ecs_service.go index 0bc7b2e3c21..9f4fb374152 100644 --- a/aws/resource_aws_ecs_service.go +++ b/aws/resource_aws_ecs_service.go @@ -311,7 +311,7 @@ func resourceAwsEcsServiceCreate(d *schema.ResourceData, meta interface{}) error log.Printf("[DEBUG] ECS service created: %s", *service.ServiceArn) d.SetId(*service.ServiceArn) - return resourceAwsEcsServiceUpdate(d, meta) + return resourceAwsEcsServiceRead(d, meta) } func resourceAwsEcsServiceRead(d *schema.ResourceData, meta interface{}) error { @@ -323,7 +323,18 @@ func resourceAwsEcsServiceRead(d *schema.ResourceData, meta interface{}) error { Cluster: aws.String(d.Get("cluster").(string)), } - out, err := conn.DescribeServices(&input) + var out *ecs.DescribeServicesOutput + err := resource.Retry(2*time.Minute, func() *resource.RetryError { + var err error + out, err = conn.DescribeServices(&input) + if err != nil { + if d.IsNewResource() && isAWSErr(err, ecs.ErrCodeServiceNotFoundException, "") { + return resource.RetryableError(err) + } + return resource.NonRetryableError(err) + } + return nil + }) if err != nil { return err }