fix: drop VOLUME declarations that shadow the home bind mount#1122
fix: drop VOLUME declarations that shadow the home bind mount#1122sayotte wants to merge 1 commit into
Conversation
The Dockerfile declared:
VOLUME ["/home/moltis/.config/moltis", "/home/moltis/.moltis", \
"/home/moltis/.npm", "/var/run/docker.sock"]
Pathological case
-----------------
A deployment bind mounts the whole home directory (e.g.
`./moltis-home:/home/moltis`). A VOLUME declared on a *nested* path is not
covered by a parent bind mount: the container runtime attaches an anonymous
volume at each declared subpath, which shadows the corresponding subtree of
the bind mount. The image ships those dirs empty (only `mkdir`, no COPY), so
on first boot moltis finds an empty data dir, seeds its default scaffolding,
and writes a fresh empty `moltis.db` into the anonymous volume. The operator's
real, populated `.moltis` (credentials, sessions, checkpoints, logs) sits
untouched but invisible behind the anonymous volume.
The user-visible symptom is severe and misleading: the app forces onboarding
again because `setup_required` is computed live from credential rows in
`moltis.db`, and the shadowing anonymous volume has an empty one. The real
data is fine; it is just not the file the process opened. The shadowing also
cannot be undone by a downstream/wrapper image -- there is no UNVOLUME -- so
it has to be fixed at the base image.
Illusion of beneficial cases
----------------------------
The only thing VOLUME buys here is an auto-created anonymous volume when the
operator mounts nothing, which sould seem to provide persistence by default.
That benefit is hollow for this image:
- The "persistence safety net" is a mirage. Anonymous volumes survive a
restart but not `rm` / `--force-recreate`, and they orphan silently. It does
not protect data; it only hides where the data went.
- Failing obviously (data in the ephemeral layer, gone on recreate) is better
operator feedback than silently accumulating orphaned anonymous volumes.
Mount points are the operator's responsibility; the dirs are still created and
chowned so an explicit mount lands with correct ownership. A comment documents
the intent without the side effects.
Greptile SummaryThis PR removes the
Confidence Score: 5/5Safe to merge — the change removes a VOLUME instruction that caused silent data loss for operators using parent bind mounts, and leaves all other runtime-stage setup untouched. The deletion is surgical: only the VOLUME line and its comment are removed; mkdir, chown, USER, WORKDIR, EXPOSE, and ENTRYPOINT are identical to before. The fix is well-documented in the new comment, matches the described failure mode exactly, and there are no regressions for operators who already bind-mount explicitly. The /var/run/docker.sock entry in the old VOLUME list was also incorrect (declaring a socket path as a volume would have created a directory there when no mount was supplied), so its removal is an additional correctness improvement. No files require special attention.
|
| Filename | Overview |
|---|---|
| Dockerfile | Removes four VOLUME declarations and replaces the single-line comment with an explanatory block; mkdir/chown and all other runtime-stage instructions are unchanged. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph BEFORE["Before this PR"]
B1["Operator: docker run\n-v ./moltis-home:/home/moltis"] --> B2["Container runtime applies\nparent bind mount /home/moltis"]
B2 --> B3["VOLUME declaration triggers\nanonymous volume on /home/moltis/.moltis\n(nested path)"]
B3 --> B4["Anonymous volume SHADOWS\nbind mount subtree"]
B4 --> B5["App opens /home/moltis/.moltis/moltis.db\nfrom empty anonymous volume"]
B5 --> B6["❌ Fresh empty DB → forced onboarding\nReal data invisible behind shadow"]
end
subgraph AFTER["After this PR"]
A1["Operator: docker run\n-v ./moltis-home:/home/moltis"] --> A2["Container runtime applies\nparent bind mount /home/moltis"]
A2 --> A3["No nested VOLUME declarations\nNo anonymous volumes created"]
A3 --> A4["App opens /home/moltis/.moltis/moltis.db\nfrom the bind mount (real data)"]
A4 --> A5["✅ Existing DB loaded\nCredentials, sessions, checkpoints intact"]
end
Reviews (1): Last reviewed commit: "fix: drop VOLUME declarations that shado..." | Re-trigger Greptile
The Dockerfile declared:
Pathological case
Deployments bind mount the whole home directory (e.g.
./moltis-home:/home/moltis). A VOLUME declared on a nested path is not covered by a parent bind mount: the container runtime attaches an anonymous volume at each declared subpath, which shadows the corresponding subtree of the bind mount. The image ships those dirs empty (onlymkdir, no COPY), so on first boot moltis finds an empty data dir, seeds its default scaffolding, and writes a fresh emptymoltis.dbinto the anonymous volume. The operator's real, populated.moltis(credentials, sessions, checkpoints, logs) sits untouched but invisible behind the anonymous volume.The user-visible symptom is severe and misleading: the app forces onboarding again because
setup_requiredis computed live from credential rows inmoltis.db, and the shadowing anonymous volume has an empty one. The real data is fine; it is just not the file the process opened. The shadowing also cannot be undone by a downstream/wrapper image -- there is no UNVOLUME -- so it has to be fixed at the base image.Illusion of beneficial cases
The only thing VOLUME buys here is an auto-created anonymous volume when the operator mounts nothing, which would seem to provide persistence by default and avoid the overlay copy-up write penalty. Both benefits are hollow for this image:
rm/--force-recreate, and they orphan silently. It does not protect data; it only hides where the data went.Mount points are the operator's responsibility; the dirs are still created and chowned so an explicit mount lands with correct ownership. A comment documents the intent without the side effects.