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 1 commit
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
15 changes: 13 additions & 2 deletions 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 @@ -11,7 +11,7 @@ import * as kms from '../../aws-kms';
import * as logs from '../../aws-logs';
import * as s3 from '../../aws-s3';
import * as cloudmap from '../../aws-servicediscovery';
import { Duration, IResource, Resource, Stack, Aspects, ArnFormat, IAspect, Token, Names } from '../../core';
import { Duration, IResource, Resource, Stack, Aspects, ArnFormat, IAspect, Token, Names, Annotations } from '../../core';

const CLUSTER_SYMBOL = Symbol.for('@aws-cdk/aws-ecs/lib/cluster.Cluster');

Expand Down 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`,
* the AsgCapacityProvider construct will not be able to enforce the option `newInstancesProtectedFromScaleIn` of the AutoScalingGroup will be enabled.
* If this property is not enable in the AutoScalingGroup, the CFN stack execution will fail.
*/
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 {
Annotations.of(this).addWarningV2('@aws-cdk/aws-ecs:cannotEnforceNewInstancesProtectedFromScaleIn', 'Cannot enforce `newInstancesProtectedFromScaleIn: true` on the AutoScalingGroup. This will have no effect if the AutoScalingGroup was created in this CDK app.');
}
}

const capacityProviderNameRegex = /^(?!aws|ecs|fargate).+/gm;
Expand Down
136 changes: 105 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,81 @@ 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 emit a warning 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');

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

// THEN
expect(getWarnings(app.synth())).toEqual([{
message: 'Cannot enforce `newInstancesProtectedFromScaleIn: true` on the AutoScalingGroup. This will have no effect if the AutoScalingGroup was created in this CDK app. [ack: @aws-cdk/aws-ecs:cannotEnforceNewInstancesProtectedFromScaleIn]',
path: '/test/provider',
}]);
});

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::ECS::CapacityProvider', {
AutoScalingGroupProvider: {
AutoScalingGroupArn: {
Ref: 'asgASG4D014670',
},
ManagedScaling: {
Status: 'ENABLED',
TargetCapacity: 100,
test('with IAutoScalingGroup should not emit warning 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',
},
});
expect(getWarnings(app.synth())).toEqual([]);
});

});

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

});

test('should throw an error if the capacity provider was created using an imported AsgCapacityProvider', () => {
// GIVEN
const app = new cdk.App();
const stack = new cdk.Stack(app, 'test');
const autoScalingGroup = autoscaling.AutoScalingGroup.fromAutoScalingGroupName(stack, 'ASG', 'my-asg');
const cluster = new ecs.Cluster(stack, 'EcsCluster');

const capacityProvider = new ecs.AsgCapacityProvider(stack, 'provider', {
autoScalingGroup,
});
// 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 +3104,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 +3125,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 +3146,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 +3168,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 +3189,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 +3212,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 +3235,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
Loading