feat(experimental-ec2-pattern): Adjust ASG rolling update properties …

…where scaling policy present

Some practical testing of `AutoScalingRollingUpdate` has demonstrated that
when an ASG has a scaling policy, it is safest to dynamically set the `MinInstancesInService` property.
Add an aspect to do that.

See also https://github.com/guardian/testing-asg-rolling-update.

src/experimental/patterns/ec2-app.test.ts

@@ -1,17 +1,17 @@
-import { Duration } from "aws-cdk-lib";
-import { Template } from "aws-cdk-lib/assertions";
+import { App, Duration } from "aws-cdk-lib";
+import { Match, Template } from "aws-cdk-lib/assertions";
 import { InstanceClass, InstanceSize, InstanceType, UserData } from "aws-cdk-lib/aws-ec2";
+import { CloudFormationStackArtifact } from "aws-cdk-lib/cx-api";
 import { AccessScope } from "../../constants";
 import { GuUserData } from "../../constructs/autoscaling";
-import type { GuStack } from "../../constructs/core";
+import { GuStack } from "../../constructs/core";
 import { simpleGuStackForTesting } from "../../utils/test";
 import type { GuEc2AppExperimentalProps } from "./ec2-app";
 import { GuEc2AppExperimental } from "./ec2-app";
 
 // TODO test User Data includes a build number
 describe("The GuEc2AppExperimental pattern", () => {
-  function initialProps(scope: GuStack): GuEc2AppExperimentalProps {
-    const app = "test-gu-ec2-app";
+  function initialProps(scope: GuStack, app: string = "test-gu-ec2-app"): GuEc2AppExperimentalProps {
     const buildNumber = 123;
 
     const { userData } = new GuUserData(scope, {
@@ -140,4 +140,67 @@ describe("The GuEc2AppExperimental pattern", () => {
     // The target group healthcheck polling should be the last thing in the user data.
     expect(endMarkerPosition).toEqual(totalLines - 1);
   });
+
+  it("should adjust properties of a horizontally scaling service", () => {
+    const cdkApp = new App();
+    const stack = new GuStack(cdkApp, "test", {
+      stack: "test-stack",
+      stage: "TEST",
+    });
+
+    const scalingApp = "my-scaling-app";
+    const { autoScalingGroup } = new GuEc2AppExperimental(stack, {
+      ...initialProps(stack, scalingApp),
+      scaling: {
+        minimumInstances: 5,
+      },
+    });
+    autoScalingGroup.scaleOnRequestCount("ScaleOnRequests", {
+      targetRequestsPerMinute: 100,
+    });
+
+    /*
+    We're ultimately testing an `Aspect`, which appear to run only at synth time.
+    As a work-around, synth the `App`, then perform assertions on the resulting template.
+
+    See also: https://github.com/aws/aws-cdk/issues/29047.
+     */
+    const { artifacts } = cdkApp.synth();
+    const cfnStack = artifacts.find((_): _ is CloudFormationStackArtifact => _ instanceof CloudFormationStackArtifact);
+
+    if (!cfnStack) {
+      throw new Error("Unable to locate a CloudFormationStackArtifact");
+    }
+
+    const template = Template.fromJSON(cfnStack.template as Record<string, unknown>);
+
+    const parameterName = `MinInstancesInServiceFor${scalingApp.replaceAll("-", "")}`;
+
+    template.hasParameter(parameterName, {
+      Type: "Number",
+      Default: 5,
+      MaxValue: 9, // (min * 2) - 1
+    });
+
+    template.hasResource("AWS::AutoScaling::AutoScalingGroup", {
+      Properties: {
+        MinSize: "5",
+        MaxSize: "10",
+        DesiredCapacity: Match.absent(),
+        Tags: Match.arrayWith([{ Key: "App", Value: scalingApp, PropagateAtLaunch: true }]),
+      },
+      UpdatePolicy: {
+        AutoScalingRollingUpdate: {
+          MaxBatchSize: 10,
+          SuspendProcesses: ["AlarmNotification"],
+          MinSuccessfulInstancesPercent: 100,
+          WaitOnResourceSignals: true,
+          PauseTime: "PT5M",
+          MinInstancesInService: {
+            Ref: parameterName,
+          },
+        },
+      },
+    });
+  });
 });

src/experimental/patterns/ec2-app.ts

@@ -1,11 +1,65 @@
-import { Duration } from "aws-cdk-lib";
+import type { IAspect } from "aws-cdk-lib";
+import { Aspects, CfnParameter, Duration } from "aws-cdk-lib";
 import type { CfnAutoScalingGroup } from "aws-cdk-lib/aws-autoscaling";
-import { UpdatePolicy } from "aws-cdk-lib/aws-autoscaling";
+import { CfnScalingPolicy, ScalingProcess, UpdatePolicy } from "aws-cdk-lib/aws-autoscaling";
 import { Effect, Policy, PolicyStatement } from "aws-cdk-lib/aws-iam";
-import type { GuStack } from "../../constructs/core";
+import type { IConstruct } from "constructs";
+import { GuAutoScalingGroup } from "../../constructs/autoscaling";
+import { GuStack } from "../../constructs/core";
 import type { GuEc2AppProps } from "../../patterns";
 import { GuEc2App } from "../../patterns";
 
+class HorizontallyScalingDeploymentProperties implements IAspect {
+  public visit(construct: IConstruct) {
+    if (construct instanceof CfnScalingPolicy) {
+      const { node } = construct;
+      const { scopes, path } = node;
+      const guStack = GuStack.of(construct);
+
+      const autoScalingGroup = scopes.find((_): _ is GuAutoScalingGroup => _ instanceof GuAutoScalingGroup);
+
+      if (!autoScalingGroup) {
+        throw new Error(`Failed to detect the autoscaling group relating to the scaling policy on path ${path}`);
+      }
+
+      const cfnAutoScalingGroup = autoScalingGroup.node.defaultChild as CfnAutoScalingGroup;
+      const currentRollingUpdate = cfnAutoScalingGroup.cfnOptions.updatePolicy?.autoScalingRollingUpdate;
+
+      if (currentRollingUpdate) {
+        /*
+        An autoscaling group that horizontally scales should not explicitly set `Desired`,
+        as a rolling update will set the current `Desired` back to the template version,
+        undoing any changes that a scale-out event may have done.
+         */
+        cfnAutoScalingGroup.desiredCapacity = undefined;
+
+        /*
+        An autoscaling group that horizontally scales should expose a CloudFormation Parameter linked to the
+        `MinInstancesInService` property of the rolling update policy.
+
+        Riff-Raff will set this parameter during deployment.
+        The value depends on the current capacity of the ASG:
+          - If the service is running normally, it'll be set to the `Minimum` capacity
+          - If the service is partially scaled, it'll be set to the current `Desired` capacity
+          - If the service is fully scaled, it'll be set to (at least) `Maximum` - 1
+         */
+        const minInstancesInService = new CfnParameter(guStack, `MinInstancesInServiceFor${autoScalingGroup.app}`, {
+          type: "Number",
+          default: parseInt(cfnAutoScalingGroup.minSize),
+          maxValue: parseInt(cfnAutoScalingGroup.maxSize) - 1,
+        });
+
+        cfnAutoScalingGroup.cfnOptions.updatePolicy = {
+          autoScalingRollingUpdate: {
+            ...currentRollingUpdate,
+            minInstancesInService: minInstancesInService.valueAsNumber,
+          },
+        };
+      }
+    }
+  }
+}
+
 export interface GuEc2AppExperimentalProps extends Omit<GuEc2AppProps, "updatePolicy"> {}
 
 /**
@@ -167,5 +221,7 @@ export class GuEc2AppExperimental extends GuEc2App {
           --exit-code $exitCode || echo 'Failed to send Cloudformation Signal'
         `,
     );
+
+    Aspects.of(scope).add(new HorizontallyScalingDeploymentProperties());
   }
 }