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: add front rendering app #8728

Merged
merged 30 commits into from
Oct 12, 2023
Merged

feat: add front rendering app #8728

merged 30 commits into from
Oct 12, 2023

Conversation

jamesgorrie
Copy link
Contributor

@jamesgorrie jamesgorrie commented Sep 1, 2023

What does this change?

Adds new front-web app to our stack.
Part of #8351

This PR also stores the ELB in a parameter that we can later read from frontend to help mitigate any manual input of these values.

Why?

When we see a degradation of 1 endpoint in DCR e.g. /Front, /AMPInteractive it can have a knock-on effect and degrade the performance of all the other stacks as DCR is currently the rendered as described here.

Some notes on creating a stack (to be stored / scripted somewhere)

  • You need to manually create some SSM parameters
    • /CODE/frontend/{app}/elb.logs.bucketName
    • /CODE/frontend/{app}/logging.stream.name

Co-authored-by: Georges Lebreton georges.lebreton@guardian.co.uk

@github-actions
Copy link

github-actions bot commented Sep 1, 2023

Size Change: 0 B 🆕

Total Size: 0 B

compressed-size-action

Comment on lines 151 to 155
new StringParameter(this, 'ec2RoleArn', {
// Annoyingly this doesn't follow the same pattern as the other SSM parameters
parameterName: `/${stack}/${stage}/${app}.loadBalancerDnsName`,
stringValue: loadBalancer.loadBalancerDnsName,
});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This autogenerates the SSM parameter that we can use here.

@jamesgorrie jamesgorrie added the run_chromatic Runs chromatic when label is applied label Sep 14, 2023
@jamesgorrie jamesgorrie marked this pull request as ready for review September 20, 2023 10:55
@jamesgorrie jamesgorrie requested a review from a team as a code owner September 20, 2023 10:55
Copy link
Contributor

@georgeblahblah georgeblahblah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - this is an exciting step!

Just a warning that this "conflicts" (although not at a file level) with #8886 - one of these PRs will need to be updated to copy DotcomRendering-front-CODE.template.json, depending on which gets merged first. I'm mentioning it because I don't think it would cause a file conflict in git, but it would break the build.

CODE: DotcomRendering-front-CODE.template.json
cloudFormationStackByTags: false
cloudFormationStackName: render-front
amiParameter: AMIRenderfront
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how does this work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JamieB-gu do you have more insight on this - I can't really see where it's used?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you run cdk synth (in this project make cdk-synth) you'll see it in the generated cloudformation template.

AFAIK (@jacobwinch can correct me if I've got any of this wrong) AMIgo1 can be used to define an AMI with particular features, known as a recipe, and "bake" (build) it on a regular schedule. RiffRaff uses amiTags defined in the riff-raff.yaml file to derive the latest version of an AMI that's baked to a particular AMIgo recipe, and gets an id for this image. It passes this id into cloudformation during deployment2 via a parameter, and this is the name of that parameter. Cloudformation needs to know what name to expect and what it means (image id), so this will also be defined in the cloudformation template.

In our case we generate the cloudformation template using CDK, which automatically sets the name of this parameter to "AMI" + "name of your app"3. Previously we only had one app, rendering, so this parameter was always AMIRendering. This PR creates a new app called render-front, so now there's a second version of this parameter called AMIRenderfront.

This PR sets up RiffRaff to deploy both apps together, so we have two sets of deployments defined:

  • One for the rendering app, using the DotcomRendering cloudformation template containing the AMIRendering parameter.
  • One for the render-fronts app, using the DotcomRendering-front cloudformation template containing the AMIRenderfronts parameter.

AFAIK @jamesgorrie you would like this new fronts app to use the same AMIgo recipe as the existing rendering app. Therefore we're able to use a shared deployment template45 to define most of the AMI configuration (like amiTags), only diverging when we hit this amiParameter field because it's different for each app as mentioned. As we always want the same recipe, I think it's preferable to only define and update it in one place, which also makes it a bit easier to run the check-node-versions script:

filepath: 'dotcom-rendering/scripts/deploy/riff-raff.yaml',
pattern: /^ +Recipe: dotcom-rendering.*-node-(\d+\.\d+\.\d+)$/m,

Footnotes

  1. https://github.com/guardian/amigo

  2. https://github.com/guardian/amigo/blob/5edc0ff7daaaee94f3697c35f2729b1a12391510/docs/riffraff-integration.md

  3. https://github.com/guardian/cdk/blob/402a5bb57f696461cd1c4f7afc2220fc525d3700/docs/migration-guide-ec2.md?plain=1#L191

  4. https://github.com/guardian/riff-raff/blob/main/riff-raff/public/docs/reference/riff-raff.yaml.md#templates

  5. https://github.com/guardian/riff-raff/blob/main/riff-raff/public/docs/reference/riff-raff.yaml.md#template

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all sounds correct to me @JamieB-gu!

@jamesgorrie
Copy link
Contributor Author

Just a warning that this "conflicts" (although not at a file level) with #8886 - one of these PRs will need to be updated to copy DotcomRendering-front-CODE.template.json

@georgeblahblah - I'll wait for that PR to be merged as that change is more relevant to this PR than that.

@jamesgorrie jamesgorrie changed the title feat: add front CODE stack feat: add front rendering app Oct 3, 2023
bucketSsmKey: /account/services/dotcom-artifact.bucket
dependencies:
- frontend-static
- frontend-cfn
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we want to depend on the render-front-cfn deployment here:

Suggested change
- frontend-cfn
- render-front-cfn

@jacobwinch
Copy link
Contributor

The latest changes look good to me 👍

@jamesgorrie jamesgorrie merged commit cf12c19 into main Oct 12, 2023
23 checks passed
@jamesgorrie jamesgorrie deleted the front-code-stack branch October 12, 2023 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dotcom-rendering run_chromatic Runs chromatic when label is applied
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants