Map named volumes to native container volumes (fixes #96)#122
Map named volumes to native container volumes (fixes #96)#122kylebrowning wants to merge 1 commit into
Conversation
The named-volume branch of `composeVolumeToRunArgs` bind-mounted a host directory and mounted it at the *parent* of the declared destination (`destination.deletingLastPathComponent()`). Two consequences: 1. A single-segment destination collapsed to `/`, so e.g. `redisdata:/data` mounted the volume over the container's root filesystem and shadowed the image's entrypoint — `redis` failed to start with "failed to find target executable docker-entrypoint.sh". 2. Even for nested destinations the host bind mount uses virtiofs, which the guest cannot `chown`. Images whose entrypoint chowns its data dir (postgres, redis, …) died with "chown: Operation not permitted". `container` 1.0.0 supports named volumes natively and auto-creates them on first `run`, so map a Compose named volume straight to a `container` named volume (`<project>_<source>`) mounted at the declared destination. Native volumes are block devices the guest owns, so chown works, and the destination is no longer rewritten. Verified end-to-end: a redis service with `redisdata:/data` now reaches "Ready to accept connections" where it previously crashed on startup. Adds tests covering the native mapping, the Mcrich23#96 root-mount regression, nested destinations with a mode, and the missing-project-name case. Co-Authored-By: Claude <noreply@anthropic.com>
Cyb3rDudu
left a comment
There was a problem hiding this comment.
Nice - this is a real fix. The destination.deletingLastPathComponent() collapsing /data down to / and shadowing the entrypoint is exactly #96, and moving to native volumes + the declared destination is the right call. The tests covering the regression and the nil-project case are great too.
Heads-up on an overlap: #119 rewrites this same branch with a more complete take — it honors the top-level volumes: block (name overrides, external: true skip-create, driver / driver_opts / labels) and replaces the createVolumeHardLink loop with a native container volume create. As written here those get dropped, and createVolumeHardLink in ComposeUp would still run, creating host dirs that nothing mounts anymore.
What this PR gets right that #119 doesn't: the <project>_<source> namespacing. #119 names the volume just data, so two projects would collide.
My plan is to take #119's handling as the base and fold your <project>_<source> default into it (I've left that suggestion on #119) - one coherent design instead of two. So I'm going to hold this one rather than merge it as-is, but the namespacing idea is yours and I'll credit it when I wire it in. Really appreciate you digging into #96; happy to ping you when the combined version is up so you can sanity-check it.
|
closing — #119 merged with your _ |
Problem
The named-volume branch of
composeVolumeToRunArgsbind-mounted a host directory and mounted it at the parent of the declared destination viadestination.deletingLastPathComponent(). Two failure modes fall out of this:/(issue volumes mounts parent directory than we expect, may tries to mount root directory #96). A single-segment destination collapses to/. Soredisdata:/datamounts the volume over the container's entire root filesystem, shadowing the image's entrypoint. Redis fails to start with:chown. Images whose entrypoint chowns its data directory (postgres, redis, …) die with:A side effect of (1) for nested paths (e.g.
pgdata:/var/lib/postgresql/data) is that data was being mounted at/var/lib/postgresql— the parent — so it wasn't actually persisted at the path the user asked for.Fix
container1.0.0 supports named volumes natively and auto-creates them on firstrun. This maps a Compose named volume straight to acontainernamed volume named<project>_<source>(mirroring Docker Compose's namespacing), mounted at the declared destination.Native volumes are block devices the guest owns, so
chownworks, and the destination is no longer rewritten to its parent. This also retires the now-outdated warning text ("thecontainertool does not support named volume references"), which predates 1.0.0.Verification
Against a real
docker-compose.ymlwith aredisservice usingredisdata:/data:failed to find target executable docker-entrypoint.sh.backend_redisdatavolume is auto-created and redis reachesReady to accept connections/redis-cli ping→PONG.Tests
Adds coverage to
ComposeVolumeTests(the named-volume branch previously had none):/All 137 static tests pass. (The
.containerDependentdynamic suites fail in my environment with a pre-existingNo such file or directoryat setup — reproduced identically on a cleanmain, unrelated to this change.)Note / out of scope
A postgres service using a named volume will still hit the postgres image's
initdb: directory ... not empty ... lost+foundguard, becausecontainer's native volumes are formatted filesystems with alost+found. That's image/platform-specific and is worked around user-side by pointingPGDATAat a subdirectory of the mount; it's independent of this fix.