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.

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

feat(ecs): add EfsVolumeConfiguration to Volume #8467

Merged
merged 6 commits into from Aug 18, 2020

Conversation

simon-castano
Copy link
Contributor

The PR is adding EfsVolumeConfiguration options to TaskDefinition as described on https://docs.aws.amazon.com/AmazonECS/latest/developerguide/efs-volumes.html#specify-efs-config

Full documentation is missing from CloudFormation documentation and is therefore not referenced.
While the unit test is passing, I haven't add the possibility to test this feature in practice.

fixes #6918
closes #8448


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@SomayaB SomayaB added the @aws-cdk/aws-ecs Related to Amazon Elastic Container label Jun 11, 2020
@farzad-xgen
Copy link

Hi,

When this will be merged? We really need this!

@Krobar
Copy link

Krobar commented Jul 21, 2020

We also need this. Any idea why it is so delayed?

@k3nnyP
Copy link

k3nnyP commented Jul 27, 2020

This sure would be nice. Resorting to ECS EC2 tasks until this is released.

@timpur
Copy link

timpur commented Aug 4, 2020

Is this ready, since https://aws.amazon.com/about-aws/whats-new/2020/08/amazon-ecs-announces-cloudformation-support-for-amazon-efs-volumes/ ?

@bvtujo
Copy link
Contributor

bvtujo commented Aug 10, 2020

Hi @simon-castano thank you so much for the contribution. Is there any chance we could add support for Fargate task definitions to this PR as well? EFS is supported for ECS in Cloudformation as of 8/3 so we should be able to use both EC2 and Fargate launch types.

@simon-castano
Copy link
Contributor Author

Hi @bvtujo,
afaik, volumes for FargateTaskDefinition are defined in FargateTaskDefinitionProps.
The later being an extension of CommonTaskDefinitionProps which already refers to Volume including EfsVolumeConfiguration, EFS volume support for FargateTaskDefinition shall be enabled with this PR.

Copy link
Contributor

@bvtujo bvtujo left a comment

Choose a reason for hiding this comment

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

Ok, with my comment addressed I'm almost ready to merge this! @simon-castano Do you have any manual test output from deploying this construct?

@simon-castano
Copy link
Contributor Author

@bvtujo please see here after manual test configuration and output, application is deploying successfully when invoked.

cdk application

#!/usr/bin/env python3

from aws_cdk import aws_ec2, aws_ecs, aws_efs, core


class EfsTestStack(core.Stack):
    def __init__(self, scope: core.Construct, id: str, **kwargs) -> None:
        super().__init__(scope, id, **kwargs)

        efs = aws_efs.FileSystem(
            self,
            "file-system",
            vpc=aws_ec2.Vpc.from_lookup(self, "vpc", vpc_id="vpc-12345678"),
            file_system_name="efs",
        )

        task_definition = aws_ecs.FargateTaskDefinition(
            self,
            "task-definition",
            volumes=[
                aws_ecs.Volume(
                    name="efs-volume",
                    efs_volume_configuration=aws_ecs.EfsVolumeConfiguration(
                        file_system_id=efs.file_system_id
                    ),
                )
            ],
        )
        task_definition.add_container(
            "hello-world",
            image=aws_ecs.ContainerImage.from_registry(name="hello-world"),
        )


app = core.App()
EfsTestStack(app, "fargate-efs")

app.synth()

synth output

