Repository navigation
Simplify Snapshot API for Actor Lifecycle v2 - #2449
Conversation
3f0365d to
a2ed6cc
Compare
| logActorCrashed(ctx, latestActor, ateattr.OperationPause) | ||
| recordActorCrash(ctx, ateattr.ActorMetricAttributes(latestActor, sandboxClass, ateattr.OperationPause)) | ||
| } | ||
| return nil, fmt.Errorf("actor %s is CRASHED: %s", actorRef, storedActor.GetStatus().GetCrash().GetMessage()) |
There was a problem hiding this comment.
🤖 should-fix 🟡 – Behavior change versus pre-#2289 (f6a1dc7). The baseline committed CRASHED here and returned the actor with no error, and its unit test asserted that. This head fails PauseActor with a bare fmt.Errorf, which maps to Unknown at the gRPC boundary and reads as retryable even though the actor is terminal. Return the CRASHED actor as before, or wrap the error as apierror.FailedPrecondition.
There was a problem hiding this comment.
Done. ensurePausedFinalized now delegates to crashActorAndFail, which crashes the actor and returns apierror.FailedPrecondition("actor %s is CRASHED: %s", ...). The crash is terminal, so the caller sees a non-retryable code instead of Internal. The three pause-finalize crash tests assert the code.
| logActorCrashed(ctx, latestActor, ateattr.OperationSuspend) | ||
| recordActorCrash(ctx, ateattr.ActorMetricAttributes(latestActor, sandboxClass, ateattr.OperationSuspend)) | ||
| } | ||
| return nil, fmt.Errorf("actor %s is CRASHED: %s", actorRef, storedActor.GetStatus().GetCrash().GetMessage()) |
There was a problem hiding this comment.
🤖 should-fix 🟡 – Behavior change versus pre-#2289 (f6a1dc7). The baseline suspend finalize always committed SUSPENDED and returned OK. This head commits CRASHED when the latest-generation snapshot has no durable entry and then fails the RPC with a bare fmt.Errorf, so the caller sees Unknown instead of the actor. If the crash is intended, return the CRASHED actor or wrap the error as apierror.FailedPrecondition. Same pattern as in ensurePausedFinalized.
There was a problem hiding this comment.
Done, same as ensurePausedFinalized. TestEnsureSuspendedFinalized_MissingDurableSnapshotClearsAssignment asserts the code.
I applied the same rule to the other places in pause and suspend that crash the actor: ensureMarkedSuspending, ensureAteletSuspended, both checks in ensurePausedSnapshotUploaded, ensurePausedFinalized, ensureSuspendedFinalized, and ensureAteletPaused. They now share a new helper, crashActorAndFail. It returns FailedPrecondition only once the crash has been written. If the crash itself fails, it returns that error, so the caller retries and the retry crashes the actor again.
| return nil, resources.SnapshotURI{}, fmt.Errorf("while building the snapshot URI for tag %s: %w", tagRef, err) | ||
| } | ||
| snap, st := findLatestSnapshotStorage(actor.GetStatus(), ateapipb.SnapshotDurability_SNAPSHOT_DURABILITY_DURABLE, ateapipb.SnapshotStorageStatus_SNAPSHOT_STORAGE_STATUS_COMPLETED) | ||
| snap := findLatestDurableSnapshot(actor.GetStatus(), ateapipb.SnapshotStorageStatus_SNAPSHOT_STORAGE_STATUS_COMPLETED) |
There was a problem hiding this comment.
🤖 question 🟢 – Kept from #2289. The pre-#2289 code derived a tag's object prefix from the tag's uid, so its objects could always be located from the row. Now a random snapshot name is minted at reserve time and the URI is the only record of it. Create and delete are equivalent, but docs/api-guide.md:369-374 still describes the uid-keyed layout and status.storageLocation, which #2289 removed. Is the random name intentional, or should this go back to the uid? Either way the docs section needs updating.
There was a problem hiding this comment.
Went back to the UID: a tag's snapshot now sits under tags/<tag uid> again. The full URI is still recorded in status.snapshot, so the objects can always be found from the tag itself, without status.storageLocation.
The store assigns the UID inside CreateTag, so creation is now two writes:
- Reserve the name.
- Record the
PENDINGsnapshot with its UID-keyed URI, before anything is copied.
A create that dies between those two writes leaves a tag with no snapshot and nothing in storage. DeleteTag now skips the storage cleanup for such a tag, and TagStatus.snapshot is marked +k8s:optional to match. The tag's snapshot uuid is the tag's UID, so it is still the last segment of the URI.
I updated the layout section of the guide. TestTagActorSnapshot_RecreateAfterRecordFailure covers the gap between the two writes, a new store contract case covers recording a snapshot on a reserved tag, and the "UID-based URI" assertions now actually compare against the tag's UID.
| if err := crashActor(ctx, w.store, actorRef, ateattr.OperationSuspend, crashMessageLocalSnapshotNodeUnknown); err != nil { | ||
| slog.ErrorContext(ctx, "Failed to crash actor", slog.String("err", err.Error())) | ||
| } | ||
| return nil, fmt.Errorf("actor is CRASHED because it was in PAUSED state with no completed local snapshot") |
There was a problem hiding this comment.
🤖 nit 🟢 – Kept from #2289. The pre-#2289 code did not inspect the local snapshot in MarkSuspending. It crashed later, in the upload step, and only when assigned_node was empty. Crashing here is stricter than the baseline, and the message says "node unknown" when the actual cause is a missing local snapshot entry. If the early crash stays, use a message that matches the cause.
There was a problem hiding this comment.
Kept the early crash. For a PAUSED actor, the durable URI is now built from the local snapshot's uuid. Without a local snapshot there is no name to build the URI from, and no snapshot to attach the pending durable snapshot to.
The baseline ended in the same CRASHED state, just one step later: atelet rejected the upload with InvalidArgument because the local snapshot name was empty, and that crashed the actor.
The crash message is now crashMessageLocalSnapshotMissing. I also split the same combined check in ensurePausedSnapshotUploaded, so a missing snapshot and an unknown node each record their own message.
| slog.LogAttrs(ctx, slog.LevelError, "Node name not found during finalize pause, crashing actor", | ||
| ateattr.ActorRefLogAttrs(actorRef)...) | ||
| crashStatus = newActorCrash(ateattr.OperationPause, crashMessageLocalSnapshotNodeUnknown) | ||
| } else if snap == nil { |
There was a problem hiding this comment.
🤖 nit 🟢 – Introduced by this PR. The pre-#2289 code tolerated a missing in-progress local snapshot name during pause finalize and still committed PAUSED. This head crashes the actor instead. Only reachable through a corrupted record, but it departs from the baseline behavior this PR is meant to keep.
There was a problem hiding this comment.
Keeping the crash on purpose. A PAUSED actor with no local snapshot can't be resumed from its pause. In the baseline:
- resume silently fell back to the durable snapshot or a cold boot, or crashed restoring an older
LocalSnapshotwhose files had already been pruned; - suspend crashed at the upload step, because suspending a paused actor uploads its local snapshot.
Crashing at finalize reports the corrupted record where it is found, instead of losing the pause without telling anyone. This path now fails with FailedPrecondition too (see the first thread).
25c23c0 to
d8c03f5
Compare
Replace the repeated Snapshot.storage list with direct fields on Snapshot (uuid, locality, fidelity, and a single optional durable_snapshot) so each snapshot generation has one identity, one fidelity, an optional node locality, and at most one durable storage location. Also rename SNAPSHOT_STORAGE_STATUS_IN_PROGRESS to SNAPSHOT_STORAGE_STATUS_PENDING, mark TagStatus.snapshot optional so a tag can be reserved before its snapshot URI is minted, and add documentation comments for atelet checkpoint request messages.
d8c03f5 to
753a0eb
Compare
| toUpdate.Status.State = ateapipb.ActorState_ACTOR_STATE_PAUSED | ||
| // Record the node locality of the local checkpoint and clear older local checkpoints. | ||
| snap := snapshotAtLatestGeneration(toUpdate.Status) | ||
| snap.Locality = toUpdate.GetStatus().GetAssignedNode() |
There was a problem hiding this comment.
🤖 nit 🟢 – The nil check for the latest-generation snapshot ran on the first GetActor, but this closure runs on the row re-fetched after the worker release. The lease makes a concurrent rewrite of status.snapshots unlikely, but if it happens this is a nil-pointer panic in the handler where the previous code returned an error. Keep a snap == nil guard inside the closure.
There was a problem hiding this comment.
Fixed by removing the second GetActor instead of adding the guard back. Freeing the worker never writes the actor row, so the re-read only moved the update's precondition past the checks. Now the precondition pins the version the checks read. If anything writes the actor in between, the update fails with Aborted instead of panicking.
This also fixes a real race. The DeleteWorker workflow crashes actors without taking the actor lease. With the re-read, finalize overwrote that CRASHED with PAUSED and still reported success. TestEnsurePausedFinalized_KeepsCrashFromWorkerDelete covers it, and fails on the old code.
| inProgressSt.Status = ateapipb.SnapshotStorageStatus_SNAPSHOT_STORAGE_STATUS_COMPLETED | ||
| } | ||
| inProgressSt := snapshotAtLatestGeneration(toUpdate.Status).GetDurableSnapshot() | ||
| inProgressSt.Status = ateapipb.SnapshotStorageStatus_SNAPSHOT_STORAGE_STATUS_COMPLETED |
There was a problem hiding this comment.
🤖 nit 🟢 – Same shape as pause finalize: the DurableSnapshot == nil check ran on the pre-release read, and this closure dereferences the re-fetched row without a guard. Separately, this step and the pre-check select the entry by snapshotAtLatestGeneration while ensureAteletSuspended and ensurePausedSnapshotUploaded select it by findLatestDurableSnapshot(PENDING). They coincide in every flow I can construct, but one lookup in all four places would make that obvious.
There was a problem hiding this comment.
Same fix as pause finalize: the second GetActor is gone, so the update is pinned to the version the DurableSnapshot check read. Suspend had the same race, with DeleteWorker's CRASHED overwritten by SUSPENDED. TestEnsureSuspendedFinalized_KeepsCrashFromWorkerDelete covers it.
Also updated the snapshot lookups in pause/resume to use snapshotAtLatestGeneration
| // the operation is pointless, or crashActor's own error if the crash did not | ||
| // commit, so the caller retries and the retry crashes the actor again. | ||
| func crashActorAndFail(ctx context.Context, st crashActorStore, actorRef resources.ActorRef, opName, message string) error { | ||
| slog.LogAttrs(ctx, slog.LevelError, "Crashing actor", |
There was a problem hiding this comment.
🤖 nit 🟢 – This logs "Crashing actor" at Error before crashActor, which already logs through logActorCrashed when the crash commits and stays silent when the actor was already CRASHED. Every new call site now logs the crash twice, and logs it even when nothing changed. Drop this line or demote it to Debug.
There was a problem hiding this comment.
Done, dropped it. The reason isn't lost: it's saved in Status.Crash.Message and carried in the FailedPrecondition the step returns. crashActor's ate.actor.crashed event and counter still record the crash itself, once per transition.
753a0eb to
037af14
Compare
Update snapshot construction, lookup, pruning, Postgres tag serialization, and kubectl-ate output to read and write Snapshot.uuid, Snapshot.locality, Snapshot.fidelity, and Snapshot.durable_snapshot directly instead of scanning repeated SnapshotStorage entries. Consolidate snapshot lookup on latestSnapshot and snapshot retention on pruneSnapshots, which clears locality on superseded entries and drops snapshots that have neither local locality nor a durable snapshot. The suspend upload steps and FinalizeSuspended select the snapshot being written by last_assigned_generation, as the pause steps do, so the snapshot a suspend uploads to is the one its finalize marks COMPLETED.
Reserve the tag in the store without a snapshot first so the store assigns the tag's UID, then record the PENDING snapshot with a URI keyed on that UID before copying any objects. This keeps the tag snapshot prefix tied to the tag's UID while persisting the URI in status.snapshot before the copy begins. If a tag is deleted after reservation but before its snapshot is recorded, skip object-storage cleanup because no snapshot URI was written.
…tate is missing Crash the actor in FinalizePaused and FinalizeSuspended when the latest local or durable snapshot entry is missing, and return FailedPrecondition whenever either finalize step commits CRASHED so callers see the crash instead of a successful pause or suspend response. Add crashActorAndFail and use it in MarkSuspending, CallAteletPause, CallAteletSuspend, and UploadPausedCheckpoint so crashing an actor on missing state consistently returns FailedPrecondition once the crash commits, or returns the store error if crashing failed. Stop re-reading the actor in FinalizePaused and FinalizeSuspended after its worker is freed, so the update's precondition stays on the version those checks read. A write that lands in between, such as the DeleteWorker workflow crashing the actor, now fails the step as Aborted instead of being overwritten with PAUSED or SUSPENDED.
Update unit, functional, validation, and store contract tests for the flattened Snapshot schema, UID-keyed tag snapshot reservation, and FailedPrecondition responses when pause or suspend crashes an actor. Add tests that a crash the DeleteWorker workflow commits while FinalizePaused or FinalizeSuspended is freeing the worker is kept, and that the step fails as Aborted. Update docs/api-guide.md to reflect Snapshot.uuid, Snapshot.locality, Snapshot.fidelity, Snapshot.durable_snapshot, and SNAPSHOT_STORAGE_STATUS_PENDING.
037af14 to
c3a5132
Compare
* cmd/atenet: Remove the router's "all" mode (agent-substrate#2441) All of our in-tree deployments explicitly deploy the router in either ingress or egress mode. Attempting to one run router instance that handles both ingress and egress seems pretty confusing, so we should get rid of the flag that lets us do so. In the future, we might want to split the ingress router and the egress router into separate atenet subcommands (or even separate binaries). * test: Fix macos unit test failures (agent-substrate#2429) # Description `make test` failed on macOS for two host-specific reasons. The DNS relay referenced Linux-only network namespace APIs from an otherwise portable file, and the object storage tests inherited exported `AWS_PROFILE` and `AWS_DEFAULT_PROFILE` values from the developer's shell. Move only the namespace-serving method behind a Linux build tag so portable DNS tests still run on macOS. Clear both AWS profile selectors in the object storage test so local AWS configuration cannot affect it. - [x] Tests pass - [x] Appropriate changes to documentation are included in the PR --------- Signed-off-by: timflannagan <[email protected]> * Update x/net to v0.60.0 to fix govulncheck failures (agent-substrate#2401) Both modules in the post-merge govulncheck matrix resolve golang.org/x/net v0.58.0 and fail on five newly published HTTP/2 vulnerabilities. Update x/net to v0.60.0 in the root module and the GCP Secret Manager plugin, include its required transitive dependency updates, and regenerate vendor. Go also updates the required x/crypto, x/sync, x/sys, x/term, and x/text versions. All first-party changes are in the two go.mod/go.sum pairs; the other changes are generated vendor updates. Affected advisories: GO-2026-6603, GO-2026-6610, GO-2026-6611, GO-2026-6612, and [GO-2026-6617](https://pkg.go.dev/vuln/GO-2026-6617). The [latest failing main run](https://github.com/agent-substrate/substrate/actions/runs/37911293778) already selects Go 1.27.2; its remaining failures are all in x/net. The earlier standard-library findings were resolved by the workflow selecting the patched stable toolchain. This change preserves the current Go version requirement. Validation after merging main `59072f7b`: - Linux/amd64 with Go 1.27.1: `go test -race -count=1 ./...` passed with PostgreSQL/envtest integration enabled (`CI=true`). - All checks in `bash hack/verify-all.sh` passed, including generated-code and vendor consistency, and the checkout remained clean. - GCP Secret Manager plugin race tests passed. - govulncheck v1.8.0 with Go 1.27.2: both root and plugin modules passed with exit 0, zero vulnerable imported packages, and zero reachable findings. Both report GO-2026-5932 only at module level in unimported x/crypto code. The merge retains main's direct x/net dependency classification at v0.60.0. Canonical v0.60.0 DNS parser content was restored in vendor after the generation check found that the automatic merge had retained three older statements. Verification ran as an unprivileged user. Root-gated mount tests and real cluster E2E were not run for this head. AI assistance was used to resolve the merge conflicts and run verification. * ROOTFS snapshot fidelity: micro-VM capture and restore, API opened for micro-VM templates (agent-substrate#2448) Step III of agent-substrate#2102, tracked in agent-substrate#2447. Step I (agent-substrate#2309) removed `on_pause`; step II (agent-substrate#2364) introduced the `SnapshotFidelity` ladder with ROOTFS defined but rejected everywhere. **ROOTFS becomes real on micro-VM.** A ROOTFS snapshot holds the durable volumes plus each container's root-filesystem changes, without guest memory: the actor cold-boots on resume but keeps what it wrote to its rootfs. gVisor keeps rootfs changes inside its memory checkpoint and cannot serve it yet; it now declares that explicitly and a gVisor template that asks for ROOTFS is rejected at admission. Four commits, meant to be read in order. Each builds and passes tests on its own, and ROOTFS stays unreachable from the public API until the third. ## 1. atelet: forward ROOTFS and stop narrowing paused uploads Atelet accepts every level of the ladder and forwards it; which levels a runtime serves is the runtime's knowledge. The paused-upload path still converted a MEMORY pause into a VOLUMES upload, a leftover from the days of separate pause and commit scopes. Pause and suspend now capture one fidelity, a paused actor's template cannot change, and golden actors are never paused, so captured and requested fidelity are always equal; the conversion is replaced by an equality check that refuses any mismatch as drift. ## 2. ateom-microvm: capture and restore ROOTFS Every piece already existed: MEMORY checkpoints tar the host-side rootfs upper per container alongside the Cloud Hypervisor snapshot, and MEMORY restores untar those uppers before assembling the overlays. ROOTFS is the same checkpoint without the VM snapshot, and the upper restore followed by the VOLUMES cold-boot path with a new `keepRootfsUpper` boot parameter so the cold boot does not wipe the restored upper (safe across its retry). Only a MEMORY restore resumes the guest's CPU counters; ROOTFS counts from zero like VOLUMES. `resources.ValidateSnapshotFidelity` now takes the runtime's supported set: micro-VM passes all three, gVisor passes VOLUMES and MEMORY and rejects ROOTFS before taking the actor lock, naming the levels it serves. ## 3. ateapi: allow ROOTFS on micro-VM templates The field-level rejection moves to the template-level cross-field hook: ROOTFS requires `sandbox_config.sandbox_class: SANDBOX_CLASS_MICROVM`. An actor can only be repointed to a template with an identical sandbox config, so a ROOTFS template never ends up under a gVisor actor. The control plane needs no logic change: a ROOTFS snapshot restores at ROOTFS, and a repointed actor drops to VOLUMES as it does for MEMORY, since a rootfs upper is only valid over the image it was written on. ## 4. e2e: prove it on the counter demo The counter app gains a third counter in a file on its root filesystem, printed in the response, so a ROOTFS snapshot is distinguishable from VOLUMES. All lifecycle rows state their rootfs expectation (MEMORY and ROOTFS keep it, VOLUMES resets it). New micro-VM-only ROOTFS rows cover pause, suspend, suspend from PAUSED, and the template repoint and revert. On the gVisor CI lane the ROOTFS rows skip; the new rootfs assertions on MEMORY and VOLUMES rows run there as a side check. ## Behavior changes - **API**: `SNAPSHOT_FIDELITY_ROOTFS` is accepted on micro-VM templates and rejected on gVisor ones with a message naming the requirement. No field or wire changes; the `preferred_fidelity` field-level custom validation tag is removed. - **Atelet**: a paused-upload whose desired fidelity differs from the captured one is now `FailedPrecondition`; previously a MEMORY capture could be downgraded to VOLUMES. Unreachable in practice, as explained in commit 1. - Unchanged: MEMORY and VOLUMES capture and restore, the golden-actor MEMORY override, metric labels (`rootfs` was already a member). - No backward compatibility is kept, per the pre-v1.0 policy. ## Not in this PR `required_fidelity`, `ResumeActorResponse.resumed_fidelity`, the resume fallback rule, gVisor ROOTFS (pending the gVisor team's separable rootfs capture), and restoring a MEMORY snapshot as a ROOTFS cold boot (the artifacts already allow it; atelet would need the rootfs file subset in the manifest to skip the memory image). ## Verification - Unit tests for atelet, ateom-microvm, ateom-gvisor, resources, ateattr, ateomphaselog, ateapi (validation, controlapi, functional), kubectl-ate, and apitool pass; the ateom tests are linux-tagged and were type-checked for linux locally. - golangci-lint, kube-api-linter, python-protos, boilerplate pass; codegen shows no drift. - The micro-VM counter demo was run manually on a GKE cluster with a nested-virtualization node pool and behaved as expected. The ROOTFS e2e rows need a micro-VM lane; the gVisor lane runs the rest. - [x] Breaking change - [x] Not backward compatible 🤖 Generated with [Claude Code](https://claude.com/claude-code) * fix(kubectl-ate): validate atespace flags before connecting (agent-substrate#2464) `get actors` and `get actor-template` now reject missing or conflicting atespace flags before loading kubeconfig or connecting to the API. Previously, a kubeconfig or connection error could hide the argument error. The tests use a missing kubeconfig to check that invalid flags are rejected first and valid queries still reach client initialization. Refs agent-substrate#2463 - [x] Tests pass: `go test -race ./cmd/kubectl-ate/...` - [x] Full Go suite passes: `env -u NO_COLOR go test -race -p 1 ./...` - [x] Lint passes: `hack/verify/golangci-lint.sh` The first full-suite run failed in the CPU timing test and in color-output tests with `NO_COLOR=1`. The serial run above passed. The Python license and proto checks could not finish because `pypi.org` DNS lookup failed. The remaining repository verifiers passed; metrics and shellcheck used local binaries at the versions pinned by the scripts. * Allow ActorTemplates to disable golden snapshot creation (agent-substrate#2328) ActorTemplates can opt out of automated golden snapshot creation with `snapshotConfig.goldenSnapshotConfig.mode: GOLDEN_SNAPSHOT_MODE_DISABLED`, avoiding unused golden actors and snapshots. Omitting the configuration or leaving the mode `UNSPECIFIED` defaults to `ENABLED`. The mode is an enum with declarative validation, following the existing snapshot fidelity pattern. Disabled templates are skipped during queue resync and reconciliation. Their regular actors can cold boot and suspend/resume using their own snapshots. `kubectl ate get actor-templates` displays `DISABLED` in the `GOLDEN TAG` column. The API guide, field table, glossary, and CLI documentation describe the opt-out. The functional regression starts a real `ActorTemplateReconciler`. An enabled control template proves it is making progress while the disabled template's golden actor and tag both remain absent. The test also verifies the disabled template's actor cold-boots, creates its own durable snapshot on suspend, and restores that snapshot on resume. The disabled reconciliation case is part of the existing table-driven unit test. Implements the golden snapshot opt-out feature discussed in agent-substrate#2068. AI assistance: Codex helped implement and test these changes. ### Validation - macOS arm64: race and coverage tests passed for defaulting, API validation, control-plane unit tests, and CLI printing. - The CLI table regression failed before adding the disabled state and passed after the change. - Linux amd64, Go 1.27.1: `go test -race ./...` passed for all 147 packages with tests. - With `CI=true`, fresh race tests for control-plane functional tests and PostgreSQL storage passed: 612 tests/subtests, zero failures and zero skips. - Removing both disabled-mode guards made the lifecycle regression fail because it found the disabled template's golden actor. The complete implementation passed. - `hack/verify-all.sh`: all 14 verifiers passed with unmodified settings. The first attempt hit a linter package-loading timeout; a longer standalone lint run reported `0 issues`, and the standard verifier run then passed after the cache warmed. - Linux builds of `ateapi` and `kubectl-ate` passed. Functional tests use real PostgreSQL and an envtest Kubernetes API with a fake atelet. Real gVisor E2E was not run. - [x] Appropriate changes to documentation are included in the PR. - [x] Race tests, repository verifiers, and affected binaries were checked. --------- Co-authored-by: cloudonly <[email protected]> * atunnel: strip X-Ate-Assignment-Stale from upstream actor responses (agent-substrate#2410) ### Motivation `atunnel` defines `StaleAssignmentHeader = "X-Ate-Assignment-Stale"` to distinguish an `atunnel` infrastructure-level routing rejection (HTTP 421 `Misdirected Request`, returned when an actor is no longer active on that worker) from an HTTP 421 returned by the guest actor application itself (`internal/atunnel/ingress.go#L45-L47`). While `newActorProxy` already strips internal Substrate headers (`ate-target-port`, `ate-target-actor`) on the request leg into the sandbox, it lacked response sanitization on the return leg. As a result, an untrusted actor workload running inside the container could return `X-Ate-Assignment-Stale: true` and spoof an infrastructure routing rejection to downstream routers or clients. ### Changes 1. **`internal/atunnel/ingress.go`**: Added a `ModifyResponse` hook to `newActorProxy()` that removes `X-Ate-Assignment-Stale` from any response received from the actor upstream. 2. **`internal/atunnel/ingress_test.go`**: Added unit test `TestServeHTTPStripsStaleAssignmentHeaderFromActor` asserting that even if the actor application returns `X-Ate-Assignment-Stale: true`, the header is stripped before the response reaches the client. ### Testing - `go test -race -count=1 ./internal/atunnel/...` - `./hack/verify/boilerplate.sh` - `./hack/verify/gofmt.sh` - `./hack/verify/golangci-lint.sh` > The PR title becomes the release note. Write it for users: what changed for them, not how the code changed. AI was used to assist in the development of these changes. - [x] Tests pass - [x] Appropriate changes to documentation are included in the PR * WorkerPools resolve their SandboxConfig and protect it from deletion (agent-substrate#2393) Part of agent-substrate#587. Step 3 of the SandboxConfig management design, after agent-substrate#2332 and agent-substrate#2333. Each WorkerPool now resolves one SandboxConfig per sandbox class, reports it in status, and protects it from being deleted while in use. Workers do not read the resolved config yet; that is the next step. ## How to review The PR is three commits. Each one builds and passes its tests on its own, so they can be reviewed in order. 1. **Resolve each WorkerPool's SandboxConfig into status.** The API change, the resolution rules, and the `Resolved` condition. No events, no finalizer. 2. **Protect SandboxConfigs in use with a finalizer.** The finalizer, the reconciler that releases it, the two-step status write for a switch, and the `ate-setup` and `install-microvm-deps.sh` changes that deal with the finalizer. 3. **Emit events for a WorkerPool's SandboxConfig and add e2e tests.** The Warning and `SandboxConfigSwitched` events, and an e2e suite that walks a pool through every move. ## Resolution Without `configRef`: the pool uses the class default. ```yaml apiVersion: ate.dev/v1alpha1 kind: WorkerPool metadata: name: counter-pool namespace: ate-demo generation: 1 spec: replicas: 2 workerImage: ateom-gvisor:v1 sandboxClasses: - name: gvisor # no configRef status: replicas: 2 readyReplicas: 2 selector: ate.dev/worker-pool=counter-pool sandboxClasses: - name: gvisor configRef: name: gvisor-default # newest gvisor config annotated is-class-default conditions: - type: Resolved status: "True" reason: Resolved message: resolved to SandboxConfig "gvisor-default" observedGeneration: 1 ``` With `configRef` pointing at a config that does not exist: the pool keeps the config it resolved last time. ```yaml apiVersion: ate.dev/v1alpha1 kind: WorkerPool metadata: name: pinned-pool namespace: ate-demo generation: 2 spec: replicas: 2 workerImage: ateom-gvisor:v1 sandboxClasses: - name: gvisor configRef: name: gvisor-release2 # does not exist status: replicas: 2 readyReplicas: 2 selector: ate.dev/worker-pool=pinned-pool sandboxClasses: - name: gvisor configRef: name: gvisor-release1 # still in use, still protected conditions: - type: Resolved status: "False" reason: SandboxConfigNotFound message: SandboxConfig "gvisor-release2" not found; keeping "gvisor-release1" observedGeneration: 2 ``` - With `configRef`: that config is used. It must exist, belong to the same class, and not be in the process of being deleted. - Without `configRef`: the newest config of the class that is annotated `sandboxconfig.ate.dev/is-class-default: "true"` and is not being deleted is used. - If no config can be resolved, the status `configRef` keeps the config resolved last time, and the `Resolved` condition is `False` with reason `SandboxConfigNotFound`, `SandboxClassMismatch`, `SandboxConfigDeleting`, or `DefaultConfigNotFound`. A Warning event is emitted when the reason changes. - `kubectl get workerpool` gains `SANDBOXCONFIG` and `RESOLVED` columns. - The counter demo no longer pins its pool to `gvisor-default`. It follows the class default. ## Switching configs When a class switches to a different config, the controller writes status twice. First it records the new config in `status.sandboxClasses[].pendingConfigRef`, which protects the new config while the current one stays in `configRef`. Then it reads the new config from the API server. If the config exists, belongs to the class, and is not being deleted, `configRef` moves to it. Otherwise the class keeps the current config and `Resolved` becomes `False`. This prevents a pool from recording a config that was deleted between the cache read and the status write. ## Events Each time a class's `configRef` changes, the controller emits a Normal event with reason `SandboxConfigSwitched`. The event is emitted after the status write that records the change, so each switch shows up once. A reconcile that leaves the config unchanged emits nothing, so the pool's events show the configs it has used, in order. A class that fails to resolve emits a Warning event with the condition reason. To see the history for one pool: ``` kubectl -n ate-demo events --for workerpool/counter-pool ``` Example for the counter pool, which has no `configRef` and so follows the class default. It first resolved to `gvisor-default`. Then a newer config `gvisor-default-v2` annotated `is-class-default: "true"` was created, and the pool switched to it on its own. Then `gvisor-default-v2` lost the annotation, got it back, and was finally deleted. A config that is being deleted is no longer a class default, so the pool moved back to `gvisor-default`, and the finalizer on `gvisor-default-v2` was released once no pool used it. ``` $ kubectl -n ate-demo-counter events --for workerpool/counter LAST SEEN TYPE REASON OBJECT MESSAGE 61s Normal SandboxConfigSwitched WorkerPool/counter sandbox class "gvisor" uses SandboxConfig "gvisor-default" 53s Normal SandboxConfigSwitched WorkerPool/counter sandbox class "gvisor" switched from SandboxConfig "gvisor-default" to "gvisor-default-v2" 46s Normal SandboxConfigSwitched WorkerPool/counter sandbox class "gvisor" switched from SandboxConfig "gvisor-default-v2" to "gvisor-default" 39s Normal SandboxConfigSwitched WorkerPool/counter sandbox class "gvisor" switched from SandboxConfig "gvisor-default" to "gvisor-default-v2" 33s Normal SandboxConfigSwitched WorkerPool/counter sandbox class "gvisor" switched from SandboxConfig "gvisor-default-v2" to "gvisor-default" ``` ## Deletion protection - Before recording a config in status, the WorkerPool controller adds the finalizer `sandboxconfig.ate.dev/workerpool-protection` to it. - A new `SandboxConfigProtectionReconciler` removes the finalizer once no WorkerPool status references the config. It checks the cache first, then confirms with a live list from the API server before removing it. - A SandboxConfig that is being deleted is never resolved for a new WorkerPool and is no longer treated as the class default. WorkerPools that already use it keep using it, and because of the finalizer the config is not actually deleted until every such WorkerPool has moved to another config or has been deleted. This is what the bundled `gvisor-default` looks like once a WorkerPool has resolved it. Nothing else in the config changes. ``` kubectl get sandboxconfig gvisor-default -o yaml ``` ```yaml apiVersion: ate.dev/v1alpha1 kind: SandboxConfig metadata: name: gvisor-default annotations: sandboxconfig.ate.dev/is-class-default: "true" finalizers: - sandboxconfig.ate.dev/workerpool-protection # added by the WorkerPool controller spec: sandboxClass: gvisor defaultVersion: v1 versions: - name: v1 state: Enabled ... ``` After `kubectl delete sandboxconfig gvisor-default` while a WorkerPool still uses it, the delete does not finish. The config gets a `deletionTimestamp` and stays in the cluster with the finalizer until the last WorkerPool moves off it: ```yaml apiVersion: ate.dev/v1alpha1 kind: SandboxConfig metadata: name: gvisor-default annotations: sandboxconfig.ate.dev/is-class-default: "true" deletionTimestamp: "2026-10-11T18:42:07Z" finalizers: - sandboxconfig.ate.dev/workerpool-protection # kept while a WorkerPool status references the config spec: ... ``` To find out which WorkerPools are holding it: ``` kubectl get workerpool -A -o custom-columns=NAMESPACE:.metadata.namespace,NAME:.metadata.name,SANDBOXCONFIG:.status.sandboxClasses[*].configRef.name ``` ## Testing - Unit tests cover resolution, the status write, the finalizer, the two-step switch, and the events, including that a switch is reported once when the first status write is rejected. - The new e2e suite `internal/e2e/suites/sandboxconfig` creates a pool with no `configRef` and no workers and walks it through the sequence in the events example above. Run it with `hack/run-e2e.sh ./internal/e2e/suites/sandboxconfig/...`. ## Breaking changes - A new WorkerPool gets no Deployment until every sandbox class has a resolved config. Before upgrading, make sure each class has a config annotated `is-class-default: "true"`, or set `configRef` on the pool. The bundled `gvisor-default` and the micro-VM config are already annotated. Existing pools keep their Deployment. - `spec.sandboxClasses[].name` is now immutable. To change the class, create a new pool. - Rolling atecontroller back to a version before this PR leaves the finalizer on SandboxConfigs. Remove it with `kubectl patch sandboxconfig <name> --type=merge -p '{"metadata":{"finalizers":null}}'`. - [x] Tests pass - [x] Appropriate changes to documentation are included in the PR * Simplify Snapshot API for Actor Lifecycle v2 (agent-substrate#2449) The initial API developed in agent-substrate#2289 had a repeated SnasphotStorage structure keyed by Durability (RESIDENT, LOCAL and DURABLE). However this was overly descriptive. RESIDENT and LOCAL storage details are local to atelet, and the promotion of snapshots from RESIDENT -> LOCAL -> DURABLE is also driven by atelet. Putting the information in Snapshot API meant that every action triggered by atelet has to invoke ateapi first to record the transition. Instead, this PR simplifies the sturcture by having a single SnapshotStorage to contain the durable storage details. Locality details will not be recorded in the Snapshot API. We only record that a snapshot also has locality on a certain node. #### Breaking change LocalSnapshot is removed and replaced by a Snapshot.locality field. Part of agent-substrate#2069 - [x] Tests pass - [x] Appropriate changes to documentation are included in the PR --------- Signed-off-by: timflannagan <[email protected]> Signed-off-by: Eitan Yarmush <[email protected]> Co-authored-by: Taahir Ahmed <[email protected]> Co-authored-by: Tim Flannagan <[email protected]> Co-authored-by: cloudonly <[email protected]> Co-authored-by: Dmitry Berkovich <[email protected]> Co-authored-by: ktsoator <[email protected]> Co-authored-by: cloudonly <[email protected]> Co-authored-by: Pancho Trujillo <[email protected]> Co-authored-by: Zoe Zhao <[email protected]> Co-authored-by: Michelle Au <[email protected]>
The initial API developed in #2289 had a repeated SnasphotStorage structure keyed by Durability (RESIDENT, LOCAL and DURABLE). However this was overly descriptive. RESIDENT and LOCAL storage details are local to atelet, and the promotion of snapshots from RESIDENT -> LOCAL -> DURABLE is also driven by atelet. Putting the information in Snapshot API meant that every action triggered by atelet has to invoke ateapi first to record the transition.
Instead, this PR simplifies the sturcture by having a single SnapshotStorage to contain the durable storage details. Locality details will not be recorded in the Snapshot API. We only record that a snapshot also has locality on a certain node.
Breaking change
LocalSnapshot is removed and replaced by a Snapshot.locality field.
Part of #2069