Describe the bug
A recent change was made to set memory at the container level, not just at the task definition level.
This was accomplished by setting the cpu and memory values of the container to be the same as the task definition, which isn't necessarily going to be the desired case.
This causes deployment to break for existing users of this construct who have added additional containers to their task definition.
Expected Behavior
Deployment not to break
Current Behavior
Deployment breaks because the total cpu and memory used in the containers exceeds the values allocated to the task definition.
Reproduction Steps
All three of these constructs fail deployment where they would previously succeed before this change. QueueProcessingFargateService will always automatically allocate the same value to both the task definition and default container, so passing in a cpu/memory value for this construct in particular isn't necessary to not be able to add additional containers.
Note: If these values are not specified on the additional containers, they will be allocated 0 cpu, which can be problematic if any cpu is desired
const qserv = new QueueProcessingFargateService(this, 'QServ', {
image: ContainerImage.fromRegistry('amazon/amazon-ecs-sample')
});
qserv.taskDefinition.addContainer('container', {
image: ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
cpu: 8
})
const nlbserv = new NetworkLoadBalancedFargateService(this, 'NLBServ', {
cpu: 256,
taskImageOptions: {
image: ecs.ContainerImage.fromRegistry("amazon/amazon-ecs-sample"),
},
});
nlbserv.taskDefinition.addContainer('container', {
image: ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
cpu: 8
});
const appserv = new ApplicationLoadBalancedFargateService(this, 'AppServ', {
cpu: 256,
taskImageOptions: {
image: ecs.ContainerImage.fromRegistry("amazon/amazon-ecs-sample"),
},
});
appserv.taskDefinition.addContainer('container', {
image: ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
cpu: 8
});
Possible Solution
The documentation states:
The number of cpu units used by the task.
No mention of container cpu units is specified. If we want to be able to allocate the value, there should be a separate property for this so as not to break existing users, and to also allow for the addition of other containers if desired.
Additional Information/Context
No response
CDK CLI Version
2.34.1
Framework Version
No response
Node.js Version
.
OS
mac
Language
Typescript
Language Version
No response
Other information
No response
Describe the bug
A recent change was made to set memory at the container level, not just at the task definition level.
This was accomplished by setting the cpu and memory values of the container to be the same as the task definition, which isn't necessarily going to be the desired case.
This causes deployment to break for existing users of this construct who have added additional containers to their task definition.
Expected Behavior
Deployment not to break
Current Behavior
Deployment breaks because the total cpu and memory used in the containers exceeds the values allocated to the task definition.
Reproduction Steps
All three of these constructs fail deployment where they would previously succeed before this change. QueueProcessingFargateService will always automatically allocate the same value to both the task definition and default container, so passing in a cpu/memory value for this construct in particular isn't necessary to not be able to add additional containers.
Note: If these values are not specified on the additional containers, they will be allocated 0 cpu, which can be problematic if any cpu is desired
Possible Solution
The documentation states:
No mention of container cpu units is specified. If we want to be able to allocate the value, there should be a separate property for this so as not to break existing users, and to also allow for the addition of other containers if desired.
Additional Information/Context
No response
CDK CLI Version
2.34.1
Framework Version
No response
Node.js Version
.
OS
mac
Language
Typescript
Language Version
No response
Other information
No response