Resources:
  filesystemECC0ED22:
    Type: AWS::EFS::FileSystem
    Properties:
      FileSystemTags:
        - Key: Name
          Value: efs
    UpdateReplacePolicy: Retain
    DeletionPolicy: Retain
    Metadata:
      aws:cdk:path: fargate-efs/file-system/Resource
  filesystemEfsSecurityGroup0505816F:
    Type: AWS::EC2::SecurityGroup
    Properties:
      GroupDescription: fargate-efs/file-system/EfsSecurityGroup
      SecurityGroupEgress:
        - CidrIp: 0.0.0.0/0
          Description: Allow all outbound traffic by default
          IpProtocol: "-1"
      Tags:
        - Key: Name
          Value: efs
      VpcId: vpc-12345678
    Metadata:
      aws:cdk:path: fargate-efs/file-system/EfsSecurityGroup/Resource
  filesystemEfsMountTarget1350424EA:
    Type: AWS::EFS::MountTarget
    Properties:
      FileSystemId:
        Ref: filesystemECC0ED22
      SecurityGroups:
        - Fn::GetAtt:
            - filesystemEfsSecurityGroup0505816F
            - GroupId
      SubnetId: subnet-87654321
    Metadata:
      aws:cdk:path: fargate-efs/file-system/EfsMountTarget1
  filesystemEfsMountTarget2782924A8:
    Type: AWS::EFS::MountTarget
    Properties:
      FileSystemId:
        Ref: filesystemECC0ED22
      SecurityGroups:
        - Fn::GetAtt:
            - filesystemEfsSecurityGroup0505816F
            - GroupId
      SubnetId: subnet-FEDCBA09
    Metadata:
      aws:cdk:path: fargate-efs/file-system/EfsMountTarget2
  taskdefinitionTaskRoleAB145429:
    Type: AWS::IAM::Role
    Properties:
      AssumeRolePolicyDocument:
        Statement:
          - Action: sts:AssumeRole
            Effect: Allow
            Principal:
              Service: ecs-tasks.amazonaws.com
        Version: "2012-10-17"
    Metadata:
      aws:cdk:path: fargate-efs/task-definition/TaskRole/Resource
  taskdefinitionBBCB03E6:
    Type: AWS::ECS::TaskDefinition
    Properties:
      ContainerDefinitions:
        - Essential: true
          Image: hello-world
          Name: hello-world
      Cpu: "256"
      Family: fargateefstaskdefinitionB82E84E0
      Memory: "512"
      NetworkMode: awsvpc
      RequiresCompatibilities:
        - FARGATE
      TaskRoleArn:
        Fn::GetAtt:
          - taskdefinitionTaskRoleAB145429
          - Arn
      Volumes:
        - Name: efs-volume
          EfsVolumeConfiguration:
            FileSystemId:
              Ref: filesystemECC0ED22
    Metadata:
      aws:cdk:path: fargate-efs/task-definition/Resource
  CDKMetadata:
    Type: AWS::CDK::Metadata
    Properties:
      Modules: aws-cdk=1.59.0,@aws-cdk/assets=1.59.0,@aws-cdk/aws-apigateway=1.59.0,@aws-cdk/aws-applicationautoscaling=1.59.0,@aws-cdk/aws-autoscaling=1.59.0,@aws-cdk/aws-autoscaling-common=1.59.0,@aws-cdk/aws-autoscaling-hooktargets=1.59.0,@aws-cdk/aws-certificatemanager=1.59.0,@aws-cdk/aws-cloudformation=1.59.0,@aws-cdk/aws-cloudfront=1.59.0,@aws-cdk/aws-cloudwatch=1.59.0,@aws-cdk/aws-codeguruprofiler=1.59.0,@aws-cdk/aws-cognito=1.59.0,@aws-cdk/aws-ec2=1.59.0,@aws-cdk/aws-ecr=1.59.0,@aws-cdk/aws-ecr-assets=1.59.0,@aws-cdk/aws-ecs=1.60.0-1,@aws-cdk/aws-efs=1.59.0,@aws-cdk/aws-elasticloadbalancing=1.59.0,@aws-cdk/aws-elasticloadbalancingv2=1.59.0,@aws-cdk/aws-events=1.59.0,@aws-cdk/aws-iam=1.59.0,@aws-cdk/aws-kms=1.59.0,@aws-cdk/aws-lambda=1.59.0,@aws-cdk/aws-logs=1.59.0,@aws-cdk/aws-route53=1.59.0,@aws-cdk/aws-route53-targets=1.59.0,@aws-cdk/aws-s3=1.59.0,@aws-cdk/aws-s3-assets=1.59.0,@aws-cdk/aws-sam=1.59.0,@aws-cdk/aws-secretsmanager=1.59.0,@aws-cdk/aws-servicediscovery=1.59.0,@aws-cdk/aws-sns=1.59.0,@aws-cdk/aws-sns-subscriptions=1.59.0,@aws-cdk/aws-sqs=1.59.0,@aws-cdk/aws-ssm=1.59.0,@aws-cdk/cloud-assembly-schema=1.59.0,@aws-cdk/core=1.59.0,@aws-cdk/custom-resources=1.59.0,@aws-cdk/cx-api=1.59.0,@aws-cdk/region-info=1.59.0,jsii-runtime=Python/3.8.2

