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

Improved caching with Docker in CI #803

Draft
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

mec
Copy link
Member

@mec mec commented Nov 12, 2024

This work is one option to improve the performance of CI here on the Rails template.

There is a long Slack thread here:

https://dxw.slack.com/archives/CD8TQ9NP9/p1731332872232279

This is by no stretch optimised. it is just a quick approach that worked for the DfE Complete team.

We kicked things off with some linter fixes to the Dockerfile and then moved on to deliver the actual GitHub Actions changes, see the commit for details.

With CI running all the same checks in the Github Action we no longer need the ci scripts so we remove them.

Things we could still do:

  • the name of the image, test_app is possibly a variable of some kind
  • find a better way to build from the cached layers without going through the whole process again?
  • anything else you can see

@mec mec force-pushed the chore/docker-cache-github-action branch 6 times, most recently from ce42b9d to aa26ae0 Compare November 12, 2024 09:51
@mec mec force-pushed the chore/docker-cache-github-action branch 2 times, most recently from 904df48 to d7df933 Compare November 12, 2024 10:17
name: Set up Docker Buildx
uses: docker/setup-buildx-action@v3
-
name: Build and cache image
Copy link
Contributor

@yndajas yndajas Nov 12, 2024

Choose a reason for hiding this comment

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

question: loading the cache

I think one of the problems with the old caching (removed here) was that it could never find the cache in the load step (documented here; example here - see the "Run actions/cache@v3" step). Are we confident that this approach doesn't have a similar issue?

Copy link
Contributor

@yndajas yndajas Nov 12, 2024

Choose a reason for hiding this comment

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

Or is the purpose of the caching to have a single build for parallelised steps that follow within a single run (the way you're splitting up the test steps makes me wonder!), rather than to build in the first run and then use the cache on subsequent runs? Or is it both?

Copy link
Member Author

@mec mec Nov 12, 2024

Choose a reason for hiding this comment

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

Yep, I am confident the cache is hit, explained below.

I think it is both in this context.

I think with mode=min we cache the Docker image layers, you can see the layers with SHAs in the cache: https://github.com/dxw/rails-template/actions/caches

The initial build ensure the cache is up-to-date, if there are changes in the early part of the image, we have to build them and it takes longer, but that does not happen often.

Each subsequent build will use the cache, you can see the hit rate for each run in the UI e.g. https://github.com/dxw/rails-template/actions/runs/11795643255/attempts/1 on this one the image had to be cached so the hit rate was 12% but you can see all the subsequent builds are 88%.

I prefer to run things in parallel if I can, it doesn't do much here, but with longer running tasks, it can save a lot of time.

There are plenty of optimisations to make here, every situation is different, but this is the general approach.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice - thanks for the explainer. Really useful!

@mec mec force-pushed the chore/docker-cache-github-action branch 6 times, most recently from 7d19c9f to 8d95d0f Compare November 12, 2024 12:12
@mec mec force-pushed the chore/docker-cache-github-action branch 13 times, most recently from 8579573 to ac1582f Compare November 12, 2024 13:19
@mec mec force-pushed the chore/docker-cache-github-action branch 4 times, most recently from 6fba480 to a5efd7a Compare November 12, 2024 14:28
@mec mec changed the title WIP: Attempt at building and caching the Docker image Improved caching with Docker in CI Nov 12, 2024
Comment on lines 40 to 57
-
name: Checkout
uses: actions/checkout@v4
-
name: Set up Docker Buildx
uses: docker/setup-buildx-action@v3
-
name: Build and cache
uses: docker/build-push-action@v6
with:
context: .
file: ./Dockerfile
build-args: |
RAILS_ENV=test
push: false
load: true
tags: app_test:latest
cache-from: type=gha
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: reuse

Speaking of reuse, it might be nice to use some anchors and aliases to DRY up these steps?

Somewhere before jobs:?

post-cache-job-common-steps: &post-cache-job-common-steps
  -
    name: Checkout
    uses: actions/checkout@v4
  -
    name: Set up Docker Buildx
    uses: docker/setup-buildx-action@v3
  -
    name: Build and cache
    uses: docker/build-push-action@v6
    with:
      context: .
      file: ./Dockerfile
      build-args: |
        RAILS_ENV=test
      push: false
      load: true
      tags: app_test:latest
      cache-from: type=gha

And in each job:

    steps:
      <<: *post-cache-job-common-steps
      - [the step(s) unique to this job]

Copy link
Contributor

Choose a reason for hiding this comment

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

Although it looks like there are only three of these jobs now - I'm sure there were more earlier!

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, see updated PR comment! Feel free to jump in and do those? :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ooh, I just went to do it and got a warning in my editor: Property post-cache-job-common-steps is not allowed. yaml-schema: GitHub Workflow

Looks like YAML anchors aren't yet supported in GitHub actions, but they're coming: actions/runner#1182

Copy link
Member Author

Choose a reason for hiding this comment

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

Do we think there is another low effort way to reuse the shared steps?

Copy link
Contributor

Choose a reason for hiding this comment

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

I've pushed something that might work, but the actions don't seem to be firing at the moment 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Alright, done - what do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Composite actions, very cool! Nice work! 👍

@mec mec force-pushed the chore/docker-cache-github-action branch 2 times, most recently from 277fc1d to 19006d0 Compare November 13, 2024 15:45
@yndajas yndajas force-pushed the chore/docker-cache-github-action branch 7 times, most recently from a8c016b to 29b3eff Compare November 14, 2024 13:00
@mec mec force-pushed the chore/docker-cache-github-action branch from 7219532 to d8386ed Compare November 18, 2024 09:14
@mec
Copy link
Member Author

mec commented Nov 18, 2024

I've dropped the independent 'build and cache' job, which gives us big time savings, I feel like there was a reason I separated that job out, but I think we'll need to merge this to test the caching on another PR.

mec and others added 4 commits November 18, 2024 09:23
We wanted to try and speed up the CI tasks for the Rails Template.

This work introduces the same approach we took to save time on the DfE
Complete project.

We use Docker supplied actions to build the image and cache the layers
to the Github actions cache.

Subsequent jobs are set to use the layers from the cache which, in most
cases, gives us a nice speed boost.

We've split the jobs so they can be run in parallel, as we know the
image is the same for each job they all use the strategy.

We've kept the Shellcheck task separate as this code is not strictly
speaking the application code, so doesn't really need to be inside the
image.
Now that we have all the CI tasks in Github Actions these scripts are
never used.
This adds a composite action to run the two cache-loading steps that are
reused across three of the jobs in the continuous integration workflow.
The second of these steps is a little long and detailed, and differs
slightly but meaningfully from a similar step in the build cache job, so
it might be useful to DRY this up. This also allows us to see the meat
of the post-cache jobs a little easier in the continuous integration
workflow

The `actions/checkout@v4` step is needed in each job in order to load
our action (and presumably also the external ones used in the composite
action)

It would be quite nice to use a YAML anchor or alias to do this kind of
reuse, but these are currently unsupported in GitHub Actions. They might
be on the way soon, so watch this space: actions/runner#1182
Our caching strategy is to cached across CI jobs with the focus on
caching the layers in the Docker image.

As each job has to build the image from (using the cache) regardless, we
think we can get rid of the independent build and cache job and let the
caching happen as we run each job.

The only way to see if this pays off is to merge the changes and run
another CI workflow to see how much caching we get.

We have also split each linter and formatter into their own `run` steps,
this is for clarity.
@mec mec force-pushed the chore/docker-cache-github-action branch from d8386ed to ca62b83 Compare November 18, 2024 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants