Updated to reflect review feedback

aws · May 11, 2021 · 5876422 · 5876422
1 parent 3627bd9
commit 5876422
Show file tree

Hide file tree

Showing 5 changed files with 33 additions and 36 deletions.
diff --git a/packages/@aws-cdk/aws-ecs/README.md b/packages/@aws-cdk/aws-ecs/README.md
@@ -172,7 +172,7 @@ To add spot instances into the cluster, you must specify the `spotPrice` in the
 
 ```ts
 // Add an AutoScalingGroup with spot instances to the existing cluster
-cluster.addCapacity('AsgSpot', {
+cluster.addCapacity('ASGSpot', {
   maxCapacity: 2,
   minCapacity: 2,
   desiredCapacity: 2,
@@ -790,12 +790,12 @@ const autoScalingGroup = new autoscaling.AutoScalingGroup(stack, 'ASG', {
   maxCapacity: 100,
 });
 
-const capacityProvider = new ecs.EC2CapacityProvider(stack, 'EC2CapacityProvider', {
+const capacityProvider = new ecs.AsgCapacityProvider(stack, 'AsgCapacityProvider', {
   autoScalingGroup,
 });
-cluster.addEC2CapacityProvider(capacityProvider);
+cluster.addAsgCapacityProvider(capacityProvider);
 
-const taskDefinition = new ecs.EC2TaskDefinition(stack, 'TaskDef');
+const taskDefinition = new ecs.Ec2TaskDefinition(stack, 'TaskDef');
 
 taskDefinition.addContainer('web', {
   image: ecs.ContainerImage.fromRegistry('amazon/amazon-ecs-sample'),    memoryReservationMiB: 256,
@@ -818,7 +818,7 @@ new ecs.Ec2Service(stack, 'EC2Service', {
 Currently, this feature is only supported for services with EC2 launch types.
 
 To add elastic inference accelerators to your EC2 instance, first add
-`inferenceAccelerators` field to the EC2TaskDefinition and set the `deviceName`
+`inferenceAccelerators` field to the Ec2TaskDefinition and set the `deviceName`
 and `deviceType` properties.
 
 ```ts

diff --git a/packages/@aws-cdk/aws-ecs/lib/cluster.ts b/packages/@aws-cdk/aws-ecs/lib/cluster.ts
@@ -124,7 +124,7 @@ export class Cluster extends Resource implements ICluster {
   /**
    * The EC2 Auto Scaling Group capacity providers associated with the cluster.
    */
-  private _ec2CapacityProviders: EC2CapacityProvider[] = [];
+  private _asgCapacityProviders: AsgCapacityProvider[] = [];
 
   /**
    * The AWS Cloud Map namespace to associate with the cluster.
@@ -193,7 +193,7 @@ export class Cluster extends Resource implements ICluster {
     // since it's harmless, but we'd prefer not to add unexpected new
     // resources to the stack which could surprise users working with
     // brown-field CDK apps and stacks.
-    Aspects.of(this).add(new MaybeCreateCapacityProviderAssociations(this, id, this._ec2CapacityProviders));
+    Aspects.of(this).add(new MaybeCreateCapacityProviderAssociations(this, id, this._asgCapacityProviders));
   }
 
   /**
@@ -288,14 +288,13 @@ export class Cluster extends Resource implements ICluster {
   }
 
   /**
-   * This method adds an Auto Scaling Group Capacity Provider to a cluster using
-   * the specified EC2 Capacity Provider.
+   * This method adds an Auto Scaling Group Capacity Provider to a cluster.
    *
    * @param provider the capacity provider to add to this cluster.
    */
-  public addEC2CapacityProvider(provider: EC2CapacityProvider, options: AddAutoScalingGroupCapacityOptions = {}) {
+  public addAsgCapacityProvider(provider: AsgCapacityProvider, options: AddAutoScalingGroupCapacityOptions = {}) {
     // Don't add the same capacity provider more than once.
-    if (this._ec2CapacityProviders.includes(provider)) {
+    if (this._asgCapacityProviders.includes(provider)) {
       return;
     }
 
@@ -306,7 +305,7 @@ export class Cluster extends Resource implements ICluster {
       taskDrainTime: provider.enableManagedTerminationProtection ? Duration.seconds(0) : options.taskDrainTime,
     });
 
-    this._ec2CapacityProviders.push(provider);
+    this._asgCapacityProviders.push(provider);
   }
 
   /**
@@ -1062,7 +1061,7 @@ capacity provider. The weight value is taken into consideration after the base v
 /**
  * The options for creating an EC2 Capacity Provider.
  */
-export interface EC2CapacityProviderProps extends AddAutoScalingGroupCapacityOptions {
+export interface AsgCapacityProviderProps extends AddAutoScalingGroupCapacityOptions {
   /**
    * The name for the capacity provider.
    *
@@ -1118,7 +1117,7 @@ export interface EC2CapacityProviderProps extends AddAutoScalingGroupCapacityOpt
  * ensure that instances are not prematurely terminated while there are still tasks
  * running on them.
  */
-export class EC2CapacityProvider extends CoreConstruct {
+export class AsgCapacityProvider extends CoreConstruct {
   /**
    * Capacity provider name
    * @default Chosen by CloudFormation
@@ -1135,7 +1134,7 @@ export class EC2CapacityProvider extends CoreConstruct {
    */
   readonly enableManagedTerminationProtection?: boolean;
 
-  constructor(scope: Construct, id: string, props: EC2CapacityProviderProps) {
+  constructor(scope: Construct, id: string, props: AsgCapacityProviderProps) {
     super(scope, id);
 
     this.autoScalingGroup = props.autoScalingGroup as autoscaling.AutoScalingGroup;
@@ -1151,8 +1150,8 @@ export class EC2CapacityProvider extends CoreConstruct {
       name: props.capacityProviderName,
       autoScalingGroupProvider: {
         autoScalingGroupArn: this.autoScalingGroup.autoScalingGroupName,
-        managedScaling: {
-          status: props.enableManagedScaling === false ? 'DISABLED' : 'ENABLED',
+        managedScaling: props.enableManagedScaling === false ? undefined : {
+          status: 'ENABLED',
           targetCapacity: props.targetCapacityPercent || 100,
           maximumScalingStepSize: props.maximumScalingStepSize,
           minimumScalingStepSize: props.minimumScalingStepSize,
@@ -1172,9 +1171,9 @@ export class EC2CapacityProvider extends CoreConstruct {
 class MaybeCreateCapacityProviderAssociations implements IAspect {
   private scope: CoreConstruct;
   private id: string;
-  private capacityProviders: EC2CapacityProvider[]
+  private capacityProviders: AsgCapacityProvider[]
 
-  constructor(scope: CoreConstruct, id: string, capacityProviders: EC2CapacityProvider[]) {
+  constructor(scope: CoreConstruct, id: string, capacityProviders: AsgCapacityProvider[]) {
     this.scope = scope;
     this.id = id;
     this.capacityProviders = capacityProviders;

diff --git a/packages/@aws-cdk/aws-ecs/test/cluster.test.ts b/packages/@aws-cdk/aws-ecs/test/cluster.test.ts
@@ -4,6 +4,7 @@ import {
   haveResource,
   haveResourceLike,
   ResourcePart,
+  ABSENT,
 } from '@aws-cdk/assert-internal';
 import * as autoscaling from '@aws-cdk/aws-autoscaling';
 import * as ec2 from '@aws-cdk/aws-ec2';
@@ -1809,7 +1810,7 @@ nodeunitShim({
     });
 
     // WHEN
-    new ecs.EC2CapacityProvider(stack, 'provider', {
+    new ecs.AsgCapacityProvider(stack, 'provider', {
       autoScalingGroup,
     });
 
@@ -1829,7 +1830,7 @@ nodeunitShim({
     test.done();
   },
 
-  'can disable managed scaling for EC2 capacity provider'(test: Test) {
+  'can disable managed scaling for ASG capacity provider'(test: Test) {
     // GIVEN
     const app = new cdk.App();
     const stack = new cdk.Stack(app, 'test');
@@ -1841,7 +1842,7 @@ nodeunitShim({
     });
 
     // WHEN
-    new ecs.EC2CapacityProvider(stack, 'provider', {
+    new ecs.AsgCapacityProvider(stack, 'provider', {
       autoScalingGroup,
       enableManagedScaling: false,
     });
@@ -1852,10 +1853,7 @@ nodeunitShim({
         AutoScalingGroupArn: {
           Ref: 'asgASG4D014670',
         },
-        ManagedScaling: {
-          Status: 'DISABLED',
-          TargetCapacity: 100,
-        },
+        ManagedScaling: ABSENT,
         ManagedTerminationProtection: 'ENABLED',
       },
     }));
@@ -1874,7 +1872,7 @@ nodeunitShim({
     });
 
     // WHEN
-    new ecs.EC2CapacityProvider(stack, 'provider', {
+    new ecs.AsgCapacityProvider(stack, 'provider', {
       autoScalingGroup,
     });
 
@@ -1897,7 +1895,7 @@ nodeunitShim({
     });
 
     // WHEN
-    new ecs.EC2CapacityProvider(stack, 'provider', {
+    new ecs.AsgCapacityProvider(stack, 'provider', {
       autoScalingGroup,
       enableManagedTerminationProtection: false,
     });
@@ -1909,7 +1907,7 @@ nodeunitShim({
     test.done();
   },
 
-  'can add EC2 capacity via Capacity Provider'(test: Test) {
+  'can add ASG capacity via Capacity Provider'(test: Test) {
     // GIVEN
     const app = new cdk.App();
     const stack = new cdk.Stack(app, 'test');
@@ -1923,7 +1921,7 @@ nodeunitShim({
     });
 
     // WHEN
-    const capacityProvider = new ecs.EC2CapacityProvider(stack, 'provider', {
+    const capacityProvider = new ecs.AsgCapacityProvider(stack, 'provider', {
       autoScalingGroup,
       enableManagedTerminationProtection: false,
     });
@@ -1932,8 +1930,8 @@ nodeunitShim({
     cluster.enableFargateCapacityProviders();
 
     // Ensure not added twice
-    cluster.addEC2CapacityProvider(capacityProvider);
-    cluster.addEC2CapacityProvider(capacityProvider);
+    cluster.addAsgCapacityProvider(capacityProvider);
+    cluster.addAsgCapacityProvider(capacityProvider);
 
     // THEN
     expect(stack).to(haveResource('AWS::ECS::ClusterCapacityProviderAssociations', {

diff --git a/packages/@aws-cdk/aws-ecs/test/ec2/ec2-service.test.ts b/packages/@aws-cdk/aws-ecs/test/ec2/ec2-service.test.ts
@@ -250,11 +250,11 @@ nodeunitShim({
       });
 
       // WHEN
-      const capacityProvider = new ecs.EC2CapacityProvider(stack, 'provider', {
+      const capacityProvider = new ecs.AsgCapacityProvider(stack, 'provider', {
         autoScalingGroup,
         enableManagedTerminationProtection: false,
       });
-      cluster.addEC2CapacityProvider(capacityProvider);
+      cluster.addAsgCapacityProvider(capacityProvider);
 
       const taskDefinition = new ecs.TaskDefinition(stack, 'ServerTask', {
         compatibility: ecs.Compatibility.EC2,

diff --git a/packages/@aws-cdk/aws-ecs/test/ec2/integ.capacity-provider.ts b/packages/@aws-cdk/aws-ecs/test/ec2/integ.capacity-provider.ts
@@ -25,11 +25,11 @@ const autoScalingGroup = new autoscaling.AutoScalingGroup(stack, 'ASG', {
   machineImage: ecs.EcsOptimizedImage.amazonLinux2(),
 });
 
-const cp = new ecs.EC2CapacityProvider(stack, 'EC2CapacityProvider', {
+const cp = new ecs.AsgCapacityProvider(stack, 'EC2CapacityProvider', {
   autoScalingGroup,
 });
 
-cluster.addEC2CapacityProvider(cp);
+cluster.addAsgCapacityProvider(cp);
 
 new ecs.Ec2Service(stack, 'EC2Service', {
   cluster,