Skip to content

Commit

Permalink
fix(ecs-patterns): memory limit is not set at the container level (aw…
Browse files Browse the repository at this point in the history
…s#21201)

I ran into an issue when deploying a Java application on Fargate, where the container kept getting killed because of out-of-memory condition. Setting  memoryLimitMiB also on container fixed the problem, based on:

https://aws.amazon.com/blogs/containers/how-amazon-ecs-manages-cpu-and-memory-resources/

> [Update 12/11/2020] Some applications are container-aware and can configure themselves to take full advantage of the resources available inside the container. The latest versions of Java are a good example of this pattern. For this reason, some of these applications work best when CPU and memory resources are explicitly configured at the container level, in addition to the configuration at the task level. In other words, while containers without specific resource configurations can nominally access all task resources, aws/amazon-ecs-agent#1735.

I initially asked about this in aws#13127

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
gnagy authored and mrgrain committed Jul 28, 2022
1 parent 4d13004 commit 372acc2
Show file tree
Hide file tree
Showing 21 changed files with 70 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,11 @@ export class ApplicationLoadBalancedFargateService extends ApplicationLoadBalanc
const containerName = taskImageOptions.containerName ?? 'web';
const container = this.taskDefinition.addContainer(containerName, {
image: taskImageOptions.image,
logging: logDriver,
cpu: props.cpu,
memoryLimitMiB: props.memoryLimitMiB,
environment: taskImageOptions.environment,
secrets: taskImageOptions.secrets,
logging: logDriver,
dockerLabels: taskImageOptions.dockerLabels,
});
container.addPortMappings({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,11 @@ export class ApplicationMultipleTargetGroupsFargateService extends ApplicationMu
const containerName = taskImageOptions.containerName ?? 'web';
const container = this.taskDefinition.addContainer(containerName, {
image: taskImageOptions.image,
logging: this.logDriver,
cpu: props.cpu,
memoryLimitMiB: props.memoryLimitMiB,
environment: taskImageOptions.environment,
secrets: taskImageOptions.secrets,
logging: this.logDriver,
dockerLabels: taskImageOptions.dockerLabels,
});
if (taskImageOptions.containerPorts) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,11 @@ export class NetworkLoadBalancedFargateService extends NetworkLoadBalancedServic
const containerName = taskImageOptions.containerName ?? 'web';
const container = this.taskDefinition.addContainer(containerName, {
image: taskImageOptions.image,
logging: logDriver,
cpu: props.cpu,
memoryLimitMiB: props.memoryLimitMiB,
environment: taskImageOptions.environment,
secrets: taskImageOptions.secrets,
logging: logDriver,
dockerLabels: taskImageOptions.dockerLabels,
});
container.addPortMappings({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,11 @@ export class NetworkMultipleTargetGroupsFargateService extends NetworkMultipleTa
const containerName = taskImageOptions.containerName ?? 'web';
const container = this.taskDefinition.addContainer(containerName, {
image: taskImageOptions.image,
logging: this.logDriver,
cpu: props.cpu,
memoryLimitMiB: props.memoryLimitMiB,
environment: taskImageOptions.environment,
secrets: taskImageOptions.secrets,
logging: this.logDriver,
dockerLabels: taskImageOptions.dockerLabels,
});
if (taskImageOptions.containerPorts) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,17 +120,22 @@ export class QueueProcessingFargateService extends QueueProcessingServiceBase {
constructor(scope: Construct, id: string, props: QueueProcessingFargateServiceProps) {
super(scope, id, props);

const cpu = props.cpu || 256;
const memoryLimitMiB = props.memoryLimitMiB || 512;

// Create a Task Definition for the container to start
this.taskDefinition = new FargateTaskDefinition(this, 'QueueProcessingTaskDef', {
memoryLimitMiB: props.memoryLimitMiB || 512,
cpu: props.cpu || 256,
cpu,
memoryLimitMiB,
family: props.family,
});

const containerName = props.containerName ?? 'QueueProcessingContainer';

this.taskDefinition.addContainer(containerName, {
image: props.image,
cpu,
memoryLimitMiB,
command: props.command,
environment: this.environment,
secrets: this.secrets,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,17 @@ export class ScheduledFargateTask extends ScheduledTaskBase {
this.taskDefinition = props.scheduledFargateTaskDefinitionOptions.taskDefinition;
} else if (props.scheduledFargateTaskImageOptions) {
const taskImageOptions = props.scheduledFargateTaskImageOptions;
const cpu = taskImageOptions.cpu || 256;
const memoryLimitMiB = taskImageOptions.memoryLimitMiB || 512;

this.taskDefinition = new FargateTaskDefinition(this, 'ScheduledTaskDef', {
memoryLimitMiB: taskImageOptions.memoryLimitMiB || 512,
cpu: taskImageOptions.cpu || 256,
memoryLimitMiB,
cpu,
});
this.taskDefinition.addContainer('ScheduledContainer', {
image: taskImageOptions.image,
memoryLimitMiB,
cpu,
command: taskImageOptions.command,
environment: taskImageOptions.environment,
secrets: taskImageOptions.secrets,
Expand Down
15 changes: 11 additions & 4 deletions packages/@aws-cdk/aws-ecs-patterns/test/ec2/l3s.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,8 @@ test('test Fargate loadbalanced construct', () => {
// WHEN
new ecsPatterns.ApplicationLoadBalancedFargateService(stack, 'Service', {
cluster,
cpu: 1024,
memoryLimitMiB: 2048,
taskImageOptions: {
image: ecs.ContainerImage.fromRegistry('test'),
environment: {
Expand All @@ -515,6 +517,11 @@ test('test Fargate loadbalanced construct', () => {
Template.fromStack(stack).hasResourceProperties('AWS::ECS::TaskDefinition', {
ContainerDefinitions: [
Match.objectLike({
Cpu: 1024,
DockerLabels: {
label1: 'labelValue1',
label2: 'labelValue2',
},
Environment: [
{
Name: 'TEST_ENVIRONMENT_VARIABLE1',
Expand All @@ -525,6 +532,7 @@ test('test Fargate loadbalanced construct', () => {
Value: 'test environment variable 2 value',
},
],
Image: 'test',
LogConfiguration: {
LogDriver: 'awslogs',
Options: {
Expand All @@ -533,12 +541,11 @@ test('test Fargate loadbalanced construct', () => {
'awslogs-region': { Ref: 'AWS::Region' },
},
},
DockerLabels: {
label1: 'labelValue1',
label2: 'labelValue2',
},
Memory: 2048,
}),
],
Cpu: '1024',
Memory: '2048',
});

Template.fromStack(stack).hasResourceProperties('AWS::ECS::Service', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,7 @@
}
}
},
"Memory": 512,
"Name": "web",
"PortMappings": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -529,6 +529,7 @@
}
}
},
"Memory": 512,
"Name": "web",
"PortMappings": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,7 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 256,
"Environment": [
{
"Name": "QUEUE_NAME",
Expand Down Expand Up @@ -504,6 +505,7 @@
}
}
},
"Memory": 512,
"Name": "QueueProcessingContainer"
}
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,7 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 512,
"Essential": true,
"Image": "amazon/amazon-ecs-sample",
"LogConfiguration": {
Expand All @@ -578,6 +579,7 @@
}
}
},
"Memory": 1024,
"Name": "web",
"PortMappings": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 512,
"Essential": true,
"Image": "amazon/amazon-ecs-sample",
"LogConfiguration": {
Expand All @@ -143,6 +144,7 @@
}
}
},
"Memory": 1024,
"Name": "web",
"PortMappings": [
{
Expand Down Expand Up @@ -775,6 +777,7 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 512,
"Essential": true,
"Image": "amazon/amazon-ecs-sample",
"LogConfiguration": {
Expand All @@ -789,6 +792,7 @@
}
}
},
"Memory": 1024,
"Name": "web",
"PortMappings": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,7 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 512,
"Essential": true,
"Image": "amazon/amazon-ecs-sample",
"LogConfiguration": {
Expand All @@ -542,6 +543,7 @@
}
}
},
"Memory": 1024,
"Name": "web",
"PortMappings": [
{
Expand Down Expand Up @@ -799,6 +801,7 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 512,
"Essential": true,
"Image": "amazon/amazon-ecs-sample",
"LogConfiguration": {
Expand All @@ -813,6 +816,7 @@
}
}
},
"Memory": 1024,
"Name": "web",
"PortMappings": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,7 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 512,
"Essential": true,
"Image": "amazon/amazon-ecs-sample",
"LogConfiguration": {
Expand All @@ -526,6 +527,7 @@
}
}
},
"Memory": 1024,
"Name": "web",
"PortMappings": [
{
Expand Down Expand Up @@ -775,6 +777,7 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 512,
"Essential": true,
"Image": "amazon/amazon-ecs-sample",
"LogConfiguration": {
Expand All @@ -789,6 +792,7 @@
}
}
},
"Memory": 1024,
"Name": "web",
"PortMappings": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,7 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 512,
"Essential": true,
"Image": "amazon/amazon-ecs-sample",
"LogConfiguration": {
Expand All @@ -529,6 +530,7 @@
}
}
},
"Memory": 1024,
"Name": "web",
"PortMappings": [
{
Expand Down Expand Up @@ -775,6 +777,7 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 512,
"Essential": true,
"Image": "amazon/amazon-ecs-sample",
"LogConfiguration": {
Expand All @@ -789,6 +792,7 @@
}
}
},
"Memory": 1024,
"Name": "web",
"PortMappings": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -530,6 +530,7 @@
}
}
},
"Memory": 512,
"Name": "web",
"PortMappings": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -770,6 +770,7 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 256,
"Environment": [
{
"Name": "QUEUE_NAME",
Expand Down Expand Up @@ -813,6 +814,7 @@
}
}
},
"Memory": 512,
"Name": "QueueProcessingContainer"
}
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,7 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 256,
"Environment": [
{
"Name": "QUEUE_NAME",
Expand Down Expand Up @@ -513,6 +514,7 @@
}
}
},
"Memory": 512,
"Name": "QueueProcessingContainer"
}
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,7 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 256,
"Environment": [
{
"Name": "QUEUE_NAME",
Expand Down Expand Up @@ -504,6 +505,7 @@
}
}
},
"Memory": 512,
"Name": "QueueProcessingContainer"
}
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,7 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 256,
"Environment": [
{
"Name": "TRIGGER",
Expand Down Expand Up @@ -323,6 +324,7 @@
}
}
},
"Memory": 512,
"Name": "ScheduledContainer"
}
],
Expand Down
Loading

0 comments on commit 372acc2

Please sign in to comment.