wp-calypso icon indicating copy to clipboard operation
wp-calypso copied to clipboard

CI: upgrade docker image from `buster` to `bullseye-slim`.

Open worldomonation opened this issue 2 years ago • 8 comments

Proposed Changes

This PR upgrades our docker base image from buster to bullseye-slim.

Key changes:

  • use bullseye-slim instead of buster.
  • removal of nvm from the image.
  • other optimizations throughout the docker build process to reduce the image size.

Context: In pdqkMK-x6-p2 I wrote about the need for the Docker base images to eventually move onto bullseye-based images. This was precipitated by Playwright, whose 1.25 release stated that buster support is slated to end in December 2022.

Additionally, seeing as buster was initially released mid-2019 it is starting to get a little long in the tooth. While the base images affected here are not the same images pushed to production servers, staying reasonably up to date is still important.

Removal of nvm I've opted to remove nvm from our dependencies because the current version of node is already baked into the image we use. It felt unnecessary to install nvm on top of this for every docker image created.

Testing Instructions

Ensure the following build configurations are passing:

  • [x] Gutenberg E2E (desktop)
  • [x] Calypso E2E (desktop)
  • [x] Calypso E2E (mobile)
  • [x] Pre-Release E2E
  • [x] Code Style
  • [x] Unit Tests
  • [x] Build base image

Pre-merge Checklist

  • [ ] Have you written new tests for your changes?
  • [ ] Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
  • [ ] Have you checked for TypeScript, React or other console errors?
  • [ ] Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
  • [ ] Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?

Related to #

worldomonation avatar Sep 16 '22 18:09 worldomonation

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

matticbot avatar Sep 16 '22 18:09 matticbot

Ran every unique-ish build configuration and everything has passed so far.

worldomonation avatar Sep 20 '22 17:09 worldomonation

While the base images affected here are not the same images pushed to production servers

This made me think a bit, and I think this is ultimately pushed to production servers. While main Dockerfile is deployed to production and not modified in this PR, it does start from the base image in most cases here:

https://github.com/Automattic/wp-calypso/blob/02648c013f4cd9143b5f7a0f9bdc55191b04be58/Dockerfile#L1-L19

Since use_cache is true when building the Dockerfile from TeamCity, we start the build from builder-cache-true, which starts from the base image, which starts from bullseye slim.

This makes me think we should change the line FROM node:${node_version}-buster as builder-cache-false to FROM node:${node_version}-bullseye-slim. (Though I don't think this is currently used anywhere)

noahtallen avatar Sep 20 '22 20:09 noahtallen

this is ultimately pushed to production servers ... t does start from the base image in most cases here

Hmm, that's a good catch - I hadn't accounted for that and had (erroneously) thought that Production images were built in a completely separate chain. Thank you for catching it.

I also ran a quick search in the codebase for builder-cache-false and like you found it doesn't seem to be used anywhere. If it ends up being used. because the base image is now slim it will require installation of a few additional packages like curl and wget. Since there doesn't seem to be anything derived from builder-cache-false, I think we're fine.

worldomonation avatar Sep 20 '22 20:09 worldomonation

thought that Production images were built in a completely separate chain

This used to be the case, but changed at the beginning of the year iirc.

noahtallen avatar Sep 20 '22 20:09 noahtallen

I was running the Docker builds again, just to verify that everything is working smoothly. It definitely appears to be, but I think I ran into an issue testing the e2e tests. When I manually trigger the e2e tests again, it doesn't utilize the Docker image build I manually triggered as well -- instead, it resolves the Docker image used in the last commit on the branch, which was published yesterday.

Apparently, that image isn't available any more, so calypso.live doesn't work and the test started failing. Sorry about that!

I think if you rebase the branch or push a small change, it will generate a new build and the tests will pass.

noahtallen avatar Sep 21 '22 23:09 noahtallen

When I manually trigger the e2e tests again, it doesn't utilize the Docker image build I manually triggered as well -- instead, it resolves the Docker image used in the last commit on the branch, which was published yesterday.

Weird - I think perhaps the parameters had been adjusted by default, which is what I did during development - and the adjusted image tag parameter just stuck around when you went to trigger it too.

I'll rebase and investigate.

worldomonation avatar Sep 22 '22 16:09 worldomonation

On 09/22 I rebased this on then-latest trunk, triggered a docker base image build with the custom tag bullseye-slim (previously unused tag), then triggered multiple E2E and other supporting tasks by specifying ci-e2e:bullseye-slim tag.

Happy to report that everything appears to be working fine and thus this PR is ready to be merged in a couple hours' time.

worldomonation avatar Sep 26 '22 21:09 worldomonation