Add State Persistence: Snapshot on Stop, Restore on Start (#335 P0-b) - #337
Conversation
… P0-b) The standalone server was in-memory only, so `cloudemu stop`/`start` (and any restart) lost every emulated resource. This adds opt-in persistence for the data-bearing services that share a cross-provider driver interface. - New persist package: schema-versioned Snapshot/ProviderState with Export and Restore that read/write through the storage/database/secrets/compute driver interfaces (Go has no generic serializer for the unexported, mutex/func-laden in-memory value types, so this goes through the same interfaces seed uses). Covers S3/Blob/GCS, DynamoDB/Firestore/Cosmos, Secrets, and compute instances across all three providers in one human-readable, git-diffable JSON file. - serve: --persist (restore after providers are built, snapshot after graceful shutdown so no in-flight request races the read), --state-file, and --persist-metadata-only to omit object bodies for a smaller snapshot. - lifecycle CLI: `start` manages the snapshot path in the run dir; `delete` removes it. Object bodies, secret values, and table items are saved by default; compute instances are recreated via RunInstances (image/type/tags preserved, fresh IDs/IPs). Services without a unified driver interface still start empty and are tracked as follow-ups under stackshy#107.
NitinKumar004
left a comment
There was a problem hiding this comment.
Deep review — state persistence
Strong, well-scoped work. Verified in an isolated worktree at the PR head (#331 has since merged to development, so the real diff is the 5 persist/serve/lifecycle/docs files). Gates green: build / vet / go test -race ./persist/... ./cmd/cloudemu/ (ok both) / gofmt clean.
What's right:
- Interface-based snapshot is the correct design — the justification (unexported fields + mutexes/funcs/nested stores defeat any reflection codec) holds; routing through the seed drivers gives one diffable multi-cloud file.
- Mirror rule satisfied by construction —
targetswires Storage/Database/Secrets/Compute for all three providers (serve.go:186/193/200); persist is provider-agnostic; k8s correctly excluded. - Ordering is right — restore after
rebuild()before serving; snapshot afterShutdown()so no in-flight request races the read. - Fail-fast / fail-closed —
--persistwithout--state-fileerrors; unknown schema rejected; missing file → start empty. Terminated instances skipped. Metadata default correctly flipped to bodies-on. - No new numeric-fidelity loss — the DynamoDB decoder already stores numbers as float64 (
server/aws/dynamodb/types.go:52), so a JSON snapshot round-trip is lossless relative to the emulator's own representation.
Holding on one Medium plus a dead-code Low; two doc-notes below.
M1 (Medium) — non-atomic snapshot write can brick start
Snapshot.WriteFile uses os.WriteFile (truncate-in-place, not atomic). If the write is interrupted — disk-full, OOM, or stop's SIGTERM→SIGKILL escalation (stopTimeout 12s) firing while a large default-bodies snapshot is still being written after the ≤10s graceful shutdown — the file is left truncated. The next start --persist then hits persist.ReadFile → json.Unmarshal fails → restoreState returns that error (not os.ErrNotExist) → runServe aborts. So a partial snapshot fails startup hard, on the exact stop→start path the feature exists for, until the user deletes the file by hand. Inline on WriteFile. Fix: write atomically (temp in the same dir + os.Rename), and/or treat an unparseable snapshot like a missing one (warn + start empty) rather than aborting.
L2 (Low) — table restore silently drops secondary indexes
persist.Table captures only Name/PartitionKey/SortKey (persist.go:74). A table created with a GSI loses it on restore, so a Query against that index that worked before stop fails after start. Please document as a known limitation (or capture indexes).
L3 (Low) — --persist-metadata-only restores zero-byte objects
By design metadata-only omits bodies; on restore PutObject(…, nil, …) (persist.go:341) recreates keys as empty objects — GetObject then returns 0 bytes. Worth an explicit doc note so it isn't a silent surprise.
No AI attribution. Requesting changes on M1 + the L1 dead-code removal (both cheap); L2/L3 are doc follow-ups.
| return err | ||
| } | ||
|
|
||
| return os.WriteFile(path, b, filePerm) |
There was a problem hiding this comment.
M1 (Medium). os.WriteFile truncates in place, so an interrupted write (disk-full, OOM, or stop's SIGTERM→SIGKILL escalation while a large default-bodies snapshot is mid-write after graceful shutdown) leaves a truncated file. Then start --persist → ReadFile → json.Unmarshal fails → restoreState returns the error (not os.ErrNotExist) → runServe aborts, wedging startup on the very stop→start path this feature serves, until the user deletes the file. Write atomically: marshal to a temp file in the same dir, then os.Rename onto the target (rename is atomic on the same filesystem). Optionally also treat an unparseable snapshot in restoreState like a missing one — warn and start empty — so a corrupt file never blocks boot.
There was a problem hiding this comment.
Fixed both halves. WriteFile now marshals to a temp file in the same dir and os.Renames it onto the target — atomic on the same filesystem, so an interrupted write (disk-full/OOM/SIGKILL) leaves the previous snapshot or none, never a truncated one. And restoreState now fails open: a corrupt/truncated/unknown-schema snapshot logs a warning to stderr and starts empty rather than returning the error and aborting runServe. So a bad file can never wedge the stop→start path.
Verified end-to-end: garbling snapshot.json and running start --persist → server comes up (status running), no manual deletion needed; a normal cycle leaves valid JSON. Added TestRestoreStateIgnoresUnreadableFile (corrupt + missing → nil). 493792f
| } | ||
| } | ||
|
|
||
| if rmErr := os.RemoveAll(assetsDir(dir)); rmErr != nil { |
There was a problem hiding this comment.
L1 (Low) — dead code. Nothing ever creates or writes an assets/ directory (object bodies are inlined as base64 in the single snapshot.json), so this RemoveAll cleans up a path that never exists, and assetsDir/assetsDirName (lines 26, 74) have no producer. Please drop the two symbols and this block — it reads as leftover scaffolding for a bodies-on-disk layout that wasn't taken.
There was a problem hiding this comment.
Removed — dropped assetsDir, assetsDirName, and the RemoveAll block. You're right, object bodies are base64-inlined in the single snapshot.json, so nothing ever writes an assets/ dir; it was leftover scaffolding for a bodies-on-disk layout we didn't take. 493792f
… GSIs Address review on the state-persistence PR: - WriteFile now writes to a temp file and os.Rename onto the target (atomic on the same filesystem), so an interrupted write (disk-full, OOM, SIGKILL) can't leave a truncated snapshot. - restoreState fails open: a corrupt/unparseable/unknown-schema snapshot logs a warning and starts empty instead of aborting startup on the stop→start path. - Capture and restore table secondary indexes (GSIs) from the table config. - Remove dead assets/ cleanup code (object bodies are inlined in the snapshot; no assets directory is ever written). - Docs: note that --persist-metadata-only restores zero-byte objects. Tests: GSI export/restore round-trip; restoreState ignores a corrupt/missing snapshot.
|
Thanks for the thorough review — all points addressed in
Gate green: build / vet / gofmt / |
NitinKumar004
left a comment
There was a problem hiding this comment.
Approving — all findings resolved
Verified the hardening commit (493792f9) in an isolated worktree at the PR head; gate green (build / vet / go test -race ./persist/... ./cmd/cloudemu/ ok both / gofmt clean).
- M1 (both halves) fixed.
Snapshot.WriteFilenow writes to a temp file in the same dir andos.Renames onto the target — atomic on one filesystem, with clean temp-cleanup on every error path — so an interrupted write can never leave a truncated snapshot. AndrestoreStatenow fails open: a corrupt / unreadable / unknown-schema snapshot warns and starts empty instead of wedging startup on the stop→start path. Covered byTestRestoreStateIgnoresUnreadableFile. (Nice detail dropping the now-unusedfilePermconst —CreateTempis 0600 by default.) - L1 fixed — the dead
assets/cleanup (assetsDir/assetsDirName+ theRemoveAll) is gone. - L2 fixed — table secondary indexes are captured (
Table.GSIs) and restored viaCreateTable;TestExportRestorePreservesGSIsproves the round-trip. - L3 — docs now note that
--persist-metadata-onlyrestores zero-byte objects.
One non-blocking, forward-looking heads-up: fail-open + overwrite-on-stop means that once SchemaVersion is ever bumped, an old-but-valid snapshot would be rejected → silent empty start → the next stop overwrites it → old data lost. Harmless today (schema v1, no migration), and fail-open was the right call for the corrupt-file case — just worth revisiting when v2 lands (e.g. rename a schema-rejected file aside rather than overwrite it).
Clean, well-tested turnaround — thanks. LGTM.
…#338) * Add cloud snapshots: named save/load/list/delete of emulator state (#335 P1) Persistence (#337) auto-saves one state on stop; this adds multiple named, restorable snapshots on a live server — a local, free equivalent of LocalStack Cloud Pods, built on the same persist engine. - persist: add optional Meta header to Snapshot and whole-emulator ExportAll / RestoreAll helpers (serve's persist-on-stop path refactored to reuse them). - admin: GET /_cloudemu/snapshot exports the whole-emulator state as JSON, POST rebuilds to empty and restores from the posted state; both act on every provider like reset. Wired through NewControl (snapshot/restore may be nil to disable, e.g. --admin=false). - CLI: `cloudemu snapshot save|load|list|delete <name>`. save/load talk to the running daemon's control plane; list/delete are file operations. Snapshots are single JSON files under ~/.cloudemu/snapshots/, atomic-written, with name sanitization to block path traversal and --force to overwrite. Covers the same services as persistence (object storage, NoSQL tables, secrets, compute). Tests: persist multi-provider ExportAll/RestoreAll; admin snapshot GET/POST/disabled; CLI name validation, flag parsing, and save/load/list/delete against a test server. * Harden cloud snapshots: broaden admin warning, list Meta, document destructive load Address review on the cloud-snapshots PR: - Broaden the non-loopback --admin warning: GET /_cloudemu/snapshot dumps all emulated state (including secret values) to any caller, not just reset wiping. - `snapshot list` now reads each file's Meta header, showing the recorded createdAt and a providers column instead of the file mtime alone. - Document that `load` is destructive (wipe then restore) and note the future staging-and-swap hardening for a failed restore.
Objective / Issue
P0-b of the "minikube for cloud resources" roadmap (#335), and the persistence half of #107. The standalone server was in-memory only, so
cloudemu stop/start(or any process restart) lost every emulated resource — the biggest dev-experience gap versus LocalStack (whose persistence + Cloud Pods are paywalled). This adds opt-in persistence so resources survive a restart.What we found
memstore.Storefields across ~77 service mocks, and the value types holdsync.Mutex,func(e.g.lambda funcData.handler), and nested stores. Go has nopickle;json/gob/msgpackall drop unexported/func fields, and reflection can't reach unexported fields withoutunsafe. So a "save everything" engine isn't feasible — persistence must go through clean interfaces.--persistoff (the default) behavior is byte-for-byte unchanged; the in-process test-double API is untouched.How we fixed it
persistpackage: schema-versionedSnapshot/ProviderStatewithExportandRestorethat read/write through the existing storage / database / secrets / compute driver interfaces (the same onesseeduses). One human-readable,git-diffable JSON file spans all three providers.serve:--persistrestores after providers are built and snapshots after graceful shutdown (so no in-flight request races the read);--state-file;--persist-metadata-onlyto omit object bodies for a smaller snapshot.startmanages the snapshot path under the run dir;deleteremoves it.RunInstances, so image/type/tags are preserved but the emulator assigns fresh instance IDs/IPs.How it works
State lives in RAM while running (fast, unchanged); on stop it's exported to one JSON file on disk, and on start it's read back in before the first request is served.
sequenceDiagram actor U as You participant CLI as cloudemu CLI participant S as serve (daemon) participant P as persist participant D as Drivers<br/>(S3 · DynamoDB · Secrets · EC2) participant F as snapshot.json<br/>(disk) U->>CLI: cloudemu start --persist CLI->>S: spawn serve --persist --state-file … S->>S: build providers (empty) alt snapshot file exists S->>P: ReadFile(state-file) P->>F: read F-->>P: Snapshot (schema-versioned JSON) P->>D: Restore — CreateBucket/PutObject,<br/>CreateTable/PutItem, CreateSecret, RunInstances else first run / no file Note over S: start empty (today's behavior) end S-->>U: endpoints up ✅ Note over U,D: normal use — all reads/writes hit in-memory stores (RAM) U->>D: aws s3 cp / put-item / create-secret / run-instances U->>CLI: cloudemu stop CLI->>S: SIGTERM S->>S: graceful shutdown (drain in-flight, then quiescent) S->>P: Export current state P->>D: ListBuckets/GetObject, Scan, ListSecrets, DescribeInstances D-->>P: buckets/objects, tables/items, secrets, instances P->>F: WriteFile(Snapshot JSON) S-->>U: "state saved to snapshot.json"Key points the diagram encodes: restore happens after providers are built but before serving (so the first request already sees prior state), snapshot happens after graceful shutdown (so no in-flight request races the read), and the whole path goes through the driver interfaces — never raw struct serialization.
Alternatives not taken
gobsnapshot of all stores — impossible: unexported fields + funcs/mutexes/nested stores.Snapshotterfor all ~77 services now — a multi-thousand-line PR that can't be TDD/E2E-certified in one pass; deferred to follow-ups.--persist-metadata-onlyas the opt-out.Docs / Tests / Playground
docs/standalone-server.md— persistence section, flag table, fidelity notes.WriteFile/ReadFilewith schema-version rejection.Test plan
go test -race ./persist/... ./cmd/cloudemu/...go test ./.../go mod tidy/ golangci-lint = clean (CodeQL: only pre-existing findings, none in changed files).awsCLI + the actualcloudemu start/stop/startlifecycle: create bucket+object, DynamoDB table+item, secret, EC2 instance → stop → start → all restored.N/BOOL/Ltypes round-trip correctly;--persist-metadata-onlydrops bodies while keeping keys; idempotent re-stop.Risk & Rollback
--persistfully reverts behavior. A missing/corrupt/old-schema snapshot fails closed (unknown schema is rejected; missing file → start empty).Conclusion
cloudemu start --persistnow keeps object storage, NoSQL tables, secrets, and compute across restarts, in one readable multi-cloud file — a free, diffable analog to LocalStack's paid persistence.Follow-ups (tracked under #107 / #335): per-service
Snapshotterfor the remaining services (VPC, IAM, Lambda, SQS, SNS, …); optional versioned/shareable snapshots ("cloud pods"); event-log replay for deterministic debugging.