Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(riff-raff.yaml): Add minInstancesInServiceParameters when applicable #2476

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/yellow-chefs-laugh.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@guardian/cdk": minor
---

feat(riff-raff.yaml): Add `minInstancesInServiceParameters` when applicable

To complement the changes in https://github.com/guardian/riff-raff/pull/1383,
add the `minInstancesInServiceParameters` property to the `riff-raff.yaml` file when applicable.
32 changes: 3 additions & 29 deletions src/experimental/patterns/ec2-app.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,38 +3,12 @@ import { Match, Template } from "aws-cdk-lib/assertions";
import type { CfnAutoScalingGroup } from "aws-cdk-lib/aws-autoscaling";
import { CfnScalingPolicy } from "aws-cdk-lib/aws-autoscaling";
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 { GuStack } from "../../constructs/core";
import { simpleGuStackForTesting } from "../../utils/test";
import { getTemplateAfterAspectInvocation, simpleGuStackForTesting } from "../../utils/test";
import type { GuEc2AppExperimentalProps } from "./ec2-app";
import { GuEc2AppExperimental, RollingUpdateDurations } from "./ec2-app";

/**
* `Aspects` appear to run only at synth time.
* This means we must synth the stack to see the results of the `Aspect`.
*
* @see https://github.com/aws/aws-cdk/issues/29047
*
* @param stack the stack to synthesise
*/
function getTemplateAfterAspectInvocation(stack: GuStack): Template {
const app = App.of(stack);

if (!app) {
throw new Error(`Unable to locate the enclosing App from GuStack ${stack.node.id}`);
}

const { artifacts } = app.synth();
const cfnStack = artifacts.find((_): _ is CloudFormationStackArtifact => _ instanceof CloudFormationStackArtifact);

if (!cfnStack) {
throw new Error("Unable to locate a CloudFormationStackArtifact");
}

return Template.fromJSON(cfnStack.template as Record<string, unknown>);
}
import { getAsgRollingUpdateCfnParameterName, GuEc2AppExperimental, RollingUpdateDurations } from "./ec2-app";

