-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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.
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 Fargate Support #2483
ECS Fargate Support #2483
Changes from 3 commits
0e6f071
16679e7
8838cbf
352b34f
030e09e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,6 +27,12 @@ func resourceAwsEcsTaskDefinition() *schema.Resource { | |
Computed: true, | ||
}, | ||
|
||
"cpu": { | ||
Type: schema.TypeString, | ||
Optional: true, | ||
ForceNew: true, | ||
}, | ||
|
||
"family": { | ||
Type: schema.TypeString, | ||
Required: true, | ||
|
@@ -59,6 +65,18 @@ func resourceAwsEcsTaskDefinition() *schema.Resource { | |
ForceNew: true, | ||
}, | ||
|
||
"execution_role_arn": { | ||
Type: schema.TypeString, | ||
Optional: true, | ||
ForceNew: true, | ||
}, | ||
|
||
"memory": { | ||
Type: schema.TypeString, | ||
Optional: true, | ||
ForceNew: true, | ||
}, | ||
|
||
"network_mode": { | ||
Type: schema.TypeString, | ||
Optional: true, | ||
|
@@ -109,6 +127,13 @@ func resourceAwsEcsTaskDefinition() *schema.Resource { | |
}, | ||
}, | ||
}, | ||
|
||
"requires_compatibilities": { | ||
Type: schema.TypeSet, | ||
Optional: true, | ||
ForceNew: true, | ||
Elem: &schema.Schema{Type: schema.TypeString}, | ||
}, | ||
}, | ||
} | ||
} | ||
|
@@ -155,6 +180,18 @@ func resourceAwsEcsTaskDefinitionCreate(d *schema.ResourceData, meta interface{} | |
input.TaskRoleArn = aws.String(v.(string)) | ||
} | ||
|
||
if v, ok := d.GetOk("execution_role_arn"); ok { | ||
input.ExecutionRoleArn = aws.String(v.(string)) | ||
} | ||
|
||
if v, ok := d.GetOk("cpu"); ok { | ||
input.Cpu = aws.String(v.(string)) | ||
} | ||
|
||
if v, ok := d.GetOk("memory"); ok { | ||
input.Memory = aws.String(v.(string)) | ||
} | ||
|
||
if v, ok := d.GetOk("network_mode"); ok { | ||
input.NetworkMode = aws.String(v.(string)) | ||
} | ||
|
@@ -185,6 +222,10 @@ func resourceAwsEcsTaskDefinitionCreate(d *schema.ResourceData, meta interface{} | |
input.PlacementConstraints = pc | ||
} | ||
|
||
if v, ok := d.GetOk("requires_compatibilities"); ok && v.(*schema.Set).Len() > 0 { | ||
input.RequiresCompatibilities = expandStringList(v.(*schema.Set).List()) | ||
} | ||
|
||
log.Printf("[DEBUG] Registering ECS task definition: %s", input) | ||
out, err := conn.RegisterTaskDefinition(&input) | ||
if err != nil { | ||
|
@@ -231,12 +272,20 @@ func resourceAwsEcsTaskDefinitionRead(d *schema.ResourceData, meta interface{}) | |
} | ||
|
||
d.Set("task_role_arn", taskDefinition.TaskRoleArn) | ||
d.Set("execution_role_arn", taskDefinition.ExecutionRoleArn) | ||
d.Set("cpu", taskDefinition.Cpu) | ||
d.Set("memory", taskDefinition.Memory) | ||
d.Set("network_mode", taskDefinition.NetworkMode) | ||
d.Set("volumes", flattenEcsVolumes(taskDefinition.Volumes)) | ||
if err := d.Set("placement_constraints", flattenPlacementConstraints(taskDefinition.PlacementConstraints)); err != nil { | ||
log.Printf("[ERR] Error setting placement_constraints for (%s): %s", d.Id(), err) | ||
} | ||
|
||
d.Set("requires_compatibilities", taskDefinition.RequiresCompatibilities) | ||
if err := d.Set("requires_compatibilities", flattenStringList(taskDefinition.RequiresCompatibilities)); err != nil { | ||
log.Printf("[ERR] Error setting requires_compatibilities for (%s): %s", d.Id(), err) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there any particular reason why we should silently ignore this error? Also why we're setting the same field twice? I think the one above won't work as we're passing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No actual reason I just copied the code from the lines above. Will return the error. |
||
} | ||
|
||
return nil | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,6 +70,7 @@ Load balancers support the following: | |
* `target_group_arn` - (Required for ALB) The ARN of the ALB target group to associate with the service. | ||
* `container_name` - (Required) The name of the container to associate with the load balancer (as it appears in a container definition). | ||
* `container_port` - (Required) The port on the container to associate with the load balancer. | ||
* `launch_type` - (Optional) The launch type on which to run your service. The valid values are `EC2` and `FARGATE`. Defaults to `EC2`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While you write There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The AWS API default to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Not sure to get it here: Is Also, we already have default values in a lot of places, whether those are booleans, floats, etc, in order to mimic the AWS API correctly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is a typo, I meant |
||
|
||
## placement_strategy | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of our acceptance tests run in
us-west-2
by default and Fargate unfortunately isn't available there yet, so we'll need to pin this test tous-east-1
, otherwise it fails:We can do so by adding a
provider
block to the config here, e.g.we do that for a couple of other services already for the same reason, like Lightsail.