Merge pull request #11298 from hashicorp/f-update-ecs-definitions-constraints

provider/aws: Update ECS task_definition and service

builtin/providers/aws/resource_aws_ecs_service.go

@@ -117,11 +117,12 @@ func resourceAwsEcsService() *schema.Resource {
 						"field": {
 							Type:     schema.TypeString,
 							ForceNew: true,
-							Required: true,
+							Optional: true,
 						},
 					},
 				},
 			},
+
 			"placement_constraints": {
 				Type:     schema.TypeSet,
 				Optional: true,
@@ -129,7 +130,7 @@ func resourceAwsEcsService() *schema.Resource {
 				MaxItems: 10,
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
-						"type": { //TODO: Add a Validation for the types
+						"type": {
 							Type:     schema.TypeString,
 							ForceNew: true,
 							Required: true,
@@ -137,7 +138,7 @@ func resourceAwsEcsService() *schema.Resource {
 						"expression": {
 							Type:     schema.TypeString,
 							ForceNew: true,
-							Required: true,
+							Optional: true,
 						},
 					},
 				},
@@ -178,6 +179,11 @@ func resourceAwsEcsServiceCreate(d *schema.ResourceData, meta interface{}) error
 		var ps []*ecs.PlacementStrategy
 		for _, raw := range strategies {
 			p := raw.(map[string]interface{})
+			t := p["type"].(string)
+			f := p["field"].(string)
+			if err := validateAwsEcsPlacementStrategy(t, f); err != nil {
+				return err
+			}
 			ps = append(ps, &ecs.PlacementStrategy{
 				Type:  aws.String(p["type"].(string)),
 				Field: aws.String(p["field"].(string)),
@@ -191,9 +197,14 @@ func resourceAwsEcsServiceCreate(d *schema.ResourceData, meta interface{}) error
 		var pc []*ecs.PlacementConstraint
 		for _, raw := range constraints {
 			p := raw.(map[string]interface{})
+			t := p["type"].(string)
+			e := p["expression"].(string)
+			if err := validateAwsEcsPlacementConstraint(t, e); err != nil {
+				return err
+			}
 			pc = append(pc, &ecs.PlacementConstraint{
-				Type:       aws.String(p["type"].(string)),
-				Expression: aws.String(p["expression"].(string)),
+				Type:       aws.String(t),
+				Expression: aws.String(e),
 			})
 		}
 		input.PlacementConstraints = pc

builtin/providers/aws/resource_aws_ecs_task_definition.go

@@ -96,7 +96,7 @@ func resourceAwsEcsTaskDefinition() *schema.Resource {
 						"expression": {
 							Type:     schema.TypeString,
 							ForceNew: true,
-							Required: true,
+							Optional: true,
 						},
 					},
 				},
@@ -154,9 +154,14 @@ func resourceAwsEcsTaskDefinitionCreate(d *schema.ResourceData, meta interface{}
 		var pc []*ecs.TaskDefinitionPlacementConstraint
 		for _, raw := range constraints {
 			p := raw.(map[string]interface{})
+			t := p["type"].(string)
+			e := p["expression"].(string)
+			if err := validateAwsEcsPlacementConstraint(t, e); err != nil {
+				return err
+			}
 			pc = append(pc, &ecs.TaskDefinitionPlacementConstraint{
-				Type:       aws.String(p["type"].(string)),
-				Expression: aws.String(p["expression"].(string)),
+				Type:       aws.String(t),
+				Expression: aws.String(e),
 			})
 		}
 		input.PlacementConstraints = pc

builtin/providers/aws/validators.go

@@ -686,3 +686,44 @@ func validateRoute53RecordType(v interface{}, k string) (ws []string, errors []e
 	}
 	return
 }
+
+// Validates that ECS Placement Constraints are set correctly
+// Takes type, and expression as strings
+func validateAwsEcsPlacementConstraint(constType, constExpr string) error {
+	switch constType {
+	case "distinctInstance":
+		// Expression can be nil for distinctInstance
+		return nil
+	case "memberOf":
+		if constExpr == "" {
+			return fmt.Errorf("Expression cannot be nil for 'memberOf' type")
+		}
+	default:
+		return fmt.Errorf("Unknown type provided: %q", constType)
+	}
+	return nil
+}
+
+// Validates that an Ecs placement strategy is set correctly
+// Takes type, and field as strings
+func validateAwsEcsPlacementStrategy(stratType, stratField string) error {
+	switch stratType {
+	case "random":
+		// random does not need the field attribute set, could error, but it isn't read at the API level
+		return nil
+	case "spread":
+		//  For the spread placement strategy, valid values are instanceId
+		// (or host, which has the same effect), or any platform or custom attribute
+		// that is applied to a container instance
+		// stratField is already cased to a string
+		return nil
+	case "binpack":
+		if stratField != "cpu" && stratField != "memory" {
+			return fmt.Errorf("Binpack type requires the field attribute to be either 'cpu' or 'memory'. Got: %s",
+				stratField)
+		}
+	default:
+		return fmt.Errorf("Unknown type %s. Must be one of 'random', 'spread', or 'binpack'.", stratType)
+	}
+	return nil
+}

builtin/providers/aws/validators_test.go

@@ -1086,3 +1086,87 @@ func TestValidateRoute53RecordType(t *testing.T) {
 		}
 	}
 }
+
+func TestValidateEcsPlacementConstraint(t *testing.T) {
+	cases := []struct {
+		constType string
+		constExpr string
+		Err       bool
+	}{
+		{
+			constType: "distinctInstance",
+			constExpr: "",
+			Err:       false,
+		},
+		{
+			constType: "memberOf",
+			constExpr: "",
+			Err:       true,
+		},
+		{
+			constType: "distinctInstance",
+			constExpr: "expression",
+			Err:       false,
+		},
+		{
+			constType: "memberOf",
+			constExpr: "expression",
+			Err:       false,
+		},
+	}
+
+	for _, tc := range cases {
+		if err := validateAwsEcsPlacementConstraint(tc.constType, tc.constExpr); err != nil && !tc.Err {
+			t.Fatalf("Unexpected validation error for \"%s:%s\": %s",
+				tc.constType, tc.constExpr, err)
+		}
+
+	}
+}
+
+func TestValidateEcsPlacementStrategy(t *testing.T) {
+	cases := []struct {
+		stratType  string
+		stratField string
+		Err        bool
+	}{
+		{
+			stratType:  "random",
+			stratField: "",
+			Err:        false,
+		},
+		{
+			stratType:  "spread",
+			stratField: "instanceID",
+			Err:        false,
+		},
+		{
+			stratType:  "binpack",
+			stratField: "cpu",
+			Err:        false,
+		},
+		{
+			stratType:  "binpack",
+			stratField: "memory",
+			Err:        false,
+		},
+		{
+			stratType:  "binpack",
+			stratField: "disk",
+			Err:        true,
+		},
+		{
+			stratType:  "fakeType",
+			stratField: "",
+			Err:        true,
+		},
+	}
+
+	for _, tc := range cases {
+		if err := validateAwsEcsPlacementStrategy(tc.stratType, tc.stratField); err != nil && !tc.Err {
+			t.Fatalf("Unexpected validation error for \"%s:%s\": %s",
+				tc.stratType, tc.stratField, err)
+		}
+
+	}
+}

website/source/docs/providers/aws/r/ecs_service.html.markdown

@@ -75,11 +75,13 @@ Load balancers support the following:
 
 `placement_constraints` support the following:
 
-* `expression` -  Cluster Query Language expression to apply to the constraint.
+* `type` - (Required) The type of constraint. The only valid values at this time are `memberOf` and `distinctInstance`.
+* `expression` -  (Optional) Cluster Query Language expression to apply to the constraint. Does not need to be specified
+for the `distinctInstance` type.
 For more information, see [Cluster Query Language in the Amazon EC2 Container
 Service Developer
 Guide](http://docs.aws.amazon.com/AmazonECS/latest/developerguide/cluster-query-language.html).
-* `type` - The type of constraint. The only valid values at this time are `memberOf` and `distinctInstance`.
+
 
 
 ## Attributes Reference

website/source/docs/providers/aws/r/ecs_task_definition.html.markdown

@@ -81,6 +81,8 @@ official [Developer Guide](https://docs.aws.amazon.com/AmazonECS/latest/develope
 * `volume` - (Optional) A volume block. See below for details about what arguments are supported.
 * `placement_constraints` - (Optional) rules that are taken into consideration during task placement. Maximum number of 
 `placement_constraints` is `10`. Defined below.
+* `placement_strategy` - (Optional) an algorithm for selecting instances for task placement or tasks for termination. Maximum
+number of `placement_strategy` blocks is `5`. Defined below.
 
 Volume block supports the following arguments:
 
@@ -92,11 +94,24 @@ parameter of container definition in the `mountPoints` section.
 
 `placement_constraints` support the following:
 
-* `expression` -  Cluster Query Language expression to apply to the constraint.
+* `type` - (Required) The type of constraint. The only valid values at this time are `memberOf` or `distinctInstance`.
+* `expression` -  (Optional) Cluster Query Language expression to apply to the constraint.  Does not need to be specified
+for the `distinctInstance` type.
 For more information, see [Cluster Query Language in the Amazon EC2 Container
 Service Developer
 Guide](http://docs.aws.amazon.com/AmazonECS/latest/developerguide/cluster-query-language.html).
-* `type` - The type of constraint. The only valid value at this time is `memberOf`
+
+## placement_strategy
+
+`placement_strategy` supports the following:
+
+* `type` - (Required) The type of placement strategy. Must be one of: `binpack`, `random`, or `spread`
+* `field` - (Optional) For the `spread` placement strategy, valid values are instanceId (or host,
+ which has the same effect), or any platform or custom attribute that is applied to a container instance.
+ 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).
+
+
 
 ## Attributes Reference