Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ecs scheduling strategy #4825

Merged
merged 5 commits into from
Jun 26, 2018
Merged

Ecs scheduling strategy #4825

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fc1679d
aws_ecs_service: add support for scheduling_strategy
Jun 13, 2018
7b0975d
auto ignore desired count for DAEMON scheduling strategy
Jun 13, 2018
6b5fa9e
update tests
Jun 13, 2018
81e7818
do not require desired_count in DAEMON scheduling strategy
Jun 13, 2018
fdcbea6
code review feedback
Jun 14, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions aws/data_source_aws_ecs_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ func dataSourceAwsEcsService() *schema.Resource {
Type: schema.TypeString,
Computed: true,
},
"scheduling_strategy": {
Type: schema.TypeString,
Computed: true,
},
"task_definition": {
Type: schema.TypeString,
Computed: true,
Expand Down Expand Up @@ -78,6 +82,7 @@ func dataSourceAwsEcsServiceRead(d *schema.ResourceData, meta interface{}) error
d.Set("cluster_arn", service.ClusterArn)
d.Set("desired_count", service.DesiredCount)
d.Set("launch_type", service.LaunchType)
d.Set("scheduling_strategy", service.SchedulingStrategy)
d.Set("task_definition", service.TaskDefinition)

return nil
Expand Down
1 change: 1 addition & 0 deletions aws/data_source_aws_ecs_service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ func TestAccAWSEcsServiceDataSource_basic(t *testing.T) {
resource.TestCheckResourceAttrPair(resourceName, "id", dataSourceName, "arn"),
resource.TestCheckResourceAttrPair(resourceName, "desired_count", dataSourceName, "desired_count"),
resource.TestCheckResourceAttrPair(resourceName, "launch_type", dataSourceName, "launch_type"),
resource.TestCheckResourceAttrPair(resourceName, "scheduling_strategy", dataSourceName, "scheduling_strategy"),
resource.TestCheckResourceAttrPair(resourceName, "name", dataSourceName, "service_name"),
resource.TestCheckResourceAttrPair(resourceName, "task_definition", dataSourceName, "task_definition"),
),
Expand Down
28 changes: 26 additions & 2 deletions aws/resource_aws_ecs_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,17 @@ func resourceAwsEcsService() *schema.Resource {
Default: "EC2",
},

"scheduling_strategy": {
Type: schema.TypeString,
Optional: true,
ForceNew: true,
Default: ecs.SchedulingStrategyReplica,
ValidateFunc: validation.StringInSlice([]string{
ecs.SchedulingStrategyDaemon,
ecs.SchedulingStrategyReplica,
}, false),
},

"iam_role": {
Type: schema.TypeString,
ForceNew: true,
Expand Down Expand Up @@ -321,6 +332,13 @@ func resourceAwsEcsServiceCreate(d *schema.ResourceData, meta interface{}) error
input.LaunchType = aws.String(v.(string))
}

schedulingStrategy := d.Get("scheduling_strategy").(string)
input.SchedulingStrategy = aws.String(schedulingStrategy)
if schedulingStrategy == ecs.SchedulingStrategyDaemon {
// unset desired count if DAEMON
input.DesiredCount = nil
}

loadBalancers := expandEcsLoadBalancers(d.Get("load_balancer").(*schema.Set).List())
if len(loadBalancers) > 0 {
log.Printf("[DEBUG] Adding ECS load balancers: %s", loadBalancers)
Expand Down Expand Up @@ -489,7 +507,11 @@ func resourceAwsEcsServiceRead(d *schema.ResourceData, meta interface{}) error {
d.Set("task_definition", taskDefinition)
}

d.Set("desired_count", service.DesiredCount)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What value for service.DesiredCount gets returned when in DAEMON mode? Is it 0 or the number of container instances? We should generally try to call d.Set() if possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my testing, it's the number of container instances, which can change periodically.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the following DiffSuppressFunc to the desired_count, deployment_minimum_percent, and deployment_maximum_percent attributes, we can continue to always call d.Set("desired_count", service.DesiredCount) (along with the others) and remove the conditional in the read function. This will remove the breaking change for outputs that depend on the desired_count attribute always existing.

DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool {
	if d.Get("scheduling_strategy").(string) == ecs.SchedulingStrategyDaemon {
		return true
	}
	return false
},

d.Set("scheduling_strategy", service.SchedulingStrategy)
// Automatically ignore desired count if DAEMON
if *service.SchedulingStrategy != ecs.SchedulingStrategyDaemon {
d.Set("desired_count", service.DesiredCount)
}
d.Set("health_check_grace_period_seconds", service.HealthCheckGracePeriodSeconds)
d.Set("launch_type", service.LaunchType)

Expand Down Expand Up @@ -709,7 +731,9 @@ func resourceAwsEcsServiceUpdate(d *schema.ResourceData, meta interface{}) error
Cluster: aws.String(d.Get("cluster").(string)),
}

if d.HasChange("desired_count") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the DesiredCount returning from the API is nil or 0 when in DAEMON mode, we should be able to leave the old logic here. 👍

Copy link
Contributor Author

@erks erks Jun 19, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my testing, it's the number of container instances.

schedulingStrategy := d.Get("scheduling_strategy").(string)
// Automatically ignore desired count if DAEMON
if schedulingStrategy != ecs.SchedulingStrategyDaemon && d.HasChange("desired_count") {
_, n := d.GetChange("desired_count")
input.DesiredCount = aws.Int64(int64(n.(int)))
}
Expand Down
108 changes: 108 additions & 0 deletions aws/resource_aws_ecs_service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ func TestAccAWSEcsService_withARN(t *testing.T) {
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEcsServiceExists("aws_ecs_service.mongo", &service),
resource.TestCheckResourceAttr("aws_ecs_service.mongo", "service_registries.#", "0"),
resource.TestCheckResourceAttr("aws_ecs_service.mongo", "scheduling_strategy", "REPLICA"),
),
},

