Skip to content

exec: fix pruning cache mounts with parent ref on no-cache#5306

Merged
tonistiigi merged 1 commit intomoby:masterfrom
tonistiigi:cache-mount-mode-prune
Sep 9, 2024
Merged

exec: fix pruning cache mounts with parent ref on no-cache#5306
tonistiigi merged 1 commit intomoby:masterfrom
tonistiigi:cache-mount-mode-prune

Conversation

@tonistiigi
Copy link
Copy Markdown
Member

fixes #5305

On a build with no-cache, cache mounts were not pruned correctly if the mount was on top of another ref. This also appeared in Dockerfile when mode/uid/gid was set because implicit parent ref is created in these cases in order to change the permissions of a subdir that is used as a cache mount base.

Because it is not possible to know ahead of time what ref will become the parent of cache mount during build, all cache mounts matching the ID that have a parent will be pruned.

On a build with no-cache, cache mounts were not pruned correctly
if the mount was on top of another ref. This also appeared in
Dockerfile when mode/uid/gid was set because implicit parent
ref is created in these cases in order to change the permissions
of a subdir that is used as a cache mount base.

Because it is not possible to know ahead of time what ref
will become the parent of cache mount during build, all cache
mounts matching the ID that have a parent will be pruned.

Signed-off-by: Tonis Tiigi <[email protected]>
@polarathene
Copy link
Copy Markdown
Contributor

Since this will remove the ability to do a build retaining the cache mounts while only discarding the layer cache, is there currently an advised way to achieve the behaviour that's deemed as a bug?

--no-cache is useful to perform a build without layer cache, but I'm not a fan of that forcing a cache mount that other Dockerfile use too. From now on will the only approach way to preserve the cache mounts be via temporarily changing the id before --no-cache is used? (such as with a build arg?)

I'm aware of the existing tracking issue related to separate control, but that's been unresolved for some time. Just curious if there will still be a viable workaround.

@tonistiigi
Copy link
Copy Markdown
Member Author

@polarathene --no-cache-filter for pruning cache mounts is tracked in #1213 . Atm. you can use --no-cache-filter with specific stages, or invalidate cache with some other methods like updating ARG. You can also change the namespace for cache mounts if your use case is to do one-off builds that do not affect cache mounts for other invocations https://github.com/moby/buildkit/blob/master/frontend/dockerfile/docs/reference.md#buildkit-built-in-build-args .

if idx == -1 {
continue
}
itemID := string(k[idx+2:])
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should we if idx+2 < len(k) { to handle out of bounds?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That should not be possible. Then the idx would be -1. In the worst case itemID can be ""

@tonistiigi tonistiigi merged commit a1993e8 into moby:master Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No-cache not working for--mount=type=cache when mode is specified

4 participants