Skip to content

Commit

Permalink
fix(autoscaling): Allow adding AutoScalingGroup to multiple target gr…
Browse files Browse the repository at this point in the history
…oups

While this doesn't solve the underlying problem, which will require an
API breakage in `scaleOnRequestCount()` (see
aws#5667 (comment)).

As `scaleOnRequestCount()` seems to be the only part that is affected by
this, we'll only issue a validation error if we have multiple
`targetGroupArns` and `scaleOnRequestCount()` has been called.

closes aws#5667
  • Loading branch information
azatoth committed Dec 17, 2022
1 parent 810d736 commit 8706cfb
Show file tree
Hide file tree
Showing 13 changed files with 2,477 additions and 56 deletions.
18 changes: 14 additions & 4 deletions packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts
Expand Up @@ -985,6 +985,7 @@ abstract class AutoScalingGroupBase extends Resource implements IAutoScalingGrou
public abstract readonly osType: ec2.OperatingSystemType;
protected albTargetGroup?: elbv2.ApplicationTargetGroup;
public readonly grantPrincipal: iam.IPrincipal = new iam.UnknownPrincipal({ resource: this });
protected hasSetScaleOnRequest: boolean = false;

/**
* Send a message to either an SQS queue or SNS topic when instances launch or terminate
Expand Down Expand Up @@ -1088,6 +1089,7 @@ abstract class AutoScalingGroupBase extends Resource implements IAutoScalingGrou
});

policy.node.addDependency(this.albTargetGroup.loadBalancerAttached);
this.hasSetScaleOnRequest = true;
return policy;
}

Expand Down Expand Up @@ -1388,6 +1390,8 @@ export class AutoScalingGroup extends AutoScalingGroupBase implements
if (props.requireImdsv2) {
Aspects.of(this).add(new AutoScalingGroupRequireImdsv2Aspect());
}

this.node.addValidation({ validate: () => this.validateTargetGroup() });
}

/**
Expand Down Expand Up @@ -1415,10 +1419,6 @@ export class AutoScalingGroup extends AutoScalingGroupBase implements
* Attach to ELBv2 Application Target Group
*/
public attachToApplicationTargetGroup(targetGroup: elbv2.IApplicationTargetGroup): elbv2.LoadBalancerTargetProps {
if (this.albTargetGroup !== undefined) {
throw new Error('Cannot add AutoScalingGroup to 2nd Target Group');
}

this.targetGroupArns.push(targetGroup.targetGroupArn);
if (targetGroup instanceof elbv2.ApplicationTargetGroup) {
// Copy onto self if it's a concrete type. We need this for autoscaling
Expand Down Expand Up @@ -1765,6 +1765,16 @@ export class AutoScalingGroup extends AutoScalingGroupBase implements
};
}
}


private validateTargetGroup(): string[] {
const errors = new Array<string>();
if (this.hasSetScaleOnRequest && this.targetGroupArns.length > 1) {
errors.push('Cannon use multiple target groups if `setScaleOnRequest()` is being used.');
}

return errors;
}
}

