Repository navigation
feat(runtime): run sessions in Cloud Hypervisor VMs behind a setting - #892
Conversation
ca04518 to
4a3b68b
Compare
fdc1d6e to
5f10e95
Compare
Before, every session ran in a Docker container. With the new server setting session-runtime=vm, the server runs each session in its own Cloud Hypervisor VM through internal/runtime/vm, while environment terminals, the assistant, the agent updater, the cleanup helper, the browser companion and image operations stay on Docker until work item 9. The default, docker, changes nothing. Each VM's monitor runs in its own systemd unit in aether.slice, so a VM survives server restarts and updates; the server finds it again from <data dir>/vm/<run id>/vm.json. Mounts reach the guest over one virtio-fs share composed from host bind mounts inside the unit's private mount namespace; host sockets are forwarded over vsock. Session preparation helpers and the dev-exec control parser move out of the Docker files unchanged so both engines share them. The OOM watcher asks the runtime for kills when it can report guest ones itself.
Docker refuses an exec into a paused container at once; a paused guest would instead hold the exec until its context ends, so closed sessions, which are paused, could stall a caller.
A restore that failed after resuming the guest kept the snapshot marked saved. The guest's disks had already moved past it, so restoring it again would pair old memory with newer disks.
4a3b68b to
8161fb3
Compare
 [Medium impact] The new VM path changes how session workloads run when operators enable it. Not ready to merge: VM commands can hang or lose output, and waking or restoring a VM can stop live work or reuse stale saved state.
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| m := &managedExec{engine: e, key: key, tty: tty, proc: p, attachment: att, identity: runtime.ExecIdentity{ | ||
| ContainerID: runtime.ID(idPrefix + key), ExecID: execID, CreationKey: spec.CreationKey, ClaimToken: claim, User: spec.User, | ||
| }} |
There was a problem hiding this comment.
Pipe executions return the raw guest attachment, whose input closer leaves the process's stdin open. runSSHConn.pump closes that writer when an SSH client finishes sending input. A command such as cat therefore never gets EOF, so it keeps running and SSH never receives an exit status. Return a pipe-specific attachment that calls Process.CloseStdin when its input closes.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/runtime/vm/managed.go
Line: 67-69
Comment:
**Pipe commands never receive EOF**
Pipe executions return the raw guest attachment, whose input closer leaves the process's stdin open. `runSSHConn.pump` closes that writer when an SSH client finishes sending input. A command such as `cat` therefore never gets EOF, so it keeps running and SSH never receives an exit status. Return a pipe-specific attachment that calls `Process.CloseStdin` when its input closes.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| } | ||
| execID := processID("managed") | ||
| argv := append([]string{coordtransport.CLIPath, devexec.Command, mode, spec.CreationKey, claim}, spec.Argv...) | ||
| p, att, err := e.spawn(ctx, key, guest.ProcessSpec{ID: execID, Argv: argv, Env: env, User: spec.User, Dir: spec.WorkingDir, TTY: size, Lossless: spec.Lossless}) |
There was a problem hiding this comment.
StartExecPipe must keep stdout lossless even when callers omit ExecSpec.Lossless. Passing that flag through unchanged lets the guest drop unread output after 256 KiB. Existing callers, including the Codex bridge and ACP keeper bridge, omit the flag. A slow reader can therefore lose part of a reply and break the conversation. Preserve lossless stdout for every pipe execution.
Knowledge Base Used: Environment construction and execution
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/runtime/vm/managed.go
Line: 63
Comment:
**Agent replies can lose bytes**
`StartExecPipe` must keep stdout lossless even when callers omit `ExecSpec.Lossless`. Passing that flag through unchanged lets the guest drop unread output after 256 KiB. Existing callers, including the Codex bridge and ACP keeper bridge, omit the flag. A slow reader can therefore lose part of a reply and break the conversation. Preserve lossless stdout for every pipe execution.
**Knowledge Base Used:** [Environment construction and execution](https://app.greptile.com/-/switch-tenant?tenant=aethercode&next=%2Fcustom-context%2Fknowledge-base%2F3xdevops%2Faether%2F-%2Fdocs%2Fenvironment-construction-and-execution.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| if err != nil { | ||
| return 0, false | ||
| } | ||
| code, err := h.Process(guest.MainID).Wait(ctx) |
There was a problem hiding this comment.
waitMain stops checking the monitor while it waits for Main. If Cloud Hypervisor crashes or is killed during that wait, the guest client retries the disconnected call and waits for a connection that will never return. The scheduler passes its long-lived superCtx, so the session can stay marked running indefinitely instead of reporting exit 255. Watch the unit alongside the guest wait, or use bounded status calls.
Knowledge Base Used: Workspace execution and synchronization
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/runtime/vm/lifecycle.go
Line: 171
Comment:
**Crashed VMs stay marked running**
`waitMain` stops checking the monitor while it waits for Main. If Cloud Hypervisor crashes or is killed during that wait, the guest client retries the disconnected call and waits for a connection that will never return. The scheduler passes its long-lived `superCtx`, so the session can stay marked running indefinitely instead of reporting exit 255. Watch the unit alongside the guest wait, or use bounded status calls.
**Knowledge Base Used:** [Workspace execution and synchronization](https://app.greptile.com/-/switch-tenant?tenant=aethercode&next=%2Fcustom-context%2Fknowledge-base%2F3xdevops%2Faether%2F-%2Fdocs%2Fworkspace-execution-and-synchronization.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| defer func() { _ = att.Close() }() | ||
| if err = p.CloseStdin(ctx); err != nil { | ||
| return 0, "", "", fmt.Errorf("vm runtime: exec input: %w", err) | ||
| } | ||
| stdout, stderr, err := readLimited(att) |
There was a problem hiding this comment.
Exec does not close its attachment when ctx ends. readLimited runs before p.Wait(ctx), and the attachment reads use their own background context. If the command hangs or the VM pauses after spawning it, the caller's deadline cannot interrupt these reads. Tie the attachment to the call's context before reading output. The same reader is used by user preparation and managed control calls.
Knowledge Base Used: Keep container faults from degrading server availability
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/runtime/vm/exec.go
Line: 107-111
Comment:
**Command reads ignore cancellation**
`Exec` does not close its attachment when `ctx` ends. `readLimited` runs before `p.Wait(ctx)`, and the attachment reads use their own background context. If the command hangs or the VM pauses after spawning it, the caller's deadline cannot interrupt these reads. Tie the attachment to the call's context before reading output. The same reader is used by user preparation and managed control calls.
**Knowledge Base Used:** [Keep container faults from degrading server availability](https://app.greptile.com/-/switch-tenant?tenant=aethercode&next=%2Fcustom-context%2Fknowledge-base%2F3xdevops%2Faether%2F-%2Freverts%2Fincident-mitigation_136-20260915-container-availability-14cb623.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| errc := make(chan error, 1) | ||
| go func() { errc <- read(att.Stderr(), &errOut) }() | ||
| outErr := read(att.Stdout(), &out) | ||
| if outErr != nil { | ||
| _ = att.Close() | ||
| } | ||
| errErr := <-errc |
There was a problem hiding this comment.
A stderr failure is not noticed until stdout ends. An endless stderr-only command can pass the output limit, causing its stderr reader to return. The guest then blocks on its full stderr buffer while the host keeps waiting on quiet stdout. execAttached has the same ordering when its stderr writer fails. Collect both read results concurrently and close the streams on the first failure.
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/runtime/vm/exec.go
Line: 150-156
Comment:
**Stderr failures leave commands stuck**
A stderr failure is not noticed until stdout ends. An endless stderr-only command can pass the output limit, causing its stderr reader to return. The guest then blocks on its full stderr buffer while the host keeps waiting on quiet stdout. `execAttached` has the same ordering when its stderr writer fails. Collect both read results concurrently and close the streams on the first failure.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| if _, err := e.requireRunning(ctx, key, "recover an execution in"); err != nil { | ||
| if errors.Is(err, runtime.ErrNotFound) { | ||
| return nil, err | ||
| } | ||
| return nil, fmt.Errorf("%w: %w: %w", runtime.ErrExecUnavailable, runtime.ErrNotFound, err) |
There was a problem hiding this comment.
This wraps every requireRunning failure as runtime.ErrNotFound, including a cancelled context or a temporary systemctl failure. Those errors do not prove that the VM or command ended. The scheduler treats this result as proof and marks the terminal gone. Preserve temporary errors as ErrExecUnavailable; add ErrNotFound only after confirming absence or exit.
Knowledge Base Used: Environment construction and execution
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/runtime/vm/managed.go
Line: 103-107
Comment:
**Recovery declares live commands gone**
This wraps every `requireRunning` failure as `runtime.ErrNotFound`, including a cancelled context or a temporary `systemctl` failure. Those errors do not prove that the VM or command ended. The scheduler treats this result as proof and marks the terminal gone. Preserve temporary errors as `ErrExecUnavailable`; add `ErrNotFound` only after confirming absence or exit.
**Knowledge Base Used:** [Environment construction and execution](https://app.greptile.com/-/switch-tenant?tenant=aethercode&next=%2Fcustom-context%2Fknowledge-base%2F3xdevops%2Faether%2F-%2Fdocs%2Fenvironment-construction-and-execution.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| unlock := e.lock(key) | ||
| defer unlock() | ||
| rec, err := e.load(key) | ||
| if err != nil { | ||
| return 0, false | ||
| } | ||
| if rec.Exited { | ||
| return rec.ExitCode, true | ||
| } | ||
| if err := e.finish(ctx, rec, code); err != nil { |
There was a problem hiding this comment.
An old wait can power off a newer boot of the same VM. Sleep stops Main while the old supervision wait remains active. If wake starts the VM before that waiter takes this lock, start has already cleared Exited; the waiter then calls finish with the previous Main's exit code. The scheduler's sleepGen check happens only after Runtime.Wait returns. Compare a boot generation before finishing the VM.
Knowledge Base Used: Workspace execution and synchronization
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/runtime/vm/lifecycle.go
Line: 175-184
Comment:
**Old waits stop new boots**
An old wait can power off a newer boot of the same VM. Sleep stops Main while the old supervision wait remains active. If wake starts the VM before that waiter takes this lock, `start` has already cleared `Exited`; the waiter then calls `finish` with the previous Main's exit code. The scheduler's `sleepGen` check happens only after `Runtime.Wait` returns. Compare a boot generation before finishing the VM.
**Knowledge Base Used:** [Workspace execution and synchronization](https://app.greptile.com/-/switch-tenant?tenant=aethercode&next=%2Fcustom-context%2Fknowledge-base%2F3xdevops%2Faether%2F-%2Fdocs%2Fworkspace-execution-and-synchronization.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| if err = api.resume(ctx); err != nil { | ||
| return fmt.Errorf("vm runtime: resume restored VM %s: %w", key, err) | ||
| } | ||
| rec.Saved = false | ||
| if err = e.store(rec); err != nil { | ||
| return err | ||
| } | ||
| if err = os.RemoveAll(dir); err != nil { |
There was a problem hiding this comment.
Restore resumes the guest before making the snapshot unusable. If saving Saved=false fails—for example, because the snapshot filled the disk—the deferred cleanup kills the resumed VM, but the old record still says Saved=true and the snapshot remains. Retrying restore then pairs old memory with disks the guest has already changed. Durably mark the snapshot consumed before resuming the guest.
Knowledge Base Used: Workspace execution and synchronization
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/runtime/vm/save.go
Line: 121-128
Comment:
**Restore can reuse stale memory**
Restore resumes the guest before making the snapshot unusable. If saving `Saved=false` fails—for example, because the snapshot filled the disk—the deferred cleanup kills the resumed VM, but the old record still says `Saved=true` and the snapshot remains. Retrying restore then pairs old memory with disks the guest has already changed. Durably mark the snapshot consumed before resuming the guest.
**Knowledge Base Used:** [Workspace execution and synchronization](https://app.greptile.com/-/switch-tenant?tenant=aethercode&next=%2Fcustom-context%2Fknowledge-base%2F3xdevops%2Faether%2F-%2Fdocs%2Fworkspace-execution-and-synchronization.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| st, err := m.proc.Status(ctx) | ||
| if errors.Is(err, guest.ErrNotFound) { | ||
| return runtime.ExecState{}, fmt.Errorf("%w: %w: execution %s", runtime.ErrExecUnavailable, runtime.ErrNotFound, m.identity.ExecID) |
There was a problem hiding this comment.
A missing guest process record does not mean the managed execution's final record is gone. The guest forgets exited processes after 32 later exits, including the short control commands created here. Once that happens, Status and recovery return ErrNotFound without reading the saved dev-exec result. Completed terminals can lose an exit code the server already knew. Fall back to the control reply when the guest forgets the process.
Knowledge Base Used: Environment construction and execution
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/runtime/vm/managed.go
Line: 172-174
Comment:
**Terminal exit codes disappear**
A missing guest process record does not mean the managed execution's final record is gone. The guest forgets exited processes after 32 later exits, including the short control commands created here. Once that happens, `Status` and recovery return `ErrNotFound` without reading the saved `dev-exec` result. Completed terminals can lose an exit code the server already knew. Fall back to the control reply when the guest forgets the process.
**Knowledge Base Used:** [Environment construction and execution](https://app.greptile.com/-/switch-tenant?tenant=aethercode&next=%2Fcustom-context%2Fknowledge-base%2F3xdevops%2Faether%2F-%2Fdocs%2Fenvironment-construction-and-execution.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| if err != nil { | ||
| return runtime.MemoryEvents{}, err | ||
| } | ||
| st, err := h.Stats(ctx) |
There was a problem hiding this comment.
A paused VM still has an active systemd unit, so this call asks a frozen helper for stats. The OOM watcher does that serially with a five-second deadline per VM, including paused and retained sessions. Each frozen VM adds five seconds to every scheduler sweep, delaying cleanup and other periodic work. Skip frozen guests or use cached counters. Apply the same check in sample so paused VMs do not exhaust its shared sampling deadline.
Knowledge Base Used: Environment construction and execution
Prompt To Fix With AI
This is a comment left during a code review.
Path: internal/runtime/vm/stats.go
Line: 35
Comment:
**Paused VMs delay periodic work**
A paused VM still has an active systemd unit, so this call asks a frozen helper for stats. The OOM watcher does that serially with a five-second deadline per VM, including paused and retained sessions. Each frozen VM adds five seconds to every scheduler sweep, delaying cleanup and other periodic work. Skip frozen guests or use cached counters. Apply the same check in `sample` so paused VMs do not exhaust its shared sampling deadline.
**Knowledge Base Used:** [Environment construction and execution](https://app.greptile.com/-/switch-tenant?tenant=aethercode&next=%2Fcustom-context%2Fknowledge-base%2F3xdevops%2Faether%2F-%2Fdocs%2Fenvironment-construction-and-execution.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Stacked on #891 (item 3), which is stacked on #890 (item 4) and #889 (the plan).
Work item 2 of the microVM plan: the VM engine.
What was wrong before, what happens now
Every session ran in a Docker container; there was no other
runtime.Runtime.Now
aether-server serve --session-runtime vm(config keysession-runtime, defaultdocker) runs each session in its own Cloud Hypervisor VM through the newinternal/runtime/vm. The scheduler drives it throughruntime.Runtimeunchanged. Withdocker, nothing changes.isSessioninvm.go): a spec whoseCreationKeyequals itsAETHER_RUN_IDis a session and becomes a VM with IDvm-<run id>. Everything else (environment terminals, assistant, agent updater, cleanup helper, browser companion, image methods) goes to the embedded Docker runtime.TestEveryContainerMethodIsRoutedfails if Docker gains a method taking a container ID that the router does not route, so item 9 can remove the Docker delegate piece by piece.aether-vm-<run id>.serviceinaether.slice(PrivateMounts=yes,OOMScoreAdjust=500,CPUQuota=the exact CPU limit), outside the server's process tree, so a VM survives server restarts and updates. The unit runsaether-server vm-monitor <dir>, which composes the share, starts virtiofsd, then execs Cloud Hypervisor, so the unit ends with the VM. Per-VM files live in<data dir>/vm/<run id>/(vm.jsonrecord, disks,monitor.log,console.log);FindByCreationKeyand recovery read the record.runtime.UserScript), thenBootruns dockerd, setup and Main in the guest. Stop sends SIGTERM to Main, kills after the grace, records the exit and powers off. Main's exit is recorded on the host, soInspectandWaitanswer after a restart; a VM that ends without one reports 255. Pause/Resume are Cloud Hypervisor's pause/resume.ManagedExecRuntime(the unchangeddev-execsupervisor started through the helper, exactly as Docker starts it throughdocker exec),SudoRuntime,CapacityRuntime(Docker's plus the VM disks' filesystem),InitPIDRuntime(Cloud Hypervisor's PID),ContainerStats(CPU, memory, pids from the helper; net and disk from Cloud Hypervisor's counters), and two new ones ininternal/runtime:MemoryEventsRuntime(guest OOM kills;oomwatch.goprefers it, Docker's path unchanged, approved by the coordinator) andSaveRuntime(Save/Restore, for item 8).CPUQuota; guest memory = memory limit,shared=on, balloon with free page reporting andpage_reporting.page_reporting_order=0(item 1: without it freed page cache stalls in the host); the process limit is applied by the helper.open_tree+mount_setattr: recursive, nosuid, nodev, read-only where asked; subpaths resolved withopenat2(RESOLVE_BENEATH), as Docker's volume subpath refuses an escaping symlink). Why one share and not one virtiofsd per mount: one process and one PCI slot per VM instead of 10-20; virtiofsd 1.10 has no--readonly, so the host kernel's read-only bind is what enforces it; single-file mounts and subpaths work, which a virtiofsd shared dir cannot express; nothing leaks onto the host's mount table. Host sockets cannot cross virtio-fs: sockets at the top of a shared directory are forwarded by the helper over vsock, and<dir>/vsock.sock_<port>is a symlink to the host socket, so Cloud Hypervisor connects straight to it with no host proxy.network.go, for item 5): one tap per VM that Cloud Hypervisor creates, a /30 per VM from10.200.0.0/16, guest DNS from the host's upstream resolvers, and NAT via three idempotent iptables rules.Savepauses, snapshots, ends the monitor;Restorestarts the monitor on the snapshot, resumes, waits for the helper and sets the guest clock. A VM with virtio-fs mounts is refused with an error naming them: measured below, a restored VM's share is dead.docker.go/docker_sudo.go/docker_managed_exec.gointosession.goandmanaged_exec.gounchanged (user script, safe.directory env, setup output masking, root PATH, dev-exec control argv and reply parsing), so both engines share them; Docker's behaviour is byte-for-byte the same.Interfaces agreed with items 3 and 4
internal/guest):Dial(vsockPath);Ping→Info{Protocol, Version, BootID, BootError};Boot(BootSpec{Main, SetupScript, NestedDocker, PidsLimit})(dockerd, setup once per boot, Main;*SetupErroron failure, masked here);Spawn(ProcessSpec{ID, Argv, Env, User, Dir, TTY, Lossless})(*StartError{126|127});Process(id)for reattach (Attach(fromStart),Signal,Status,Wait,Resize,CloseStdin);Statsincl.OOMKills;SyncClock;PowerOff;Config{Hostname, Network, Mounts(share|data), Sockets}.KernelArgs(base). PID 1 forwards HUP/INT/QUIT/TERM/USR1/USR2/ALRM/WINCH to Main.internal/vmimage):Open(<data>/vm/images, kernel, docker);Kernel;BaseDisk;NewRootDisk(qcow2,backing_files=on);NewDataDisk(raw ext4);HelperDisk(read-only, first disk,/initis the helper). Boot line:root=/dev/vda ro rootfstype=ext4 init=/init console=ttyS0 quiet page_reporting.page_reporting_order=0+ item 3's args.--vm-kernel(config keyvm-kernel, default/usr/local/lib/aether/vmlinux, item 10 installs it).How I verified it
Unit tests (
go test ./internal/runtime/vm/, also under-race) with a fake Cloud Hypervisor REST API served on the VM's real API socket, fake systemd units, a fake helper and a fake image store: create/start/stop/wait/inspect/destroy, setup failure masking, a VM dying while booting (error carriesmonitor.log), SIGKILL after the grace, a second engine finding a running VM, exec limits and missing commands, managed exec through dev-exec and its recovery, save refusal and save/restore, stats, the mount plan and socket links, addressing and NAT rules, and the routing guard.oomwatch_test.gocovers both OOM paths.On the KVM test host (AWS m8id.4xlarge, nested KVM, Ubuntu 24.04, Cloud Hypervisor v54.1, virtiofsd 1.10.0, item 4's kernel and store, item 3's helper),
TestKVMSessionLifecycleandTestKVMSaveRestoreKeepsProcesses(-tags integration, skipped withoutAETHER_VM_TEST_*and/dev/kvm) pass:ubuntu:24.04session: create 1.1 s cold (base disk built), 14 ms warm; start through unit, boot, setup and Main 0.68-0.72 s; attach replays Main's output; exec sees the checkout both ways,pwdis the working dir, the read-only mount refuses writes, 1.5 CPUs gives 2 vCPUs, the forwarded socket exists at/run/aether/coord3.sock, DNS resolves; stop 0.23-0.45 s.Earlier runs of the same engine code with a stand-in guest init (before item 3's helper existed) also showed: the unit's main PID is Cloud Hypervisor in
aether.slice/aether-vm-<id>.service; the composed share enforces read-only for a file and for the share root; an escaping subpath is refused at boot withresolve escape beneath ...: invalid cross-device link; the guest reaches the internet through NAT (wget http://example.com); a guest connection to vsock port 1025 reached the host socket through the symlink; vsock still answers after pause/resume; save of a 1 GiB guest wrote 132 MB and a 64 MB file in the guest's/tmpcame back byte-identical.Save with virtio-fs (the plan's Open question): pause + snapshot succeeds, but after restore every access to the share returns EBADF, because a fresh virtiofsd has none of the guest's FUSE state; remounting in the guest works (16 ms) only when no process holds an fd or cwd in the share, which an agent always does in
/workspace. Hence the refusal.Checks:
make fmt-check,go vet,make lint(also with-tags integration) clean for these packages;go testofinternal/runtime/...,internal/scheduler,internal/server,cmd/aether-serverpass;make public-auditpasses.Greptile CLI:
greptile: command not foundin this environment, so it did not run; the Greptile bot reviews the pushes.Left open
ContainerSizesis Docker's).ContainerStatsruns onesystemctl showper recorded VM per sample; fine for tens of VMs.This PR was made with the help of Claude Code.
Opened from Aether run run-pmvnsrnaj7