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): Support multi-targetgroup applications on different containers #24013

Open
1 of 2 tasks
ajbeach2 opened this issue Feb 4, 2023 · 1 comment
Open
1 of 2 tasks
Labels
@aws-cdk/aws-ecs-patterns Related to ecs-patterns library feature-request A feature should be added or improved. p2

Comments

@ajbeach2
Copy link

ajbeach2 commented Feb 4, 2023

Describe the feature

The ApplicationMultipleTargetGroupsEc2Service module only passes in the default task definition container name

https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/aws-ecs-patterns/lib/ecs/application-multiple-target-groups-ecs-service.ts#L149.

All ecs loadbalancer configurations will end up using only the default container name for all target groups.

For example, if i have a task def that has 2 containers, for a SPA. where i have and api and spa server separately.

	taskDef.AddContainer(jsii.String("api-container"), &awsecs.ContainerDefinitionOptions{
		PortMappings: &[]*awsecs.PortMapping{
			{
				ContainerPort: jsii.Number(80),
				Protocol:      awsecs.Protocol_TCP,
			},
		},
		Image: awsecs.ContainerImage_FromEcrRepository(
			getRepository(scope, "app-api"),
			jsii.String("latest"),
		),
		Environment: &map[string]*string{
			"DEBUG":              jsii.String("false"),
		},
		MemoryLimitMiB: jsii.Number(512),
		Secrets:        &secrets,
	})

	taskDef.AddContainer(jsii.String("client-container"), &awsecs.ContainerDefinitionOptions{
		PortMappings: &[]*awsecs.PortMapping{
			{
				ContainerPort: jsii.Number(8080),
				Protocol:      awsecs.Protocol_TCP,
			},
		},
		MemoryLimitMiB: jsii.Number(256),
		Image: awsecs.ContainerImage_FromEcrRepository(
			getRepository(scope, "app-client"),
			jsii.String("latest"),
		),
	})

If you configure multiple target groups:

			TargetGroups: &[]*awsecspatterns.ApplicationTargetProps{
				{
					ContainerPort: jsii.Number(80),
					Listener:      jsii.String("api-container"),
				},
				{
					ContainerPort: jsii.Number(8080),
					PathPattern:   jsii.String("/app"),
					Listener:      jsii.String("spa-container"),
					Priority:      jsii.Number(1),
				},
			},

the result is incorrectly assigning the loadbalancers to the same container:

aws ecs describe-services --cluster ${CLUSTER_NAME} --services web-app
            "loadBalancers": [
                {
                    "targetGroupArn": "arn:aws:elasticloadbalancing:us-east-1:xxxxxxxx:targetgroup/target-group2",
                    "containerName": "app-api",
                    "containerPort": 80
                },
                {
                    "targetGroupArn": "arn:aws:elasticloadbalancing:us-east-1:xxxxxxx:targetgroup/target-group1",
                    "containerName": "app-api",
                    "containerPort": 8080
                }
            ],

Use Case

The use case is outlined here:
https://docs.aws.amazon.com/AmazonECS/latest/developerguide/register-multiple-targetgroups.html#multiple-targetgroups-example3

Proposed Solution

Update ApplicationTargetGroup to include Container name otherwise use the default container name.

this line would then be updated to:

 targets: [
          service.loadBalancerTarget({
            containerName: targetProps.containerName,
            containerPort: targetProps.containerPort,
            protocol: targetProps.protocol,
          }),
        ],

Other Information

No response

Acknowledgements

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.62.1 (build 8641449)

Environment details (OS name and version, etc.)

Ubuntu 22.04.1 LTS

@ajbeach2 ajbeach2 added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Feb 4, 2023
@github-actions github-actions bot added the @aws-cdk/aws-ecs-patterns Related to ecs-patterns library label Feb 4, 2023
@pahud pahud added p2 and removed needs-triage This issue or PR still needs to be triaged. labels Feb 6, 2023
@weiluo8791
Copy link

ApplicationMultipleTargetGroupsFargateService also seems to have the same problem.
if you have
taskDefinition.addContainer('first', {
image: firstImage,
memoryLimitMiB: process.env.ECS_CONTAINER_MEMORY_LIMIT_MIB,
cpu: process.env.ECS_CONTAINER_CPU_LIMIT_VALUE,
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,
memoryLimitMiB: process.env.ECS_CONTAINER_MEMORY_LIMIT_MIB,
cpu: process.env.ECS_CONTAINER_CPU_LIMIT_VALUE,
essential: true,
portMappings: [
{
containerPort: 8002, // main api
protocol: ecs.Protocol.TCP,
}
],
});

const fargateService = new ecs_patterns.ApplicationMultipleTargetGroupsFargateService(this, 'MyFargateService', {
...
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,
},
],
...
});

it will generate CF template as
taskdefinition8D2D9F59:
Type: AWS::ECS::TaskDefinition
Properties:
ContainerDefinitions:
- Cpu: 1024
Essential: true
Image: first:latest
Memory: 4096
Name: first
PortMappings:
- ContainerPort: 8081
Protocol: tcp
- ContainerPort: 8003
Protocol: tcp
- ContainerPort: 8002
Protocol: tcp
- Cpu: 1024
Essential: true
Image: second:latest
Memory: 4096
Name: second
PortMappings:
- ContainerPort: 8002
Protocol: tcp

which will result in deploy error as it is complaining TCP host port '8002' is mapped multiple times in task.

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.

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 feature-request A feature should be added or improved. p2
Projects
None yet
Development

No branches or pull requests

3 participants