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

[aws-ecs-patterns] Cannot Create ApplicationLoadBalancedEc2Service Separate From Cluster #4267

Closed
jd-carroll opened this issue Sep 27, 2019 · 4 comments
Labels
@aws-cdk/aws-ecs-patterns Related to ecs-patterns library bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/medium Medium work item – several days of effort p2

Comments

@jd-carroll
Copy link
Contributor

jd-carroll commented Sep 27, 2019

I am trying to define a generic Cluster stack which would be able to accept any number of service tasks. However, when using two different stacks to define the Cluster and Service, CDK fails with a circular reference error.

Note: This is using the ApplicationLoadBalancedEc2Service from @aws-cdk/aws-ecs-patterns.

Reproduction Steps

I would expect the following to deploy successfully:

const app = new cdk.App();
// ---- Create the cluster stack with all the compute availability
const clusterStack = new cdk.Stack(app, 'aws-ecs-cluster-stack');

const vpc = new ec2.Vpc(clusterStack, 'Vpc', { maxAzs: 2 });

const cluster = new ecs.Cluster(clusterStack, 'EcsCluster', { vpc });
cluster.addCapacity('DefaultAutoScalingGroup', {
  instanceType: ec2.InstanceType.of(ec2.InstanceClass.T2, ec2.InstanceSize.MICRO)
});

// ---- Create a service stack defining some task
const serviceStack = new cdk.Stack(app, 'aws-ecs-service-stack');

new ApplicationLoadBalancedEc2Service(serviceStack, 'InboundSmtpAppService', {
  cluster,
  memoryLimitMiB: 512,
  image: ecs.ContainerImage.fromRegistry("amazon/amazon-ecs-sample")
});

app.synth();

Error Log

Fails with: (original bug report contained wrong stack trace)

Error: 'aws-ecs-service-stack' depends on 'aws-ecs-cluster-stack' (aws-ecs-service-stack/LB/Resource -> aws-ecs-cluster-stack/Vpc/PrivateSubnet1/Subnet.Ref). Adding this dependency (aws-ecs-cluster-stack/EcsCluster/DefaultAutoScalingGroup/InstanceSecurityGroup/from awsecsservicestackLBSecurityGroup45FF69AB:32768-65535 -> aws-ecs-service-stack/LB/SecurityGroup/Resource.GroupId) would create a cyclic reference.
    at Stack.addDependency (.../ecs-service-with-advanced-alb-config/node_modules/@aws-cdk/core/lib/stack.ts:283:15)
    at CfnReference.consumeFromStack (.../ecs-service-with-advanced-alb-config/node_modules/@aws-cdk/core/lib/private/cfn-reference.ts:131:22)
    at Stack.prepare (.../ecs-service-with-advanced-alb-config/node_modules/@aws-cdk/core/lib/stack.ts:501:23)
    at Function.prepare (.../ecs-service-with-advanced-alb-config/node_modules/@aws-cdk/core/lib/construct.ts:84:28)
    at Function.synth (.../ecs-service-with-advanced-alb-config/node_modules/@aws-cdk/core/lib/construct.ts:41:10)
    at App.synth (.../ecs-service-with-advanced-alb-config/node_modules/@aws-cdk/core/lib/app.ts:128:36)
    at Object.<anonymous> (.../ecs-service-with-advanced-alb-config/index.ts:106:5)
    at Module._compile (internal/modules/cjs/loader.js:778:30)
    at Module.m._compile (.../ecs-service-with-advanced-alb-config/node_modules/ts-node/src/index.ts:493:23)
    at Module._extensions..js (internal/modules/cjs/loader.js:789:10)

Environment

  • CLI Version : 1.9.0
  • Framework Version: 1.9.0
  • OS : macOS 10.14.6
  • Language : all

This is 🐛 Bug Report

@jd-carroll jd-carroll added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Sep 27, 2019
@SomayaB SomayaB added the @aws-cdk/aws-ecs-patterns Related to ecs-patterns library label Sep 27, 2019
@rix0rrr rix0rrr removed the needs-triage This issue or PR still needs to be triaged. label Sep 30, 2019
@rix0rrr rix0rrr added the p2 label Oct 24, 2019
@iamhopaul123
Copy link
Contributor

iamhopaul123 commented Oct 24, 2019

Hi @jd-carroll, thanks for discovering this issue!

Error reason

Passes the cluster as a parameter in creating the ecs_pattern construct so the serviceStack has to depend on the clusterStack (also corresponds to the purpose of the user).

However, if no load balancer is provided, a new load balancer will be created by default and the vpc for this load balancer should be the same using to create the cluster. When creating the load balancer, security groups will also be created since there is no security groups provided to create the load balancer ("ApplicationLoadBalancedEc2Service" does not support providing security group as property for creating the default load balancer). And the security groups created inside load balancer constructor registered security group id in the vpc (create resources in vpc which is in clusterStack), causing the cyclic reference.

Solutions

  1. define the load balancer in the clusterStack and pass the defined load balancer as a parameter when creating the higher-level construct.
const lb = new elbv2.ApplicationLoadBalancer(clusterStack, 'lb', { vpc });

// ---- Create a service stack defining some task
const serviceStack = new cdk.Stack(app, 'aws-ecs-service-stack');

new ecs_patterns.ApplicationLoadBalancedEc2Service(serviceStack, 'InboundSmtpAppService', {
  cluster,
  memoryLimitMiB: 512,
  taskImageOptions: {
    image: ecs.ContainerImage.fromRegistry("amazon/amazon-ecs-sample")
  },
  loadBalancer: lb
});
  1. Create security group outside and pass in as a new property (in parallel with publicLoadBalancer), so that when "ApplicationLoadBalancedEc2Service" create a new load balancer, it does not need to create new security group in the "serviceStack". (Not supported yet)

Suggestion

For now I think solution 1 can help you solve this problem. However, we should discuss if this behavior is a common use case (separating as clusterStack and serviceStack when using ecs_pattern constructs). If so, we need to think about adding the security group as a new property for creating a new load balancer in the construct.

@iamhopaul123
Copy link
Contributor

@pkandasamy91 any thoughts on this?

@piradeepk
Copy link
Contributor

@iamhopaul123 thanks for investigating the issue! I think the first solution would definitely unblock @jd-carroll

Looking at the bigger picture though, we should see if there's a way around this without requiring the user to do either. I don't think it's intuitive for users to know that they need to pass in a security group if their cluster is created in a separate stack. Nor should they be required to create a load balancer if the construct should be able to create one for them.

@rix0rrr rix0rrr assigned MrArnoldPalmer and unassigned rix0rrr Jan 23, 2020
@MrArnoldPalmer MrArnoldPalmer added the effort/medium Medium work item – several days of effort label Aug 17, 2020
@SoManyHs SoManyHs changed the title Cannot Create ApplicationLoadBalancedEc2Service Separate From Cluster [aws-ecs-patterns] Cannot Create ApplicationLoadBalancedEc2Service Separate From Cluster May 13, 2021
@github-actions
Copy link

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels May 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ecs-patterns Related to ecs-patterns library bug This issue is a bug. closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. effort/medium Medium work item – several days of effort p2
Projects
None yet
Development

No branches or pull requests

6 participants