bvtujo
bvtujo previously approved these changes Aug 18, 2020
Copy link
Contributor

@bvtujo bvtujo left a comment

Choose a reason for hiding this comment

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

Thanks so much for the test output. I'm satisfied. And thank you so much for contributing!

@mergify mergify bot dismissed bvtujo’s stale review August 18, 2020 16:35

Pull request has been modified.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
  • Commit ID: c4fb1d7
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Copy link
Contributor

@bvtujo bvtujo left a comment

Choose a reason for hiding this comment

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

This CI build failed due to a rate limit from Dockerhub when building the aws-s3-assets package. We may have to try merging from master again several times today.

@skinny85
Copy link
Contributor

This CI build failed due to a rate limit from Dockerhub when building the aws-s3-assets package. We may have to try merging from master again several times today.

@bvtujo restarted the build for you.

@mergify
Copy link
Contributor

mergify bot commented Aug 18, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit 85ff9fd into aws:master Aug 18, 2020
misterjoshua pushed a commit to misterjoshua/aws-cdk that referenced this pull request Aug 19, 2020
The PR is adding EfsVolumeConfiguration options to TaskDefinition as described on https://docs.aws.amazon.com/AmazonECS/latest/developerguide/efs-volumes.html#specify-efs-config

Full documentation is missing from CloudFormation documentation and is therefore not referenced.
While the unit test is passing, I haven't add the possibility to test this feature in practice.

fixes aws#6918
closes aws#8448

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@wsill
Copy link

wsill commented Aug 21, 2020

Does anyone know how this works. I tried adding the following addVolume to my Taskdefinition but I get the error -> FargateService/Service (FargateServiceAC2B3B85) One or more of the requested capabilities are not supported. (Service: AmazonECS; Status Code: 400; Error Code: PlatformTaskDefinitionIncompatibilityException; Request ID: 50096aa9-77ca-454a-bdf4-82896701554f; Proxy: null)

Here is the code I'm using:

const appTask = new ecs.TaskDefinition(this, 'TaskDefinition', {
family: ${this.stackName}-Service,
cpu: '1024',
memoryMiB: '2048',
compatibility: ecs.Compatibility.FARGATE,
});
appTask.addVolume({
name: 'efs',
efsVolumeConfiguration: {
fileSystemId: fileSystem.fileSystemId,
transitEncryption: 'ENABLED',
rootDirectory: /${props.namespace},
},
});

@bvtujo
Copy link
Contributor

bvtujo commented Aug 21, 2020

Hi @wsill, is your Fargate service using the LATEST platform version, or 1.4.0? EFS support was enabled in 1.4.0 but LATEST still refers to 1.3.0. You can read more about platform versions in our docs.

@wsill
Copy link

wsill commented Aug 21, 2020

Thanks @bvtujo. I added the latest (I assumed we were using the latest since I didn't specify) and it seems to be working. This is what I ended up using for those interested:

const appService = new ecs.FargateService(this, 'FargateService', {
  taskDefinition: appTask,
  cluster,
  vpcSubnets: {subnetType: SubnetType.PRIVATE},
  desiredCount: 1,
  securityGroup: appSecurityGroup,
  platformVersion: FargatePlatformVersion.VERSION1_4,
}); 

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add EfsVolumeConfiguration to ecs.Volume efsVolumeConfiguration is unsupported in ecs.CfnTaskDefinition