Skip to content

Commit

Permalink
Revert "fix(ecs-patterns): memory limit is not set at the container l…
Browse files Browse the repository at this point in the history
…evel" (aws#21530)

This reverts aws#21201 due to aws#21484: Recent update breaks FargateService L3 constructs which have added additional containers to the task definition

Closes aws#21484
Re-opens aws#13127 

----

### All Submissions:

* [ ] 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
TheRealAmazonKendra authored and josephedward committed Aug 30, 2022
1 parent 7722816 commit 3e3cfa1
Show file tree
Hide file tree
Showing 29 changed files with 12 additions and 81 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -82,11 +82,9 @@ export class ApplicationLoadBalancedFargateService extends ApplicationLoadBalanc
const containerName = taskImageOptions.containerName ?? 'web';
const container = this.taskDefinition.addContainer(containerName, {
image: taskImageOptions.image,
cpu: props.cpu,
memoryLimitMiB: props.memoryLimitMiB,
logging: logDriver,
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 @@ -74,11 +74,9 @@ export class ApplicationMultipleTargetGroupsFargateService extends ApplicationMu
const containerName = taskImageOptions.containerName ?? 'web';
const container = this.taskDefinition.addContainer(containerName, {
image: taskImageOptions.image,
cpu: props.cpu,
memoryLimitMiB: props.memoryLimitMiB,
logging: this.logDriver,
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 @@ -72,11 +72,9 @@ export class NetworkLoadBalancedFargateService extends NetworkLoadBalancedServic
const containerName = taskImageOptions.containerName ?? 'web';
const container = this.taskDefinition.addContainer(containerName, {
image: taskImageOptions.image,
cpu: props.cpu,
memoryLimitMiB: props.memoryLimitMiB,
logging: logDriver,
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 @@ -74,11 +74,9 @@ export class NetworkMultipleTargetGroupsFargateService extends NetworkMultipleTa
const containerName = taskImageOptions.containerName ?? 'web';
const container = this.taskDefinition.addContainer(containerName, {
image: taskImageOptions.image,
cpu: props.cpu,
memoryLimitMiB: props.memoryLimitMiB,
logging: this.logDriver,
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 @@ -68,13 +68,10 @@ 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', {
cpu,
memoryLimitMiB,
memoryLimitMiB: props.memoryLimitMiB || 512,
cpu: props.cpu || 256,
family: props.family,
runtimePlatform: props.runtimePlatform,
});
Expand All @@ -83,8 +80,6 @@ export class QueueProcessingFargateService extends QueueProcessingServiceBase {

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 @@ -73,18 +73,12 @@ 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,
cpu,
runtimePlatform: props.runtimePlatform,
memoryLimitMiB: taskImageOptions.memoryLimitMiB || 512,
cpu: taskImageOptions.cpu || 256,
});
this.taskDefinition.addContainer('ScheduledContainer', {
image: taskImageOptions.image,
memoryLimitMiB,
cpu,
command: taskImageOptions.command,
environment: taskImageOptions.environment,
secrets: taskImageOptions.secrets,
Expand Down
15 changes: 4 additions & 11 deletions packages/@aws-cdk/aws-ecs-patterns/test/ec2/l3s.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -500,8 +500,6 @@ 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 @@ -518,11 +516,6 @@ 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 @@ -533,7 +526,6 @@ test('test Fargate loadbalanced construct', () => {
Value: 'test environment variable 2 value',
},
],
Image: 'test',
LogConfiguration: {
LogDriver: 'awslogs',
Options: {
Expand All @@ -542,11 +534,12 @@ test('test Fargate loadbalanced construct', () => {
'awslogs-region': { Ref: 'AWS::Region' },
},
},
Memory: 2048,
DockerLabels: {
label1: 'labelValue1',
label2: 'labelValue2',
},
}),
],
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 @@ -640,7 +640,6 @@
}
}
},
"Memory": 512,
"Name": "web",
"PortMappings": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1124,7 +1124,6 @@
{
"essential": true,
"image": "amazon/amazon-ecs-sample",
"memory": 512,
"name": "web",
"portMappings": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,6 @@
}
}
},
"Memory": 512,
"Name": "web",
"PortMappings": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,6 @@
}
}
},
"Memory": 512,
"Name": "web",
"PortMappings": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,6 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 256,
"Environment": [
{
"Name": "QUEUE_NAME",
Expand Down Expand Up @@ -505,7 +504,6 @@
}
}
},
"Memory": 512,
"Name": "QueueProcessingContainer"
}
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -564,7 +564,6 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 512,
"Essential": true,
"Image": "amazon/amazon-ecs-sample",
"LogConfiguration": {
Expand All @@ -579,7 +578,6 @@
}
}
},
"Memory": 1024,
"Name": "web",
"PortMappings": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -654,7 +654,6 @@
}
}
},
"Memory": 512,
"Name": "web",
"PortMappings": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1150,7 +1150,6 @@
{
"essential": true,
"image": "amazon/amazon-ecs-sample",
"memory": 512,
"name": "web",
"portMappings": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,6 @@
}
}
},
"Memory": 512,
"Name": "web",
"PortMappings": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -960,7 +960,6 @@
{
"essential": true,
"image": "amazon/amazon-ecs-sample",
"memory": 512,
"name": "web",
"portMappings": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 512,
"Essential": true,
"Image": "amazon/amazon-ecs-sample",
"LogConfiguration": {
Expand All @@ -144,7 +143,6 @@
}
}
},
"Memory": 1024,
"Name": "web",
"PortMappings": [
{
Expand Down Expand Up @@ -777,7 +775,6 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 512,
"Essential": true,
"Image": "amazon/amazon-ecs-sample",
"LogConfiguration": {
Expand All @@ -792,7 +789,6 @@
}
}
},
"Memory": 1024,
"Name": "web",
"PortMappings": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,6 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 512,
"Essential": true,
"Image": "amazon/amazon-ecs-sample",
"LogConfiguration": {
Expand All @@ -543,7 +542,6 @@
}
}
},
"Memory": 1024,
"Name": "web",
"PortMappings": [
{
Expand Down Expand Up @@ -801,7 +799,6 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 512,
"Essential": true,
"Image": "amazon/amazon-ecs-sample",
"LogConfiguration": {
Expand All @@ -816,7 +813,6 @@
}
}
},
"Memory": 1024,
"Name": "web",
"PortMappings": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,6 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 512,
"Essential": true,
"Image": "amazon/amazon-ecs-sample",
"LogConfiguration": {
Expand All @@ -527,7 +526,6 @@
}
}
},
"Memory": 1024,
"Name": "web",
"PortMappings": [
{
Expand Down Expand Up @@ -777,7 +775,6 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 512,
"Essential": true,
"Image": "amazon/amazon-ecs-sample",
"LogConfiguration": {
Expand All @@ -792,7 +789,6 @@
}
}
},
"Memory": 1024,
"Name": "web",
"PortMappings": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,6 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 512,
"Essential": true,
"Image": "amazon/amazon-ecs-sample",
"LogConfiguration": {
Expand All @@ -530,7 +529,6 @@
}
}
},
"Memory": 1024,
"Name": "web",
"PortMappings": [
{
Expand Down Expand Up @@ -777,7 +775,6 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 512,
"Essential": true,
"Image": "amazon/amazon-ecs-sample",
"LogConfiguration": {
Expand All @@ -792,7 +789,6 @@
}
}
},
"Memory": 1024,
"Name": "web",
"PortMappings": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,6 @@
}
}
},
"Memory": 512,
"Name": "web",
"PortMappings": [
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -770,7 +770,6 @@
"Properties": {
"ContainerDefinitions": [
{
"Cpu": 256,
"Environment": [
{
"Name": "QUEUE_NAME",
Expand Down Expand Up @@ -814,7 +813,6 @@
}
}
},
"Memory": 512,
"Name": "QueueProcessingContainer"
}
],
Expand Down
Loading

0 comments on commit 3e3cfa1

Please sign in to comment.