Skip to content

Commit 715158f

Browse files
tea-418tea-418
authored
fix(ecs): ec2Service placement strategies use incorrect casing which causes drift (#20946)
Fixes #20812 Switches lowercase cpu and memory placement strategy to uppercase as required by cloudformation: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-ecs-service-placementstrategy.html ---- ### 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*
1 parent e1a7f9e commit 715158f

14 files changed

Lines changed: 3588 additions & 10 deletions

File tree

‎packages/@aws-cdk/aws-ecs-patterns/test/ec2/l3s-v2.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ describe('When Application Load Balancer', () => {
195195
PropagateTags: 'SERVICE',
196196
ServiceName: 'myService',
197197
PlacementConstraints: [{ Type: 'memberOf', Expression: 'attribute:ecs.instance-type =~ m5a.*' }],
198-
PlacementStrategies: [{ Field: 'instanceId', Type: 'spread' }, { Field: 'cpu', Type: 'binpack' }, { Type: 'random' }],
198+
PlacementStrategies: [{ Field: 'instanceId', Type: 'spread' }, { Field: 'CPU', Type: 'binpack' }, { Type: 'random' }],
199199
});
200200

201201
Template.fromStack(stack).hasResourceProperties('AWS::ECS::TaskDefinition', {
@@ -1173,7 +1173,7 @@ describe('When Network Load Balancer', () => {
11731173
SchedulingStrategy: 'REPLICA',
11741174
ServiceName: 'myService',
11751175
PlacementConstraints: [{ Type: 'memberOf', Expression: 'attribute:ecs.instance-type =~ m5a.*' }],
1176-
PlacementStrategies: [{ Field: 'instanceId', Type: 'spread' }, { Field: 'cpu', Type: 'binpack' }, { Type: 'random' }],
1176+
PlacementStrategies: [{ Field: 'instanceId', Type: 'spread' }, { Field: 'CPU', Type: 'binpack' }, { Type: 'random' }],
11771177
});
11781178

11791179
Template.fromStack(stack).hasResourceProperties('AWS::ECS::TaskDefinition', {

‎packages/@aws-cdk/aws-ecs-patterns/test/ec2/queue-processing-ecs-service.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,7 @@ testDeprecated('test ECS queue worker service construct - with optional props',
334334
Type: 'ECS',
335335
},
336336
PlacementConstraints: [{ Type: 'memberOf', Expression: 'attribute:ecs.instance-type =~ m5a.*' }],
337-
PlacementStrategies: [{ Field: 'instanceId', Type: 'spread' }, { Field: 'cpu', Type: 'binpack' }, { Type: 'random' }],
337+
PlacementStrategies: [{ Field: 'instanceId', Type: 'spread' }, { Field: 'CPU', Type: 'binpack' }, { Type: 'random' }],
338338
});
339339

340340
Template.fromStack(stack).hasResourceProperties('AWS::SQS::Queue', {

‎packages/@aws-cdk/aws-ecs/lib/placement.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@ export enum BinPackResource {
88
/**
99
* Fill up hosts' CPU allocations first
1010
*/
11-
CPU = 'cpu',
11+
CPU = 'CPU',
1212

1313
/**
1414
* Fill up hosts' memory allocations first
1515
*/
16-
MEMORY = 'memory',
16+
MEMORY = 'MEMORY',
1717
}
1818

1919
/**

‎packages/@aws-cdk/aws-ecs/test/ec2/ec2-service.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1936,7 +1936,7 @@ describe('ec2 service', () => {
19361936
// THEN
19371937
Template.fromStack(stack).hasResourceProperties('AWS::ECS::Service', {
19381938
PlacementStrategies: [{
1939-
Field: 'cpu',
1939+
Field: 'CPU',
19401940
Type: 'binpack',
19411941
}],
19421942
});
@@ -1967,7 +1967,7 @@ describe('ec2 service', () => {
19671967
// THEN
19681968
Template.fromStack(stack).hasResourceProperties('AWS::ECS::Service', {
19691969
PlacementStrategies: [{
1970-
Field: 'memory',
1970+
Field: 'MEMORY',
19711971
Type: 'binpack',
19721972
}],
19731973
});
@@ -1998,7 +1998,7 @@ describe('ec2 service', () => {
19981998
// THEN
19991999
Template.fromStack(stack).hasResourceProperties('AWS::ECS::Service', {
20002000
PlacementStrategies: [{
2001-
Field: 'memory',
2001+
Field: 'MEMORY',
20022002
Type: 'binpack',
20032003
}],
20042004
});

‎packages/@aws-cdk/aws-ecs/test/ec2/integ.placement-strategies.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import * as ec2 from '@aws-cdk/aws-ec2';
2+
import * as cdk from '@aws-cdk/core';
3+
import { Construct } from 'constructs';
4+
import * as ecs from '../../lib';
5+
6+
const app = new cdk.App();
7+
8+
class EcsStack extends cdk.Stack {
9+
constructor(scope: Construct, id: string, props?: cdk.StackProps) {
10+
super(scope, id, props);
11+
12+
const vpc = new ec2.Vpc(this, 'VPC');
13+
14+
const cluster = new ecs.Cluster(this, 'EcsCluster', { vpc });
15+
cluster.addCapacity('DefaultAutoScalingGroup', {
16+
instanceType: ec2.InstanceType.of(ec2.InstanceClass.T2, ec2.InstanceSize.MICRO),
17+
});
18+
19+
const taskDefinition = new ecs.Ec2TaskDefinition(this, 'TaskDef');
20+
taskDefinition.addContainer('web', {
21+
image: ecs.ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
22+
memoryLimitMiB: 256,
23+
});
24+
25+
new ecs.Ec2Service(this, 'Test_Stack', {
26+
cluster,
27+
taskDefinition,
28+
placementStrategies: [
29+
ecs.PlacementStrategy.packedByCpu(),
30+
ecs.PlacementStrategy.packedByMemory(),
31+
],
32+
});
33+
}
34+
}
35+
36+
new EcsStack(app, 'aws-cdk-ecs-integration-test-stack');
37+
38+
app.synth();

‎packages/@aws-cdk/aws-ecs/test/ec2/placement-strategies.integ.snapshot/PlacementStrategiesDefaultTestDeployAssert6383DC1B.template.json‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
{
2+
"Resources": {
3+
"AwsApiCallCloudformationgetTemplate": {
4+
"Type": "Custom::DeployAssert@SdkCallCloudformationgetTemplate",
5+
"Properties": {
6+
"ServiceToken": {
7+
"Fn::GetAtt": [
8+
"SingletonFunction1488541a7b23466481b69b4408076b81HandlerCD40AE9F",
9+
"Arn"
10+
]
11+
},
12+
"service": "Cloudformation",
13+
"api": "getTemplate",
14+
"parameters": {
15+
"StackName": "aws-cdk-ecs-integration-test-stack"
16+
},
17+
"flattenResponse": "false",
18+
"salt": "1658455048048"
19+
},
20+
"UpdateReplacePolicy": "Delete",
21+
"DeletionPolicy": "Delete"
22+
},
23+
"AwsApiCallCloudformationgetTemplateAssertEqualsCloudformationgetTemplate14A66D97": {
24+
"Type": "Custom::DeployAssert@AssertEquals",
25+
"Properties": {
26+
"ServiceToken": {
27+
"Fn::GetAtt": [
28+
"SingletonFunction1488541a7b23466481b69b4408076b81HandlerCD40AE9F",
29+
"Arn"
30+
]
31+
},
32+
"actual": {
33+
"Fn::GetAtt": [
34+
"AwsApiCallCloudformationgetTemplate",
35+
"apiCallResponse"
36+
]
37+
},
38+
"expected": "{\"$StringLike\":\"/PlacementStrategies:s*- Field: CPUs*Type: binpacks*- Field: MEMORYs*Type: binpack/\"}",
39+
"salt": "1658455048048"
40+
},
41+
"UpdateReplacePolicy": "Delete",
42+
"DeletionPolicy": "Delete"
43+
},
44+
"SingletonFunction1488541a7b23466481b69b4408076b81Role37ABCE73": {
45+
"Type": "AWS::IAM::Role",
46+
"Properties": {
47+
"AssumeRolePolicyDocument": {
48+
"Version": "2012-10-17",
49+
"Statement": [
50+
{
51+
"Action": "sts:AssumeRole",
52+
"Effect": "Allow",
53+
"Principal": {
54+
"Service": "lambda.amazonaws.com"
55+
}
56+
}
57+
]
58+
},
59+
"ManagedPolicyArns": [
60+
{
61+
"Fn::Sub": "arn:${AWS::Partition}:iam::aws:policy/service-role/AWSLambdaBasicExecutionRole"
62+
}
63+
],
64+
"Policies": [
65+
{
66+
"PolicyName": "Inline",
67+
"PolicyDocument": {
68+
"Version": "2012-10-17",
69+
"Statement": [
70+
{
71+
"Action": [
72+
"cloudformation:GetTemplate"
73+
],
74+
"Effect": "Allow",
75+
"Resource": [
76+
"*"
77+
]
78+
}
79+
]
80+
}
81+
}
82+
]
83+
}
84+
},
85+
"SingletonFunction1488541a7b23466481b69b4408076b81HandlerCD40AE9F": {
86+
"Type": "AWS::Lambda::Function",
87+
"Properties": {
88+
"Runtime": "nodejs14.x",
89+
"Code": {
90+
"S3Bucket": {
91+
"Ref": "AssetParametersec094b96e98289a8faed4f4280a8531224c0191f583bc684c21c91a65319e4a3S3Bucket5F1832C4"
92+
},
93+
"S3Key": {
94+
"Fn::Join": [
95+
"",
96+
[
97+
{
98+
"Fn::Select": [
99+
0,
100+
{
101+
"Fn::Split": [
102+
"||",
103+
{
104+
"Ref": "AssetParametersec094b96e98289a8faed4f4280a8531224c0191f583bc684c21c91a65319e4a3S3VersionKeyA04E23E6"
105+
}
106+
]
107+
}
108+
]
109+
},
110+
{
111+
"Fn::Select": [
112+
1,
113+
{
114+
"Fn::Split": [
115+
"||",
116+
{
117+
"Ref": "AssetParametersec094b96e98289a8faed4f4280a8531224c0191f583bc684c21c91a65319e4a3S3VersionKeyA04E23E6"
118+
}
119+
]
120+
}
121+
]
122+
}
123+
]
124+
]
125+
}
126+
},
127+
"Timeout": 120,
128+
"Handler": "index.handler",
129+
"Role": {
130+
"Fn::GetAtt": [
131+
"SingletonFunction1488541a7b23466481b69b4408076b81Role37ABCE73",
132+
"Arn"
133+
]
134+
}
135+
}
136+
}
137+
},
138+
"Outputs": {
139+
"AssertionResultsAssertEqualsCloudformationgetTemplate": {
140+
"Value": {
141+
"Fn::GetAtt": [
142+
"AwsApiCallCloudformationgetTemplateAssertEqualsCloudformationgetTemplate14A66D97",
143+
"data"
144+
]
145+
}
146+
}
147+
},
148+
"Parameters": {
149+
"AssetParametersec094b96e98289a8faed4f4280a8531224c0191f583bc684c21c91a65319e4a3S3Bucket5F1832C4": {
150+
"Type": "String",
151+
"Description": "S3 bucket for asset \"ec094b96e98289a8faed4f4280a8531224c0191f583bc684c21c91a65319e4a3\""
152+
},
153+
"AssetParametersec094b96e98289a8faed4f4280a8531224c0191f583bc684c21c91a65319e4a3S3VersionKeyA04E23E6": {
154+
"Type": "String",
155+
"Description": "S3 key for asset version \"ec094b96e98289a8faed4f4280a8531224c0191f583bc684c21c91a65319e4a3\""
156+
},
157+
"AssetParametersec094b96e98289a8faed4f4280a8531224c0191f583bc684c21c91a65319e4a3ArtifactHash000AF521": {
158+
"Type": "String",
159+
"Description": "Artifact hash for asset \"ec094b96e98289a8faed4f4280a8531224c0191f583bc684c21c91a65319e4a3\""
160+
}
161+
}
162+
}

0 commit comments

Comments
 (0)