Skip to content

mountutil: give an actionable error for unshared bind-propagation source - #5119

Open
pujitha24 wants to merge 1 commit into
containerd:mainfrom
pujitha24:auto/issue-4423
Open

mountutil: give an actionable error for unshared bind-propagation source#5119
pujitha24 wants to merge 1 commit into
containerd:mainfrom
pujitha24:auto/issue-4423

Conversation

@pujitha24

Copy link
Copy Markdown
Contributor

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 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

Fixes #4423

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>
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.

Unable to start node_exporter from official docker compose (mountpoint "/" doesn't have optional field neither of [shared: master:])

1 participant