aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.5k stars 3.85k forks source link

[Feature] ecs.FargateService should accept array of subnets #1951

Closed robertd closed 5 years ago

robertd commented 5 years ago

It would be nice if we can pass existing subnets to a FargateServiceProps. Our SecOps prevents us from creating subnets ad-hoc, thus we cannot use ecs.FargateService construct. Thanks!

const service = new ecs.FargateService(this, "FargateService", {
    cluster: cluster,
    taskDefinition: fargateTaskDefinition,
    desiredCount: 1,
    securityGroup: sg
});
rix0rrr commented 5 years ago

You can import an existing VPC network (see ec2.VpcNetwork.importFromContext).

The Fargate Service will use the private subnets from the imported VPC network. Should you need the public subnets, you pass the vpcPlacement property.

allankp commented 5 years ago

Need a bit of guidance on a related issue. Subnets for my fargate service are never populated @rix0rrr @robertd

  networkConfiguration:
   { awsvpcConfiguration:
      { assignPublicIp: 'DISABLED',
        subnets: [],
        securityGroups: [Token] } } }
      const vpc = ec2.VpcNetwork.import(this, 'vpc', {
            vpcId: 'vpc-xxxxx',
            availabilityZones: ['eu-west-1a', 'eu-west-1b'],
            privateSubnetIds: ['subnet-xxxxx', 'subnet-xxxx'],
        });

        const cluster = ecs.Cluster.import(this, 'Cluster', {
            clusterName: "cluster_name",
            vpc: vpc,
            securityGroups: [{securityGroupId: 'securityGroupId'}]
        })

        const fargateService = new ecs.FargateService(this, 'FargateService', {
            cluster: cluser,
            taskDefinition: taskname
            desiredCount: 2,
        });
rix0rrr commented 5 years ago

@allankp Cluster.import does not take an IVpcNetwork, it takes VpcNetworkImportProps. Correct code:

 const cluster = ecs.Cluster.import(this, 'Cluster', {
            clusterName: "cluster_name",
            vpc: {
              vpcId: 'vpc-xxxxx',
              availabilityZones: ['eu-west-1a', 'eu-west-1b'],
              privateSubnetIds: ['subnet-xxxxx', 'subnet-xxxx'],
            },
            securityGroups: [{securityGroupId: 'securityGroupId'}]
        })
allankp commented 5 years ago

@rix0rrr Rookie mistake, much appreciated

robertd commented 5 years ago

@rix0rrr Thank you for clarifying that. Originally, I've used imported vpc (via importFromContext) and passed it down to cluster. I was surprised that cluster/VSCode didn't yell at me (see screenshot attached).

image

But anyway, after your last reply to @allankp I've changed things around

    //Import existing Security Group
     const sg = ec2.SecurityGroup.import(this, "SecurityGroup", {
      securityGroupId: props.securityGroupId
    });

    const cluster = ecs.Cluster.import(this, "Cluster", {
      clusterName: props.clusterName,
      securityGroups: [ sg ],
      vpc: {
        vpcId: props.vpcId,
          availabilityZones: ['us-west-2'],
          privateSubnetIds: subnets       
      }
    });

... and my CFN looks like this

...
  ServiceD69D759B:
    Type: AWS::ECS::Service
    Properties:
      TaskDefinition:
        Ref: TaskDefinitionB36D86D9
      Cluster: DlvAPPEcsClustersStack-AppF1B96344-5WK03WOT8LSV
      DeploymentConfiguration:
        MaximumPercent: 200
        MinimumHealthyPercent: 50
      DesiredCount: 1
      LaunchType: FARGATE
      LoadBalancers: []
      NetworkConfiguration:
        AwsvpcConfiguration:
          AssignPublicIp: DISABLED
          SecurityGroups:
            - sg-xxxxxxx
          Subnets:
            - subnet-xxxxxxx
            - subnet-xxxxxxx
            - subnet-xxxxxxx
...

TY!

robertd commented 5 years ago

@rix0rrr Although, is this statement still correct?

You can import an existing VPC network (see ec2.VpcNetwork.importFromContext).

The Fargate Service will use the private subnets from the imported VPC network. Should you need the public subnets, you pass the vpcPlacement property.

Because if I do it that way:

    //Import existing VPC
    const vpc = ec2.VpcNetwork.importFromContext(this, "VPC", {
      vpcId: props.vpcId    
    });

    //Import existing Security Group
     const sg = ec2.SecurityGroup.import(this, "SecurityGroup", {
      securityGroupId: props.securityGroupId
    });

    const cluster = ecs.Cluster.import(this, "Cluster", {
      clusterName: props.clusterName,
      securityGroups: [ sg ],
      vpc: vpc
    });

My CFN is still missing subnets

  ServiceD69D759B:
    Type: AWS::ECS::Service
    Properties:
      TaskDefinition:
        Ref: TaskDefinitionB36D86D9
      Cluster: DlvAPPEcsClustersStack-AppF1B96344-5WK03WOT8LSV
      DeploymentConfiguration:
        MaximumPercent: 200
        MinimumHealthyPercent: 50
      DesiredCount: 1
      LaunchType: FARGATE
      LoadBalancers: []
      NetworkConfiguration:
        AwsvpcConfiguration:
          AssignPublicIp: DISABLED
          SecurityGroups:
            - sg-xxxxxxxx
          Subnets: []

Just wanted to make sure the info is correct for the folks who stumble upon this issue.

allankp commented 5 years ago

Do you need to define a vpcplacement group on the fargateservice with the subnet type:

vpcPlacement: {subnetsToUse: SubnetType.Private}

FYI that does not work for me, subnets are still empty on the fargateservice subnets.

robertd commented 5 years ago

@allankp Same here. The only way I was able to populate subnets was using this in cluster construct.

            vpc: {
              vpcId: 'vpc-xxxxx',
              availabilityZones: ['eu-west-1a', 'eu-west-1b'],
              privateSubnetIds: ['subnet-xxxxx', 'subnet-xxxx'],
            }