Skip to content

feat(ateapi): send snapshot files and scope from the control plane, remove the snapshot manifest - #1907

Open
Shrutiya Mohan (shrutiyam-glitch) wants to merge 14 commits into
agent-substrate:mainfrom
shrutiyam-glitch:remove-snapshot-manifest
Open

Shrutiya Mohan (shrutiyam-glitch) wants to merge 14 commits into
agent-substrate:mainfrom
shrutiyam-glitch:remove-snapshot-manifest

Conversation

@shrutiyam-glitch

@shrutiyam-glitch Shrutiya Mohan (shrutiyam-glitch) commented Sep 26, 2026 •

Copy link
Copy Markdown
Collaborator

Follow up on #1835
Moving sandbox assets to RestoreRequest - #1866

Why
atelet wrote a manifest.json beside every snapshot and read it back to learn which files make up the snapshot, what scope it captured, which sandbox class took it, and whose it was. That made snapshot data in object storage an authoritative input to restore. #1866 moved the sandbox binaries and pause image onto the request; this PR moves the rest and deletes the manifest. It also takes a manifest GET off the restore path before any download and the final manifest PUT off suspend.

Result: the control plane is the single source of truth for snapshots. atelet takes everything it needs from the request and never reads or writes a manifest.

Where each manifest field comes from now

Manifest field Now
snapshotFiles New: reported by ateom in CheckpointResponse, stored on ExternalSnapshot.snapshot_files / LocalSnapshot.snapshot_files (copied into tags), sent as RestoreRequest.snapshot_files / UploadPausedCheckpointRequest.snapshot_files.
dataSnapshotFiles New: reported by ateom in CheckpointResponse.data_snapshot_files, stored on LocalSnapshot.data_snapshot_files, sent as UploadPausedCheckpointRequest.data_snapshot_files. A FULL capture uploaded as DATA writes exactly these files; an empty list is FailedPrecondition. This replaces the manifest field from #2028 and the per-class durable-dir.tar rule.
scope RestoreRequest / UploadPausedCheckpointRequest captured and desired scope, from the actor and its template.
sandboxClass From the ActorTemplate (immutable, so the class is the one the pause captured with). On upload it only labels metrics. It no longer decides the FULL→DATA conversion.
golden vs. latest No wire field. atelet derives the restore metric label from the snapshot URI: the snapshot is golden when the owner atespace is ate-golden.
sandbox assets / pause image Already on the request since #1866.

Breaking change

Snapshots taken before this change have no recorded file list and can't be restored. After upgrading, regenerate golden tags and cold-boot any actors that were suspended or paused before the upgrade.

  • Tests pass
  • Appropriate changes to documentation are included in the PR

ateom reports the exact files a checkpoint consists of, but atelet only recorded them in the snapshot manifest. Return them to the caller so the control plane can store the file list itself.
Add snapshot_files to LocalSnapshot and ExternalSnapshot. A pause stores the files that atelet's CheckpointResponse lists on the actor's LocalSnapshot. A suspend from running stores them on its ExternalSnapshot, and a tag copies them from the snapshot it references.

This moves the list of files that make up a snapshot into the control plane, so manifest.json can later be removed. A suspend from PAUSED doesn't record files yet, because atelet doesn't report what it uploaded. Nothing reads the lists yet.
ateapi now sends the recorded file lists back to atelet: RestoreRequest.snapshot_files and golden_snapshot_files on restore, and the paused snapshot's files on UploadPausedCheckpointRequest.

atelet reports the files it uploaded in UploadPausedCheckpointResponse, after narrowing a FULL capture to its durable-dir tar for a DATA upload. ateapi records that list on the ExternalSnapshot, so the narrowing lives only in atelet. atelet still reads manifest.json for everything else.
When UploadPausedCheckpoint is retried after a previous attempt uploaded
every file and pruned the local snapshot, atelet recognized the finished
upload by fetching the remote manifest. Check for the uploaded objects
themselves instead, so the retry no longer depends on the manifest.

ateapi now sends captured_scope. With desired_scope and snapshot_files it
determines the files the upload wrote, and atelet looks each one up with a
metadata-only lookup (GCS Attrs, S3 HeadObject). The lookups run
concurrently, so the check takes about one round trip. The destination is
minted once per suspend and each object becomes visible only once fully
written, so finding every file means the previous attempt finished, and
the retry returns those files like a fresh upload. Any missing file fails
the upload as before.
Snapshots no longer carry a manifest.json. The files a snapshot holds are
recorded by ateapi on the actor's ExternalSnapshot (and copied onto tags),
and every restore and upload request now carries them, so the manifest had
no remaining reader. atelet stops writing it at checkpoint and upload, and
stops fetching it at restore; the manifest_fetch restore phase is removed
from the metrics registry.

