Repository navigation
ateapi: reject VOLUMES snapshots without supported volume mounts - #1972
Open
Luomu (cloudonly) wants to merge 1 commit into
Open
Luomu (cloudonly) wants to merge 1 commit into
Luomu (cloudonly) wants to merge 1 commit into
Conversation
Luomu (cloudonly)
marked this pull request as ready for review
September 29, 2026 03:53
Maya Wang (mayawang)
added
area/api
Maya Wang (mayawang)
requested a review
from Julian Gutierrez Oschmann (juli4n)
September 29, 2026 23:16
Luomu (cloudonly)
force-pushed
the
fix-data-snapshot-template-validation
branch
from
October 8, 2026 02:53
40891a4 to
05ff652
Compare
Luomu (cloudonly)
changed the title
This was referenced Oct 10, 2026
Closed
Closed
Merged
Luomu (cloudonly)
force-pushed
the
fix-data-snapshot-template-validation
branch
2 times, most recently
from
October 13, 2026 18:00
28fb830 to
78ee1d7
Compare
This was referenced Oct 13, 2026
Luomu (cloudonly)
force-pushed
the
fix-data-snapshot-template-validation
branch
from
October 14, 2026 03:40
78ee1d7 to
930be81
Compare
Contributor
Author
|
Julian Gutierrez Oschmann (@juli4n), gentle ping on this one. It is rebased on current main with no conflicts and CI is green. It also unblocks the golden-snapshot stack that builds on it (the warmup/capture consistency PR and the atespace migration), so a first look would help a lot. Happy to adjust anything you'd like changed. |
Luomu (cloudonly)
force-pushed
the
fix-data-snapshot-template-validation
branch
5 times, most recently
from
October 17, 2026 15:55
d215060 to
491cd80
Compare
A template whose preferred fidelity is VOLUMES but that mounts no volume a snapshot can capture (durable-dir, or a CSI volume on the micro-VM class) fails at runtime on every pause and suspend. Reject it at template admission instead, in the custom validation that already checks volume references and the ROOTFS sandbox class. Document the requirement next to the ROOTFS rule in the API guide, including that an external volume's data stays on the volume rather than in the snapshot.
Luomu (cloudonly)
force-pushed
the
fix-data-snapshot-template-validation
branch
from
October 18, 2026 02:05
491cd80 to
cd0ce64
Compare
|
Since #2543 merged, golden capture uses the template's fidelity, so a VOLUMES template with no durable volume passes validation and then ends in GoldenActorCrashed. We run files-only templates, so catching this at validation would help us. |
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
3 participants
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.
Fixes #1936
Reject ActorTemplates whose
snapshot_config.preferred_fidelityis VOLUMES unless a container mounts a durable-dir volume, or a CSI volume on microVM. This catches invalid configurations before suspend crashes the actor. Update the volume snapshot resume fixture with a mounted durable-dir volume.The validation covers both sandbox classes and MEMORY/VOLUMES fidelity across eight mount configurations, including durable-dir plus CSI on gVisor and a mounted systemInfo volume. It retains the upstream ROOTFS rejection case.
Document the requirement beside the ROOTFS rule in the
snapshotConfigrow of the API guide, noting that an external volume's data stays on the volume rather than in the snapshot.Supersedes #2550, which implements the same rule; that PR is closed in favor of this one.
Validation
Rebased onto main
5d7a89070fabee769656d23877353b29c98777b8on 2026-10-16. Resolved the API-guide conflict by keeping the VOLUMES requirement, main's durable-dir permission documentation, and themicrovmsandbox-class spelling.CI=true, non-root test user:go test -race -count=1passed forcmd/ateapi/internal/apivalidation,cmd/ateapi/internal/controlapi, and the completecmd/ateapi/internal/controlapi/functionaltestsuite. All 454 top-level tests passed, with 2,009 passing test entries and no skips, includingTestResumeActor_DataSnapshotIgnoresGolden.hack/verify/benchmarking-python.shpassed after the final main sync.The Linux tests and build ran on
c0eb40f7c9f5d421f5e8f6e5c836ca3dcbc4779f. The final main sync changes only benchmarking Python and documentation; every Go source file and API test input is identical to that tested commit. The Python verifier ran on the final PR commit.Real gVisor/micro-VM E2E was not run.
AI assistance: Codex helped implement, review, and validate the original change; Claude Code rebased it onto current main and added the API-guide documentation. Codex resolved this rebase and ran the validation listed above.