Aws-cdk: AWS ECS: Ec2Service too many resources

[peterdeme]

Problem

Current ECS design is close to unusable when one wants to re-use an ECS Cluster across multiple ECS services (across multiple git repos) because there is too much going on under the hood.

Please make the flow less strict so that the CDK can have greater ECS adoption.

Description

Hey Guys,

So a couple of months ago year ago I tried CDK but realized it wants to create too many unwanted resources (https://github.com/aws/aws-cdk/issues/2234). Today, I gave it another chance.

I tied to create an ECS Ec2Service but I'm failing at the point where I need to provide the cluster. So we (I guess other companies, too) have a single cluster where we host a lot of services. These services live in different Git repos.

As you can see in the official documentation, the cluster parameter of EcsService is a string where aws-cdk asks for a goddamn ICluster.

I guess it's needless to say how unconvenient this is when you only know the name of the cluster. Well, if I want to import my existing cluster with Cluster.fromClusterAttributes (ref), I must know it's VpcId and SecurityGroups which I really don't want to provide, it's too low-level information. (My company has a raw cloudformation template right now where we only provide the cluster name.) Also: why? In the official CF docs, nothing is required.

So I took a look at why Ec2Service needs a full-fledged ICluster. When looking at the constructor I sadly realized that once again the CDK wants to create all kinds of resources here and here. Needless to say that Troposphere only requires a string.

Now, I'm genuinely confused about the general philosophy of the CDK. We have ecs-patterns module and I thought that one is for providing really high-level constructs and if someone is a fan of creating unwanted resouces, it's for those people. Now, looks like we can't even create an ECS Ec2Service conveniently.

So, let me ask: what's the goal of CDK? It's for companies who don't want to control all their AWS resources and only care about higher level abstractions? It's supposed to serve every use case? By looking at the constructor of Ec2Service, I can't really decide. It feels like it dances around the edge of high and low level abstractions.

Recommendations

• In Cluster nothing should be required, maximum clusterName. Dumb down Cluster.fromClusterAttributes to only require a clusterName, this means vpc and securityGroups should be optional.
• In the constructor of Ec2Cluster, if theICluster does not have vpc and securityGroups defined, do not create additional resources like a new security group. I think this part of the code is too magical anyway.

edit.: I think I solved it with a nasty workaround.

import { CfnService } from '@aws-cdk/aws-ecs';

new CfnService(scope, "MobileService", {
    taskDefinition: taskDefinition.taskDefinitionArn,
    cluster: "clusterName-as-a-string"
});

This gets the job done but attaching a load balancer is not easy...

edit2: never mind guys, load balancer also require importing an existing Vpc instead of VpcId. I guess if I want to keep use CDK I'll need to use raw Cfn* sources. :(

[peterdeme]

FYI @SomayaB I got it figured out in a funky way, haha. It's a bit risky, because there's multiple fake values provided that are not used in the CF template (and I can just hope it stays that way).

Here it is:

const cluster = Cluster.fromClusterAttributes(scope, "Cluster", {
        clusterName: cfParameter(scope, ParameterName.ClusterName).toString(),
        securityGroups: [],
        vpc: {} as IVpc
    });

And the fact that this works perfectly makes my point even stronger:

• either securityGroups and vpc should be optional in Cluster.fromClusterAttributes
• or we need a new method called: Cluster.fromClusterName(scope, clusterName)

_(Disclaimer: cfParameter is a helper method that wrapps CfnParameter)_

I also figured out how to create a load balancer if you only know the VpcId and the subnets. This one also uses a couple of fake inputs which is a bit risky. :/

The whole thing:

import cdk = require('@aws-cdk/core');
import { TaskDefinition, Compatibility,  ContainerImage, Cluster, Ec2Service } from '@aws-cdk/aws-ecs';
import { GenericLogDriver } from "@aws-cdk/aws-ecs/lib/log-drivers/generic-log-driver"
import { Repository, IRepository } from '@aws-cdk/aws-ecr';
import { PolicyStatement, Effect } from '@aws-cdk/aws-iam';
import { cfParameter } from "../../parameters/parameters"
import { ParameterName } from '../../parameters/parametername';
import { ApplicationLoadBalancer, ApplicationProtocol, ApplicationTargetGroup, ApplicationListener, IApplicationLoadBalancer, TargetType } from '@aws-cdk/aws-elasticloadbalancingv2';
import { queue } from "./sqs"
import { Vpc, IVpc, SecurityGroup } from '@aws-cdk/aws-ec2';
import { Fn } from '@aws-cdk/core';

export function createEcsService(scope: cdk.Stack) {
    const svc = setupEcsService(scope);   

    setupLoadBalancer(scope, svc);
}

function setupEcsService(scope: cdk.Stack) {
    const taskDefinition = new TaskDefinition(scope, "mobile-taskdefinition", {
        compatibility: Compatibility.EC2
    });

    const role = taskDefinition.obtainExecutionRole();

    role.addToPolicy(new PolicyStatement({
        effect: Effect.ALLOW,
        actions: ["sqs:*"],
        resources: [queue.queueArn]
    }));

    const appVersion = cfParameter(scope, ParameterName.AppVersion);

    const container = taskDefinition.addContainer("MobileContainer", {
        image: ContainerImage.fromEcrRepository(getRepository(scope), appVersion.toString()),
        memoryReservationMiB: 512,
        environment: {
            "EnvironmentName": cfParameter(scope, ParameterName.EnvironmentName).toString()
        },
        logging: new GenericLogDriver({
            logDriver: 'sumologic',
            options: {
                "sumo-source-category": 'example-tag',
                "sumo-url": "https://my-sumo-url.com"
            }
        })
    });

    container.addPortMappings({ containerPort: 5000});

    const cluster = Cluster.fromClusterAttributes(scope, "Cluster", {
        clusterName: cfParameter(scope, ParameterName.ClusterName).toString(),
        securityGroups: [],
        vpc: {} as IVpc
    });

    const svc = new Ec2Service(scope, "Service", {
        cluster: cluster,
        taskDefinition: taskDefinition
    })

    return svc;
}

function setupLoadBalancer(scope: cdk.Stack, ecsService: Ec2Service) {    
    const vpc = Vpc.fromVpcAttributes(scope, "EnvVpc", {                
        vpcId: cfParameter(scope, ParameterName.VpcId).toString(),
        publicSubnetIds: [
            cfParameter(scope, ParameterName.EnvironmentName) + Fn.importValue("PublicASubnet"),
            cfParameter(scope, ParameterName.EnvironmentName) + Fn.importValue("PublicBSubnet")
        ],
        // The following values are mandatory to provide
        // but they won't be present in the CF template because the loadbalancer is public, not private
    // so don't worry about them
        availabilityZones: ['not_used1', 'not_used2'],        
        privateSubnetIds: ["not_used1", "not_used2"],
        isolatedSubnetIds: ["not_used1", "not_used2"],
        isolatedSubnetRouteTableIds: ["not_used1", "not_used2"],
        privateSubnetRouteTableIds: ["not_used1", "not_used2"],
        publicSubnetRouteTableIds: ["not_used1", "not_used2"]
    });

    const lb = new ApplicationLoadBalancer(scope, 'LB', {
        vpc: vpc,
        internetFacing: true,
        securityGroup: SecurityGroup.fromSecurityGroupId(scope, "SecurityGroup", "sg-434")
      });

    const targetGroup = new ApplicationTargetGroup(scope, "TargetGroup", {
        vpc: vpc,
        protocol: ApplicationProtocol.HTTPS,
        targetType: TargetType.INSTANCE     
    });

    new ApplicationListener(scope, "Listener", {
        loadBalancer: lb,
        protocol: ApplicationProtocol.HTTPS,
        certificateArns: ["certificateArn"],
        defaultTargetGroups: [targetGroup]
    });

    ecsService
        .loadBalancerTarget({
            containerName: "MobileContainer",
            containerPort: 5000
        })
        .attachToApplicationTargetGroup(targetGroup);
}

function getRepository(scope: cdk.Stack): IRepository {    
    return Repository.fromRepositoryAttributes(scope, "existingrepo", {
        "repositoryName": "mobilerepository",
        "repositoryArn": "arn:aws:ecr:eu-central-1:123456789012:repository/mobilerepository"
    });
}

[hencrice]

@peterdeme Really appreciate the detailed feedback and providing your workarounds! I will bring this back to the team to discuss.

[geof2001]

First off let me apologize if this sounds a bit ranty. I was looking for this error explicitly though regarding setting up ECS services with existing resources. I'm just not sure CDK is intended for shops that have already heavily invested in cloudformation. Importing and using existing resources seems to be as much or more code than is necessary. Especially with all the required ITypes. We export a lot of string values already using Cfn exports like sgs, vpcs, subnets, azs, lb's, log groups, clusters, roles and so on. All of these are just either resource id's or ARN's and I'm finding I have to practically reconstruct resources which were just referable directly in the property as a Fn::ImportValue. Complex workarounds to essentially refer a property to an IType instead of just a string is bulky to say the least.

I really want to use this tool I'm just finding it hard to justify the learning curve right now when everyone is already familiar with yaml and their pre-existing stack resources.

[peterdeme]

Yeah, absolutely, I have similar experience.

[SomayaB]

@geof2001 @peterdeme I'm so sorry that has been your experience. This issue is on our radar and we are currently gathering customer feedback so that we can improve the overall experience of migrating from Cloudformation to CDK. Thank you for sharing some of the pain points you've experienced and if you have any more feedback please let us know.