Skip to content

Commit 4034adb

Browse files
authored
feat(applicationautoscaling): validate evaluationPeriods and datapointsToAlarm for step scaling policy (#28880)
This PR adds validations for `evaluationPeriods` and `datapointsToAlarm` for step scaling policy. It is based on [the PR for datapointsToAlarm in autoscaling step scaling policy](#28792). Check the following cases: - `evaluationPeriods` < 1 - `datapointsToAlarm` is set, and - `evaluationPeriods` is not set - `datapointsToAlarm` < 1 - `evaluationPeriods` < `datapointsToAlarm` These validations also consider whether those parameters are tokens or not. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent a441d37 commit 4034adb

File tree

2 files changed

+78
-3
lines changed

2 files changed

+78
-3
lines changed

packages/aws-cdk-lib/aws-applicationautoscaling/lib/step-scaling-policy.ts

+17-3
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ export interface BasicStepScalingPolicyProps {
7272
*
7373
* Only has meaning if `evaluationPeriods != 1`.
7474
*
75-
* @default `evaluationPeriods`
75+
* @default - Same as `evaluationPeriods`
7676
*/
7777
readonly datapointsToAlarm?: number;
7878

@@ -117,8 +117,22 @@ export class StepScalingPolicy extends Construct {
117117
throw new Error(`'scalingSteps' can have at most 40 steps, got ${props.scalingSteps.length}`);
118118
}
119119

120-
if (props.datapointsToAlarm !== undefined && props.datapointsToAlarm < 1) {
121-
throw new RangeError(`datapointsToAlarm cannot be less than 1, got: ${props.datapointsToAlarm}`);
120+
if (props.evaluationPeriods !== undefined && !cdk.Token.isUnresolved(props.evaluationPeriods) && props.evaluationPeriods < 1) {
121+
throw new Error(`evaluationPeriods cannot be less than 1, got: ${props.evaluationPeriods}`);
122+
}
123+
if (props.datapointsToAlarm !== undefined) {
124+
if (props.evaluationPeriods === undefined) {
125+
throw new Error('evaluationPeriods must be set if datapointsToAlarm is set');
126+
}
127+
if (!cdk.Token.isUnresolved(props.datapointsToAlarm) && props.datapointsToAlarm < 1) {
128+
throw new Error(`datapointsToAlarm cannot be less than 1, got: ${props.datapointsToAlarm}`);
129+
}
130+
if (!cdk.Token.isUnresolved(props.datapointsToAlarm)
131+
&& !cdk.Token.isUnresolved(props.evaluationPeriods)
132+
&& props.evaluationPeriods < props.datapointsToAlarm
133+
) {
134+
throw new Error(`datapointsToAlarm must be less than or equal to evaluationPeriods, got datapointsToAlarm: ${props.datapointsToAlarm}, evaluationPeriods: ${props.evaluationPeriods}`);
135+
}
122136
}
123137

124138
const adjustmentType = props.adjustmentType || AdjustmentType.CHANGE_IN_CAPACITY;

packages/aws-cdk-lib/aws-applicationautoscaling/test/step-scaling-policy.test.ts

+61
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,26 @@ describe('step scaling policy', () => {
218218

219219
});
220220

221+
test('step scaling with invalid evaluation period throws error', () => {
222+
// GIVEN
223+
const stack = new cdk.Stack();
224+
const target = createScalableTarget(stack);
225+
226+
// THEN
227+
expect(() => {
228+
target.scaleOnMetric('Tracking', {
229+
metric: new cloudwatch.Metric({ namespace: 'Test', metricName: 'Metric', statistic: 'p99' }),
230+
scalingSteps: [
231+
{ upper: 0, change: -1 },
232+
{ lower: 100, change: +1 },
233+
{ lower: 500, change: +5 },
234+
],
235+
evaluationPeriods: 0,
236+
metricAggregationType: appscaling.MetricAggregationType.MAXIMUM,
237+
});
238+
}).toThrow(/evaluationPeriods cannot be less than 1, got: 0/);
239+
});
240+
221241
test('step scaling with evaluation period & data points to alarm configured', () => {
222242
// GIVEN
223243
const stack = new cdk.Stack();
@@ -274,6 +294,47 @@ describe('step scaling policy', () => {
274294
}).toThrow('datapointsToAlarm cannot be less than 1, got: 0');
275295
});
276296

297+
test('step scaling with datapointsToAlarm is greater than evaluationPeriods throws error', () => {
298+
// GIVEN
299+
const stack = new cdk.Stack();
300+
const target = createScalableTarget(stack);
301+
302+
// THEN
303+
expect(() => {
304+
target.scaleOnMetric('Tracking', {
305+
metric: new cloudwatch.Metric({ namespace: 'Test', metricName: 'Metric', statistic: 'p99' }),
306+
scalingSteps: [
307+
{ upper: 0, change: -1 },
308+
{ lower: 100, change: +1 },
309+
{ lower: 500, change: +5 },
310+
],
311+
evaluationPeriods: 10,
312+
datapointsToAlarm: 15,
313+
metricAggregationType: appscaling.MetricAggregationType.MAXIMUM,
314+
});
315+
}).toThrow(/datapointsToAlarm must be less than or equal to evaluationPeriods, got datapointsToAlarm: 15, evaluationPeriods: 10/);
316+
});
317+
318+
test('step scaling with datapointsToAlarm without evaluationPeriods throws error', () => {
319+
// GIVEN
320+
const stack = new cdk.Stack();
321+
const target = createScalableTarget(stack);
322+
323+
// THEN
324+
expect(() => {
325+
target.scaleOnMetric('Tracking', {
326+
metric: new cloudwatch.Metric({ namespace: 'Test', metricName: 'Metric', statistic: 'p99' }),
327+
scalingSteps: [
328+
{ upper: 0, change: -1 },
329+
{ lower: 100, change: +1 },
330+
{ lower: 500, change: +5 },
331+
],
332+
datapointsToAlarm: 15,
333+
metricAggregationType: appscaling.MetricAggregationType.MAXIMUM,
334+
});
335+
}).toThrow(/evaluationPeriods must be set if datapointsToAlarm is set/);
336+
});
337+
277338
test('scalingSteps must have at least 2 steps', () => {
278339
// GIVEN
279340
const stack = new cdk.Stack();

0 commit comments

Comments
 (0)