Skip to content

cache: do not ignore readonly option#2562

Merged
sipsma merged 1 commit intomoby:masterfrom
ktock:refmountcachero
Jan 17, 2022
Merged

cache: do not ignore readonly option#2562
sipsma merged 1 commit intomoby:masterfrom
ktock:refmountcachero

Conversation

@ktock
Copy link
Collaborator

@ktock ktock commented Jan 17, 2022

Fixes #2491 (comment)

Currently, read-only option for a mount is sometimes ignored and this seems to lead to conflicts between (unexpectedly-)writable overlay mounts (related: #2334).

cc @tonistiigi @crazy-max

Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
Copy link
Collaborator

@sipsma sipsma left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for finding+fixing

@sipsma sipsma merged commit 6471f31 into moby:master Jan 17, 2022
@ktock ktock deleted the refmountcachero branch January 17, 2022 00:40
sipsma added a commit to sipsma/buildkit that referenced this pull request Jan 18, 2022
This adds test coverage for ensuring the readonly parameter is honored
as expected in the ref Mount methods. There was a regression introduced
during the moby#2335 that went unnoticed until identified and fixed in moby#2562.
This test coverage should help prevent similar regressions in the
future.

Signed-off-by: Erik Sipsma <erik@sipsma.dev>
sipsma added a commit to sipsma/buildkit that referenced this pull request Jan 18, 2022
This adds test coverage for ensuring the readonly parameter is honored
as expected in the ref Mount methods. There was a regression introduced
during moby#2335 that went unnoticed until identified and fixed in moby#2562.
This test coverage should help prevent similar regressions in the
future.

Signed-off-by: Erik Sipsma <erik@sipsma.dev>
@crazy-max crazy-max added this to the v0.10.0 milestone Feb 4, 2022
@tonistiigi
Copy link
Member

@ktock Does this fix #2334?

@ktock
Copy link
Collaborator Author

ktock commented Feb 15, 2022

@tonistiigi No, this patch doesn't fully fix #2334.
Proposed a fix at #2637.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants