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.67k stars 3.92k forks source link

(aws-ecspatterns): ApplicationMultipleTargetGroupsFargateService generate incorrect CF template when use multiple container in a task #24582

Open weiluo8791 opened 1 year ago

weiluo8791 commented 1 year ago

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

pahud commented 1 year ago

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

sakurai-ryo commented 1 year ago

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. https://github.com/aws/aws-cdk/blob/2bb49bf58361fbf382a03644021b123a69d5badf/packages/aws-cdk-lib/aws-ecs-patterns/lib/base/application-multiple-target-groups-service-base.ts#L213

sanderk92 commented 1 year ago

I'm running into the same issue currently. See this issue describing the same bug as well: https://github.com/aws/aws-cdk/issues/24013. Is there any progress?

gboudreau commented 6 months ago

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...