Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(ecs): let AsgCapacityProvider use IAutoScalingGroup only when Managed Termination Protection is disable #30335

Merged
merged 12 commits into from
Jun 25, 2024
Merged

fix(ecs): let AsgCapacityProvider use IAutoScalingGroup only when Managed Termination Protection is disable #30335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
1d78413
Let AsgCapacityProvider use IAutoScalingGroup even when Managed Termi…
scorbiere May 24, 2024
65aa924
Merge branch 'aws:main' into fix_asg_capacity_provider
scorbiere Jun 17, 2024
d0d05dd
fix(core): overrideLogicalId validation (#29708)
nmussy Jun 17, 2024
d85ce7b
Prevent the usage of an imported ASG with Cluster and ASGProvider
scorbiere Jun 18, 2024
1bf9568
Merge branch 'aws:main' into fix_asg_capacity_provider
scorbiere Jun 20, 2024
197e8d8
Fix the doc.
scorbiere Jun 20, 2024
e821362
Some rewording to clarify the test.
scorbiere Jun 21, 2024
7fd9c3c
Update cluster.ts doc string
scorbiere Jun 21, 2024
375dfb5
Merge branch 'main' into fix_asg_capacity_provider
mergify[bot] Jun 24, 2024
1ff1c70
Merge branch 'main' into fix_asg_capacity_provider
scorbiere Jun 24, 2024
d600661
Merge branch 'main' into fix_asg_capacity_provider
paulhcsun Jun 24, 2024
d7d6b58
Merge branch 'main' into fix_asg_capacity_provider
scorbiere Jun 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion packages/aws-cdk-lib/aws-ecs/lib/cluster.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,9 @@ export class Cluster extends Resource implements ICluster {
}

private configureAutoScalingGroup(autoScalingGroup: autoscaling.AutoScalingGroup, options: AddAutoScalingGroupCapacityOptions = {}) {
if (!(autoScalingGroup instanceof autoscaling.AutoScalingGroup)) {
throw new Error('Cannot configure the AutoScalingGroup because it is an imported resource.');
}
if (autoScalingGroup.osType === ec2.OperatingSystemType.WINDOWS) {
this.configureWindowsAutoScalingGroup(autoScalingGroup, options);
} else {
Expand Down Expand Up @@ -1177,6 +1180,10 @@ export interface AsgCapacityProviderProps extends AddAutoScalingGroupCapacityOpt

/**
* The autoscaling group to add as a Capacity Provider.
*
* Warning: When passing an imported resource using `AutoScalingGroup.fromAutoScalingGroupName` along with `enableManagedTerminationProtection: true`,
* the `AsgCapacityProvider` construct will not be able to enforce the option `newInstancesProtectedFromScaleIn` of the `AutoScalingGroup`.
* In this case the constructor of `AsgCapacityProvider` will throw an exception.
*/
readonly autoScalingGroup: autoscaling.IAutoScalingGroup;

Expand Down Expand Up @@ -1306,7 +1313,11 @@ export class AsgCapacityProvider extends Construct {
throw new Error('Cannot enable Managed Termination Protection on a Capacity Provider when Managed Scaling is disabled. Either enable Managed Scaling or disable Managed Termination Protection.');
}
if (this.enableManagedTerminationProtection) {
this.autoScalingGroup.protectNewInstancesFromScaleIn();
if (this.autoScalingGroup instanceof autoscaling.AutoScalingGroup) {
this.autoScalingGroup.protectNewInstancesFromScaleIn();
} else {
throw new Error('Cannot enable Managed Termination Protection on a Capacity Provider when providing an imported AutoScalingGroup.');
}
}

const capacityProviderNameRegex = /^(?!aws|ecs|fargate).+/gm;
Expand Down
132 changes: 101 additions & 31 deletions packages/aws-cdk-lib/aws-ecs/test/cluster.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import * as logs from '../../aws-logs';
import * as s3 from '../../aws-s3';
import * as cloudmap from '../../aws-servicediscovery';
import * as cdk from '../../core';
import { getWarnings } from '../../core/test/util';
import * as cxapi from '../../cx-api';
import * as ecs from '../lib';

Expand Down Expand Up @@ -2194,36 +2195,76 @@ describe('cluster', () => {

});

test('creates ASG capacity providers with expected defaults', () => {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'test');
const vpc = new ec2.Vpc(stack, 'Vpc');
const autoScalingGroup = new autoscaling.AutoScalingGroup(stack, 'asg', {
vpc,
instanceType: new ec2.InstanceType('bogus'),
machineImage: ecs.EcsOptimizedImage.amazonLinux2(),
describe('creates ASG capacity providers ', () => {
test('with expected defaults', () => {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'test');
const vpc = new ec2.Vpc(stack, 'Vpc');
const autoScalingGroup = new autoscaling.AutoScalingGroup(stack, 'asg', {
vpc,
instanceType: new ec2.InstanceType('bogus'),
machineImage: ecs.EcsOptimizedImage.amazonLinux2(),
});

// WHEN
new ecs.AsgCapacityProvider(stack, 'provider', {
autoScalingGroup,
});

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::ECS::CapacityProvider', {
AutoScalingGroupProvider: {
AutoScalingGroupArn: {
Ref: 'asgASG4D014670',
},
ManagedScaling: {
Status: 'ENABLED',
TargetCapacity: 100,
},
ManagedTerminationProtection: 'ENABLED',
},
});
});

// WHEN
new ecs.AsgCapacityProvider(stack, 'provider', {
autoScalingGroup,
test('with IAutoScalingGroup should throw an error if Managed Termination Protection is enabled.', () => {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'test');
const autoScalingGroup = autoscaling.AutoScalingGroup.fromAutoScalingGroupName(stack, 'ASG', 'my-asg');

// THEN
expect(() => {
new ecs.AsgCapacityProvider(stack, 'provider', {
autoScalingGroup,
});
}).toThrow('Cannot enable Managed Termination Protection on a Capacity Provider when providing an imported AutoScalingGroup.');
});

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::ECS::CapacityProvider', {
AutoScalingGroupProvider: {
AutoScalingGroupArn: {
Ref: 'asgASG4D014670',
},
ManagedScaling: {
Status: 'ENABLED',
TargetCapacity: 100,
test('with IAutoScalingGroup should not throw an error if Managed Termination Protection is disabled.', () => {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'test');
const autoScalingGroup = autoscaling.AutoScalingGroup.fromAutoScalingGroupName(stack, 'ASG', 'my-asg');

// WHEN
new ecs.AsgCapacityProvider(stack, 'provider', {
autoScalingGroup,
enableManagedTerminationProtection: false,
});

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::ECS::CapacityProvider', {
AutoScalingGroupProvider: {
AutoScalingGroupArn: 'my-asg',
ManagedScaling: {
Status: 'ENABLED',
TargetCapacity: 100,
},
ManagedTerminationProtection: 'DISABLED',
},
ManagedTerminationProtection: 'ENABLED',
},
});
});

});

test('can disable Managed Scaling and Managed Termination Protection for ASG capacity provider', () => {
Expand Down Expand Up @@ -2483,6 +2524,23 @@ describe('cluster', () => {

});

test('throws when calling Cluster.addAsgCapacityProvider with an AsgCapacityProvider created with an imported ASG', () => {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'test');
const importedAsg = autoscaling.AutoScalingGroup.fromAutoScalingGroupName(stack, 'ASG', 'my-asg');
const cluster = new ecs.Cluster(stack, 'EcsCluster');

const capacityProvider = new ecs.AsgCapacityProvider(stack, 'provider', {
autoScalingGroup: importedAsg,
enableManagedTerminationProtection: false,
pahud marked this conversation as resolved.
Show resolved Hide resolved
});
// THEN
expect(() => {
cluster.addAsgCapacityProvider(capacityProvider);
}).toThrow('Cannot configure the AutoScalingGroup because it is an imported resource.');
});

test('should throw an error if capacity provider with default strategy is not present in capacity providers', () => {
// GIVEN
const app = new cdk.App();
Expand Down Expand Up @@ -3042,11 +3100,17 @@ test('throws when InstanceWarmupPeriod is greater than 10000', () => {
describe('Accessing container instance role', function () {

const addUserDataMock = jest.fn();
const autoScalingGroup: autoscaling.AutoScalingGroup = {
addUserData: addUserDataMock,
addToRolePolicy: jest.fn(),
protectNewInstancesFromScaleIn: jest.fn(),
} as unknown as autoscaling.AutoScalingGroup;

function getAutoScalingGroup(stack: cdk.Stack): autoscaling.AutoScalingGroup {
const vpc = new ec2.Vpc(stack, 'Vpc');
const asg = new autoscaling.AutoScalingGroup(stack, 'asg', {
vpc,
instanceType: new ec2.InstanceType('bogus'),
machineImage: ecs.EcsOptimizedImage.amazonLinux2(),
});
asg.addUserData = addUserDataMock;
return asg;
}

afterEach(() => {
addUserDataMock.mockClear();
Expand All @@ -3057,11 +3121,12 @@ describe('Accessing container instance role', function () {
const app = new cdk.App();
const stack = new cdk.Stack(app, 'test');
const cluster = new ecs.Cluster(stack, 'EcsCluster');
const autoScalingGroup = getAutoScalingGroup(stack);

// WHEN

const capacityProvider = new ecs.AsgCapacityProvider(stack, 'Provider', {
autoScalingGroup: autoScalingGroup,
autoScalingGroup,
});

cluster.addAsgCapacityProvider(capacityProvider);
Expand All @@ -3077,10 +3142,11 @@ describe('Accessing container instance role', function () {
const app = new cdk.App();
const stack = new cdk.Stack(app, 'test');
const cluster = new ecs.Cluster(stack, 'EcsCluster');
const autoScalingGroup = getAutoScalingGroup(stack);

// WHEN
const capacityProvider = new ecs.AsgCapacityProvider(stack, 'Provider', {
autoScalingGroup: autoScalingGroup,
autoScalingGroup,
});

cluster.addAsgCapacityProvider(capacityProvider, {
Expand All @@ -3098,6 +3164,7 @@ describe('Accessing container instance role', function () {
const app = new cdk.App();
const stack = new cdk.Stack(app, 'test');
const cluster = new ecs.Cluster(stack, 'EcsCluster');
const autoScalingGroup = getAutoScalingGroup(stack);

// WHEN
const capacityProvider = new ecs.AsgCapacityProvider(stack, 'Provider', {
Expand All @@ -3118,6 +3185,7 @@ describe('Accessing container instance role', function () {
const app = new cdk.App();
const stack = new cdk.Stack(app, 'test');
const cluster = new ecs.Cluster(stack, 'EcsCluster');
const autoScalingGroup = getAutoScalingGroup(stack);

// WHEN
const capacityProvider = new ecs.AsgCapacityProvider(stack, 'Provider', {
Expand All @@ -3140,6 +3208,7 @@ describe('Accessing container instance role', function () {
const app = new cdk.App();
const stack = new cdk.Stack(app, 'test');
const cluster = new ecs.Cluster(stack, 'EcsCluster');
const autoScalingGroup = getAutoScalingGroup(stack);

// WHEN
const capacityProvider = new ecs.AsgCapacityProvider(stack, 'Provider', {
Expand All @@ -3162,6 +3231,7 @@ describe('Accessing container instance role', function () {
const app = new cdk.App();
const stack = new cdk.Stack(app, 'test');
const cluster = new ecs.Cluster(stack, 'EcsCluster');
const autoScalingGroup = getAutoScalingGroup(stack);

// WHEN
const capacityProvider = new ecs.AsgCapacityProvider(stack, 'Provider', {
Expand Down