Merge branch 'main' into fix_instances_reg_exp

aws · Jan 30, 2024 · fa18285 · fa18285
2 parents 880721a + 4034adb
commit fa18285
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 3 deletions.
diff --git a/packages/aws-cdk-lib/aws-applicationautoscaling/lib/step-scaling-policy.ts b/packages/aws-cdk-lib/aws-applicationautoscaling/lib/step-scaling-policy.ts
@@ -72,7 +72,7 @@ export interface BasicStepScalingPolicyProps {
    *
    * Only has meaning if `evaluationPeriods != 1`.
    *
-   * @default `evaluationPeriods`
+   * @default - Same as `evaluationPeriods`
    */
   readonly datapointsToAlarm?: number;
 
@@ -117,8 +117,22 @@ export class StepScalingPolicy extends Construct {
       throw new Error(`'scalingSteps' can have at most 40 steps, got ${props.scalingSteps.length}`);
     }
 
-    if (props.datapointsToAlarm !== undefined && props.datapointsToAlarm < 1) {
-      throw new RangeError(`datapointsToAlarm cannot be less than 1, got: ${props.datapointsToAlarm}`);
+    if (props.evaluationPeriods !== undefined && !cdk.Token.isUnresolved(props.evaluationPeriods) && props.evaluationPeriods < 1) {
+      throw new Error(`evaluationPeriods cannot be less than 1, got: ${props.evaluationPeriods}`);
+    }
+    if (props.datapointsToAlarm !== undefined) {
+      if (props.evaluationPeriods === undefined) {
+        throw new Error('evaluationPeriods must be set if datapointsToAlarm is set');
+      }
+      if (!cdk.Token.isUnresolved(props.datapointsToAlarm) && props.datapointsToAlarm < 1) {
+        throw new Error(`datapointsToAlarm cannot be less than 1, got: ${props.datapointsToAlarm}`);
+      }
+      if (!cdk.Token.isUnresolved(props.datapointsToAlarm)
+        && !cdk.Token.isUnresolved(props.evaluationPeriods)
+        && props.evaluationPeriods < props.datapointsToAlarm
+      ) {
+        throw new Error(`datapointsToAlarm must be less than or equal to evaluationPeriods, got datapointsToAlarm: ${props.datapointsToAlarm}, evaluationPeriods: ${props.evaluationPeriods}`);
+      }
     }
 
     const adjustmentType = props.adjustmentType || AdjustmentType.CHANGE_IN_CAPACITY;

diff --git a/packages/aws-cdk-lib/aws-applicationautoscaling/test/step-scaling-policy.test.ts b/packages/aws-cdk-lib/aws-applicationautoscaling/test/step-scaling-policy.test.ts
@@ -218,6 +218,26 @@ describe('step scaling policy', () => {
 
   });
 
+  test('step scaling with invalid evaluation period throws error', () => {
+    // GIVEN
+    const stack = new cdk.Stack();
+    const target = createScalableTarget(stack);
+
+    // THEN
+    expect(() => {
+      target.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 },
+        ],
+        evaluationPeriods: 0,
+        metricAggregationType: appscaling.MetricAggregationType.MAXIMUM,
+      });
+    }).toThrow(/evaluationPeriods cannot be less than 1, got: 0/);
+  });
+
   test('step scaling with evaluation period & data points to alarm configured', () => {
     // GIVEN
     const stack = new cdk.Stack();
@@ -274,6 +294,47 @@ describe('step scaling policy', () => {
     }).toThrow('datapointsToAlarm cannot be less than 1, got: 0');
   });
 
+  test('step scaling with datapointsToAlarm is greater than evaluationPeriods throws error', () => {
+    // GIVEN
+    const stack = new cdk.Stack();
+    const target = createScalableTarget(stack);
+
+    // THEN
+    expect(() => {
+      target.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 },
+        ],
+        evaluationPeriods: 10,
+        datapointsToAlarm: 15,
+        metricAggregationType: appscaling.MetricAggregationType.MAXIMUM,
+      });
+    }).toThrow(/datapointsToAlarm must be less than or equal to evaluationPeriods, got datapointsToAlarm: 15, evaluationPeriods: 10/);
+  });
+
+  test('step scaling with datapointsToAlarm without evaluationPeriods throws error', () => {
+    // GIVEN
+    const stack = new cdk.Stack();
+    const target = createScalableTarget(stack);
+
+    // THEN
+    expect(() => {
+      target.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 },
+        ],
+        datapointsToAlarm: 15,
+        metricAggregationType: appscaling.MetricAggregationType.MAXIMUM,
+      });
+    }).toThrow(/evaluationPeriods must be set if datapointsToAlarm is set/);
+  });
+
   test('scalingSteps must have at least 2 steps', () => {
     // GIVEN
     const stack = new cdk.Stack();