-
Notifications
You must be signed in to change notification settings - Fork 3.8k
oci: Use WithReadonlyTempMount when adding users/groups #8351
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
Conversation
Signed-off-by: Djordje Lukic <[email protected]>
thaJeztah
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
I added cherry-pick labels for this, as it reduces the risk of code diverging |
|
derp; new code isn't doing the same, but actually fixing the overlay mounts (updated my previous comment) |
| // tryReadonlyMounts is used by the options which are trying to get user/group | ||
| // information from container's rootfs. Since the option does read operation | ||
| // only, this helper will append ReadOnly mount option to prevent linux kernel | ||
| // from syncing whole filesystem in umount syscall. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should keep the comments here to know why we use readonly mount .... no just remove it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The #8259 change added a new
WithReadonlyTempMountthat will force the mount to be readonly.This change removes the
tryReadonlyMountsand uses that function.Relates to #8259
Related: rumpl/moby#92 and moby/moby#45267
Note: once this PR is good to go I will backport these to 1.6 and 1.7, this is needed for moby's migration to the containerd image store and moby uses 1.6.x