Add API for "full" container snapshots#6430
Conversation
There was a problem hiding this comment.
I'm Bonk, and I've done a quick review of your PR.
This PR adds snapshotContainer() API and local dev Docker implementation for full container snapshots, alongside renaming the existing directory snapshot fields for clarity.
- [MEDIUM] No stale container snapshot image cleanup:
warnAboutStaleSnapshotVolumeshandles stale Docker volumes for directory snapshots, but there's no equivalent mechanism for staleworkerd-container-snap-*images. Over time in local dev, these could accumulate. Not blocking, but worth tracking. - [LOW] Missing
KJ_LOGinsnapshotContainer:snapshotDirectorylogsKJ_LOG(INFO, "created snapshot volume", ...)on success, butsnapshotContainerhas no corresponding log entry. Minor consistency/debugging gap.
This review was generated by an AI assistant and may contain inaccuracies.
|
I'm Bonk, and I've done a quick review of your PR. This PR adds Summary of findings posted to PR #6430:
Things that looked good:
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6430 +/- ##
==========================================
+ Coverage 70.70% 70.89% +0.19%
==========================================
Files 427 428 +1
Lines 117263 119644 +2381
Branches 18903 18962 +59
==========================================
+ Hits 82905 84819 +1914
- Misses 23102 23556 +454
- Partials 11256 11269 +13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
20eadae to
60182d2
Compare
|
Split out the local dev ( |
60182d2 to
564d7dd
Compare
564d7dd to
83a16ed
Compare
83a16ed to
e7f24df
Compare
|
The generated output of |
snapshotContainer creates a "full container" snapshot, which includes the entire disk/filesystem and (optionally) the state of the container memory when the snapshot is created. Unlike directory snapshots, only a single container snapshot can be restored (but multiple directory snapshots can be restored alongside a container snapshot).
This uses the "docker commit" feature to create a new image from a running container. Restoring a full container snapshot is then as simple as using the new image. By design, local dev cannot support memory snapshots, so we explicitly forbid those with an error.
e7f24df to
d1c04ba
Compare
Since @fhanau resolved the Windows release build issues, I've added the container-client.c++ modifications back into this PR so that we have the local dev implementation now. |
This is part 2 of the containers snapshot API, following up from #6376 which implemented
snapshotDirectory()to create a "directory snapshot". This PR implementssnapshotContainer()to create a "full container" snapshot.Conceptually, a "directory snapshot" can be thought of as an (immutable) Docker volume which can be mounted (restored) to other containers. More than one directory snapshot can be restored to a container.
A "full container snapshot" can be thought of as creating a new Docker image based on the current state of the container (this is how it is even implemented in local dev, using the Docker commit API). Only one full container snapshot can be restored into a new container, which effectively replaces the image of the container.
Directory snapshots can be restored alongside a full container snapshot.
cc @IRCody @spahl