Skip to content

Commit 5bb58e4

Browse files
kichikkichik
authored
fix: Custom capacity provider for ECS was broken (#336)
Fixes #333
1 parent c7313ac commit 5bb58e4

File tree

3 files changed

+113
-48
lines changed

3 files changed

+113
-48
lines changed

‎API.md‎

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

‎src/providers/ecs.ts‎

Lines changed: 51 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,8 @@ export interface EcsRunnerProviderProps extends RunnerProviderProps {
8383
/**
8484
* Existing capacity provider to use.
8585
*
86+
* Make sure the AMI used by the capacity provider is compatible with ECS.
87+
*
8688
* @default new capacity provider
8789
*/
8890
readonly capacityProvider?: ecs.AsgCapacityProvider;
@@ -325,54 +327,59 @@ export class EcsRunnerProvider extends BaseProvider implements IRunnerProvider {
325327
const imageBuilder = props?.imageBuilder ?? EcsRunnerProvider.imageBuilder(this, 'Image Builder');
326328
const image = this.image = imageBuilder.bindDockerImage();
327329

328-
if (props?.capacityProvider && (props?.minInstances || props?.maxInstances || props?.instanceType || props?.storageSize)) {
329-
cdk.Annotations.of(this).addWarning('When using a custom capacity provider, minInstances, maxInstances, instanceType and storageSize will be ignored.');
330-
}
330+
if (props?.capacityProvider) {
331+
if (props?.minInstances || props?.maxInstances || props?.instanceType || props?.storageSize || props?.spot || props?.spotMaxPrice) {
332+
cdk.Annotations.of(this).addWarning('When using a custom capacity provider, minInstances, maxInstances, instanceType, storageSize, spot, and spotMaxPrice will be ignored.');
333+
}
331334

332-
const spot = props?.spot ?? props?.spotMaxPrice !== undefined;
333-
334-
const launchTemplate = new ec2.LaunchTemplate(this, 'Launch Template', {
335-
machineImage: this.defaultClusterInstanceAmi(),
336-
instanceType: props?.instanceType ?? this.defaultClusterInstanceType(),
337-
blockDevices: props?.storageSize ? [
338-
{
339-
deviceName: '/dev/sda1',
340-
volume: {
341-
ebsDevice: {
342-
volumeSize: props?.storageSize?.toGibibytes(),
343-
deleteOnTermination: true,
335+
this.capacityProvider = props.capacityProvider;
336+
} else {
337+
const spot = props?.spot ?? props?.spotMaxPrice !== undefined;
338+
339+
const launchTemplate = new ec2.LaunchTemplate(this, 'Launch Template', {
340+
machineImage: this.defaultClusterInstanceAmi(),
341+
instanceType: props?.instanceType ?? this.defaultClusterInstanceType(),
342+
blockDevices: props?.storageSize ? [
343+
{
344+
deviceName: '/dev/sda1',
345+
volume: {
346+
ebsDevice: {
347+
volumeSize: props?.storageSize?.toGibibytes(),
348+
deleteOnTermination: true,
349+
},
344350
},
345351
},
346-
},
347-
] : undefined,
348-
spotOptions: spot ? {
349-
requestType: ec2.SpotRequestType.ONE_TIME,
350-
maxPrice: props?.spotMaxPrice ? parseFloat(props?.spotMaxPrice) : undefined,
351-
} : undefined,
352-
requireImdsv2: true,
353-
securityGroup: this.securityGroups[0],
354-
role: new iam.Role(this, 'Launch Template Role', {
355-
assumedBy: new iam.ServicePrincipal('ec2.amazonaws.com'),
356-
}),
357-
userData: ec2.UserData.forOperatingSystem(image.os.is(Os.WINDOWS) ? ec2.OperatingSystemType.WINDOWS : ec2.OperatingSystemType.LINUX),
358-
});
359-
this.securityGroups.slice(1).map(sg => launchTemplate.connections.addSecurityGroup(sg));
360-
361-
const autoScalingGroup = new autoscaling.AutoScalingGroup(this, 'Auto Scaling Group', {
362-
vpc: this.vpc,
363-
launchTemplate,
364-
vpcSubnets: this.subnetSelection,
365-
minCapacity: props?.minInstances ?? 0,
366-
maxCapacity: props?.maxInstances ?? 5,
367-
});
368-
autoScalingGroup.addUserData(this.loginCommand(), this.pullCommand());
369-
autoScalingGroup.role.addManagedPolicy(iam.ManagedPolicy.fromAwsManagedPolicyName('AmazonSSMManagedInstanceCore'));
370-
image.imageRepository.grantPull(autoScalingGroup);
352+
] : undefined,
353+
spotOptions: spot ? {
354+
requestType: ec2.SpotRequestType.ONE_TIME,
355+
maxPrice: props?.spotMaxPrice ? parseFloat(props?.spotMaxPrice) : undefined,
356+
} : undefined,
357+
requireImdsv2: true,
358+
securityGroup: this.securityGroups[0],
359+
role: new iam.Role(this, 'Launch Template Role', {
360+
assumedBy: new iam.ServicePrincipal('ec2.amazonaws.com'),
361+
}),
362+
userData: ec2.UserData.forOperatingSystem(image.os.is(Os.WINDOWS) ? ec2.OperatingSystemType.WINDOWS : ec2.OperatingSystemType.LINUX),
363+
});
364+
this.securityGroups.slice(1).map(sg => launchTemplate.connections.addSecurityGroup(sg));
371365

372-
this.capacityProvider = props?.capacityProvider ?? new ecs.AsgCapacityProvider(this, 'Capacity Provider', {
373-
autoScalingGroup,
374-
spotInstanceDraining: false, // waste of money to restart jobs as the restarted job won't have a token
375-
});
366+
const autoScalingGroup = new autoscaling.AutoScalingGroup(this, 'Auto Scaling Group', {
367+
vpc: this.vpc,
368+
launchTemplate,
369+
vpcSubnets: this.subnetSelection,
370+
minCapacity: props?.minInstances ?? 0,
371+
maxCapacity: props?.maxInstances ?? 5,
372+
});
373+
374+
this.capacityProvider = props?.capacityProvider ?? new ecs.AsgCapacityProvider(this, 'Capacity Provider', {
375+
autoScalingGroup,
376+
spotInstanceDraining: false, // waste of money to restart jobs as the restarted job won't have a token
377+
});
378+
}
379+
380+
this.capacityProvider.autoScalingGroup.addUserData(this.loginCommand(), this.pullCommand());
381+
this.capacityProvider.autoScalingGroup.role.addManagedPolicy(iam.ManagedPolicy.fromAwsManagedPolicyName('AmazonSSMManagedInstanceCore'));
382+
image.imageRepository.grantPull(this.capacityProvider.autoScalingGroup);
376383

377384
this.cluster.addAsgCapacityProvider(
378385
this.capacityProvider,

‎test/providers.test.ts‎

Lines changed: 60 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
import * as cdk from 'aws-cdk-lib';
2-
import {
3-
aws_ec2 as ec2,
4-
} from 'aws-cdk-lib';
2+
import { aws_ec2 as ec2, aws_ecs as ecs } from 'aws-cdk-lib';
53
import { Match, Template } from 'aws-cdk-lib/assertions';
6-
import { CodeBuildRunnerProvider, FargateRunnerProvider, LambdaRunnerProvider } from '../src';
4+
import * as autoscaling from 'aws-cdk-lib/aws-autoscaling';
5+
import { CodeBuildRunnerProvider, EcsRunnerProvider, FargateRunnerProvider, LambdaRunnerProvider } from '../src';
76

87
test('CodeBuild provider', () => {
98
const app = new cdk.App();
@@ -90,3 +89,60 @@ test('Fargate provider', () => {
9089
],
9190
}));
9291
});
92+
93+
describe('ECS provider', () => {
94+
test('Basic', () => {
95+
const app = new cdk.App();
96+
const stack = new cdk.Stack(app, 'test');
97+
98+
const vpc = new ec2.Vpc(stack, 'vpc');
99+
const sg = new ec2.SecurityGroup(stack, 'sg', { vpc });
100+
101+
new EcsRunnerProvider(stack, 'provider', {
102+
vpc: vpc,
103+
securityGroups: [sg],
104+
});
105+
106+
const template = Template.fromStack(stack);
107+
108+
template.resourceCountIs('AWS::ECS::Cluster', 1);
109+
template.resourceCountIs('AWS::AutoScaling::AutoScalingGroup', 1);
110+
111+
template.hasResourceProperties('AWS::ECS::TaskDefinition', Match.objectLike({
112+
NetworkMode: 'bridge',
113+
RequiresCompatibilities: ['EC2'],
114+
ContainerDefinitions: [
115+
{
116+
Name: 'runner',
117+
},
118+
],
119+
}));
120+
});
121+
122+
test('Custom capacity provider', () => {
123+
const app = new cdk.App();
124+
const stack = new cdk.Stack(app, 'test');
125+
126+
const vpc = new ec2.Vpc(stack, 'vpc');
127+
const sg = new ec2.SecurityGroup(stack, 'sg', { vpc });
128+
129+
new EcsRunnerProvider(stack, 'provider', {
130+
vpc: vpc,
131+
securityGroups: [sg],
132+
capacityProvider: new ecs.AsgCapacityProvider(stack, 'Capacity Provider', {
133+
autoScalingGroup: new autoscaling.AutoScalingGroup(stack, 'Auto Scaling Group', {
134+
vpc: vpc,
135+
instanceType: ec2.InstanceType.of(ec2.InstanceClass.T3, ec2.InstanceSize.MICRO),
136+
machineImage: ecs.EcsOptimizedImage.amazonLinux2(),
137+
minCapacity: 1,
138+
maxCapacity: 3,
139+
}),
140+
}),
141+
});
142+
143+
const template = Template.fromStack(stack);
144+
145+
// don't create our own autoscaling group when capacity provider is specified
146+
template.resourceCountIs('AWS::AutoScaling::AutoScalingGroup', 1);
147+
});
148+
});

0 commit comments

Comments
 (0)