Skip to content

Commit a7de7fe

Browse files
authored
fix(autoscaling): step scaling without adjustment type fails (#29158)
### Reason for this change Step Scaling without `adjustmentType` fails with CFn error `You must specify an AdjustmentType for policy type: StepScaling`. - Reproduction code ```ts const asg = new autoscaling.AutoScalingGroup(stack, 'ASG', { vpc, instanceType: ec2.InstanceType.of(ec2.InstanceClass.BURSTABLE2, ec2.InstanceSize.MICRO), machineImage: new ec2.AmazonLinuxImage({ generation: ec2.AmazonLinuxGeneration.AMAZON_LINUX_2 }), }); asg.scaleOnMetric('StepScaling', { metric: new cloudwatch.Metric({ namespace: 'AWS/EC2', metricName: 'CPUUtilization', dimensionsMap: { AutoScalingGroupName: asg.autoScalingGroupName } }), scalingSteps: [ { upper: 10, change: -1 }, { lower: 50, change: +1 }, { lower: 90, change: +2 }, ], evaluationPeriods: 10, datapointsToAlarm: 5, metricAggregationType: autoscaling.MetricAggregationType.MAXIMUM, // adjustmentType: autoscaling.AdjustmentType.CHANGE_IN_CAPACITY, }); ``` ### Description of changes According to [the CDK code](https://github.com/aws/aws-cdk/blob/v2.122.0/packages/aws-cdk-lib/aws-autoscaling/lib/step-scaling-policy.ts#L105-L115), `CHANGE_IN_CAPACITY` will be used if `adjustmentType` is not specified in the prop. But the variable is not passed into `StepScalingAction` construct (instead `props.adjustmentType` is passed as is). [The documentation](https://github.com/aws/aws-cdk/blob/v2.122.0/packages/aws-cdk-lib/aws-autoscaling/lib/step-scaling-policy.ts#L26) also says that the default value is `CHANGE_IN_CAPACITY`. So we should use `adjustmentType` instead of `props.adjustmentType`. ```ts const adjustmentType = props.adjustmentType || AdjustmentType.CHANGE_IN_CAPACITY; const changesAreAbsolute = adjustmentType === AdjustmentType.EXACT_CAPACITY; // ... // ... this.lowerAction = new StepScalingAction(this, 'LowerPolicy', { // adjustmentType: props.adjustmentType, adjustmentType, ``` **The fact that the error occurs if `props.adjustmentType` is not specified means that the user's successful existing stack always specifies `props.adjustmentType`. In other words, no change to the existing resource will occur due to this change, so there is no need for a feature flag.** ### Description of how you validated changes ~~Unit tests without integ tests, because this PR could cover this bug by unit tests.~~ Both unit and integ tests. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 7e8239b commit a7de7fe

File tree

7 files changed

+330
-5
lines changed

7 files changed

+330
-5
lines changed

Diff for: packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.js.snapshot/autoscaling-step-scaling.assets.json

+2-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.js.snapshot/autoscaling-step-scaling.template.json

+93
Original file line numberDiff line numberDiff line change
@@ -643,6 +643,99 @@
643643
"Statistic": "Average",
644644
"Threshold": 50
645645
}
646+
},
647+
"ASGStepScalingWithDefaultAdjustmentTypeLowerPolicyA6C1EA33": {
648+
"Type": "AWS::AutoScaling::ScalingPolicy",
649+
"Properties": {
650+
"AdjustmentType": "ChangeInCapacity",
651+
"AutoScalingGroupName": {
652+
"Ref": "ASG46ED3070"
653+
},
654+
"MetricAggregationType": "Maximum",
655+
"PolicyType": "StepScaling",
656+
"StepAdjustments": [
657+
{
658+
"MetricIntervalUpperBound": 0,
659+
"ScalingAdjustment": -1
660+
}
661+
]
662+
}
663+
},
664+
"ASGStepScalingWithDefaultAdjustmentTypeLowerAlarmF9F52487": {
665+
"Type": "AWS::CloudWatch::Alarm",
666+
"Properties": {
667+
"AlarmActions": [
668+
{
669+
"Ref": "ASGStepScalingWithDefaultAdjustmentTypeLowerPolicyA6C1EA33"
670+
}
671+
],
672+
"AlarmDescription": "Lower threshold scaling alarm",
673+
"ComparisonOperator": "LessThanOrEqualToThreshold",
674+
"DatapointsToAlarm": 5,
675+
"Dimensions": [
676+
{
677+
"Name": "AutoScalingGroupName",
678+
"Value": {
679+
"Ref": "ASG46ED3070"
680+
}
681+
}
682+
],
683+
"EvaluationPeriods": 10,
684+
"MetricName": "DiskWriteOps",
685+
"Namespace": "AWS/EC2",
686+
"Period": 300,
687+
"Statistic": "Average",
688+
"Threshold": 100
689+
}
690+
},
691+
"ASGStepScalingWithDefaultAdjustmentTypeUpperPolicy08CC2D99": {
692+
"Type": "AWS::AutoScaling::ScalingPolicy",
693+
"Properties": {
694+
"AdjustmentType": "ChangeInCapacity",
695+
"AutoScalingGroupName": {
696+
"Ref": "ASG46ED3070"
697+
},
698+
"MetricAggregationType": "Maximum",
699+
"PolicyType": "StepScaling",
700+
"StepAdjustments": [
701+
{
702+
"MetricIntervalLowerBound": 0,
703+
"MetricIntervalUpperBound": 200,
704+
"ScalingAdjustment": 1
705+
},
706+
{
707+
"MetricIntervalLowerBound": 200,
708+
"ScalingAdjustment": 2
709+
}
710+
]
711+
}
712+
},
713+
"ASGStepScalingWithDefaultAdjustmentTypeUpperAlarm2379E17B": {
714+
"Type": "AWS::CloudWatch::Alarm",
715+
"Properties": {
716+
"AlarmActions": [
717+
{
718+
"Ref": "ASGStepScalingWithDefaultAdjustmentTypeUpperPolicy08CC2D99"
719+
}
720+
],
721+
"AlarmDescription": "Upper threshold scaling alarm",
722+
"ComparisonOperator": "GreaterThanOrEqualToThreshold",
723+
"DatapointsToAlarm": 5,
724+
"Dimensions": [
725+
{
726+
"Name": "AutoScalingGroupName",
727+
"Value": {
728+
"Ref": "ASG46ED3070"
729+
}
730+
}
731+
],
732+
"EvaluationPeriods": 10,
733+
"MetricName": "DiskWriteOps",
734+
"Namespace": "AWS/EC2",
735+
"Period": 300,
736+
"Statistic": "Average",
737+
"Threshold": 300
738+
}
646739
}
647740
},
648741
"Parameters": {

Diff for: packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.js.snapshot/manifest.json

+25-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.js.snapshot/tree.json

+175
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Diff for: packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.ts

+11
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,17 @@ asg.scaleOnMetric('StepScaling', {
3030
datapointsToAlarm: 5,
3131
metricAggregationType: autoscaling.MetricAggregationType.MAXIMUM,
3232
});
33+
asg.scaleOnMetric('StepScalingWithDefaultAdjustmentType', {
34+
metric: new cloudwatch.Metric({ namespace: 'AWS/EC2', metricName: 'DiskWriteOps', dimensionsMap: { AutoScalingGroupName: asg.autoScalingGroupName } }),
35+
scalingSteps: [
36+
{ upper: 100, change: -1 },
37+
{ lower: 300, change: +1 },
38+
{ lower: 500, change: +2 },
39+
],
40+
evaluationPeriods: 10,
41+
datapointsToAlarm: 5,
42+
metricAggregationType: autoscaling.MetricAggregationType.MAXIMUM,
43+
});
3344

3445
new integ.IntegTest(app, 'autoscaling-step-scaling-integ', {
3546
testCases: [stack],

Diff for: packages/aws-cdk-lib/aws-autoscaling/lib/step-scaling-policy.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ export class StepScalingPolicy extends Construct {
147147
const threshold = intervals[alarms.lowerAlarmIntervalIndex].upper;
148148

149149
this.lowerAction = new StepScalingAction(this, 'LowerPolicy', {
150-
adjustmentType: props.adjustmentType,
150+
adjustmentType,
151151
cooldown: props.cooldown,
152152
estimatedInstanceWarmup: props.estimatedInstanceWarmup,
153153
metricAggregationType: props.metricAggregationType ?? aggregationTypeFromMetric(props.metric),
@@ -179,7 +179,7 @@ export class StepScalingPolicy extends Construct {
179179
const threshold = intervals[alarms.upperAlarmIntervalIndex].lower;
180180

181181
this.upperAction = new StepScalingAction(this, 'UpperPolicy', {
182-
adjustmentType: props.adjustmentType,
182+
adjustmentType,
183183
cooldown: props.cooldown,
184184
estimatedInstanceWarmup: props.estimatedInstanceWarmup,
185185
metricAggregationType: props.metricAggregationType ?? aggregationTypeFromMetric(props.metric),

0 commit comments

Comments
 (0)