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

Clarify when last-modified time is considered for caching #13702

Closed

Conversation

rpygithub
Copy link

Proposed changes

This change updates the "Leverage build cache" section of the "Dockerfile best practices" to make it explicit that the last-modified time of image file(s) is considered first, before examining file content. Otherwise, the reader might form an impression that the last-modified time of file(s) is not considered and checksums are calculated in any case. See this comment on a BuildKit issue for an example where this caused confusion.

Before examining the contents of cached image file(s),
the last-modified time of the file(s) is considered.

The "basic rules" in the "Leverage build cache" section
of the Dockerfile best practices guide are updated
to make this explicit.
@rpygithub rpygithub changed the title Clarify when last-modified time considered for caching Clarify when last-modified time is considered for caching Oct 15, 2021
@netlify
Copy link

netlify bot commented Oct 15, 2021

✔️ Deploy Preview for docsdocker ready!

🔨 Explore the source changes: 372b1ff

🔍 Inspect the deploy log: https://app.netlify.com/sites/docsdocker/deploys/616977f11b63af000787b040

😎 Browse the preview: https://deploy-preview-13702--docsdocker.netlify.app

@rpygithub
Copy link
Author

@usha-mandya Not sure if you are the right person to address this to, but I would like to know if there is anything I can do to help move this change along? I see there are some workflow actions requiring approval. Does it require any input at all from me?

@thaJeztah thaJeztah added the area/build Relates to Dockerfiles or docker build command label Nov 11, 2021
@crazy-max
Copy link
Member

Ok I digged up a bit around that phrasing and the actual behavior in BuildKit vs the legacy builder. As we want to make BuildKit the default experience I'm good with this change but I think we could instead add an "info/warn block" under that paragraph about the current BuildKit caching behavior. We could also point to moby/buildkit#1058 to explain it could be handled when implemented at frontend level.

@docker-robott
Copy link
Collaborator

Thanks for the pull request. We'd like to make our product docs better, but haven’t been able to review all the suggestions.
As our docs have also diverged, we do not have the bandwidth to review and rebase old pull requests.

If the updates are still relevant, review our contribution guidelines and rebase your pull request against the latest version of the docs, then mark it as fresh with a /remove-lifecycle stale comment.
If not, this pull request will be closed in 30 days. This helps our maintainers focus on the active pull requests.

Prevent pull requests from auto-closing with a /lifecycle frozen comment.

/lifecycle stale

@knkarthik
Copy link

/remove-lifecycle stale

@knkarthik
Copy link

This is an important piece of information and I would very much want to see this in the docs!

cc: @rpygithub @crazy-max

@tonistiigi
Copy link
Member

I think the current proposal is misunderstanding the behavior and the comment it links to. There are two completely separate topics: the build context upload from local directory to the daemon and then content-based cache checksumming for ADD/COPY commands. The first uses file metadata comparisons between build invocations for the files in the build context(Dockerfiles are always uploaded). The second uses a checksum algorithm that includes file metadata(except timestamp) and file content.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build Relates to Dockerfiles or docker build command
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants