Skip to content

Commit

Permalink
resource/ecs_service: Fixes normalization issues in placement_strategy (
Browse files Browse the repository at this point in the history
#1025)

Fixes: #938

When aws_ecs_service has a placement_strategy of type `spread`, you can
use a field of `instanceId` or `host` interchangable. BUT AWS will normalize the
field value to return `instanceId`.

We have introduced a custom Set func to manage this work so that we
understand when someone uses `host`, we won't get a diff when AWS return
`instanceId` -> if there is a diff, a ForceNew is applied

```
% make testacc TEST=./aws TESTARGS='-run=TestAccAWSEcsService'                                                                  ✹
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSEcsService -timeout 120m
=== RUN   TestAccAWSEcsServiceWithARN
--- PASS: TestAccAWSEcsServiceWithARN (147.06s)
=== RUN   TestAccAWSEcsServiceWithUnnormalizedPlacementStrategy
--- PASS: TestAccAWSEcsServiceWithUnnormalizedPlacementStrategy (116.97s)
=== RUN   TestAccAWSEcsServiceWithFamilyAndRevision
--- PASS: TestAccAWSEcsServiceWithFamilyAndRevision (125.69s)
=== RUN   TestAccAWSEcsServiceWithRenamedCluster
--- PASS: TestAccAWSEcsServiceWithRenamedCluster (268.02s)
=== RUN   TestAccAWSEcsService_withIamRole
--- PASS: TestAccAWSEcsService_withIamRole (155.57s)
=== RUN   TestAccAWSEcsService_withDeploymentValues
--- PASS: TestAccAWSEcsService_withDeploymentValues (128.80s)
=== RUN   TestAccAWSEcsService_withLbChanges
--- PASS: TestAccAWSEcsService_withLbChanges (242.82s)
=== RUN   TestAccAWSEcsService_withEcsClusterName
--- PASS: TestAccAWSEcsService_withEcsClusterName (113.21s)
=== RUN   TestAccAWSEcsService_withAlb
--- PASS: TestAccAWSEcsService_withAlb (349.69s)
=== RUN   TestAccAWSEcsServiceWithPlacementStrategy
--- PASS: TestAccAWSEcsServiceWithPlacementStrategy (238.08s)
=== RUN   TestAccAWSEcsServiceWithPlacementConstraints
--- PASS: TestAccAWSEcsServiceWithPlacementConstraints (123.41s)
=== RUN   TestAccAWSEcsServiceWithPlacementConstraints_emptyExpression
--- PASS: TestAccAWSEcsServiceWithPlacementConstraints_emptyExpression (126.25s)
PASS
ok	github.com/terraform-providers/terraform-provider-aws/aws	2138.968s
```
  • Loading branch information
stack72 authored Jun 30, 2017
1 parent d4d97e0 commit 7068601
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 0 deletions.
14 changes: 14 additions & 0 deletions aws/resource_aws_ecs_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,20 @@ func resourceAwsEcsService() *schema.Resource {
},
},
},
Set: func(v interface{}) int {
var buf bytes.Buffer
m := v.(map[string]interface{})
buf.WriteString(fmt.Sprintf("%s-", m["type"].(string)))
if m["field"] != nil {
field := m["field"].(string)
if field == "host" {
buf.WriteString("instanceId-")
} else {
buf.WriteString(fmt.Sprintf("%s-", field))
}
}
return hashcode.String(buf.String())
},
},

"placement_constraints": {
Expand Down
51 changes: 51 additions & 0 deletions aws/resource_aws_ecs_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,23 @@ func TestAccAWSEcsServiceWithARN(t *testing.T) {
})
}

func TestAccAWSEcsServiceWithUnnormalizedPlacementStrategy(t *testing.T) {
rInt := acctest.RandInt()
resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSEcsServiceDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSEcsServiceWithInterchangeablePlacementStrategy(rInt),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEcsServiceExists("aws_ecs_service.mongo"),
),
},
},
})
}

func TestAccAWSEcsServiceWithFamilyAndRevision(t *testing.T) {
rName := acctest.RandomWithPrefix("tf-test")
resource.Test(t, resource.TestCase{
Expand Down Expand Up @@ -431,6 +448,40 @@ resource "aws_ecs_service" "mongo" {
`, rInt, rInt)
}

func testAccAWSEcsServiceWithInterchangeablePlacementStrategy(rInt int) string {
return fmt.Sprintf(`
resource "aws_ecs_cluster" "default" {
name = "terraformecstest%d"
}
resource "aws_ecs_task_definition" "mongo" {
family = "mongodb"
container_definitions = <<DEFINITION
[
{
"cpu": 128,
"essential": true,
"image": "mongo:latest",
"memory": 128,
"name": "mongodb"
}
]
DEFINITION
}
resource "aws_ecs_service" "mongo" {
name = "mongodb-%d"
cluster = "${aws_ecs_cluster.default.id}"
task_definition = "${aws_ecs_task_definition.mongo.arn}"
desired_count = 1
placement_strategy {
field = "host"
type = "spread"
}
}
`, rInt, rInt)
}

func testAccAWSEcsServiceWithPlacementStrategy(rInt int) string {
return fmt.Sprintf(`
resource "aws_ecs_cluster" "default" {
Expand Down
2 changes: 2 additions & 0 deletions website/docs/r/ecs_service.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ Load balancers support the following:
For the `binpack` type, valid values are `memory` and `cpu`. For the `random` type, this attribute is not
needed. For more information, see [Placement Strategy](http://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_PlacementStrategy.html).

-> **Note:** for `spread`, `host` and `instanceId` will be normalized, by AWS, to be `instanceId`. This means the statefile will show `instanceId` but your config will differ if you use `host`.

## placement_constraints

`placement_constraints` support the following:
Expand Down

0 comments on commit 7068601

Please sign in to comment.