fix(compose): mount named volumes so data survives --force-recreate - #86
Open
itxtoledo wants to merge 1 commit into
Open
fix(compose): mount named volumes so data survives --force-recreate#86itxtoledo wants to merge 1 commit into
itxtoledo wants to merge 1 commit into
Conversation
Named volumes declared in compose files were silently skipped: the volume store directory was created, but never bind-mounted into the container, so writes went to the container's ephemeral layer and were discarded on `compose up --force-recreate` (and on any container recreation). That made stateful services like MySQL lose their data whenever the stack was recreated. Resolve named volumes to their backing directory (`<volumesPath>/<runtimeName>/_data`, applying the project prefix or an explicit `name:`/`external:` like Docker does) and bind-mount it, so the data lives in the volume store and survives recreation. `down --volumes` continues to remove them as before. A previous attempt (a87cf87) was reverted (c0934d8) over concerns that Apple's virtiofs cannot chown from inside the container, breaking images like postgres that chown their data dir on init. That concern applies equally to regular bind mounts, which compose already supports and which work; silently discarding data on every recreation is the worse failure mode. The volume store directory is owned by the host user, matching what `mocker run -v name:/path` already does. Tests: added unit coverage for declared, custom-named, external and undeclared volume specs, plus an integration check that a file written to a named volume survives `compose up --force-recreate`.
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.
Problem
Named volumes declared in a
docker-compose.yamlare silently skipped bymocker compose up. The volume store directory is created (you can see it under~/.mocker/volumes/<project>-<name>/_data), but it is never bind-mounted into the container.The consequence is severe for stateful services:
compose up --force-recreate, a changed config hash,compose down && compose up— discards the data.I hit this in production dev: a MySQL stack lost its entire database on
--force-recreate, and every named volume under~/.mocker/volumes/was 0 bytes — proof they were never written to.Root cause
ComposeOrchestrator.resolveVolumeMounts(ComposeOrchestrator.swift:564) intentionally drops named volumes:The skip was added in
c0934d8as a revert ofa87cf87(which had the right idea). The virtiofs/chown concern is real but applies equally to regular bind mounts, which compose already supports and which demonstrably work — including for postgres-style images. Silently losing data on every recreation is the worse failure mode, and the volume store directory is owned by the host user (exactly whatmocker run -v name:/pathalready relies on).Fix
Named volumes are now resolved to their backing directory and bind-mounted, exactly like Docker does internally:
name:/container/path→-v <volumesPath>/<runtimeName>/_data:/container/pathruntimeNameapplies the project prefix unless the volume declares an explicitname:or isexternal:(reusing the existingComposeVolume.runtimeName(projectName:)logic, socompose down --volumesremoval stays in sync).Changes
Sources/MockerKit/Compose/ComposeOrchestrator.swiftresolveVolumeMountsnow takesprojectName,declaredVolumesandvolumesPath; declared named volumes resolve to<volumesPath>/<runtimeName>/_data. Undeclared bare names are still dropped (previous behaviour).Sources/MockerKit/Volume/VolumeManager.swiftmountpointPath(nonisolated) so compose can resolve volume backing dirs.Tests/MockerKitTests/ComposeOrchestratorTests.swiftCHANGELOG.mdVerification
swift test— 448 tests in 35 suites, all green.services: app: image: alpine:3.20 command: ["sh", "-c", "echo hello > /data/hello.txt && sleep 5 && cat /data/hello.txt"] volumes: - mydata:/data volumes: mydata:mocker compose up→hello.txtappears in~/.mocker/volumes/<project>-mydata/_data/(previously the volume stayed empty).mocker compose up --force-recreate→hello.txtsurvives (previously the data was lost).mocker compose down --volumes→ volume removed, as expected.Notes / trade-offs
postgres) may need the host directory to be writable by the container user — the same constraint regular bind mounts already impose today. If virtiofs chown support is added later, no further changes are needed here.down --volumes.