Add API for "full" container snapshots by gpanders · Pull Request #6430 · cloudflare/workerd

gpanders · 2026-03-26T16:40:28Z

This is part 2 of the containers snapshot API, following up from #6376 which implemented snapshotDirectory() to create a "directory snapshot". This PR implements snapshotContainer() 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.

NOTE: This PR intentionally does not implement full container snapshots in local dev. That will be deferred to a future PR (cc @danlapid).

cc @IRCody @spahl

ask-bonk

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: warnAboutStaleSnapshotVolumes handles stale Docker volumes for directory snapshots, but there's no equivalent mechanism for stale workerd-container-snap-* images. Over time in local dev, these could accumulate. Not blocking, but worth tracking.
[LOW] Missing KJ_LOG in snapshotContainer: snapshotDirectory logs KJ_LOG(INFO, "created snapshot volume", ...) on success, but snapshotContainer has no corresponding log entry. Minor consistency/debugging gap.

This review was generated by an AI assistant and may contain inaccuracies.

src/workerd/server/container-client.c++

ask-bonk · 2026-03-26T16:44:18Z

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.

Summary of findings posted to PR #6430:

[MEDIUM] No stale container snapshot image cleanup — There's a warnAboutStaleSnapshotVolumes mechanism for directory snapshot volumes, but no equivalent for workerd-container-snap-* Docker images that could accumulate over time in local dev.
[LOW] Missing KJ_LOG in snapshotContainer — snapshotDirectory logs on success for observability, but snapshotContainer doesn't. Posted a suggestion comment with the fix.
[LOW] User-facing error message — When restoring a container snapshot with a non-existent ID, the error from inspectImage ("Failed to inspect Docker image") is less clear than the directory snapshot equivalent. Posted an inline comment.

Things that looked good:

Cap'n Proto schema changes are wire-compatible (ordinals preserved, struct renames don't affect wire format, new fields/methods are additive)
The KJ_DEFER cleanup logic in snapshotContainer is correct — it protects the window between commitContainer and the end of the function
parseSnapshotId using UUID::fromString is a good tightening vs. the old ad-hoc hex validation
The validateSnapshotSize refactor eliminates duplication
Test coverage is thorough — round-trip, named, memory unsupported, non-existent ID, directory overlay, exclude mounts, relayer, and cross-DO scenarios
TypeScript types are correctly generated with proper experimental gating

github run

codecov-commenter · 2026-03-26T17:13:30Z

Codecov Report

❌ Patch coverage is 0% with 53 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.77%. Comparing base (9a696e4) to head (83a16ed).
⚠️ Report is 21 commits behind head on main.

Files with missing lines	Patch %	Lines
src/workerd/api/container.c++	0.00%	40 Missing ⚠️
src/workerd/server/container-client.c++	0.00%	13 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6430      +/-   ##
==========================================
+ Coverage   70.70%   70.77%   +0.07%     
==========================================
  Files         427      427              
  Lines      117263   117905     +642     
  Branches    18903    18943      +40     
==========================================
+ Hits        82905    83442     +537     
- Misses      23102    23193      +91     
- Partials    11256    11270      +14

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

gpanders · 2026-03-27T16:43:58Z

Split out the local dev (container-client.c++) implementation, so this PR only adds the API. Will open a separate PR for the local dev implementation for when the Windows compile errors are resolved.

kentonv · 2026-03-27T17:37:21Z

types/generated-snapshot/experimental/index.d.ts

 }
+interface ContainerSnapshot {
+  id: string;
+  size: number;


Hmm, why does this size property need to be here? Why doesn't the container service get the size along with the content? What happens if an application falsifies the size?

What happens if an application falsifies the size?

size here is functionally read-only, it is only provided to the user so they know how large the snapshot is (for whatever purpose). It's not read or used on the restore path: only the id is.

Why does it get passed back to the container capnp API then? You added code in this PR to validate the size passed back from the app. Why not just ignore the size passed back from the app?

Yes you're right -- good catch. I'll remove it (and the validation).

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).

kentonv · 2026-03-27T19:10:07Z

src/workerd/io/container.capnp

+    directorySnapshots @6 :List(DirectorySnapshotRestoreParams);
    # Directory snapshots to restore before the container starts.
+
+    containerSnapshot @7 :ContainerSnapshot;


Let's document that the size is ignored.

What about the name? Does the container runtime actually use that, or just the id?

If the container runtime only uses the id, maybe this should just be :Text and it's just the ID alone?

I considered dropping name as well but I could see that being useful for debugging.

If the container runtime only uses the id, maybe this should just be :Text and it's just the ID alone?

I'm not opposed but the idea behind this was that the user could pass the same object they got back from snapshotContainer() (or snapshotDirectory()) right back into container.start() without needing to know any details about the type (but the fields are there for inspection if needed).

The JS API can still allow that, I'm just talking about the underlying capnp API.

I am worried that we are passing back information that the application could have tampered with, and it would be easy to accidentally write code in the containers runtime that doesn't realize this information can be tampered with. If we pass the ID raw, then that risk seems greatly reduced.

gpanders requested review from a team as code owners March 26, 2026 16:40

ask-bonk bot reviewed Mar 26, 2026

View reviewed changes

src/workerd/server/container-client.c++ Show resolved Hide resolved

src/workerd/server/container-client.c++ Outdated Show resolved Hide resolved

gpanders mentioned this pull request Mar 26, 2026

Add docs and changelog for containers snapshots cloudflare/cloudflare-docs#29385

Open

2 tasks

gpanders force-pushed the ganders/container-snapshots branch 2 times, most recently from 20eadae to 60182d2 Compare March 27, 2026 16:42

gpanders changed the title ~~Add API and local dev implementation for "full" container snapshots~~ Add API for "full" container snapshots Mar 27, 2026

gpanders force-pushed the ganders/container-snapshots branch from 60182d2 to 564d7dd Compare March 27, 2026 16:44

anonrig approved these changes Mar 27, 2026

View reviewed changes

kentonv reviewed Mar 27, 2026

View reviewed changes

gpanders force-pushed the ganders/container-snapshots branch from 564d7dd to 83a16ed Compare March 27, 2026 18:03

kentonv reviewed Mar 27, 2026

View reviewed changes

Conversation

gpanders commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ask-bonk bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ask-bonk bot commented Mar 26, 2026

Uh oh!

codecov-commenter commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gpanders commented Mar 27, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gpanders commented Mar 26, 2026 •

edited

Loading

codecov-commenter commented Mar 26, 2026 •

edited

Loading