Skip to content

Commit dcdd1f7

Browse files
committed
fix(aws-ecs): allow empty placementStrategies on EC2Service
Same fix as #30382 but for `placementStrategies`. Apparently this was not done at that time because CloudFormation [previously did not have the appropriate semantics](#27572 (comment)) for setting PlacementStrategies to the empty array as it did for PlacementConstraints, but that is no longer the case. The CloudFormation docs [state](https://docs.aws.amazon.com/AWSCloudFormation/latest/TemplateReference/aws-resource-ecs-service.html#cfn-ecs-service-placementstrategies): > To remove this property from your service resource, specify an empty PlacementStrategy array. And indeed that works, with this CDK change applied.
1 parent d24e612 commit dcdd1f7

File tree

9 files changed

+137
-54
lines changed

9 files changed

+137
-54
lines changed

packages/@aws-cdk-testing/framework-integ/test/aws-ecs/test/ec2/integ.placement-strategies.js.snapshot/aws-cdk-ecs-integration-test-stack.assets.json

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-ecs/test/ec2/integ.placement-strategies.js.snapshot/aws-cdk-ecs-integration-test-stack.template.json

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -968,6 +968,28 @@
968968
"DependsOn": [
969969
"TaskDefTaskRole1EDB4A67"
970970
]
971+
},
972+
"TestEmptyService1DB8D95A": {
973+
"Type": "AWS::ECS::Service",
974+
"Properties": {
975+
"Cluster": {
976+
"Ref": "EcsCluster97242B84"
977+
},
978+
"DeploymentConfiguration": {
979+
"MaximumPercent": 200,
980+
"MinimumHealthyPercent": 50
981+
},
982+
"EnableECSManagedTags": false,
983+
"LaunchType": "EC2",
984+
"PlacementStrategies": [],
985+
"SchedulingStrategy": "REPLICA",
986+
"TaskDefinition": {
987+
"Ref": "TaskDef54694570"
988+
}
989+
},
990+
"DependsOn": [
991+
"TaskDefTaskRole1EDB4A67"
992+
]
971993
}
972994
},
973995
"Parameters": {

packages/@aws-cdk-testing/framework-integ/test/aws-ecs/test/ec2/integ.placement-strategies.js.snapshot/cdk.out

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-ecs/test/ec2/integ.placement-strategies.js.snapshot/integ.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-ecs/test/ec2/integ.placement-strategies.js.snapshot/manifest.json

Lines changed: 70 additions & 41 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/@aws-cdk-testing/framework-integ/test/aws-ecs/test/ec2/integ.placement-strategies.js.snapshot/tree.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,12 @@ class EcsStack extends cdk.Stack {
3737
ecs.PlacementStrategy.packedByMemory(),
3838
],
3939
});
40+
41+
new ecs.Ec2Service(this, 'Test_Empty', {
42+
cluster,
43+
taskDefinition,
44+
placementStrategies: [],
45+
});
4046
}
4147
}
4248

packages/aws-cdk-lib/aws-ecs/lib/ec2/ec2-service.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ export class Ec2Service extends BaseService implements IEc2Service {
163163
}
164164

165165
private constraints?: CfnService.PlacementConstraintProperty[];
166-
private readonly strategies: CfnService.PlacementStrategyProperty[];
166+
private strategies?: CfnService.PlacementStrategyProperty[];
167167
private readonly daemon: boolean;
168168
private readonly availabilityZoneRebalancingEnabled: boolean;
169169

@@ -212,15 +212,13 @@ export class Ec2Service extends BaseService implements IEc2Service {
212212
cluster: props.cluster.clusterName,
213213
taskDefinition: props.deploymentController?.type === DeploymentControllerType.EXTERNAL ? undefined : props.taskDefinition.taskDefinitionArn,
214214
placementConstraints: Lazy.any({ produce: () => this.constraints }),
215-
placementStrategies: Lazy.any({ produce: () => this.strategies }, { omitEmptyArray: true }),
215+
placementStrategies: Lazy.any({ produce: () => this.strategies }),
216216
schedulingStrategy: props.daemon ? 'DAEMON' : 'REPLICA',
217217
availabilityZoneRebalancing: props.availabilityZoneRebalancing,
218218
}, props.taskDefinition);
219219
// Enhanced CDK Analytics Telemetry
220220
addConstructMetadata(this, props);
221221

222-
this.constraints = undefined;
223-
this.strategies = [];
224222
this.daemon = props.daemon || false;
225223
this.availabilityZoneRebalancingEnabled = props.availabilityZoneRebalancing === AvailabilityZoneRebalancing.ENABLED;
226224

@@ -249,7 +247,9 @@ export class Ec2Service extends BaseService implements IEc2Service {
249247
if (props.placementConstraints) {
250248
this.addPlacementConstraints(...props.placementConstraints);
251249
}
252-
this.addPlacementStrategies(...props.placementStrategies || []);
250+
if (props.placementStrategies) {
251+
this.addPlacementStrategies(...props.placementStrategies);
252+
}
253253

254254
this.node.addValidation({
255255
validate: () => !this.taskDefinition.defaultContainer ? ['A TaskDefinition must have at least one essential container'] : [],
@@ -272,13 +272,14 @@ export class Ec2Service extends BaseService implements IEc2Service {
272272
throw new ValidationError("Can't configure placement strategies when daemon=true", this);
273273
}
274274

275-
if (strategies.length > 0 && this.strategies.length === 0 && this.availabilityZoneRebalancingEnabled) {
275+
if (strategies.length > 0 && !this.strategies && this.availabilityZoneRebalancingEnabled) {
276276
const [placement] = strategies[0].toJson();
277277
if (placement.type !== 'spread' || placement.field !== BuiltInAttributes.AVAILABILITY_ZONE) {
278278
throw new ValidationError(`AvailabilityZoneBalancing.ENABLED requires that the first placement strategy, if any, be 'spread across "${BuiltInAttributes.AVAILABILITY_ZONE}"'`, this);
279279
}
280280
}
281281

282+
this.strategies = [];
282283
for (const strategy of strategies) {
283284
this.strategies.push(...strategy.toJson());
284285
}

packages/aws-cdk-lib/aws-ecs/test/ec2/ec2-service.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2307,6 +2307,31 @@ describe('ec2 service', () => {
23072307
});
23082308
});
23092309

2310+
test('with empty [] placement strategies', () => {
2311+
// GIVEN
2312+
const stack = new cdk.Stack();
2313+
const vpc = new ec2.Vpc(stack, 'MyVpc', {});
2314+
const cluster = new ecs.Cluster(stack, 'EcsCluster', { vpc });
2315+
addDefaultCapacityProvider(cluster, stack, vpc);
2316+
const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'Ec2TaskDef');
2317+
2318+
taskDefinition.addContainer('web', {
2319+
image: ecs.ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),
2320+
memoryLimitMiB: 512,
2321+
});
2322+
2323+
new ecs.Ec2Service(stack, 'Ec2Service', {
2324+
cluster,
2325+
taskDefinition,
2326+
placementStrategies: [],
2327+
});
2328+
2329+
// THEN
2330+
Template.fromStack(stack).hasResourceProperties('AWS::ECS::Service', {
2331+
PlacementStrategies: Match.arrayEquals([]),
2332+
});
2333+
});
2334+
23102335
testDeprecated('with both propagateTags and propagateTaskTagsFrom defined', () => {
23112336
// GIVEN
23122337
const stack = new cdk.Stack();

0 commit comments

Comments
 (0)