Expand All @@ -111,6 +112,7 @@ func TestAccAWSEcsService_withARN(t *testing.T) {
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEcsServiceExists("aws_ecs_service.mongo", &service),
resource.TestCheckResourceAttr("aws_ecs_service.mongo", "service_registries.#", "0"),
resource.TestCheckResourceAttr("aws_ecs_service.mongo", "scheduling_strategy", "REPLICA"),
),
},
},
Expand Down Expand Up @@ -626,6 +628,54 @@ func TestAccAWSEcsService_withLaunchTypeEC2AndNetworkConfiguration(t *testing.T)
})
}

func TestAccAWSEcsService_withDaemonSchedulingStrategy(t *testing.T) {
var service ecs.Service
rString := acctest.RandString(8)

clusterName := fmt.Sprintf("tf-acc-cluster-svc-w-ss-daemon-%s", rString)
tdName := fmt.Sprintf("tf-acc-td-svc-w-ss-daemon-%s", rString)
svcName := fmt.Sprintf("tf-acc-svc-w-ss-daemon-%s", rString)

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSEcsServiceDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSEcsServiceWithDaemonSchedulingStrategy(clusterName, tdName, svcName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEcsServiceExists("aws_ecs_service.ghost", &service),
resource.TestCheckResourceAttr("aws_ecs_service.ghost", "scheduling_strategy", "DAEMON"),
),
},
},
})
}

func TestAccAWSEcsService_withReplicaSchedulingStrategy(t *testing.T) {
var service ecs.Service
rString := acctest.RandString(8)

clusterName := fmt.Sprintf("tf-acc-cluster-svc-w-ss-replica-%s", rString)
tdName := fmt.Sprintf("tf-acc-td-svc-w-ss-replica-%s", rString)
svcName := fmt.Sprintf("tf-acc-svc-w-ss-replica-%s", rString)

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSEcsServiceDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSEcsServiceWithReplicaSchedulingStrategy(clusterName, tdName, svcName),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSEcsServiceExists("aws_ecs_service.ghost", &service),
resource.TestCheckResourceAttr("aws_ecs_service.ghost", "scheduling_strategy", "REPLICA"),
),
},
},
})
}

