feat(compose): Add EFS volume support to ECS Params. #1019
Conversation
|
Manually deployed container with an EFS volume specified using the following files. ecs-params.ymlversion: 1
task_definition:
task_execution_role: ecsTaskExecutionRole
ecs_network_mode: awsvpc
task_size:
mem_limit: 1.0GB
cpu_limit: 512
efs_volumes:
- name: "myEFSVolume"
filesystem_id: "fs-fedc8554"
root_directory: /
transit_encryption: DISABLED
iam: DISABLED
run_params:
network_configuration:
awsvpc_configuration:
subnets:
- "subnet-0b24acd73f534bb4f"
- "subnet-0f0e20022e2cccd67"
security_groups:
- "sg-0fb24ebc7dd5254b0"
assign_public_ip: "ENABLED"docker-compose.ymlversion: '3'
services:
web:
image: 898620229039.dkr.ecr.us-west-2.amazonaws.com/otter:latest
volumes:
- myEFSVolume:/mount/efs
ports:
- "80:80"
logging:
driver: awslogs
options:
awslogs-group: otter
awslogs-region: us-west-2
awslogs-stream-prefix: web
volumes:
myEFSVolume:Deployment./ecs-cli compose service up --aws-profile system-test --cluster-config -fargate-tutorial-config --create-log-groups --launch-type FARGATE --force-deployment
INFO[0000] Using ECS task definition TaskDefinition="oaas:12"
WARN[0000] Failed to create log group otter in us-west-2: The specified log group already exists
INFO[0017] (service oaas) has started 1 tasks: (task 7f6bbbf9-b004-4c93-8c9e-c8be663a8e57). timestamp="2020-05-08 21:02:52 +0000 UTC"
INFO[0062] Service status desiredCount=1 runningCount=1 serviceName=oaas
INFO[0062] ECS Service has reached a stable state desiredCount=1 runningCount=1 serviceName=oaas
INFO[0062] Created an ECS service service=oaas taskDefinition="oaas:12"Task definition (Excerpted for EFS) "volumes": [
{
"efsVolumeConfiguration": {
"transitEncryptionPort": null,
"fileSystemId": "fs-fedc8554",
"authorizationConfig": {
"iam": "DISABLED",
"accessPointId": null
},
"transitEncryption": "DISABLED",
"rootDirectory": "/"
},
"name": "myEFSVolume",
"host": null,
"dockerVolumeConfiguration": null
}
] |
|
+1 I am very excited to see this land. |
|
for those of us in the cheap seats, how do we verify that we are on 1.4.*? |
| for _, efsVol := range ecsParams.TaskDefinition.EFSVolumes { | ||
| if efsVol.Name != "" { | ||
| volumesWithoutHost[efsVol.Name] = Volume{EFSVolumeConfig: efsVol} | ||
| } |
There was a problem hiding this comment.
should we return an error similar to that on line 282 if no name is specified?
| @@ -1405,6 +1406,61 @@ func TestConvertToTaskDefinitionWithECSParamsVolumeWithoutNameError(t *testing.T | |||
| assert.Error(t, err, "Expected error converting Task Definition with ECS Params volume without name") | |||
| } | |||
|
|
|||
| func TestConvertToTaskDefinitionWithEFSVolume(t *testing.T) { | |||
There was a problem hiding this comment.
can we add tests for the error cases as well? (no system id, encryption required but missing)
There was a problem hiding this comment.
+1 to @SoManyHs and @allisaurus 's comments. Also I am not sure if we should do those validation for EFS configuration, since they might change in the future and we'll have to sync with them.
| if efsVol.Name != "" { | ||
| volumesWithoutHost[efsVol.Name] = Volume{EFSVolumeConfig: efsVol} | ||
| } else { | ||
| return nil, fmt.Errorf("Name is required when specifying an docker volume") |
There was a problem hiding this comment.
c/p insidiousness: should be "...when specifying an EFS volume"
| platformVersion := aws.String("LATEST") | ||
| if len(ecsParams.TaskDefinition.EFSVolumes) != 0 { | ||
| log.Warnf("Detected an EFS Volume in task definition %s", taskDefName) | ||
| log.Warn("EFS requires Fargate platform version 1.4.0, which includes changes to the networking flows for VPC endpoint customers.") |
There was a problem hiding this comment.
not clear from this statement alone that we're using 1.4.0 now, just that it's required. Since it comes right after the msg re: EFS volume detected, does it make sense in context to say "Using required Fargate platform version 1.4.0, which..." ?
README.md
Outdated
| @@ -502,6 +502,18 @@ task_definition: | |||
| string: string | |||
| labels: | |||
| string: string | |||
| efs_volumes: | |||
| - name: string // Required | |||
There was a problem hiding this comment.
since this is a top-level ecs-params.yml example, including REQUIRED doesn't seem right (since the parent obj is itself optional). Suggest only commenting on fields that have finite valid values (example: line 498)
|
any ETA for this feature release? |
Adds new struct,
EFSVolume, toecs_params.yaml. This allows users to link existing EFS volumes in the VPC to new Fargate or EC2 tasks via docker mount points, following the Docker volume configuration pattern.NOTE: Does not add the ability to create EFS volumes through the CLI, just link new services to existing EFS volumes in a VPC.
This PR hardcodes the Fargate platform version to 1.4.0. When "LATEST" refers to 1.4.0 instead of 1.3.0, we can change it back.
Addresses #1009.
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Documentation
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.