mountutil: give an actionable error for unshared bind-propagation source - #5119
Open
pujitha24 wants to merge 1 commit into
Open
mountutil: give an actionable error for unshared bind-propagation source#5119pujitha24 wants to merge 1 commit into
pujitha24 wants to merge 1 commit into
Conversation
Motivation: Bind-mounting with `bind-propagation=shared|rshared|slave|rslave` (e.g. `-v /:/host:rslave,ro`, as used by node_exporter's official compose file) requires the mount source to already be a "shared" or "slave" mount on the host, per the kernel's shared subtree rules. When it isn't (common on non-systemd hosts such as Alpine/OpenRC, where "/" is mounted private by default), nerdctl correctly rejects the request, but the error dumped the internal marker strings it checks for instead of telling the user what to do: mountpoint "/" doesn't have optional field neither of [shared: master:] This is not a bug in the propagation check itself: it mirrors the same kernel-level restriction moby's volume parser enforces, and a container runtime cannot make a private mount propagate without the host operator changing it first. Report: containerd#4423 Approach: Reword the error raised by ensureMountOptionalValue() to name the requested propagation mode and suggest the actual fix, `mount --make-rshared <path>` on the host. --make-rshared satisfies both the "shared" and "slave" checks (a slave mount is accepted if its source has either a "shared:" or "master:" peer group), so the same suggestion is correct for both branches. No decision logic changes: the same condition still triggers the error, only the message improves. Validation: This package's tests live in a _linux.go-suffixed file, and this repo's own docs/testing/README.md states unit tests "must be run on a supported OS (linux, windows, or freebsd)" - this change was made on macOS, so `go test` cannot execute them here. What was actually run: - `go build ./...` (darwin) - passes - `GOOS=linux go build ./pkg/mountutil/...` and `GOOS=linux go vet ./pkg/mountutil/...` - pass - `GOOS=linux go test -c ./pkg/mountutil/` - the test binary, including the two updated cases in TestParseVolumeOptions, compiles cleanly (not executed, since it's a linux binary on a macOS host) - `gofmt -l` on both changed files - no output (already formatted) - Separately, in a scratch directory outside the repo, ran a standalone Go program reproducing ensureMountOptionalValue()'s exact logic against the real (OS-agnostic) mount.Info type: mounting "/" with an empty Optional field (the reported host state) reproduces the original opaque error and now yields the new actionable message for both "rslave" and "rshared", while mounting "/" with Optional="shared:1" succeeds with no error - confirming the fixed condition and message end-to-end, short of running it through this repo's own `go test`. This is a pure error-message improvement: user-visible behavior for valid propagation requests is unchanged, and the previously-failing case still fails, just with a clear next step instead of an internal implementation detail. Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation:
Bind-mounting with
bind-propagation=shared|rshared|slave|rslave(e.g.-v /:/host:rslave,ro, as used by node_exporter's official compose file)requires the mount source to already be a "shared" or "slave" mount on
the host, per the kernel's shared subtree rules. When it isn't (common on
non-systemd hosts such as Alpine/OpenRC, where "/" is mounted private by
default), nerdctl correctly rejects the request, but the error dumped the
internal marker strings it checks for instead of telling the user what to
do:
mountpoint "/" doesn't have optional field neither of [shared: master:]
This is not a bug in the propagation check itself: it mirrors the same
kernel-level restriction moby's volume parser enforces, and a container
runtime cannot make a private mount propagate without the host operator
changing it first. Report: #4423
Approach:
Reword the error raised by ensureMountOptionalValue() to name the
requested propagation mode and suggest the actual fix,
mount --make-rshared <path>on the host. --make-rshared satisfies both the"shared" and "slave" checks (a slave mount is accepted if its source has
either a "shared:" or "master:" peer group), so the same suggestion is
correct for both branches. No decision logic changes: the same condition
still triggers the error, only the message improves.
Validation:
This package's tests live in a _linux.go-suffixed file, and this repo's
own docs/testing/README.md states unit tests "must be run on a supported
OS (linux, windows, or freebsd)" - this change was made on macOS, so
go testcannot execute them here. What was actually run:go build ./...(darwin) - passesGOOS=linux go build ./pkg/mountutil/...andGOOS=linux go vet ./pkg/mountutil/...- passGOOS=linux go test -c ./pkg/mountutil/- the test binary, includingthe two updated cases in TestParseVolumeOptions, compiles cleanly
(not executed, since it's a linux binary on a macOS host)
gofmt -lon both changed files - no output (already formatted)Go program reproducing ensureMountOptionalValue()'s exact logic against
the real (OS-agnostic) mount.Info type: mounting "/" with an empty
Optional field (the reported host state) reproduces the original opaque
error and now yields the new actionable message for both "rslave" and
"rshared", while mounting "/" with Optional="shared:1" succeeds with no
error - confirming the fixed condition and message end-to-end, short of
running it through this repo's own
go test.This is a pure error-message improvement: user-visible behavior for valid
propagation requests is unchanged, and the previously-failing case still
fails, just with a clear next step instead of an internal implementation
detail.
Signed-off-by: Pujitha Paladugu 10557236+pujitha24@users.noreply.github.com
Fixes #4423