Skip to content

feat(volumes): optional fail-fast guard that the volume path is on the expected filesystem (distills #3196) - #3732

Open
AdaAibaby wants to merge 1 commit into
e2b-dev:mainfrom
AdaAibaby:feat/volume-mount-fstype-guard
Open

AdaAibaby wants to merge 1 commit into
e2b-dev:mainfrom
AdaAibaby:feat/volume-mount-fstype-guard

Conversation

@AdaAibaby

Copy link
Copy Markdown
Contributor

Distilled minimal core of #3196 (closed inactive). Instead of re-proposing that PR's six-backend plugin framework, this ships only the part with concrete, non-speculative value — and leaves the broader pluggable-backend design to the maintainers.

Problem (still on main)

CreateVolume does os.MkdirAll on the path derived from PERSISTENT_VOLUME_MOUNTS, assuming local disk (volume_create.go → raw os.MkdirAll(fullPath, 0o700)). When an operator intends a distributed filesystem (JuiceFS, CephFS, GlusterFS, SeaweedFS, BeeGFS, …) for volumes but it is not actually mounted, MkdirAll silently creates the volume on the local root disk: the create "succeeds", data isn't shared across sandboxes, and the failure only surfaces later when the local disk fills.

Fix

Optional VOLUME_EXPECTED_FSTYPE. When set, CreateVolume verifies — before MkdirAll — that the volume path is on a mount of that filesystem type (read from /proc/mounts, matching the longest mount-point prefix so a not-yet-created leaf resolves to its enclosing mount), and fails fast otherwise. Empty (the default) keeps today's local behavior with no mount check — a no-op unless opted in.

VOLUME_EXPECTED_FSTYPE=fuse.juicefs   # or ceph, fuse.glusterfs, fuse.seaweedfs, beegfs, …
PERSISTENT_VOLUME_MOUNTS=default:/mnt/juicefs

Why only this, not #3196's six backends

#3196 added a local/juicefs/cephfs/glusterfs/seaweedfs/beegfs backend framework (22 files, ~1338 lines). But the five distributed backends were identical wrappers over a shared MountpointBackend, differing only in the /proc/mounts fstype string checked in Healthy() — every one delegated create/delete to the same directory operations. The real value is exactly that mount-type verification; it's implemented here in one file + test and one config field, no backend zoo, no default behavior change. The broader design is left for maintainer direction — #3195 stays open.

Tests

  • mountFSTypeFrom: inside mount, mount point itself, prefix-colliding sibling (/mnt/jfs vs /mnt/jfs-backup), fallback to root, root.
  • verifyVolumeMountFrom: no-op when unset; pass on expected fs; reject when the distributed FS isn't mounted and the path lands on the local root (the footgun); reject wrong fstype; error on missing mounts file.

4 files, +184. go build ./... and go vet pass.

Relates to #3195. Distills #3196.

…e expected filesystem

Volume create does os.MkdirAll on the path derived from PERSISTENT_VOLUME_MOUNTS
assuming local disk. When an operator intends a distributed filesystem (JuiceFS,
CephFS, GlusterFS, SeaweedFS, BeeGFS, ...) for volumes but it is not actually
mounted, MkdirAll silently creates the volume on the local root disk: the create
"succeeds", data is not shared across sandboxes, and the failure only surfaces
later when the local disk fills.

Add an optional guard: VOLUME_EXPECTED_FSTYPE. When set, CreateVolume verifies,
before MkdirAll, that the volume path is on a mount of that filesystem type
(read from /proc/mounts, matching the longest mount-point prefix so a not-yet-
created leaf resolves to its enclosing mount), and fails fast otherwise. Empty
(the default) keeps today's local behavior with no mount check, so this is a
no-op unless opted in.

Distilled minimal core of the earlier e2b-dev#3196: that PR added a six-backend plugin
framework (local/juicefs/cephfs/glusterfs/seaweedfs/beegfs, 22 files, ~1338
lines), but the five distributed backends were identical wrappers differing only
in the /proc/mounts fstype string they check - every one delegated create/delete
to the same directory operations. The real, non-speculative value is exactly
that mount-type verification, which this implements in one file + test and one
config field, without the backend zoo or any default behavior change. The broader
pluggable-backend design is left to the maintainers (issue e2b-dev#3195 stays open).

Tests cover fstype resolution (inside mount, mount point itself, prefix-colliding
sibling, fallback to root, root), and the guard (no-op when unset, pass on
expected fs, reject when the distributed FS is not mounted and the path lands on
the local root, reject wrong fstype, error on missing mounts file).

Relates to e2b-dev#3195. Distills e2b-dev#3196.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants