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

ref(docker): use cache mounts for build cache #8796

Merged
merged 5 commits into from
Sep 5, 2024
Merged

Conversation

gustavovalverde
Copy link
Member

@gustavovalverde gustavovalverde commented Aug 22, 2024

Motivation

We've been using external tools and custom approaches to improve the caching layers in Rust, and avoid invalidating the caches if no Rust-code is changed.

This update eliminates the need for external tools like cargo-chef to leverage caching layers, resulting in an average build time reduction of 4m30s (~36% improvement).

While this solution doesn't fully resolve the issues mentioned in #6169 (comment), it represents the best possible approach without resorting to custom solutions, which we'd prefer to avoid.

Depends-On: #8817
Closes #6169

Specifications & References

Solution

  • Use cache mounts instead of cargo-chef
  • Remove custom rsync steps

Others

  • Add a RUST_VERSION arg so we don't invalidate our cache so frequently with Rust upgrades
  • Use --link in our runtime stage, to reduce the resulting artifact
  • Remove the installation of Google OS Config agent, as this is no longer required

Tests

  • Tests are passing for CI and CD deployments
  • Rebuilding is taking less time (~8 minutes instead of ~13 minutes)

Follow-up Work

PR Author's Checklist

  • The PR name will make sense to users.
  • The PR provides a CHANGELOG summary.
  • The solution is tested.
  • The documentation is up to date.
  • The PR has a priority label.

PR Reviewer's Checklist

  • The PR Author's checklist is complete.
  • The PR resolves the issue.

@gustavovalverde gustavovalverde added A-devops Area: Pipelines, CI/CD and Dockerfiles C-enhancement Category: This is an improvement I-heavy Problems with excessive memory, disk, or CPU usage P-Low ❄️ labels Aug 22, 2024
@gustavovalverde gustavovalverde self-assigned this Aug 22, 2024
@gustavovalverde gustavovalverde force-pushed the refactor-docker-caching branch 2 times, most recently from a134823 to 99f1eb6 Compare August 26, 2024 15:45
@gustavovalverde gustavovalverde marked this pull request as ready for review August 26, 2024 16:23
@gustavovalverde gustavovalverde requested a review from a team as a code owner August 26, 2024 16:23
@gustavovalverde gustavovalverde requested review from arya2 and removed request for a team August 26, 2024 16:23
@arya2 arya2 added do-not-merge Tells Mergify not to merge this PR and removed do-not-merge Tells Mergify not to merge this PR labels Aug 27, 2024
@gustavovalverde gustavovalverde marked this pull request as draft August 28, 2024 19:36
@gustavovalverde gustavovalverde force-pushed the refactor-docker-caching branch 2 times, most recently from 85ad65a to bd923da Compare August 29, 2024 20:00
This update eliminates the need for external tools like `cargo-chef` to leverage caching layers, resulting in an average build time reduction of 4m30s (~36% improvement).

While this solution doesn't fully resolve the issues mentioned in #6169 (comment), it represents the best possible approach without resorting to custom solutions, which we'd prefer to avoid.
@gustavovalverde gustavovalverde force-pushed the refactor-docker-caching branch from 50d53ca to 2bd886c Compare August 29, 2024 20:20
@gustavovalverde gustavovalverde marked this pull request as ready for review August 29, 2024 20:40
@gustavovalverde
Copy link
Member Author

@arya2 this one is ready now

@mpguerra
Copy link
Contributor

mpguerra commented Sep 2, 2024

Do we want to close #6169 with this PR or are we planning on making any other improvements in the short term?

@gustavovalverde
Copy link
Member Author

I've just linked #6169, for automatic closure.

This is not solving the underlying issue, but this is as far as we can get right now, until the mtime issue is solved in Rust.

arya2
arya2 previously approved these changes Sep 3, 2024
Copy link
Contributor

@arya2 arya2 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 good!

docker/Dockerfile Outdated Show resolved Hide resolved
@mergify mergify bot merged commit d31eea5 into main Sep 5, 2024
131 of 132 checks passed
@mergify mergify bot deleted the refactor-docker-caching branch September 5, 2024 13:29
dmidem pushed a commit to QED-it/zebra that referenced this pull request Oct 29, 2024
* ref(docker): leverage cache mount with bind mounts

This update eliminates the need for external tools like `cargo-chef` to leverage caching layers, resulting in an average build time reduction of 4m30s (~36% improvement).

While this solution doesn't fully resolve the issues mentioned in ZcashFoundation#6169 (comment), it represents the best possible approach without resorting to custom solutions, which we'd prefer to avoid.

* chore: remove extra `WORKDIR` and imp comments

* chore: improve comment legibility

Co-authored-by: Arya <[email protected]>

---------

Co-authored-by: Pili Guerra <[email protected]>
Co-authored-by: Arya <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-devops Area: Pipelines, CI/CD and Dockerfiles C-enhancement Category: This is an improvement I-heavy Problems with excessive memory, disk, or CPU usage P-Low ❄️
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docker: improve Rust dependency caching in CI to avoid rebuilding
3 participants