// TODO test User Data includes a build number
describe("The GuEc2AppExperimental pattern", () => {
Expand Down Expand Up @@ -176,7 +150,7 @@ describe("The GuEc2AppExperimental pattern", () => {
const template = getTemplateAfterAspectInvocation(stack);

// The scaling ASG should NOT have `DesiredCapacity` set, and `MinInstancesInService` set via a CFN Parameter
const parameterName = `MinInstancesInServiceFor${scalingApp.replaceAll("-", "")}`;
const parameterName = getAsgRollingUpdateCfnParameterName(autoScalingGroup);
template.hasParameter(parameterName, {
Type: "Number",
Default: 5,
Expand Down
13 changes: 10 additions & 3 deletions src/experimental/patterns/ec2-app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,10 @@ class AutoScalingRollingUpdateTimeout implements IAspect {
*
* @see https://github.com/guardian/testing-asg-rolling-update
*/
class HorizontallyScalingDeploymentProperties implements IAspect {
// eslint-disable-next-line custom-rules/experimental-classes -- this class is not indented for public use
export class HorizontallyScalingDeploymentProperties implements IAspect {
public readonly stack: GuStack;
private readonly asgToParamMap: Map<string, CfnParameter>;
public readonly asgToParamMap: Map<string, CfnParameter>;
private static instance: HorizontallyScalingDeploymentProperties | undefined;

private constructor(scope: GuStack) {
Expand Down Expand Up @@ -168,9 +169,10 @@ class HorizontallyScalingDeploymentProperties implements IAspect {
const asgNodeId = autoScalingGroup.node.id;

if (!this.asgToParamMap.has(asgNodeId)) {
const cfnParameterName = getAsgRollingUpdateCfnParameterName(autoScalingGroup);
this.asgToParamMap.set(
asgNodeId,
new CfnParameter(this.stack, `MinInstancesInServiceFor${autoScalingGroup.app}`, {
new CfnParameter(this.stack, cfnParameterName, {
type: "Number",
default: parseInt(cfnAutoScalingGroup.minSize),
maxValue: parseInt(cfnAutoScalingGroup.maxSize) - 1,
Expand All @@ -191,6 +193,11 @@ class HorizontallyScalingDeploymentProperties implements IAspect {
}
}

export function getAsgRollingUpdateCfnParameterName(autoScalingGroup: GuAutoScalingGroup) {
const { app } = autoScalingGroup;
return `MinInstancesInServiceFor${app.replaceAll("-", "")}`;
}

/**
* An IAM Policy allowing the sending of a CloudFormation signal.
*
Expand Down
33 changes: 18 additions & 15 deletions src/riff-raff-yaml-file/deployments/cloudformation.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { GuAutoScalingGroup } from "../../constructs/autoscaling";
import type { CdkStacksDifferingOnlyByStage, RiffRaffDeployment, RiffRaffDeploymentProps } from "../types";
import { getAsgRollingUpdateCfnParameterName } from "../../experimental/patterns/ec2-app";
import type { CdkStacksDifferingOnlyByStage, RiffRaffDeployment, RiffRaffDeploymentParameters } from "../types";

export function cloudFormationDeployment(
cdkStacks: CdkStacksDifferingOnlyByStage,
Expand Down Expand Up @@ -47,11 +48,8 @@ export function cloudFormationDeployment(
};
}

export function addAmiParametersToCloudFormationDeployment(
cfnDeployment: RiffRaffDeployment,
autoScalingGroups: GuAutoScalingGroup[],
): RiffRaffDeploymentProps {
const amiParametersToTags = autoScalingGroups.reduce((acc, asg) => {
export function getAmiParameters(autoScalingGroups: GuAutoScalingGroup[]): RiffRaffDeploymentParameters {
return autoScalingGroups.reduce<RiffRaffDeploymentParameters>((acc, asg) => {
const { imageRecipe, app, amiParameter } = asg;

if (!imageRecipe) {
Expand All @@ -74,14 +72,19 @@ export function addAmiParametersToCloudFormationDeployment(
},
};
}, {});
}

return {
...cfnDeployment.props,
parameters: {
...cfnDeployment.props.parameters,

// only add the `amiParametersToTags` property if there are some
...(autoScalingGroups.length > 0 && { amiParametersToTags }),
},
};
export function getMinInstancesInServiceParameters(
autoScalingGroups: GuAutoScalingGroup[],
): RiffRaffDeploymentParameters {
return autoScalingGroups.reduce<RiffRaffDeploymentParameters>((acc, asg) => {
const { app } = asg;
const cfnParameter = getAsgRollingUpdateCfnParameterName(asg);
return {
...acc,
[cfnParameter]: {
App: app,
},
};
}, {});
}
108 changes: 106 additions & 2 deletions src/riff-raff-yaml-file/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
import { App, Duration } from "aws-cdk-lib";
import { UpdatePolicy } from "aws-cdk-lib/aws-autoscaling";
import { CfnScalingPolicy, UpdatePolicy } from "aws-cdk-lib/aws-autoscaling";
import { InstanceClass, InstanceSize, InstanceType } from "aws-cdk-lib/aws-ec2";
import { Schedule } from "aws-cdk-lib/aws-events";
import { Runtime } from "aws-cdk-lib/aws-lambda";
import { AccessScope } from "../constants";
import type { GuAutoScalingGroup } from "../constructs/autoscaling";
import type { GuStackProps } from "../constructs/core";
import { GuStack } from "../constructs/core";
import { GuLambdaFunction } from "../constructs/lambda";
import { GuEc2AppExperimental } from "../experimental/patterns/ec2-app";
import { getAsgRollingUpdateCfnParameterName, GuEc2AppExperimental } from "../experimental/patterns/ec2-app";
import { GuEc2App, GuNodeApp, GuPlayApp, GuScheduledLambda } from "../patterns";
import { getTemplateAfterAspectInvocation } from "../utils/test";
import { RiffRaffYamlFile } from "./index";

describe("The RiffRaffYamlFile class", () => {
Expand Down Expand Up @@ -1413,4 +1415,106 @@ describe("The RiffRaffYamlFile class", () => {
"
`);
});

it("Should include minInstancesInServiceParameters when GuEc2AppExperimental has a scaling policy", () => {
const app = new App({ outdir: "/tmp/cdk.out" });

class MyApplicationStack extends GuStack {
public readonly asg: GuAutoScalingGroup;

// eslint-disable-next-line custom-rules/valid-constructors -- unit testing
constructor(app: App, id: string, props: GuStackProps) {
super(app, id, props);

const appName = "my-app";

const { autoScalingGroup } = new GuEc2AppExperimental(this, {
app: appName,
instanceType: InstanceType.of(InstanceClass.T4G, InstanceSize.MICRO),
access: { scope: AccessScope.PUBLIC },
userData: {
distributable: {
fileName: `${appName}.deb`,
executionStatement: `dpkg -i /${appName}/${appName}.deb`,
},
},
certificateProps: {
domainName: "rip.gu.com",
},
monitoringConfiguration: { noMonitoring: true },
scaling: {
minimumInstances: 1,
},
applicationPort: 9000,
imageRecipe: "arm64-bionic-java11-deploy-infrastructure",
buildIdentifier: "TEST",
});

new CfnScalingPolicy(autoScalingGroup, "ScaleOut", {
autoScalingGroupName: autoScalingGroup.autoScalingGroupName,
policyType: "SimpleScaling",
adjustmentType: "ChangeInCapacity",
scalingAdjustment: 1,
});

this.asg = autoScalingGroup;
}
}

const guStack = new MyApplicationStack(app, "test-stack", {
stack: "test",
stage: "TEST",
env: { region: "eu-west-1" },
});

// Ensure the Aspects are invoked...
getTemplateAfterAspectInvocation(guStack);

// ...so that the CFN Parameters are added to the template, to then be processed by the `RiffRaffYamlFile`
const actual = new RiffRaffYamlFile(app).toYAML();

const cfnParameterName = getAsgRollingUpdateCfnParameterName(guStack.asg);

expect(actual).toMatchInlineSnapshot(`
"allowedStages:
- TEST
deployments:
asg-upload-eu-west-1-test-my-app:
type: autoscaling
actions:
- uploadArtifacts
regions:
- eu-west-1
stacks:
- test
app: my-app
parameters:
bucketSsmLookup: true
prefixApp: true
contentDirectory: my-app
cfn-eu-west-1-test-my-application-stack:
type: cloud-formation
regions:
- eu-west-1
stacks:
- test
app: my-application-stack
contentDirectory: /tmp/cdk.out
parameters:
templateStagePaths:
TEST: test-stack.template.json
amiParametersToTags:
AMIMyapp:
BuiltBy: amigo
AmigoStage: PROD
Recipe: arm64-bionic-java11-deploy-infrastructure
Encrypted: 'true'
minInstancesInServiceParameters:
${cfnParameterName}:
App: my-app
dependencies:
- asg-upload-eu-west-1-test-my-app
"
`);
});
});
29 changes: 24 additions & 5 deletions src/riff-raff-yaml-file/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,13 @@ import { dump } from "js-yaml";
import { GuAutoScalingGroup } from "../constructs/autoscaling";
import { GuStack } from "../constructs/core";
import { GuLambdaFunction } from "../constructs/lambda";
import { HorizontallyScalingDeploymentProperties } from "../experimental/patterns/ec2-app";
import { autoscalingDeployment, uploadAutoscalingArtifact } from "./deployments/autoscaling";
import { addAmiParametersToCloudFormationDeployment, cloudFormationDeployment } from "./deployments/cloudformation";
import {
cloudFormationDeployment,
getAmiParameters,
getMinInstancesInServiceParameters,
} from "./deployments/cloudformation";
import { updateLambdaDeployment, uploadLambdaArtifact } from "./deployments/lambda";
import { groupByClassNameStackRegionStage } from "./group-by";
import type {
Expand Down Expand Up @@ -269,10 +274,24 @@ export class RiffRaffYamlFile {
deployments.set(asgDeployment.name, asgDeployment.props);
});

deployments.set(
cfnDeployment.name,
addAmiParametersToCloudFormationDeployment(cfnDeployment, autoscalingGroups),
);
const amiParametersToTags = getAmiParameters(autoscalingGroups);

const minInServiceParamMap = HorizontallyScalingDeploymentProperties.getInstance(stack).asgToParamMap;
const minInServiceAsgs = autoscalingGroups.filter((asg) => minInServiceParamMap.has(asg.node.id));
const minInstancesInServiceParameters = getMinInstancesInServiceParameters(minInServiceAsgs);

deployments.set(cfnDeployment.name, {
...cfnDeployment.props,
parameters: {
...cfnDeployment.props.parameters,

// only add the `amiParametersToTags` property if there are some
...(autoscalingGroups.length > 0 && { amiParametersToTags }),

// only add the `minInstancesInServiceParameters` property if there are some
...(minInServiceAsgs.length > 0 && { minInstancesInServiceParameters }),
},
});
});
});
});
Expand Down
4 changes: 3 additions & 1 deletion src/riff-raff-yaml-file/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@ export interface RiffRaffYaml {

export type RiffRaffDeploymentName = string;

export type RiffRaffDeploymentParameters = Record<string, string | boolean | Record<string, unknown>>;

export interface RiffRaffDeploymentProps {
type: string;
regions: Set<Region>;
stacks: Set<StackTag>;
app: string;
contentDirectory: string;
parameters: Record<string, string | boolean | Record<string, string>>;
parameters: RiffRaffDeploymentParameters;
dependencies?: RiffRaffDeploymentName[];
actions?: string[];
}
Expand Down
1 change: 1 addition & 0 deletions src/utils/test/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from "./assertions";
export * from "./simple-gu-stack";
export * from "./attach-policy-to-test-role";
export * from "./template";
29 changes: 29 additions & 0 deletions src/utils/test/template.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { App } from "aws-cdk-lib";
import { Template } from "aws-cdk-lib/assertions";
import { CloudFormationStackArtifact } from "aws-cdk-lib/cx-api";
import type { GuStack } from "../../constructs/core";

/**
* `Aspects` appear to run only at synth time.
* This means we must synth the stack to see the results of the `Aspect`.
*
* @see https://github.com/aws/aws-cdk/issues/29047
*
* @param stack the stack to synthesise
*/
export function getTemplateAfterAspectInvocation(stack: GuStack): Template {
const app = App.of(stack);

if (!app) {
throw new Error(`Unable to locate the enclosing App from GuStack ${stack.node.id}`);
}

const { artifacts } = app.synth();
const cfnStack = artifacts.find((_): _ is CloudFormationStackArtifact => _ instanceof CloudFormationStackArtifact);

if (!cfnStack) {
throw new Error("Unable to locate a CloudFormationStackArtifact");
}

return Template.fromJSON(cfnStack.template as Record<string, unknown>);
}