fix(docker): Stop resetting the cargo-chef cache in the Dockerfile

[teor2345]

Motivation

We're getting very slow build speeds with the Docker cache, as if it isn't working at all.

Reference / Solution

A cargo-chef user recommends removing COPY . . commands, because they overwrite the cache:
LukeMathWalker/cargo-chef#180 (comment)

This solution isn't complete, because we still need to reset the cargo-chef changes somehow. rsync can do this reset, only updating the timestamps of the changed files:
https://serverfault.com/questions/211005/rsync-difference-between-checksum-and-ignore-times-options
https://www.man7.org/linux/man-pages/man1/rsync.1.html

It isn't possible to do this with Dockerfile commands, due to a Docker bug:
https://stackoverflow.com/questions/36553502/is-there-a-way-to-add-only-changed-files-to-a-docker-image-as-a-new-layer-with#36561768

Testing

I checked that these commands worked locally, and only updated the timestamps on the files modified by cargo-chef.

Review

This is important but not a release blocker.

We might want to merge it separately to PR #6933, so we can see the cache speed improvement. The full speed improvement won't be visible until this PR merges to main.

Reviewer Checklist

• Are the PR labels correct?
• Does the code do what the ticket and PR says?
• How do you know it works? Does it have tests?

[gustavovalverde]

This is a very interesting fix. And something new for me to consider as I've built several Dockerfile and I don't recall this being an issue. But that's commonly the case as the build tools create new files, and do not modify existing ones.

In any case, this was an oversight anyways as the COPY command was already used in the first stage, and we're inheriting the whole stage and not specific output files from it.

[teor2345]

This is a very interesting fix. And something new for me to consider as I've built several Dockerfile and I don't recall this being an issue. But that's commonly the case as the build tools create new files, and do not modify existing ones.

Yep, it's very unexpected. cargo-chef has to do it this way because cargo uses file timestamps for caching.

We still need to reset the files changed by cargo-chef, I did that using git because it detects changes and updates timestamps correctly.

[teor2345]

Temporary network error:

buildx failed with: ERROR: failed to solve: failed to push us-docker.pkg.dev/zfnd-dev-zebra/zebra/zebrad-test:pr-6934: failed to copy: io: read/write on closed pipe

[gustavovalverde]

After the last changes including the .git directory, I do not feel as comfortable as before, I think it's too hacky and we might find a better approach, like using Docker mount caches.

I'll go through the documentation you shared and validate each option https://github.com/ZcashFoundation/zebra/pull/6934/files#diff-2f754321d62f08ba8392b9b168b83e24ea2852bb5d815d63e767f6c3d23c6ac5R31-R32

[teor2345]

After the last changes including the .git directory, I do not feel as comfortable as before, I think it's too hacky and we might find a better approach, like using Docker mount caches.

I don't think any purely Docker-based solution will work. Lots of people have tried, but they are still running into this known Docker bug:
https://stackoverflow.com/questions/36553502/is-there-a-way-to-add-only-changed-files-to-a-docker-image-as-a-new-layer-with#36561768

I have replaced git with COPY and rsync.

[teor2345]

The testnet failure is a known issue related to some ongoing testing, we'll fix it in #6936.

Requested changes were made

[gustavovalverde]

How are we measuring the caching improvements in this PR? I'd also all the ls commands from the Dockerfile

[teor2345]

How are we measuring the caching improvements in this PR?

The full speed improvement won't be visible until this PR merges to main. So I suggest we merge it, then check the build speed on a Rust PR that doesn't change any dependencies.

Currently it takes around 70 minutes to do a full rebuild, which is the same as the Dockerfile in the main branch.

The best speed we can achieve rebuilding Rust changes is 42 minutes, because that's how long it takes to rebuild the test binaries (and the caching layer only caches dependencies, not binaries):
https://github.com/ZcashFoundation/zebra/actions/runs/5273943072/jobs/9604406765?pr=6934#step:10:1521

If this PR makes things slower we can fix the Dockerfile so useless steps aren't being re-run, or revert this PR.

I'd also all the ls commands from the Dockerfile

I was using them to check whether caching is active, but sure.

[teor2345]

Failed due to temporary error #7015, I'll restart the jobs.

Requested changes were made

[teor2345]

This is not a high priority, not sure why I thought it was.

[gustavovalverde]

LGTM

[teor2345]

This appears to have reduced the build time by about 14 minutes:

PR before:
https://github.com/ZcashFoundation/zebra/actions/runs/5329582704/jobs/9655409075

PR after:
https://github.com/ZcashFoundation/zebra/actions/runs/5347717722/jobs/9696610852

That's good, but there might be more we can do here to improve caching.