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

Image caching logic doesn't take into account dependencies from intermediate stages #1455

Open
mthalman opened this issue Oct 2, 2024 · 3 comments

Comments

@mthalman
Copy link
Member

mthalman commented Oct 2, 2024

There is a scenario where the image caching logic incorrectly skips building a Dockerfile when that Dockerfile has an intermediate stage based on an image that has a cache miss. An example of this scenario is the this build (internal link). That build was triggered from this PR: dotnet/dotnet-buildtools-prereqs-docker#1207. In that PR, a change was made only to the crossdeps-builder Dockerfile. The build only ended up publishing that crossdeps-builder image and none of the images that are dependent on it.

Here is an example Dockerfile with such a dependency: https://github.com/dotnet/dotnet-buildtools-prereqs-docker/blob/642ddfa47486760a15781758c7e8c0709c4c96c0/src/azurelinux/3.0/net9.0/cross/amd64/Dockerfile#L3. The image caching logic only accounts for the dependency of the final image. In this case, the final image is based on crossdeps-llvm, which has not changed. So the cross Dockerfile never gets built. But it needs to be built because the content it contains is copied from crossdeps-builder, which has changed.

@lbussell
Copy link
Contributor

lbussell commented Oct 2, 2024

Instead of adding lots of complexity to our custom caching mechanism, maybe it's time to evaluate using Docker's built-in caching. There have been lots of changes since BuildKit became the default. If we store the cache on the registry, Docker should cache most or all layers using the normal cache invalidation rules, which includes base image changes and individual Dockerfile instructions. If we use the "max" cache option, then it'll also store intermediate stages/layers on the registry.

@mthalman
Copy link
Member Author

mthalman commented Oct 2, 2024

Instead of adding lots of complexity to our custom caching mechanism, maybe it's time to evaluate using Docker's built-in caching.

Maybe. But that defeats the whole purpose of #1449. 🤷‍♂️

@lbussell
Copy link
Contributor

lbussell commented Oct 7, 2024

[Triage] One way to fix this would be to include all FROM images in the Dockerfile in the image-info file on https://github.com/dotnet/versions. Then we could re-build when any of those images change. However, this is not a small amount of work for this one scenario that really only affects the buildtools-prereqs repo. Therefore, we should close this as not planned for now, and consider re-opening in the future if it becomes a bigger issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Sprint
Development

No branches or pull requests

2 participants