/**
Expand Down
3 changes: 2 additions & 1 deletion packages/@aws-cdk/aws-autoscaling/package.json
Expand Up @@ -82,10 +82,11 @@
"devDependencies": {
"@aws-cdk/assertions": "0.0.0",
"@aws-cdk/cdk-build-tools": "0.0.0",
"@aws-cdk/integ-runner": "0.0.0",
"@aws-cdk/cfn2ts": "0.0.0",
"@aws-cdk/cloud-assembly-schema": "0.0.0",
"@aws-cdk/cx-api": "0.0.0",
"@aws-cdk/integ-runner": "0.0.0",
"@aws-cdk/integ-tests": "0.0.0",
"@aws-cdk/pkglint": "0.0.0",
"@types/jest": "^27.5.2",
"jest": "^27.5.1"
Expand Down
38 changes: 38 additions & 0 deletions packages/@aws-cdk/aws-autoscaling/test/auto-scaling-group.test.ts
Expand Up @@ -2,6 +2,7 @@ import { Annotations, Match, Template } from '@aws-cdk/assertions';
import * as cloudwatch from '@aws-cdk/aws-cloudwatch';
import * as ec2 from '@aws-cdk/aws-ec2';
import { AmazonLinuxCpuType, AmazonLinuxGeneration, AmazonLinuxImage, InstanceType, LaunchTemplate } from '@aws-cdk/aws-ec2';
import { ApplicationLoadBalancer, ApplicationTargetGroup } from '@aws-cdk/aws-elasticloadbalancingv2';
import * as iam from '@aws-cdk/aws-iam';
import * as sns from '@aws-cdk/aws-sns';
import { testDeprecated } from '@aws-cdk/cdk-build-tools';
Expand Down Expand Up @@ -1864,6 +1865,43 @@ describe('auto scaling group', () => {
})).not.toThrow();

});

test('Should validate multiple target groups i conjunction with `setScaleOnRequest()`', () => {
const stack = new cdk.Stack(undefined, 'MyStack', { env: { region: 'us-east-1', account: '1234' } });


const vpc = mockVpc(stack);

const alb = new ApplicationLoadBalancer(stack, 'alb', {
vpc,
internetFacing: true,
});

const listener = alb.addListener('Listener', {
port: 80,
open: true,
});
const asg = new autoscaling.AutoScalingGroup(stack, 'MyFleet', {
instanceType: ec2.InstanceType.of(ec2.InstanceClass.M4, ec2.InstanceSize.MICRO),
machineImage: new ec2.AmazonLinuxImage(),
vpc,
});

const atg1 = new ApplicationTargetGroup(stack, 'ATG1', { port: 443 });
const atg2 = new ApplicationTargetGroup(stack, 'ATG2', { port: 443 });

listener.addTargetGroups('tgs', { targetGroups: [atg1, atg2] });

asg.attachToApplicationTargetGroup(atg1);
asg.attachToApplicationTargetGroup(atg2);

expect(asg.node.validate()).toEqual([]);

asg.scaleOnRequestCount('requests-per-minute', { targetRequestsPerMinute: 60 });

expect(asg.node.validate()).toContainEqual('Cannon use multiple target groups if `setScaleOnRequest()` is being used.');

});
});

function mockVpc(stack: cdk.Stack) {
Expand Down
@@ -0,0 +1,19 @@
{
"version": "22.0.0",
"files": {
"21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22": {
"source": {
"path": "LambdaTestDefaultTestDeployAssert1AF2B360.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "21fbb51d7b23f6a6c262b46a9caee79d744a3ac019fd45422d988b96d44b2a22.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
}
},
"dockerImages": {}
}
@@ -0,0 +1,36 @@
{
"Parameters": {
"BootstrapVersion": {
"Type": "AWS::SSM::Parameter::Value<String>",
"Default": "/cdk-bootstrap/hnb659fds/version",
"Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]"
}
},
"Rules": {
"CheckBootstrapVersion": {
"Assertions": [
{
"Assert": {
"Fn::Not": [
{
"Fn::Contains": [
[
"1",
"2",
"3",
"4",
"5"
],
{
"Ref": "BootstrapVersion"
}
]
}
]
},
"AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
}
]
}
}
}
@@ -0,0 +1,19 @@
{
"version": "22.0.0",
"files": {
"6e4ba6d1c10cb8a0be78abd30ef94d231f8913f1cd4b02c0b62b8626f3425c00": {
"source": {
"path": "aws-cdk-asg-atg-integ.template.json",
"packaging": "file"
},
"destinations": {
"current_account-current_region": {
"bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}",
"objectKey": "6e4ba6d1c10cb8a0be78abd30ef94d231f8913f1cd4b02c0b62b8626f3425c00.json",
"assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}"
}
}
}
},
"dockerImages": {}
}

0 comments on commit 8706cfb

Please sign in to comment.