func TestAccAWSEcsService_withServiceRegistries(t *testing.T) {
var service ecs.Service
rString := acctest.RandString(8)
Expand Down Expand Up @@ -1955,3 +2005,61 @@ resource "aws_ecs_service" "test" {
}
`, rName, rName, rName, clusterName, tdName, svcName)
}

func testAccAWSEcsServiceWithDaemonSchedulingStrategy(clusterName, tdName, svcName string) string {
return fmt.Sprintf(`
resource "aws_ecs_cluster" "default" {
name = "%s"
}
resource "aws_ecs_task_definition" "ghost" {
family = "%s"
container_definitions = <<DEFINITION
[
{
"cpu": 128,
"essential": true,
"image": "ghost:latest",
"memory": 128,
"name": "ghost"
}
]
DEFINITION
}
resource "aws_ecs_service" "ghost" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test resource needs to reduce the deployment_maximum_percent from the default of 200:

=== RUN   TestAccAWSEcsService_withDaemonSchedulingStrategy
--- FAIL: TestAccAWSEcsService_withDaemonSchedulingStrategy (5.71s)
    testing.go:518: Step 0 error: Error applying: 1 error(s) occurred:
        
        * aws_ecs_service.ghost: 1 error(s) occurred:
        
        * aws_ecs_service.ghost: InvalidParameterException: The daemon scheduling strategy does not support values above 100 for maximumPercent on a deployment configuration. Change the maximumPercent value to 100, and try again

e.g. deployment_maximum_percent = 100

It might also be worth noting this in the attribute documentation and/or adding a daemon example to the documentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done and added the doc/example.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This acceptance test is still failing:

=== RUN   TestAccAWSEcsService_withDaemonSchedulingStrategy
--- FAIL: TestAccAWSEcsService_withDaemonSchedulingStrategy (5.99s)
    testing.go:518: Step 0 error: Error applying: 1 error(s) occurred:
        
        * aws_ecs_service.ghost: 1 error(s) occurred:
        
        * aws_ecs_service.ghost: InvalidParameterException: Both maximumPercent and minimumHealthyPercent cannot be 100 as this will block deployments.
            status code: 400, request id: d5daf50b-7949-11e8-a362-3ff63025fa34 "tf-acc-svc-w-ss-daemon-8yeejuf7"

By changing the create function to never include DeploymentConfiguration when set in DAEMON scheduling strategy, it then received this error during acceptance testing:

=== RUN   TestAccAWSEcsService_withDaemonSchedulingStrategy
--- FAIL: TestAccAWSEcsService_withDaemonSchedulingStrategy (12.56s)
	testing.go:579: Error destroying resource! WARNING: Dangling resources
		may exist. The full state and error is shown below.

		Error: Error applying: 1 error(s) occurred:

		* aws_ecs_service.ghost (destroy): 1 error(s) occurred:

		* aws_ecs_service.ghost: InvalidParameterException: The daemon scheduling strategy does not support a desired count for services. Remove the desired count value and try again
			status code: 400, request id: d91107b6-794b-11e8-b194-535f28f11fde

The deletion function tries to drain any ACTIVE status services by setting their DesiredCount to 0 before deletion. Ignoring DAEMON scheduling strategy for that call, the testing passes as expected:

make testacc TEST=./aws TESTARGS='-run=TestAccAWSEcsService_withDaemonSchedulingStrategy'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -run=TestAccAWSEcsService_withDaemonSchedulingStrategy -timeout 120m
=== RUN   TestAccAWSEcsService_withDaemonSchedulingStrategy
--- PASS: TestAccAWSEcsService_withDaemonSchedulingStrategy (15.51s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	15.549s

name = "%s"
cluster = "${aws_ecs_cluster.default.id}"
task_definition = "${aws_ecs_task_definition.ghost.family}:${aws_ecs_task_definition.ghost.revision}"
scheduling_strategy = "DAEMON"
deployment_maximum_percent = 100
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of requiring a special configuration for min/max, we'll setup the resource to properly ignore those configurations and differences 👍

}
`, clusterName, tdName, svcName)
}

func testAccAWSEcsServiceWithReplicaSchedulingStrategy(clusterName, tdName, svcName string) string {
return fmt.Sprintf(`
resource "aws_ecs_cluster" "default" {
name = "%s"
}
resource "aws_ecs_task_definition" "ghost" {
family = "%s"
container_definitions = <<DEFINITION
[
{
"cpu": 128,
"essential": true,
"image": "ghost:latest",
"memory": 128,
"name": "ghost"
}
]
DEFINITION
}
resource "aws_ecs_service" "ghost" {
name = "%s"
cluster = "${aws_ecs_cluster.default.id}"
task_definition = "${aws_ecs_task_definition.ghost.family}:${aws_ecs_task_definition.ghost.revision}"
scheduling_strategy = "REPLICA"
desired_count = 1
}
`, clusterName, tdName, svcName)
}
1 change: 1 addition & 0 deletions website/docs/d/ecs_service.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,5 @@ The following attributes are exported:
* `arn` - The ARN of the ECS Service
* `desired_count` - The number of tasks for the ECS Service
* `launch_type` - The launch type for the ECS Service
* `scheduling_strategy` - The scheduling strategy for the ECS Service
* `task_definition` - The family for the latest ACTIVE revision
17 changes: 15 additions & 2 deletions website/docs/r/ecs_service.html.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,30 @@ resource "aws_ecs_service" "example" {
}
```

### Daemon Scheduling Strategy

```hcl
resource "aws_ecs_service" "bar" {
name = "bar"
cluster = "${aws_ecs_cluster.foo.id}"
task_definition = "${aws_ecs_task_definition.bar.arn}"
scheduling_strategy = "DAEMON"
deployment_maximum_percent = 100
}
```

## Argument Reference

The following arguments are supported:

* `name` - (Required) The name of the service (up to 255 letters, numbers, hyphens, and underscores)
* `task_definition` - (Required) The family and revision (`family:revision`) or full ARN of the task definition that you want to run in your service.
* `desired_count` - (Required) The number of instances of the task definition to place and keep running
* `desired_count` - (Optional) The number of instances of the task definition to place and keep running. Defaults to 0. Do not specify if using the `DAEMON` scheduling strategy.
* `launch_type` - (Optional) The launch type on which to run your service. The valid values are `EC2` and `FARGATE`. Defaults to `EC2`.
* `scheduling_strategy` - (Optional) The scheduling strategy to use for the service. The valid values are `REPLICA` and `DAEMON`. Defaults to `REPLICA`. Note that [*Fargate tasks do not support the `DAEMON` scheduling strategy*](https://docs.aws.amazon.com/AmazonECS/latest/developerguide/scheduling_tasks.html).
* `cluster` - (Optional) ARN of an ECS cluster
* `iam_role` - (Optional) ARN of the IAM role that allows Amazon ECS to make calls to your load balancer on your behalf. This parameter is required if you are using a load balancer with your service, but only if your task definition does not use the `awsvpc` network mode. If using `awsvpc` network mode, do not specify this role. If your account has already created the Amazon ECS service-linked role, that role is used by default for your service unless you specify a role here.
* `deployment_maximum_percent` - (Optional) The upper limit (as a percentage of the service's desiredCount) of the number of running tasks that can be running in a service during a deployment.
* `deployment_maximum_percent` - (Optional) The upper limit (as a percentage of the service's desiredCount) of the number of running tasks that can be running in a service during a deployment. When using the `DAEMON` scheduling strategy, this should be set to 100.
* `deployment_minimum_healthy_percent` - (Optional) The lower limit (as a percentage of the service's desiredCount) of the number of running tasks that must remain running and healthy in a service during a deployment.
* `placement_strategy` - (Optional) **Deprecated**, use `ordered_placement_strategy` instead.
* `ordered_placement_strategy` - (Optional) Service level strategy rules that are taken into consideration during task placement. List from top to bottom in order of precedence. The maximum number of `ordered_placement_strategy` blocks is `5`. Defined below.
Expand Down