fix(localnet): restore requires a prior up; never create a volume out of band - #273
Merged
Conversation
… of band
RunRestore loaded a pg_dumpall stream into <project>_postgres via a
throwaway `docker run -v <vol>:...`. When the target instance had never
been `up`, that `docker run` CREATED the volume out of band, with no
`com.docker.compose.*` labels. A later `up` adopts it (the "volume ...
already exists but was not created by Docker Compose" warning), and
`docker compose down --volumes` (used by `down` and `remove --force`)
refuses to delete a volume Compose did not create — so the volume is
stranded. This is the root cause behind the M1-RMV-001 e2e failure
("FAIL step 3b: volumes remain after remove").
Require the target instance to already be registered (it was `up`, so
Compose owns the volume) before restoring; refuse otherwise and tell the
user to run `localnet up <name>` first. Applies to cross-name restore
too. This keeps the "never create a volume outside Compose" invariant.
The precondition is enforced via the registry (a proxy for "the
Compose-owned volume exists"). A stricter Docker volume-ownership label
check is deferred and documented in docs/limitations.md.
Tests: seed a stopped instance in the round-trip, cross-name, and
content-SHA restore tests; add TestRestore_RefusesUnknownInstance.
Restore now loads into the instance's EXISTING Compose-owned Postgres volume and refuses to create one out of band. The snapshot/restore test tore the instance down with `remove` (which reclaims the volume), then restored — which the new contract correctly rejects with "instance not found — run localnet up first". Switch the teardown to `down`, which preserves the volume, so restore has a Compose-owned volume to load into.
The test brings up a fresh `e2e-version-test` but had no precondition cleanup, so a leftover instance from a prior/aborted run caused step 1 to abort with "instance already running". Add a best-effort `e2e_cleanup_instance` before `up`, matching the self-defending pattern used by other lifecycle tests. Cleanup lives in the test's precondition (not teardown) because M1-LST-001 depends on e2e-version-test staying up.
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
1 participant
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
The M1-RMV-001 e2e test fails with
FAIL step 3b: volumes remain after remove:volume "canton-e2e-test-default_postgres" already exists but was not created by Docker ComposeRemoved "e2e-test-default"yetFAIL step 3b: volumes remain after remove→canton-e2e-test-default_postgresRoot cause
RunRestoreloads apg_dumpallstream into<project>_postgresvia a throwawaydocker run -v <vol>:.... When the target instance was neverup, thatdocker runcreates the volume out of band, with nocom.docker.compose.*labels. A laterupadopts it (hence the warning), anddocker compose down --volumes— used by bothlocalnet downandlocalnet remove --force— only deletes volumes Compose itself created, so the adopted volume is stranded. On a persistent self-hosted runner, a detached volume left by an earlier job (e.g. a restore test) then poisons M1-RMV-001's assertion.Fix
Enforce the invariant "never create a volume outside Compose" at the source:
restorenow requires the target instance to already be registered (it wasup, so Compose owns the volume). If not, it refuses and tells the user to runlocalnet up <name>first. This also applies to cross-name restore (the target name must have beenuptoo).The precondition is enforced via the registry (a proxy for "the Compose-owned volume exists"). A stricter Docker volume-ownership label check (
docker volume ls --filter label=com.docker.compose.project=... --filter label=com.docker.compose.volume=postgres) is intentionally deferred and documented as a known gap indocs/limitations.md.Behavior change
upinstance) and no-prior-upcross-name restore are no longer allowed — runlocalnet up <name>first.Relationship to related PRs
fix(localnet): remove prunes adopted external volumes) and fix(localnet): scrub adopted volumes on remove #254 (fix(localnet): scrub adopted volumes on remove) fix the symptom at teardown (reclaiming already-adopted external volumes). This PR fixes the root cause sorestorenever produces such a volume in the first place. They are complementary.Tests
TestRestore_RefusesUnknownInstance.go build ./...,go test ./internal/localnet/... ./internal/cli/... ./internal/ui/handlers/...all pass.Note for reviewers / CI
The product fix prevents new orphaned volumes but does not retroactively delete the stale
canton-e2e-test-default_postgresvolume already present on the self-hosted runner. The first green M1-RMV-001 run may need a one-timedocker volume rm canton-e2e-test-default_postgres(or a clean runner).