Skip to content

tests/int/mounts: use container paths for mount destinations - #5503

Open
kolyshkin wants to merge 2 commits into
opencontainers:mainfrom
kolyshkin:test-mount-order-dest
Open

kolyshkin wants to merge 2 commits into
opencontainers:mainfrom
kolyshkin:test-mount-order-dest

Conversation

@kolyshkin

Copy link
Copy Markdown
Contributor

The test_mount_order test used host rootfs paths as mount destinations, relying on runc stripping the rootfs prefix from a destination. This is legacy runc behavior (dating back to commit 087caf6, when Docker passed full host paths), not something described by the runtime spec, which says the destination is a path inside the container.

Use container paths for destinations, so the test can be used with other runtimes (e.g. crun). Bind mount sources inside the container rootfs are still used, as this is what the test is about.

While at it, add a comment explaining the legacy behavior in the code.

The runtime spec says the mount destination is a path inside the
container, but runc also accepts a destination prefixed with the host
rootfs path, stripping the prefix. This dates back to commit 087caf6,
when Docker used to pass the full host path to the mountpoint.

Add a comment explaining it.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The test_mount_order test used host rootfs paths as mount destinations,
relying on runc stripping the rootfs prefix, which is a legacy runc
behavior not described by the runtime spec.

Use paths inside the container instead, as the spec says, so the test
can also be used with other runtimes (e.g. crun). Bind mount sources
inside the container rootfs are still used, as this is what the test
is about.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
rootfs := root.Name()
// The runtime spec says the destination is a path inside the container,
// but for legacy reasons (see commit 087caf69) a destination with the
// host rootfs path prefix is also accepted, and the prefix is stripped.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Maybe add a warning for such paths in 1.6 and remove those in 1.8?

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.

1 participant