UploadPausedCheckpointRequest gains sandbox_class, filled from the actor's
template, so turning a FULL capture into a DATA upload still applies the
per-class rule that used to read the class from the manifest. captured_scope
is now required on upload.

ateapi labels a restore golden when its snapshot is owned in the golden
atespace. An actor still borrowing the golden tag's snapshot keeps the
golden label it had before.

The on-node sandbox record written at Run/Restore stays; it loses only the
fields that existed for the manifest.

Snapshots taken before this change have no recorded file list and cannot be
restored. Golden tags must be regenerated, and suspended or paused actors
from before the upgrade must be cold-booted.
@shrutiyam-glitch
Shrutiya Mohan (shrutiyam-glitch) marked this pull request as ready for review September 29, 2026 16:25
@bowei Bowei Du (bowei) added area/api-machinery area/node area/security Security related issue/pr area/storage kind/cleanup Small fixes that are not bugs, for example a typo in a code comment labels Sep 30, 2026

@zoez7 Zoe Zhao (zoez7) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also the PR description should not say "refactor". This is a behavioral change rather than pure refactoring.

Comment thread docs/metrics/registry/metrics.yaml Resolved
Comment thread internal/proto/ateletpb/atelet.proto Resolved
@shrutiyam-glitch Shrutiya Mohan (shrutiyam-glitch) changed the title refactor: send snapshot files and scope from the control plane, remove the snapshot manifest feat(ateapi): send snapshot files and scope from the control plane, remove the snapshot manifest Oct 1, 2026
main now takes the DATA snapshot file set from ateom and keeps it in the
snapshot manifest, which this branch removes. Record it in ateapi
instead: atelet reports data_snapshot_files in CheckpointResponse, the
pause records them on LocalSnapshot, and the suspend of a paused actor
sends them in UploadPausedCheckpointRequest. A FULL capture uploaded as
DATA writes exactly those files; sandbox_class now only labels the
upload metrics.
Also drop RestoreRequest.golden_snapshot_files, left without a reader
by the removal of the DATA_ON_GOLDEN restore scope.
Snapshots no longer carry a manifest: the control plane records each
snapshot's files, and restores take the sandbox from the template's
SandboxConfig. Remove the manifest_fetch restore phase, stop describing
the manifest as part of a snapshot or as where the sandbox version and
pause image are pinned, document ExternalSnapshot.snapshotFiles, and say
that a restore uses the SandboxConfig as it is at resume time.
A snapshot URI names its owner's atespace, so atelet can tell a golden
restore from the URI it already receives: the golden actor's snapshots
and the golden tag an actor borrows until its first suspend are owned in
the golden atespace. Drop RestoreRequest.from_golden_snapshot, which the
control plane set only to carry that bit.
// +k8s:listType=set
// +k8s:eachVal=+k8s:format=k8s-path-segment-name
// +k8s:eachVal=+k8s:maxLength=255
repeated string data_snapshot_files = 5;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok with this as a temporary field, we'll need to update this soon with the Actor Lifecycle v2 proposal that represents the layers of snapshot in the api.

Comment thread internal/proto/ateletpb/atelet.proto Outdated
Comment on lines +517 to +520
// The sandbox class the pause checkpoint was captured with (e.g.
// "gvisor"), from the ActorTemplate's sandbox_config. It labels the upload
// metrics.
string sandbox_class = 11;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be removed?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It only labels atelet's checkpoint-duration histogram for paused uploads (previously read from the manifest). The label is conditionally required, and ateapi's suspend metrics already carry the class from the template, so removed it.

