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-ecspatterns): ApplicationMultipleTargetGroupsFargateService generate incorrect CF template when use multiple container in a task #24582

Open
weiluo8791 opened this issue Mar 12, 2023 · 4 comments
Assignees
Labels
@aws-cdk/aws-ecs-patterns Related to ecs-patterns library bug This issue is a bug. effort/medium Medium work item – several days of effort p1

Comments

@weiluo8791
Copy link

weiluo8791 commented Mar 12, 2023

Describe the bug

When creating task with multiple container bind to multiple port when creating targetGroups with ApplicationMultipleTargetGroupsFargateService it incorrectly generated cloudformation template matching wrong container and port.

Consider the following code

 taskDefinition.addContainer('first', {
      image: firstImage,
      essential: true,
      portMappings: [
        {
          containerPort: 8081, // main web server port
          protocol: ecs.Protocol.TCP,
        },
        {
          containerPort: 8003, // admin port
          protocol: ecs.Protocol.TCP,
        },
      ],
    });

    taskDefinition.addContainer('second', {
      image: secondImage,
      portMappings: [
        {
          containerPort: 8002, // api port
          protocol: ecs.Protocol.TCP,
        },
      ],
    });

    const fargateService = new ecs_patterns.ApplicationMultipleTargetGroupsFargateService(this, 'MyFargateService', {
      serviceName: process.env.ECS_SERVICE_NAME ?? 'ecs-service-name',
      desiredCount: minNumberOfInstances,
      cluster,
      taskDefinition,
      assignPublicIp: false,
      loadBalancers: [applicationLoadBalancerProps],
      targetGroups: [
        {
          containerPort: 8081,
          protocol: ecs.Protocol.TCP,
        },
        {
          containerPort: 8002,
          pathPattern: '/api/*',
          priority: 2,
          protocol: ecs.Protocol.TCP,
        },
        {
          containerPort: 8003,
          pathPattern: '/admin/*',
          priority: 3,
          protocol: ecs.Protocol.TCP,
        },
      ],
    });

this will generate the following cloudformation template

  taskdefinition8D2D9F59:
    Type: AWS::ECS::TaskDefinition
    Properties:
      ContainerDefinitions:
        - Cpu: 1024
          Essential: true
          Image: firstImage:latest
          Memory: 4096
          Name: first
          PortMappings:
            - ContainerPort: 8081
              Protocol: tcp
            - ContainerPort: 8003
              Protocol: tcp
            - ContainerPort: 8002
              Protocol: tcp
        - Cpu: 1024
          Essential: true
          Image: secondImage:latest
          Memory: 4096
          Name: second
          PortMappings:
            - ContainerPort: 8002
              Protocol: tcp

Note that container first should not have PortMappings of 8002

Expected Behavior

It should generate something like

  taskdefinition8D2D9F59:
    Type: AWS::ECS::TaskDefinition
    Properties:
      ContainerDefinitions:
        - Cpu: 1024
          Essential: true
          Image: firstImage:latest
          Memory: 4096
          Name: first
          PortMappings:
            - ContainerPort: 8081
              Protocol: tcp
            - ContainerPort: 8003
              Protocol: tcp
        - Cpu: 1024
          Essential: true
          Image: secondImage:latest
          Memory: 4096
          Name: sapi
          PortMappings:
            - ContainerPort: 8002
              Protocol: tcp

or have flexibility in ApplicationMultipleTargetGroupsFargateService targetGroups to specific which port mapped to which container

Current Behavior

Seems like when using the ApplicationMultipleTargetGroupsFargateService construct, you can specify an array of ApplicationTargetProps objects to create multiple target groups. Each ApplicationTargetProps object specifies the container name and port to use for the target group.

However, the construct incorrectly uses the first containerPort value specified in the array for all target groups, instead of using the containerPort value specified for each individual target group. As a result, the CloudFormation template generated by the CDK has the wrong containerPort values assigned to each target group, which can cause issues when the service is deployed because it will complain about

(taskdefinition8D2D9F59) Resource handler returned message: "Invalid request provided: Create TaskDefinition: TCP host port '8002' is mapped multiple times in task. (Service: AmazonECS; Status Code: 400; Error Code: ClientException; Request ID: 6fc9bf75-47d2-4a46-a1ac-efe8b125dc80; Proxy: null)" (RequestToken: 844bb1f7-c1c3-f4a2-593d-316e5561991b, HandlerErrorCode: InvalidRequest)

Reproduction Steps

  1. create taskdefinition with multiple container and multiple port binding
  2. create service with ApplicationMultipleTargetGroupsFargateService with targetGroups matching all the port in the task
  3. observe cdk synth output

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.38.0

Framework Version

No response

Node.js Version

16.15.0

OS

Mac OS 13.2.1

Language

Typescript

Language Version

No response

Other information

No response

@weiluo8791 weiluo8791 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 12, 2023
@github-actions github-actions bot added the @aws-cdk/aws-ecs-patterns Related to ecs-patterns library label Mar 12, 2023
@pahud pahud self-assigned this Mar 13, 2023
@pahud pahud added investigating This issue is being investigated and/or work is in progress to resolve the issue. and removed needs-triage This issue or PR still needs to be triaged. labels Mar 13, 2023
@pahud
Copy link
Contributor

pahud commented Mar 15, 2023

Yes I can reproduce this bug. Thank you for your report.

@pahud pahud added p1 effort/medium Medium work item – several days of effort and removed investigating This issue is being investigated and/or work is in progress to resolve the issue. labels Mar 15, 2023
@sakurai-ryo
Copy link
Contributor

sakurai-ryo commented Apr 3, 2023

Having the same issue today.
I think we need to add a unique container name as a property of ApplicationTargetProps to identify which TargetGroup the container maps to.
But it requires breaking changes.

@sanderk92
Copy link

sanderk92 commented Oct 31, 2023

I'm running into the same issue currently. See this issue describing the same bug as well: #24013. Is there any progress?

@gboudreau
Copy link

I am also having this issue. Anyone found a work-around for this? Seems to me having multiple containers using different ports would be a pretty common use-case. Not sure why there is not more ppl flagged this issue...

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. effort/medium Medium work item – several days of effort p1
Projects
None yet
Development

No branches or pull requests

5 participants