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

Merged
merged 6 commits into from
Dec 13, 2024
Merged

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
@mec mec force-pushed the chore/docker-cache-github-action branch 20 times, most recently from ac1582f to a984dfc Compare November 12, 2024 13:23
@mec mec force-pushed the chore/docker-cache-github-action branch 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 mec force-pushed the chore/docker-cache-github-action branch from d8386ed to ca62b83 Compare November 18, 2024 09:23
@jdudley1123
Copy link
Member

@mec I'd like to get these improvements merged before you leave. Is that doable?

@mec
Copy link
Member Author

mec commented Dec 12, 2024

@jdudley1123 I would think so, lets give it a try!

mec added 3 commits December 12, 2024 14:30
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.
@mec mec force-pushed the chore/docker-cache-github-action branch from ca62b83 to 398ac5e Compare December 12, 2024 14:30
@mec mec marked this pull request as ready for review December 12, 2024 14:34
@mec mec requested a review from jdudley1123 December 12, 2024 14:35
@mec
Copy link
Member Author

mec commented Dec 12, 2024

@jdudley1123 if we merge this, we'll want to swing back and update the branch protection rules.

@mec mec force-pushed the chore/docker-cache-github-action branch from 398ac5e to 89bd47f Compare December 12, 2024 15:14
Copy link
Member

@jdudley1123 jdudley1123 left a comment

Choose a reason for hiding this comment

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

This looks really good. Every time I left a comment you addressed it in the next commit 😅

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
@mec mec force-pushed the chore/docker-cache-github-action branch from 89bd47f to 56c594a Compare December 13, 2024 12:43
mec added 2 commits December 13, 2024 12:48
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.
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 56c594a to b33d141 Compare December 13, 2024 12:50
@mec mec merged commit bf78275 into develop Dec 13, 2024
5 checks passed
@mec mec deleted the chore/docker-cache-github-action branch December 13, 2024 12:54
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.

3 participants