feat: add Docker-native OverlayBD writable live snapshots - #390
Open
andrewathud wants to merge 3 commits into
Open
feat: add Docker-native OverlayBD writable live snapshots#390andrewathud wants to merge 3 commits into
andrewathud wants to merge 3 commits into
Conversation
Introduce dockerWritableMode=native with a collapsed Docker init owner device, stable random device IDs, protected runtime labels, and safe reattach so Docker can restack incremental filesystem checkpoints. Signed-off-by: Andrew Ho <andyminhtuanho@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Docker-native init owners always have a parent image layer. Leaving vsize at 0 made TCMU enable fail for native writable attaches. Co-authored-by: Cursor <cursoragent@cursor.com>
Moby Commit replaces the label map via WithLabels, which dropped runtime-owned OverlayBD live-snapshot labels and forced the overlayfs fallback. Native init also mkfs'd the stacked device, wiping the image filesystem from lowers. Mount the existing FS and re-apply runtime labels after Commit opts. Co-authored-by: Cursor <cursoragent@cursor.com>
Author
|
@Asadullo mind reviewing when you get a chance? |
Author
|
@solvemproblr mind reviewing when you get a chance? (GitHub won't let me formally request review on this upstream PR.) |
There was a problem hiding this comment.
Pull request overview
This PR adds a Docker-runtime-specific path for native OverlayBD writable uppers and live-snapshot semantics, allowing Docker init + container layers to share a single mutable OverlayBD device and enabling recovery across restarts via persisted metadata.
Changes:
- Introduces live-snapshot metadata + labels and wires them into Docker init/container layer preparation, mounts, commit, and removal flows.
- Extends the OverlayBD attach path to optionally pass a stable device ID and to reuse previously attached devices via a persisted backstore mark file.
- Fixes writable upper virtual size defaults to avoid vsize=0 for Docker-native writable attaches.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/snapshot/storage.go | Adds devID support to attaches and refactors attach/mount flow with backstore mark reuse. |
| pkg/snapshot/overlay.go | Adds dockerWritableMode config, label-mutation protection, and mount/commit/remove adjustments for live-snapshot behavior. |
| pkg/snapshot/live_snapshot.go | New live-snapshot metadata + label helpers and mutation rejection logic. |
| pkg/snapshot/live_snapshot_test.go | New unit tests for live-snapshot helpers, metadata lifecycle, and config validation. |
| pkg/snapshot/docker.go | Implements Docker “native” writable/live-snapshot layer preparation and shared-mount aliasing. |
| pkg/snapshot/docker_test.go | Expands Docker gating tests for overlayfs vs native modes. |
| pkg/label/label.go | Adds new OverlayBD live-snapshot label keys. |
| cmd/overlaybd-attacher/main.go | Updates CLI attacher to the new devID-based AttachDeviceParams API. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
516
to
+521
| if err := os.WriteFile(devSavedPath, []byte(device), 0644); err != nil { | ||
| // o.DetachDevice(ctx, snID) | ||
| return fmt.Errorf("failed to create backstore mark file of snapshot %s: %w", snID, err) | ||
| } | ||
| log.G(ctx).Debugf("write device name: %s into file: %s", device, devSavedPath) | ||
| return o.mountBlockDevice(ctx, snID, device, writable, fsType, mkfs) |
Comment on lines
+420
to
+423
| fsType := o.defaultFsType | ||
| if err := o.attachAndMountBlockDeviceWithDevID(ctx, ownerID, RwDir, fsType, false, meta.DeviceID); err != nil { | ||
| return fmt.Errorf("ensure owner overlaybd device mounted: %w", err) | ||
| } |
Member
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.
Summary
Context
Used by HUD sandbox filesystem checkpoints (see hud-evals/hud-monorepo#1103). We intend to pin a HUD-owned fork once
hud-evals/accelerated-container-imageexists; this PR lands the upstream-ready changes for review.Tip
0548cbfc4b014fcb3b93697758594b585a625c2fTest plan
go test ./pkg/snapshot/...in Linux Docker (Go 1.26)Made with Cursor