From a7de7feb6a14658ec25f4cfda434d5e1d69157d2 Mon Sep 17 00:00:00 2001 From: "k.goto" <24818752+go-to-k@users.noreply.github.com> Date: Thu, 29 Feb 2024 01:32:59 +0900 Subject: [PATCH] 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* --- .../autoscaling-step-scaling.assets.json | 4 +- .../autoscaling-step-scaling.template.json | 93 ++++++++++ .../manifest.json | 26 ++- .../tree.json | 175 ++++++++++++++++++ .../test/integ.asg-step-scaling.ts | 11 ++ .../lib/step-scaling-policy.ts | 4 +- .../aws-autoscaling/test/scaling.test.ts | 22 +++ 7 files changed, 330 insertions(+), 5 deletions(-) diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.js.snapshot/autoscaling-step-scaling.assets.json b/packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.js.snapshot/autoscaling-step-scaling.assets.json index 82a1625220d4e..0e8e3a9ec8b31 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.js.snapshot/autoscaling-step-scaling.assets.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.js.snapshot/autoscaling-step-scaling.assets.json @@ -1,7 +1,7 @@ { "version": "36.0.0", "files": { - "b6351ec1c05eef818c53fedccb18fb96ff55ee77e2049a05d2b5b15903984b19": { + "9275d003ca02cbf717d5af0130f852ba74ab89d57dbc60b2edf6737afc7f4757": { "source": { "path": "autoscaling-step-scaling.template.json", "packaging": "file" @@ -9,7 +9,7 @@ "destinations": { "current_account-current_region": { "bucketName": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}", - "objectKey": "b6351ec1c05eef818c53fedccb18fb96ff55ee77e2049a05d2b5b15903984b19.json", + "objectKey": "9275d003ca02cbf717d5af0130f852ba74ab89d57dbc60b2edf6737afc7f4757.json", "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-file-publishing-role-${AWS::AccountId}-${AWS::Region}" } } diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.js.snapshot/autoscaling-step-scaling.template.json b/packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.js.snapshot/autoscaling-step-scaling.template.json index d15d364a6c214..d0a582898d7c3 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.js.snapshot/autoscaling-step-scaling.template.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.js.snapshot/autoscaling-step-scaling.template.json @@ -643,6 +643,99 @@ "Statistic": "Average", "Threshold": 50 } + }, + "ASGStepScalingWithDefaultAdjustmentTypeLowerPolicyA6C1EA33": { + "Type": "AWS::AutoScaling::ScalingPolicy", + "Properties": { + "AdjustmentType": "ChangeInCapacity", + "AutoScalingGroupName": { + "Ref": "ASG46ED3070" + }, + "MetricAggregationType": "Maximum", + "PolicyType": "StepScaling", + "StepAdjustments": [ + { + "MetricIntervalUpperBound": 0, + "ScalingAdjustment": -1 + } + ] + } + }, + "ASGStepScalingWithDefaultAdjustmentTypeLowerAlarmF9F52487": { + "Type": "AWS::CloudWatch::Alarm", + "Properties": { + "AlarmActions": [ + { + "Ref": "ASGStepScalingWithDefaultAdjustmentTypeLowerPolicyA6C1EA33" + } + ], + "AlarmDescription": "Lower threshold scaling alarm", + "ComparisonOperator": "LessThanOrEqualToThreshold", + "DatapointsToAlarm": 5, + "Dimensions": [ + { + "Name": "AutoScalingGroupName", + "Value": { + "Ref": "ASG46ED3070" + } + } + ], + "EvaluationPeriods": 10, + "MetricName": "DiskWriteOps", + "Namespace": "AWS/EC2", + "Period": 300, + "Statistic": "Average", + "Threshold": 100 + } + }, + "ASGStepScalingWithDefaultAdjustmentTypeUpperPolicy08CC2D99": { + "Type": "AWS::AutoScaling::ScalingPolicy", + "Properties": { + "AdjustmentType": "ChangeInCapacity", + "AutoScalingGroupName": { + "Ref": "ASG46ED3070" + }, + "MetricAggregationType": "Maximum", + "PolicyType": "StepScaling", + "StepAdjustments": [ + { + "MetricIntervalLowerBound": 0, + "MetricIntervalUpperBound": 200, + "ScalingAdjustment": 1 + }, + { + "MetricIntervalLowerBound": 200, + "ScalingAdjustment": 2 + } + ] + } + }, + "ASGStepScalingWithDefaultAdjustmentTypeUpperAlarm2379E17B": { + "Type": "AWS::CloudWatch::Alarm", + "Properties": { + "AlarmActions": [ + { + "Ref": "ASGStepScalingWithDefaultAdjustmentTypeUpperPolicy08CC2D99" + } + ], + "AlarmDescription": "Upper threshold scaling alarm", + "ComparisonOperator": "GreaterThanOrEqualToThreshold", + "DatapointsToAlarm": 5, + "Dimensions": [ + { + "Name": "AutoScalingGroupName", + "Value": { + "Ref": "ASG46ED3070" + } + } + ], + "EvaluationPeriods": 10, + "MetricName": "DiskWriteOps", + "Namespace": "AWS/EC2", + "Period": 300, + "Statistic": "Average", + "Threshold": 300 + } } }, "Parameters": { diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.js.snapshot/manifest.json b/packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.js.snapshot/manifest.json index 877089e7ae7f2..c3907637c2628 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.js.snapshot/manifest.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.js.snapshot/manifest.json @@ -18,7 +18,7 @@ "validateOnSynth": false, "assumeRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-deploy-role-${AWS::AccountId}-${AWS::Region}", "cloudFormationExecutionRoleArn": "arn:${AWS::Partition}:iam::${AWS::AccountId}:role/cdk-hnb659fds-cfn-exec-role-${AWS::AccountId}-${AWS::Region}", - "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/b6351ec1c05eef818c53fedccb18fb96ff55ee77e2049a05d2b5b15903984b19.json", + "stackTemplateAssetObjectUrl": "s3://cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}/9275d003ca02cbf717d5af0130f852ba74ab89d57dbc60b2edf6737afc7f4757.json", "requiresBootstrapStackVersion": 6, "bootstrapStackVersionSsmParameter": "/cdk-bootstrap/hnb659fds/version", "additionalDependencies": [ @@ -226,6 +226,30 @@ "data": "ASGStepScalingUpperAlarmD989D805" } ], + "/autoscaling-step-scaling/ASG/StepScalingWithDefaultAdjustmentType/LowerPolicy/Resource": [ + { + "type": "aws:cdk:logicalId", + "data": "ASGStepScalingWithDefaultAdjustmentTypeLowerPolicyA6C1EA33" + } + ], + "/autoscaling-step-scaling/ASG/StepScalingWithDefaultAdjustmentType/LowerAlarm/Resource": [ + { + "type": "aws:cdk:logicalId", + "data": "ASGStepScalingWithDefaultAdjustmentTypeLowerAlarmF9F52487" + } + ], + "/autoscaling-step-scaling/ASG/StepScalingWithDefaultAdjustmentType/UpperPolicy/Resource": [ + { + "type": "aws:cdk:logicalId", + "data": "ASGStepScalingWithDefaultAdjustmentTypeUpperPolicy08CC2D99" + } + ], + "/autoscaling-step-scaling/ASG/StepScalingWithDefaultAdjustmentType/UpperAlarm/Resource": [ + { + "type": "aws:cdk:logicalId", + "data": "ASGStepScalingWithDefaultAdjustmentTypeUpperAlarm2379E17B" + } + ], "/autoscaling-step-scaling/SsmParameterValue:--aws--service--ami-amazon-linux-latest--amzn2-ami-hvm-x86_64-gp2:C96584B6-F00A-464E-AD19-53AFF4B05118.Parameter": [ { "type": "aws:cdk:logicalId", diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.js.snapshot/tree.json b/packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.js.snapshot/tree.json index ddc20e3a7c9b3..b983ee1e8ae20 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.js.snapshot/tree.json +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.js.snapshot/tree.json @@ -1067,6 +1067,181 @@ "fqn": "constructs.Construct", "version": "10.3.0" } + }, + "StepScalingWithDefaultAdjustmentType": { + "id": "StepScalingWithDefaultAdjustmentType", + "path": "autoscaling-step-scaling/ASG/StepScalingWithDefaultAdjustmentType", + "children": { + "LowerPolicy": { + "id": "LowerPolicy", + "path": "autoscaling-step-scaling/ASG/StepScalingWithDefaultAdjustmentType/LowerPolicy", + "children": { + "Resource": { + "id": "Resource", + "path": "autoscaling-step-scaling/ASG/StepScalingWithDefaultAdjustmentType/LowerPolicy/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::AutoScaling::ScalingPolicy", + "aws:cdk:cloudformation:props": { + "adjustmentType": "ChangeInCapacity", + "autoScalingGroupName": { + "Ref": "ASG46ED3070" + }, + "metricAggregationType": "Maximum", + "policyType": "StepScaling", + "stepAdjustments": [ + { + "metricIntervalUpperBound": 0, + "scalingAdjustment": -1 + } + ] + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + }, + "LowerAlarm": { + "id": "LowerAlarm", + "path": "autoscaling-step-scaling/ASG/StepScalingWithDefaultAdjustmentType/LowerAlarm", + "children": { + "Resource": { + "id": "Resource", + "path": "autoscaling-step-scaling/ASG/StepScalingWithDefaultAdjustmentType/LowerAlarm/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::CloudWatch::Alarm", + "aws:cdk:cloudformation:props": { + "alarmActions": [ + { + "Ref": "ASGStepScalingWithDefaultAdjustmentTypeLowerPolicyA6C1EA33" + } + ], + "alarmDescription": "Lower threshold scaling alarm", + "comparisonOperator": "LessThanOrEqualToThreshold", + "datapointsToAlarm": 5, + "dimensions": [ + { + "name": "AutoScalingGroupName", + "value": { + "Ref": "ASG46ED3070" + } + } + ], + "evaluationPeriods": 10, + "metricName": "DiskWriteOps", + "namespace": "AWS/EC2", + "period": 300, + "statistic": "Average", + "threshold": 100 + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + }, + "UpperPolicy": { + "id": "UpperPolicy", + "path": "autoscaling-step-scaling/ASG/StepScalingWithDefaultAdjustmentType/UpperPolicy", + "children": { + "Resource": { + "id": "Resource", + "path": "autoscaling-step-scaling/ASG/StepScalingWithDefaultAdjustmentType/UpperPolicy/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::AutoScaling::ScalingPolicy", + "aws:cdk:cloudformation:props": { + "adjustmentType": "ChangeInCapacity", + "autoScalingGroupName": { + "Ref": "ASG46ED3070" + }, + "metricAggregationType": "Maximum", + "policyType": "StepScaling", + "stepAdjustments": [ + { + "metricIntervalLowerBound": 0, + "metricIntervalUpperBound": 200, + "scalingAdjustment": 1 + }, + { + "metricIntervalLowerBound": 200, + "scalingAdjustment": 2 + } + ] + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + }, + "UpperAlarm": { + "id": "UpperAlarm", + "path": "autoscaling-step-scaling/ASG/StepScalingWithDefaultAdjustmentType/UpperAlarm", + "children": { + "Resource": { + "id": "Resource", + "path": "autoscaling-step-scaling/ASG/StepScalingWithDefaultAdjustmentType/UpperAlarm/Resource", + "attributes": { + "aws:cdk:cloudformation:type": "AWS::CloudWatch::Alarm", + "aws:cdk:cloudformation:props": { + "alarmActions": [ + { + "Ref": "ASGStepScalingWithDefaultAdjustmentTypeUpperPolicy08CC2D99" + } + ], + "alarmDescription": "Upper threshold scaling alarm", + "comparisonOperator": "GreaterThanOrEqualToThreshold", + "datapointsToAlarm": 5, + "dimensions": [ + { + "name": "AutoScalingGroupName", + "value": { + "Ref": "ASG46ED3070" + } + } + ], + "evaluationPeriods": 10, + "metricName": "DiskWriteOps", + "namespace": "AWS/EC2", + "period": 300, + "statistic": "Average", + "threshold": 300 + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } + } + }, + "constructInfo": { + "fqn": "constructs.Construct", + "version": "10.3.0" + } } }, "constructInfo": { diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.ts b/packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.ts index c49ed5d312e98..081d85b8c6ec8 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.ts +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-autoscaling/test/integ.asg-step-scaling.ts @@ -30,6 +30,17 @@ asg.scaleOnMetric('StepScaling', { datapointsToAlarm: 5, metricAggregationType: autoscaling.MetricAggregationType.MAXIMUM, }); +asg.scaleOnMetric('StepScalingWithDefaultAdjustmentType', { + metric: new cloudwatch.Metric({ namespace: 'AWS/EC2', metricName: 'DiskWriteOps', dimensionsMap: { AutoScalingGroupName: asg.autoScalingGroupName } }), + scalingSteps: [ + { upper: 100, change: -1 }, + { lower: 300, change: +1 }, + { lower: 500, change: +2 }, + ], + evaluationPeriods: 10, + datapointsToAlarm: 5, + metricAggregationType: autoscaling.MetricAggregationType.MAXIMUM, +}); new integ.IntegTest(app, 'autoscaling-step-scaling-integ', { testCases: [stack], diff --git a/packages/aws-cdk-lib/aws-autoscaling/lib/step-scaling-policy.ts b/packages/aws-cdk-lib/aws-autoscaling/lib/step-scaling-policy.ts index eed4950e7e720..937c92c0b80b1 100644 --- a/packages/aws-cdk-lib/aws-autoscaling/lib/step-scaling-policy.ts +++ b/packages/aws-cdk-lib/aws-autoscaling/lib/step-scaling-policy.ts @@ -147,7 +147,7 @@ export class StepScalingPolicy extends Construct { const threshold = intervals[alarms.lowerAlarmIntervalIndex].upper; this.lowerAction = new StepScalingAction(this, 'LowerPolicy', { - adjustmentType: props.adjustmentType, + adjustmentType, cooldown: props.cooldown, estimatedInstanceWarmup: props.estimatedInstanceWarmup, metricAggregationType: props.metricAggregationType ?? aggregationTypeFromMetric(props.metric), @@ -179,7 +179,7 @@ export class StepScalingPolicy extends Construct { const threshold = intervals[alarms.upperAlarmIntervalIndex].lower; this.upperAction = new StepScalingAction(this, 'UpperPolicy', { - adjustmentType: props.adjustmentType, + adjustmentType, cooldown: props.cooldown, estimatedInstanceWarmup: props.estimatedInstanceWarmup, metricAggregationType: props.metricAggregationType ?? aggregationTypeFromMetric(props.metric), diff --git a/packages/aws-cdk-lib/aws-autoscaling/test/scaling.test.ts b/packages/aws-cdk-lib/aws-autoscaling/test/scaling.test.ts index dc4a564d75221..f8dd19b431812 100644 --- a/packages/aws-cdk-lib/aws-autoscaling/test/scaling.test.ts +++ b/packages/aws-cdk-lib/aws-autoscaling/test/scaling.test.ts @@ -294,6 +294,28 @@ test('step scaling from percentile metric', () => { }); }); +test('step scaling with adjustmentType by default', () => { + // GIVEN + const stack = new cdk.Stack(); + const fixture = new ASGFixture(stack, 'Fixture'); + + // WHEN + fixture.asg.scaleOnMetric('Tracking', { + metric: new cloudwatch.Metric({ namespace: 'Test', metricName: 'Metric', statistic: 'p99' }), + scalingSteps: [ + { upper: 0, change: -1 }, + { lower: 100, change: +1 }, + { lower: 500, change: +5 }, + ], + }); + + // THEN + Template.fromStack(stack).hasResourceProperties('AWS::AutoScaling::ScalingPolicy', { + PolicyType: 'StepScaling', + AdjustmentType: 'ChangeInCapacity', + }); +}); + test('step scaling with evaluation period configured', () => { // GIVEN const stack = new cdk.Stack();