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.

Sign up for GitHub

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

Jump to bottom

[24.0 backport] c8d integration: Use refcount mounter for diff and export #46266

Merged
merged 3 commits into from Aug 21, 2023
Merged

[24.0 backport] c8d integration: Use refcount mounter for diff and export #46266

merged 3 commits into from Aug 21, 2023

Conversation

vvoland
Copy link
Contributor

@vvoland vvoland commented Aug 18, 2023

- What I did
Use refcounted mounter introduced in #45698 when doing diff and export. This is to reuse already mounted rootfs of a container and avoid breaking overlay by mounting an workdir layer as a lowerdir of other mounts.

Also added an integration test to verify.

- How I did it
See individual commits.

- How to verify it

$ make DOCKER_GRAPHDRIVER=overlayfs TEST_INTEGRATION_USE_SNAPSHOTTER=1 TEST_FILTER='TestOverlayfs' test-integration
Test with #45698 reverted 
INFO: Testing against a local daemon
=== RUN   TestOverlayfs
=== RUN   TestOverlayfs/diff
    overlayfs_test.go:80: diff caused overlayfs kernel warning: <4>[520803.140901] overlayfs: lowerdir is in-use as upperdir/workdir of another mount, accessing files from both mounts will result in undefined behavior.
=== RUN   TestOverlayfs/export
    overlayfs_test.go:80: export caused overlayfs kernel warning: <4>[520803.156318] overlayfs: upperdir is in-use as upperdir/workdir of another mount, accessing files from both mounts will result in undefined behavior.
    overlayfs_test.go:80: export caused overlayfs kernel warning: <4>[520803.156462] overlayfs: workdir is in-use as upperdir/workdir of another mount, accessing files from both mounts will result in undefined behavior.
=== RUN   TestOverlayfs/cp_to_container
    overlayfs_test.go:80: cp to container caused overlayfs kernel warning: <4>[520803.177945] overlayfs: upperdir is in-use as upperdir/workdir of another mount, accessing files from both mounts will result in undefined behavior.
    overlayfs_test.go:80: cp to container caused overlayfs kernel warning: <4>[520803.178382] overlayfs: workdir is in-use as upperdir/workdir of another mount, accessing files from both mounts will result in undefined behavior.
=== RUN   TestOverlayfs/cp_from_container
    overlayfs_test.go:80: cp from container caused overlayfs kernel warning: <4>[520803.193088] overlayfs: upperdir is in-use as upperdir/workdir of another mount, accessing files from both mounts will result in undefined behavior.
    overlayfs_test.go:80: cp from container caused overlayfs kernel warning: <4>[520803.193471] overlayfs: workdir is in-use as upperdir/workdir of another mount, accessing files from both mounts will result in undefined behavior.
--- FAIL: TestOverlayfs (1.07s)
    --- FAIL: TestOverlayfs/diff (0.02s)
    --- FAIL: TestOverlayfs/export (0.02s)
    --- FAIL: TestOverlayfs/cp_to_container (0.02s)
    --- FAIL: TestOverlayfs/cp_from_container (0.02s)
FAIL
Before this PR 
INFO: Testing against a local daemon
=== RUN   TestOverlayfs
=== RUN   TestOverlayfs/diff
    overlayfs_test.go:80: diff caused overlayfs kernel warning: <4>[520341.301322] overlayfs: lowerdir is in-use as upperdir/workdir of another mount, accessing files from both mounts will result in undefined behavior.
=== RUN   TestOverlayfs/export
    overlayfs_test.go:80: export caused overlayfs kernel warning: <4>[520341.314918] overlayfs: upperdir is in-use as upperdir/workdir of another mount, accessing files from both mounts will result in undefined behavior.
    overlayfs_test.go:80: export caused overlayfs kernel warning: <4>[520341.315250] overlayfs: workdir is in-use as upperdir/workdir of another mount, accessing files from both mounts will result in undefined behavior.
=== RUN   TestOverlayfs/cp_to_container
=== RUN   TestOverlayfs/cp_from_container
--- FAIL: TestOverlayfs (1.02s)
    --- FAIL: TestOverlayfs/diff (0.02s)
    --- FAIL: TestOverlayfs/export (0.02s)
    --- PASS: TestOverlayfs/cp_to_container (0.01s)
    --- PASS: TestOverlayfs/cp_from_container (0.01s)
FAIL
After this PR 
INFO: Testing against a local daemon
=== RUN   TestOverlayfs
=== RUN   TestOverlayfs/diff
=== RUN   TestOverlayfs/export
=== RUN   TestOverlayfs/cp_to_container
=== RUN   TestOverlayfs/cp_from_container
--- PASS: TestOverlayfs (1.07s)
    --- PASS: TestOverlayfs/diff (0.04s)
    --- PASS: TestOverlayfs/export (0.02s)
    --- PASS: TestOverlayfs/cp_to_container (0.01s)
    --- PASS: TestOverlayfs/cp_from_container (0.01s)
PASS

DONE 5 tests in 3.427s

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@vvoland
integration: Add test for not breaking overlayfs
54953f2 
Check that operations that could potentially perform overlayfs mounts
that could cause undefined behaviors.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
(cherry picked from commit 303e2b1)
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
@vvoland
c8d/export: Use ref counted mounter
b76a0c7 
To prevent mounting the container rootfs in a rw mode if it's already
mounted.  This can't use `mount.WithReadonlyTempMount` because the
archive code does a chroot with a pivot_root, which creates a new
directory in the rootfs.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
(cherry picked from commit 051d51b)
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
@vvoland
c8d/diff: Reuse mount, mount parent as read-only
74bf46a 
The container rw layer may already be mounted, so it's not safe to use
it in another overlay mount. Use the ref counted mounter (which will
reuse the existing mount if it exists) to avoid that.

Also, mount the parent mounts (layers of the base image) in a read-only
mode.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
(cherry picked from commit 6da42ca)
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
@vvoland vvoland added area/runtime kind/bugfix PR's that fix bugs labels Aug 18, 2023
@vvoland vvoland added this to the 24.0.6 milestone Aug 18, 2023
@thaJeztah thaJeztah added status/2-code-review containerd-integration Issues and PRs related to containerd integration labels Aug 18, 2023
thaJeztah
thaJeztah approved these changes Aug 18, 2023
View reviewed changes
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@vvoland vvoland merged commit 2774296 into moby:24.0 Aug 21, 2023
102 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rumpl rumpl rumpl approved these changes

@thaJeztah thaJeztah thaJeztah approved these changes

Assignees
No one assigned
Labels
area/runtime containerd-integration Issues and PRs related to containerd integration kind/bugfix PR's that fix bugs status/4-merge
Projects
None yet
Milestone
24.0.6
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@vvoland @rumpl @thaJeztah