tele.SnapshotKind = ateattr.SnapshotKindLatest
scope := actorSnapshotContentScopeToAtelet(src.Scope)
if src.TemplateReplaced {
scope = ateletpb.SnapshotScope_SNAPSHOT_SCOPE_DATA

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When doing a data only restore, does ate-apiserver need to pass the entire "src.SnapshotFiles" or just "DataSnapshotFiles"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just the data snapshot files should actually be enough
But on main via the manifest, atelet downloads the whole set and ateom ignores the memory images at DATA scope.

I'll record data_snapshot_files on ExternalSnapshot too, the same temporary field as on LocalSnapshot until Lifecycle v2, and send only those when a FULL snapshot is restored as DATA. but that's another API field and touches suspend, tag and resume, so can i do that in a follow-up ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And since this will be updated soon as well ?

The field only labeled atelet's checkpoint-duration histogram for
uploads of paused actors. That label is conditionally required, and
ateapi's suspend metrics already carry the sandbox class from the
template, so the request no longer needs to carry it.
A FULL snapshot restored under a replaced template goes out at DATA
scope, but the restore still listed every file, so atelet downloaded
the memory images that ateom then ignores. Record the data-scope subset
on ExternalSnapshot at suspend, copy it into tags, and send only those
files when a FULL snapshot is restored as DATA.

@aramase Anish Ramasekar (aramase) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A DATA snapshot with no durable-dir volumes now has zero objects, so tagging it fails in CopyPrefix. Copying snapshot.GetSnapshotFiles() and treating an empty list as success would fix that.

The objectstoresnapshot proto still describes the manifest as the commit marker and should be updated in this PR.

nit: this comment still mentions the manifest.

Comment thread cmd/atelet/main.go
// leaves only orphaned files, never a manifest pointing at files that never
// landed; retries overwrite the deterministic object names.
func (s *AteomHerder) uploadSnapshot(ctx context.Context, uri resources.SnapshotURI, srcDir string, rec *sandboxAssetsRecord, templateAtespace, templateName string) error {
// uploadSnapshot uploads snapshotFiles from srcDir to uri, each

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit point is now ateapi's finalize write, but the objectstoresnapshot proto still documents the manifest. Maybe something like "A snapshot is committed only when the control plane records it after UploadSnapshot succeeds; plugins must not infer commit state from object presence."

Comment thread cmd/atelet/main.go

tPersist := time.Now()
sandboxClass, err := s.uploadLocalCheckpointDir(ctx, req, localDir, uri)
snapshotFiles, err := s.uploadLocalCheckpointDir(ctx, req, localDir, uri)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pruning before ateapi commits is why retries have to check object existence, and the plugin API has no RPC for that. Could we prune after commit instead, so a retry just re-uploads and ObjectExists/StatObject can go?

Comment thread cmd/atelet/main.go
}
return nil

// A DATA snapshot of an actor without durable-dir volumes has no files;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A pre-upgrade DATA snapshot with durable-dir volumes passes this check with no files, then fails late in ateom's untarDurableVolumes after a worker is assigned. Could this require files when the spec has a durable-dir volume, like shouldHaveSnapshots does? A note in docs/upgrade.md would help too.

@@ -203,8 +204,9 @@ func isPausedOriginSuspend(actor *ateapipb.Actor) bool {
// the request is keyed by the actor UID, the worker pod UID, and the
// once-minted snapshot location, so a re-entered workflow re-sends the same
// semantic request; once atelet's Checkpoint is idempotent on those keys this

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Fully reentrant with no changes here" no longer holds, since nothing durable records snapshot_files for external checkpoints now. Same comment in workflow_pause.go.

// +k8s:listType=set
// +k8s:eachVal=+k8s:format=k8s-path-segment-name
// +k8s:eachVal=+k8s:maxLength=255
repeated string snapshot_files = 4;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: could the comment say these names are opaque, so clients don't depend on ateom's file layout?

@msau42

Copy link
Copy Markdown
Collaborator

I'm not sure if we should actually do this.

The actual file structure of checkpoints are hypervisor and storage-specific, so we end up leaking a lot of implementation details into the api.

The two primary use cases mentioned in #1835 should be resolved in other ways:

  • Risk of getting access to other actor's files. We're adding support for per-actor authentication as part of: Per-snapshot authorization #777
  • Being able to downscope the snapshot from FULL -> DATA: I think we should leave this up to the hypervisor + storage to figure out how to take a subset of files. Otherwise if we store in in ateapi, that needs ateapi needs to understand each hypervisor's files and which files are rootfs, memory vs data. Also in terms of api scalability, we start imposing restrictions on how many files can represent a snapshot.

Davanum Srinivas (dims) pushed a commit to dims/substrate that referenced this pull request Oct 9, 2026
…-substrate#2432)

Renames the object-store plugin proto package from
`objectstoresnapshot.v1` to `objectstore.v1`. It is split out of agent-substrate#2385
as its own refactor, since that PR adds a non-snapshot service to the
same API.

- Moves `pkg/proto/objectstoresnapshotpb/v1` to
`pkg/proto/objectstorepb/v1` and `objectstoresnapshot.proto` to
`objectstore.proto`, regenerated with `hack/update/codegen.sh`.
- Updates every in-tree caller, the protoredact pinned field names, and
`docs/architecture.md`.
- Services, messages and fields keep their names. The
`cmd/snapshot-plugin` binary, image and manifests are unchanged.

Open agent-substrate#1907 will need a rebase. It adds no references to the old package,
so only its existing conflicts with main apply.

#### Breaking change

gRPC method names change from
`/objectstoresnapshot.v1.<Service>/<Method>` to
`/objectstore.v1.<Service>/<Method>`. Out-of-tree plugins must
regenerate from `objectstore.proto` and roll out with atelet and
ate-api-server. Please add the `breaking-change` label.

- [x] Tests pass
- [x] Appropriate changes to documentation are included in the PR

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/api-machinery area/node area/security Security related issue/pr area/storage kind/cleanup Small fixes that are not bugs, for example a typo in a code comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants