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.

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

chore(ecs-service-extensions): Deprecate scale on CPU utilization extension #17802

Merged
merged 10 commits into from
Jan 12, 2022
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,13 @@ import * as ecs from '@aws-cdk/aws-ecs';
import * as cdk from '@aws-cdk/core';
import { ServiceExtension, ServiceBuild } from './extension-interfaces';


/**
* The autoscaling settings.
*
* @deprecated use the `minTaskCount` and `maxTaskCount` properties of `autoScaleTaskCount` in the `Service` construct
* to configure the auto scaling target for the service. For more information, please refer
* https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk-containers/ecs-service-extensions/README.md#task-auto-scaling .
*/
export interface CpuScalingProps {
/**
Expand Down Expand Up @@ -61,6 +66,9 @@ const cpuScalingPropsDefault = {

/**
* This extension helps you scale your service according to CPU utilization.
*
* @deprecated To enable target tracking based on CPU utilization, use the `targetCpuUtilization` property of `autoScaleTaskCount` in the `Service` construct.
* For more information, please refer https://github.com/aws/aws-cdk/blob/master/packages/%40aws-cdk-containers/ecs-service-extensions/README.md#task-auto-scaling .
*/
export class ScaleOnCpuUtilization extends ServiceExtension {
/**
Expand Down Expand Up @@ -126,6 +134,9 @@ export class ScaleOnCpuUtilization extends ServiceExtension {
// This hook utilizes the resulting service construct
// once it is created.
public useService(service: ecs.Ec2Service | ecs.FargateService) {
if (this.parentService.scalableTaskCount) {
throw Error('Cannot specify \'minTaskCount\' or \'maxTaskCount\' in the Service construct and also provide a \'ScaleOnCpuUtilization\' extension. \'ScaleOnCpuUtilization\' is deprecated. Please only provide \'minTaskCount\' and \'maxTaskCount\'.');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realized we can make this error a little bit shorter and clearer by referring to the property name that contains minTaskCount and maxTaskCount.

Suggested change
throw Error('Cannot specify \'minTaskCount\' or \'maxTaskCount\' in the Service construct and also provide a \'ScaleOnCpuUtilization\' extension. \'ScaleOnCpuUtilization\' is deprecated. Please only provide \'minTaskCount\' and \'maxTaskCount\'.');
throw Error('Cannot specify \'autoScaleTaskCount\' in the Service construct and also provide a \'ScaleOnCpuUtilization\' extension. \'ScaleOnCpuUtilization\' is deprecated. Please only provide \'autoScaleTaskCount\'.');

}
const scalingTarget = service.autoScaleTaskCount({
minCapacity: this.minTaskCount,
maxCapacity: this.maxTaskCount,
Expand All @@ -136,5 +147,6 @@ export class ScaleOnCpuUtilization extends ServiceExtension {
scaleInCooldown: this.scaleInCooldown,
scaleOutCooldown: this.scaleOutCooldown,
});
this.parentService.enableAutoScalingPolicy();
}
}
25 changes: 15 additions & 10 deletions packages/@aws-cdk-containers/ecs-service-extensions/lib/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,17 +221,14 @@ export class Service extends Construct {
}
}

// Set desiredCount to `undefined` if auto scaling is configured for the service
const desiredCount = props.autoScaleTaskCount ? undefined : (props.desiredCount || 1);

// Give each extension a chance to mutate the service props before
// service creation
let serviceProps = {
cluster: this.cluster,
taskDefinition: this.taskDefinition,
minHealthyPercent: 100,
maxHealthyPercent: 200,
desiredCount,
desiredCount: props.desiredCount ?? 1,
} as ServiceBuild;

for (const extensions in this.serviceDescription.extensions) {
Expand Down Expand Up @@ -273,6 +270,14 @@ export class Service extends Construct {
}
}

// Set desiredCount to `undefined` if auto scaling is configured for the service
if (props.autoScaleTaskCount || this.autoScalingPoliciesEnabled) {
serviceProps = {
...serviceProps,
desiredCount: undefined,
};
}

// Now that the service props are determined we can create
// the service
if (this.capacityType === EnvironmentCapacityType.EC2) {
Expand All @@ -291,17 +296,17 @@ export class Service extends Construct {
});

if (props.autoScaleTaskCount.targetCpuUtilization) {
const targetUtilizationPercent = props.autoScaleTaskCount.targetCpuUtilization;
this.scalableTaskCount.scaleOnCpuUtilization(`${this.id}-target-cpu-utilization-${targetUtilizationPercent}`, {
targetUtilizationPercent,
const targetCpuUtilizationPercent = props.autoScaleTaskCount.targetCpuUtilization;
this.scalableTaskCount.scaleOnCpuUtilization(`${this.id}-target-cpu-utilization-${targetCpuUtilizationPercent}`, {
targetUtilizationPercent: targetCpuUtilizationPercent,
});
this.enableAutoScalingPolicy();
}

if (props.autoScaleTaskCount.targetMemoryUtilization) {
const targetUtilizationPercent = props.autoScaleTaskCount.targetMemoryUtilization;
this.scalableTaskCount.scaleOnMemoryUtilization(`${this.id}-target-memory-utilization-${targetUtilizationPercent}`, {
targetUtilizationPercent,
const targetMemoryUtilizationPercent = props.autoScaleTaskCount.targetMemoryUtilization;
this.scalableTaskCount.scaleOnMemoryUtilization(`${this.id}-target-memory-utilization-${targetMemoryUtilizationPercent}`, {
targetUtilizationPercent: targetMemoryUtilizationPercent,
});
this.enableAutoScalingPolicy();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import '@aws-cdk/assert-internal/jest';
import * as appmesh from '@aws-cdk/aws-appmesh';
import * as ecs from '@aws-cdk/aws-ecs';
import * as cdk from '@aws-cdk/core';
import { AppMeshExtension, Container, Environment, ScaleOnCpuUtilization, ServiceDescription, Service } from '../lib';
import { AppMeshExtension, Container, Environment, ServiceDescription, Service } from '../lib';

describe('appmesh', () => {
test('should be able to add AWS App Mesh to a service', () => {
Expand Down Expand Up @@ -276,9 +276,6 @@ describe('appmesh', () => {
trafficPort: 80,
image: ecs.ContainerImage.fromRegistry('nathanpeck/name'),
}));
serviceDescription.add(new ScaleOnCpuUtilization({
initialTaskCount: 1,
}));

const mesh = new appmesh.Mesh(stack, 'my-mesh');

Expand All @@ -289,6 +286,7 @@ describe('appmesh', () => {
new Service(stack, 'my-service', {
environment,
serviceDescription,
desiredCount: 1,
});

expect(stack).toHaveResourceLike('AWS::ECS::Service', {
Expand Down Expand Up @@ -317,9 +315,6 @@ describe('appmesh', () => {
trafficPort: 80,
image: ecs.ContainerImage.fromRegistry('nathanpeck/name'),
}));
serviceDescription.add(new ScaleOnCpuUtilization({
initialTaskCount: 2,
}));

const mesh = new appmesh.Mesh(stack, 'my-mesh');

Expand All @@ -330,6 +325,7 @@ describe('appmesh', () => {
new Service(stack, 'my-service', {
environment,
serviceDescription,
desiredCount: 2,
});

expect(stack).toHaveResourceLike('AWS::ECS::Service', {
Expand Down Expand Up @@ -358,9 +354,6 @@ describe('appmesh', () => {
trafficPort: 80,
image: ecs.ContainerImage.fromRegistry('nathanpeck/name'),
}));
serviceDescription.add(new ScaleOnCpuUtilization({
initialTaskCount: 3,
}));

const mesh = new appmesh.Mesh(stack, 'my-mesh');

Expand All @@ -371,6 +364,7 @@ describe('appmesh', () => {
new Service(stack, 'my-service', {
environment,
serviceDescription,
desiredCount: 3,
});

expect(stack).toHaveResourceLike('AWS::ECS::Service', {
Expand Down Expand Up @@ -399,9 +393,6 @@ describe('appmesh', () => {
trafficPort: 80,
image: ecs.ContainerImage.fromRegistry('nathanpeck/name'),
}));
serviceDescription.add(new ScaleOnCpuUtilization({
initialTaskCount: 4,
}));

const mesh = new appmesh.Mesh(stack, 'my-mesh');

Expand All @@ -412,6 +403,7 @@ describe('appmesh', () => {
new Service(stack, 'my-service', {
environment,
serviceDescription,
desiredCount: 4,
});

expect(stack).toHaveResourceLike('AWS::ECS::Service', {
Expand Down Expand Up @@ -440,9 +432,6 @@ describe('appmesh', () => {
trafficPort: 80,
image: ecs.ContainerImage.fromRegistry('nathanpeck/name'),
}));
serviceDescription.add(new ScaleOnCpuUtilization({
initialTaskCount: 8,
}));

const mesh = new appmesh.Mesh(stack, 'my-mesh');

Expand All @@ -453,6 +442,7 @@ describe('appmesh', () => {
new Service(stack, 'my-service', {
environment,
serviceDescription,
desiredCount: 8,
});

expect(stack).toHaveResourceLike('AWS::ECS::Service', {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,4 +149,32 @@ describe('scale on cpu utilization', () => {

});

test('should error if configuring autoscaling target both in the extension and the Service', () => {
// GIVEN
const stack = new cdk.Stack();

// WHEN
const environment = new Environment(stack, 'production');
const serviceDescription = new ServiceDescription();

serviceDescription.add(new Container({
cpu: 256,
memoryMiB: 512,
trafficPort: 80,
image: ecs.ContainerImage.fromRegistry('nathanpeck/name'),
}));

serviceDescription.add(new ScaleOnCpuUtilization());
// THEN
expect(() => {
new Service(stack, 'my-service', {
environment,
serviceDescription,
autoScaleTaskCount: {
maxTaskCount: 5,
},
});
}).toThrow('Cannot specify \'minTaskCount\' or \'maxTaskCount\' in the Service construct and also provide a \'ScaleOnCpuUtilization\' extension. \'ScaleOnCpuUtilization\' is deprecated. Please only provide \'minTaskCount\' and \'maxTaskCount\'.');
});

});