From 0b1dae2a905739f7ce24fa18d78f5412092c505d Mon Sep 17 00:00:00 2001 From: chris lee Date: Sun, 11 Oct 2026 17:49:10 -0400 Subject: [PATCH 01/11] macOS guests 3/6: Darwin guest service, readiness and exec Shared GuestService on host-CID vsock 2222, root system agent, agent readiness probe, agent-gated exec, copy and stat, and a macOS stop floor. Darwin exec runs its own bounded lifecycle in exec_lifecycle.go. Linux exec, boot markers, stop and the guest client are unchanged. --- cmd/api/api/cp.go | 5 + cmd/api/api/exec.go | 21 +- cmd/api/api/exec_disconnect_test.go | 45 +++ cmd/api/api/guest_unsupported.go | 13 + cmd/api/api/instances.go | 7 +- cmd/api/api/macos_test.go | 36 +- docs/macos-experimental.md | 19 +- docs/macos-guest-agent.md | 115 ++++++ docs/macos-support.md | 3 +- lib/diskutilization/diskutilization.go | 2 +- lib/diskutilization/diskutilization_test.go | 6 +- lib/guest/client.go | 9 +- lib/guest/keep_pooled_conn_test.go | 48 +++ lib/images/types.go | 3 + lib/instances/create.go | 17 +- lib/instances/delete.go | 4 +- .../guest_agent_ready_command_test.go | 13 + lib/instances/guest_os.go | 23 ++ lib/instances/guest_os_test.go | 23 ++ lib/instances/macos.go | 112 +++++- lib/instances/macos_health_check_test.go | 66 ++++ lib/instances/macos_test.go | 116 ++++++ lib/instances/manager.go | 2 +- lib/instances/query.go | 36 +- lib/instances/start.go | 53 +-- lib/instances/stop.go | 10 +- lib/instances/storage.go | 1 + lib/instances/vm_storage_path_test.go | 2 +- lib/instances/vsock.go | 4 +- lib/paths/paths.go | 8 + lib/system/guest_agent/darwin_test.go | 19 + lib/system/guest_agent/exec.go | 3 + .../guest_agent/exec_background_child_test.go | 52 +++ lib/system/guest_agent/exec_lifecycle.go | 347 ++++++++++++++++++ .../exec_lifecycle_disconnect_test.go | 95 +++++ .../guest_agent/exec_send_cancel_test.go | 48 +++ lib/system/guest_agent/exec_sender_test.go | 59 +++ lib/system/guest_agent/main.go | 19 +- lib/system/guest_agent/network.go | 2 + lib/system/guest_agent/network_darwin.go | 15 + lib/system/guest_agent/ready_darwin.go | 7 + lib/system/guest_agent/ready_linux.go | 7 + lib/system/guest_agent/service_test.go | 295 +++++++++++++++ lib/system/guest_agent/shutdown.go | 2 + lib/system/guest_agent/shutdown_darwin.go | 41 +++ lib/system/guest_agent/shutdown_policy.go | 28 ++ .../guest_agent/shutdown_policy_test.go | 52 +++ .../shutdown_schedule_darwin_test.go | 19 + lib/system/guest_agent/vsock_darwin.go | 125 +++++++ lib/system/guest_agent/vsock_darwin_nocgo.go | 12 + .../guest_agent/vsock_darwin_nocgo_test.go | 15 + lib/system/guest_agent/vsock_darwin_test.go | 62 ++++ lib/system/guest_agent/vsock_linux.go | 13 + 53 files changed, 2059 insertions(+), 100 deletions(-) create mode 100644 cmd/api/api/exec_disconnect_test.go create mode 100644 cmd/api/api/guest_unsupported.go create mode 100644 docs/macos-guest-agent.md create mode 100644 lib/guest/keep_pooled_conn_test.go create mode 100644 lib/instances/guest_agent_ready_command_test.go create mode 100644 lib/instances/guest_os.go create mode 100644 lib/instances/guest_os_test.go create mode 100644 lib/instances/macos_health_check_test.go create mode 100644 lib/system/guest_agent/darwin_test.go create mode 100644 lib/system/guest_agent/exec_background_child_test.go create mode 100644 lib/system/guest_agent/exec_lifecycle.go create mode 100644 lib/system/guest_agent/exec_lifecycle_disconnect_test.go create mode 100644 lib/system/guest_agent/exec_send_cancel_test.go create mode 100644 lib/system/guest_agent/exec_sender_test.go create mode 100644 lib/system/guest_agent/network_darwin.go create mode 100644 lib/system/guest_agent/ready_darwin.go create mode 100644 lib/system/guest_agent/ready_linux.go create mode 100644 lib/system/guest_agent/service_test.go create mode 100644 lib/system/guest_agent/shutdown_darwin.go create mode 100644 lib/system/guest_agent/shutdown_policy.go create mode 100644 lib/system/guest_agent/shutdown_policy_test.go create mode 100644 lib/system/guest_agent/shutdown_schedule_darwin_test.go create mode 100644 lib/system/guest_agent/vsock_darwin.go create mode 100644 lib/system/guest_agent/vsock_darwin_nocgo.go create mode 100644 lib/system/guest_agent/vsock_darwin_nocgo_test.go create mode 100644 lib/system/guest_agent/vsock_darwin_test.go create mode 100644 lib/system/guest_agent/vsock_linux.go diff --git a/cmd/api/api/cp.go b/cmd/api/api/cp.go index bfa64e45c..e9ea42b34 100644 --- a/cmd/api/api/cp.go +++ b/cmd/api/api/cp.go @@ -104,6 +104,11 @@ func (s *ApiService) CpHandler(w http.ResponseWriter, r *http.Request) { return } + if reason := inst.GuestOperationUnsupportedReason(instances.GuestOpFileCopy); reason != "" { + writeGuestUnsupported(w, reason) + return + } + if inst.State != instances.StateRunning { http.Error(w, fmt.Sprintf(`{"code":"invalid_state","message":"instance must be running (current state: %s)"}`, inst.State), http.StatusConflict) return diff --git a/cmd/api/api/exec.go b/cmd/api/api/exec.go index 9313e5372..9a0ef21d7 100644 --- a/cmd/api/api/exec.go +++ b/cmd/api/api/exec.go @@ -71,8 +71,8 @@ func (s *ApiService) ExecHandler(w http.ResponseWriter, r *http.Request) { return } - if inst.MacOS != nil { - http.Error(w, `{"code":"unsupported","message":"exec is not implemented for experimental macOS instances"}`, http.StatusNotImplemented) + if reason := inst.GuestOperationUnsupportedReason(instances.GuestOpExec); reason != "" { + writeGuestUnsupported(w, reason) return } @@ -151,6 +151,14 @@ func (s *ApiService) ExecHandler(w http.ResponseWriter, r *http.Request) { // Create WebSocket read/writer wrapper that handles resize messages wsConn := &wsReadWriter{ws: ws, ctx: ctx, resizeChan: resizeChan} + if inst.MacOS != nil { + // The macOS guest agent cancels a command when its stream ends, so a lost + // websocket ends the session instead of leaving the command running. + var cancel context.CancelFunc + ctx, cancel = context.WithCancel(ctx) + defer cancel() + wsConn.ctx, wsConn.cancel = ctx, cancel + } dialer, err := s.InstanceManager.GetVsockDialer(ctx, inst.Id) if err != nil { @@ -221,6 +229,7 @@ type wsReadWriter struct { reader io.Reader mu sync.Mutex resizeChan chan<- *guest.WindowSize // Channel to send resize events (nil if not TTY) + cancel context.CancelFunc // ends a macOS exec session when the websocket fails } func (w *wsReadWriter) Read(p []byte) (n int, err error) { @@ -241,6 +250,7 @@ func (w *wsReadWriter) Read(p []byte) (n int, err error) { // Read next WebSocket message messageType, data, err := w.ws.ReadMessage() if err != nil { + w.cancelSession() return 0, err } @@ -273,7 +283,14 @@ func (w *wsReadWriter) Read(p []byte) (n int, err error) { func (w *wsReadWriter) Write(p []byte) (n int, err error) { if err := w.ws.WriteMessage(websocket.BinaryMessage, p); err != nil { + w.cancelSession() return 0, err } return len(p), nil } + +func (w *wsReadWriter) cancelSession() { + if w.cancel != nil { + w.cancel() + } +} diff --git a/cmd/api/api/exec_disconnect_test.go b/cmd/api/api/exec_disconnect_test.go new file mode 100644 index 000000000..ad16654f5 --- /dev/null +++ b/cmd/api/api/exec_disconnect_test.go @@ -0,0 +1,45 @@ +package api + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + "github.com/gorilla/websocket" + "github.com/stretchr/testify/require" +) + +func TestExecWebsocketDisconnectCancelsSession(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + readDone := make(chan error, 1) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + conn, err := upgrader.Upgrade(w, r, nil) + if err != nil { + readDone <- err + return + } + defer conn.Close() + wrapped := &wsReadWriter{ws: conn, ctx: ctx, cancel: cancel} + _, err = wrapped.Read(make([]byte, 1)) + readDone <- err + })) + defer server.Close() + conn, _, err := websocket.DefaultDialer.Dial("ws"+strings.TrimPrefix(server.URL, "http"), nil) + require.NoError(t, err) + require.NoError(t, conn.Close()) + select { + case <-ctx.Done(): + case <-time.After(5 * time.Second): + t.Fatal("WebSocket disconnect did not cancel exec session") + } + select { + case err := <-readDone: + require.Error(t, err) + case <-time.After(5 * time.Second): + t.Fatal("WebSocket reader did not exit") + } +} diff --git a/cmd/api/api/guest_unsupported.go b/cmd/api/api/guest_unsupported.go new file mode 100644 index 000000000..1e24d5ea9 --- /dev/null +++ b/cmd/api/api/guest_unsupported.go @@ -0,0 +1,13 @@ +package api + +import ( + "encoding/json" + "net/http" +) + +// writeGuestUnsupported writes the shared 501 "unsupported" body for raw HTTP handlers. +func writeGuestUnsupported(w http.ResponseWriter, reason string) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusNotImplemented) + _ = json.NewEncoder(w).Encode(map[string]string{"code": "unsupported", "message": reason}) +} diff --git a/cmd/api/api/instances.go b/cmd/api/api/instances.go index 15a622749..d5aa47fcf 100644 --- a/cmd/api/api/instances.go +++ b/cmd/api/api/instances.go @@ -988,11 +988,8 @@ func (s *ApiService) StatInstancePath(ctx context.Context, request oapi.StatInst }, nil } - if inst.MacOS != nil { - return oapi.StatInstancePath501JSONResponse{ - Code: "unsupported", - Message: "stat is not implemented for experimental macOS instances", - }, nil + if reason := inst.GuestOperationUnsupportedReason(instances.GuestOpStat); reason != "" { + return oapi.StatInstancePath501JSONResponse{Code: "unsupported", Message: reason}, nil } if inst.State != instances.StateRunning { diff --git a/cmd/api/api/macos_test.go b/cmd/api/api/macos_test.go index 5d324c0e3..69743fe56 100644 --- a/cmd/api/api/macos_test.go +++ b/cmd/api/api/macos_test.go @@ -36,20 +36,50 @@ func TestMacOSExecRejectedBeforeWebsocketUpgrade(t *testing.T) { w := httptest.NewRecorder() s.ExecHandler(w, r) require.Equal(t, http.StatusNotImplemented, w.Code) - require.Contains(t, w.Body.String(), "not implemented") + require.Contains(t, w.Body.String(), "requires the shared guest agent") } -func TestMacOSStatRejectedBeforeGuestDial(t *testing.T) { +func TestMacOSStatRequiresGuestAgentBeforeDial(t *testing.T) { s := &ApiService{} inst := &instances.Instance{StoredMetadata: instances.StoredMetadata{MacOS: &images.MacOSImage{}}, State: instances.StateRunning} ctx := mw.WithResolvedInstance(context.Background(), "test", inst) resp, err := s.StatInstancePath(ctx, oapi.StatInstancePathRequestObject{Id: "test"}) require.NoError(t, err) unsupported, ok := resp.(oapi.StatInstancePath501JSONResponse) - require.True(t, ok, "macOS stat must be rejected with 501, got %T", resp) + require.True(t, ok, "macOS stat without the agent must be rejected with 501, got %T", resp) require.Equal(t, "unsupported", unsupported.Code) } +func TestMacOSGuestAgentAdmissionBeforeUpgrade(t *testing.T) { + for _, tc := range []struct { + name string + declared, skipped bool + status int + }{ + {"unmanaged", false, false, http.StatusNotImplemented}, + {"disabled", true, true, http.StatusNotImplemented}, + {"enabled but stopped", true, false, http.StatusConflict}, + } { + for _, operation := range []string{"exec", "cp"} { + t.Run(tc.name+"/"+operation, func(t *testing.T) { + s := &ApiService{} + inst := &instances.Instance{StoredMetadata: instances.StoredMetadata{ + MacOS: &images.MacOSImage{GuestAgent: tc.declared}, SkipGuestAgent: tc.skipped, + }, State: instances.StateStopped} + ctx := mw.WithResolvedInstance(context.Background(), "test", inst) + r := httptest.NewRequest(http.MethodGet, "/instances/test/"+operation, nil).WithContext(ctx) + w := httptest.NewRecorder() + if operation == "exec" { + s.ExecHandler(w, r) + } else { + s.CpHandler(w, r) + } + require.Equal(t, tc.status, w.Code) + }) + } + } +} + func TestLinuxCreateDefaultsMatchSchema(t *testing.T) { spec, err := oapi.GetSwagger() require.NoError(t, err) diff --git a/docs/macos-experimental.md b/docs/macos-experimental.md index 630993ff9..0c183a0e9 100644 --- a/docs/macos-experimental.md +++ b/docs/macos-experimental.md @@ -73,13 +73,19 @@ Use the usual JWT-authenticated API with a VZ-capable, signed build: ``` CPU and memory default to the imported template (minimum 2 CPUs and 4GB). -Each instance gets its own writable complete boot disk and auxiliary storage; -no Linux kernel, initrd, configuration disk, overlay filesystem or balloon is +Each instance gets its own writable complete boot disk (`boot.raw`) and auxiliary +storage; instances created before that naming must be deleted and recreated. No +Linux kernel, initrd, configuration disk, overlay filesystem or balloon is attached. NAT uses the preserved MAC. The IP is observed from the host's VZ DHCP leases, which may retain an old lease before the guest is ready. -**`Running` means the VMM is running, not that SSH, the desktop or Chrome is -ready.** Readiness is currently tested over SSH. Startup does not launch Chrome. +**Without the guest agent, `Running` means the VMM is running, not that SSH, +the desktop or Chrome is ready.** Unmanaged templates still need an external +readiness check. Templates provisioned with the shared Darwin GuestService can opt +in with `guest_agent: true` in their platform configuration; those instances stay +`Initializing` until the agent answers, as Linux guests do, so waiting for +`Running` waits for the agent. See [Darwin GuestService](macos-guest-agent.md). +Startup does not launch Chrome. A shim killed without cleanup (for example, SIGKILL) leaves its control socket behind; when nothing accepts on it and the recorded shim is gone, the instance reads as Stopped and its resources are released for admission. If the API dies @@ -91,8 +97,9 @@ instance with a given Mac identifier is allowed. Do not run the source bundle or an external clone concurrently. Production provisioning needs identity and credential rekeying; this spike is not a multi-tenant image format. -`POST /instances/{id}/stop` shuts down the VMM; without a Darwin guest agent, -it does **not** guarantee an orderly guest OS/application shutdown. Use a human +`POST /instances/{id}/stop` attempts orderly GuestService shutdown for opted-in +images, then falls back to VMM shutdown if necessary. Without an enabled Darwin +guest agent it does **not** guarantee an orderly guest OS/application shutdown. Use a human or an authorized guest shutdown workflow before destructive operations when application consistency matters. Start cold-boots the instance's existing disk. Delete removes instance storage, not the imported image. Deleting the imported diff --git a/docs/macos-guest-agent.md b/docs/macos-guest-agent.md new file mode 100644 index 000000000..219b14b9a --- /dev/null +++ b/docs/macos-guest-agent.md @@ -0,0 +1,115 @@ +# Experimental Darwin GuestService + +The existing guest-agent executable now builds for macOS and serves the same +`guest.GuestService` gRPC contract as Linux on vsock port **2222**. It reuses the +copy-to/from-guest and stat implementations. Exec on Darwin runs a separate +lifecycle (`exec_lifecycle.go`): a client disconnect cancels the command, +cancellation kills its process group, and output drains are bounded. Linux exec +is unchanged. + +## Build + +Build on a macOS development machine with the macOS SDK and cgo enabled: + +```sh +CGO_ENABLED=1 GOOS=darwin GOARCH=arm64 go build \ + -o guest-agent-darwin-arm64 ./lib/system/guest_agent +``` + +Darwin native AF_VSOCK requires cgo. A cgo-disabled Darwin executable builds but +refuses to listen with an explicit error. Linux keeps its existing Go vsock +listener and cgo-disabled build path. The Darwin listener accepts only host +CID 2, marks descriptors close-on-exec, and provides net.Conn deadlines for +gRPC. Do not start the executable on the development host to test guest services. + +## Provisioning boundary + +Install the binary **inside a stopped-template provisioning guest**, not on the +host. System operations need a root LaunchDaemon. Use a root-owned, non-writable +binary location and launchd configuration; provision permissions and logs +explicitly. The default readiness file is `/var/run/hypeman/guest-agent-ready` +(`HYPEMAN_AGENT_READY_FILE` can override it). + +A listening system agent does not establish autologin, desktop readiness, TCC +permissions, or browser readiness. + +The host API and hypervisor still enforce authorization. The vsock host-CID +check is transport admission, not a replacement for instance authority checks. +As with the Linux guest agent, commands and file operations run as root with no +per-caller credential switch; the host API alone decides who may reach them. +An exec `timeout_seconds` ends the command; it does not bound a healthy stream. +Do not expose this privileged service through unauthenticated host forwarding. + +## Image declaration and host integration + +Set `"guest_agent": true` in the image's experimental macOS platform configuration +only after provisioning the root shared agent on vsock 2222. This field is omitted +by default, so existing local templates remain unmanaged. The normal instance +`skip_guest_agent` option can disable agent integration even for a declared image. + +For declared/enabled instances, the normal exec and file-copy handlers use the +shared GuestService and stop attempts its Shutdown RPC before the existing +forced-stop fallback. An unavailable agent still produces a timeout/failure, +not an assertion that it is ready. Readiness probing records the existing +`guest_agent_ready_at` marker without requiring or inventing a Linux workload +start marker. An enabled instance is `Initializing` until that marker is recorded, +then `Running`; this does not certify desktop or browser readiness. This readiness timestamp is a boot observation, +not continuous agent health. Start clears the prior boot's readiness marker. + +Undeclared/disabled images reject exec/copy before WebSocket upgrade and skip +agent shutdown/probes. The declaration is an image capability claim, not a +verified live guest handshake or a security credential. Runtime connectivity +and privileged provisioning still require the validation below. + +## OS-specific operations + +- **Shutdown:** root-only `/sbin/shutdown -h now`, not a signal to launchd/PID 1. + Signal 0 (default) and SIGTERM mean orderly shutdown; other signals are rejected. + Permission, cancellation, and command errors are reported. An RPC response is + not proof the VMM has exited: the host must wait for teardown. +- **Network identity reconfiguration:** returns gRPC `Unimplemented` on Darwin. + Current VZ NAT uses guest DHCP; no static-address, MAC-rekey, ingress, or network + policy parity is claimed. +- **GPU status:** Linux NVIDIA initialization reporting is not macOS graphics + readiness. A Darwin guest without that device reports the existing unknown + state. + +## Validation and remaining integration + +In-process gRPC tests exercise exec stdout/stderr/exit/env, disconnect +cancellation, and file copy/stat round-trips. Darwin-specific tests exercise +shutdown policy without executing a real shutdown, explicit network rejection, +and transport deadline errors using a local socket pair. These do not prove a +live guest AF_VSOCK handshake for this executable. + +## Isolated live validation (2026-10-10) + +A manually provisioned root LaunchDaemon in the isolated QA guest survives normal +API cold boots. Authenticated normal API exec reports UID0 in both TTY modes; +private root-owned copy-to/from round trips and command/descendant timeout cleanup +pass repeated race-instrumented client tests. The readiness probe uses +`/usr/bin/true` on Darwin (`/bin/true` remains the Linux command); a live regression +first failed with the Linux path, then passed after this correction. A normal GET +persisted `GuestAgentReadyAt` while leaving `ProgramStartedAt` absent. + +Darwin can disconnect vsock while shutdown is in progress, before the RPC reply +arrives. The host therefore retains the configured grace period on Darwin even +when that reply is unavailable, instead of Linux's short failure fallback. Success +still requires the owned VMM process to exit. A live normal API stop confirmed +shutdown RPC acceptance, VMM exit and closed storage without forced fallback after +the correction. HTTP stop200 or `Stopped` alone is not a build/export receipt. + +This is manual provisioning of one QA instance, not automatic image installation, +public macOS builds, output publication or reusable-image sanitation proof. The +immutable imported base remains unchanged and does not acquire this capability. + +Remaining gates: + +- Reusable image provisioning and guest-agent version compatibility. +- Strict build/export stop receipts and API recovery with the root deployment; + bounded readiness wait semantics. +- Broader backpressure/large-output validation of bounded non-TTY streaming, + PTY/disconnect and descendant-process cleanup, transfer failure/size handling, + and privilege/logging security review. +- Linux test execution on an appropriate runner, and independent authenticated + review. diff --git a/docs/macos-support.md b/docs/macos-support.md index f8c6fdb61..18cbe1c61 100644 --- a/docs/macos-support.md +++ b/docs/macos-support.md @@ -19,7 +19,8 @@ owns the behavior. | Standby, memory snapshot, memory fork | Unsupported | Rejected with a stable error. | — | | Stopped clones, fork from snapshot | Unsupported | Needs identity rebinding and guest rekeying. | PR 4 (later) | | Update (PATCH), VM restart policy | Partial | Expiration/TTL and `restart_policy: always` or `never` are supported. Manual stop suppresses automatic restart. A shim that dies without removing its socket (for example, SIGKILL) reads as Stopped, so `always` restarts it. `on_failure` is rejected: no workload exit status is available. | PR 1 | -| Volumes, env and commands, credentials, health and auto-standby policies, passthrough, shaping | Unsupported | Rejected at create, start, or update. | — | +| Exec health checks | Agent-gated | Supported at create and PATCH; `type: none` disables them. HTTP/TCP checks remain unsupported. | PR 3 | +| Volumes, env and commands, credentials and auto-standby policies, passthrough, shaping | Unsupported | Rejected at create, start, or update. | — | | Ingress | Unsupported | Refused in macOS-only mode and for macOS targets. | — | | Snapshot schedules | Unsupported | Refused at admission. | — | diff --git a/lib/diskutilization/diskutilization.go b/lib/diskutilization/diskutilization.go index a75783a0b..996e81642 100644 --- a/lib/diskutilization/diskutilization.go +++ b/lib/diskutilization/diskutilization.go @@ -88,7 +88,7 @@ func Collect(p *paths.Paths) (Breakdown, error) { instanceID := guest.Name() - breakdown.RootfsOverlays += allocatedBytesForPath(p.InstanceOverlay(instanceID)) + breakdown.RootfsOverlays += allocatedBytesForPath(p.InstanceOverlay(instanceID)) + allocatedBytesForPath(p.InstanceMacOSBootDisk(instanceID)) volumeOverlays, err := sumMatchingFilesAllocatedBytes(p.InstanceVolumeOverlaysDir(instanceID), func(path string, entry fs.DirEntry) bool { return !entry.IsDir() && filepath.Ext(entry.Name()) == ".raw" diff --git a/lib/diskutilization/diskutilization_test.go b/lib/diskutilization/diskutilization_test.go index e19db8d6f..fab0a54bc 100644 --- a/lib/diskutilization/diskutilization_test.go +++ b/lib/diskutilization/diskutilization_test.go @@ -41,6 +41,10 @@ func TestCollect_UsesAllocatedBytesAndClassifiesSnapshots(t *testing.T) { require.NoError(t, createSparseTestFile(rootfsOverlayPath, 64*1024*1024, []sparseWrite{ {offset: 4 * 1024 * 1024, data: bytes.Repeat([]byte("r"), 4096)}, })) + macOSBootDiskPath := p.InstanceMacOSBootDisk("inst-mac") + require.NoError(t, createSparseTestFile(macOSBootDiskPath, 64*1024*1024, []sparseWrite{ + {offset: 0, data: bytes.Repeat([]byte("b"), 4096)}, + })) volumeOverlayPath := p.InstanceVolumeOverlay("inst-1", "vol-1") require.NoError(t, createSparseTestFile(volumeOverlayPath, 64*1024*1024, []sparseWrite{ @@ -80,7 +84,7 @@ func TestCollect_UsesAllocatedBytesAndClassifiesSnapshots(t *testing.T) { require.Equal(t, allocatedBytesForPath(imagePath)+allocatedBytesForPath(machineImagePath), utilization.Images) require.Equal(t, allocatedBytesForPath(volumePath), utilization.Volumes) - require.Equal(t, allocatedBytesForPath(rootfsOverlayPath), utilization.RootfsOverlays) + require.Equal(t, allocatedBytesForPath(rootfsOverlayPath)+allocatedBytesForPath(macOSBootDiskPath), utilization.RootfsOverlays) require.Equal(t, allocatedBytesForPath(volumeOverlayPath), utilization.VolumeOverlays) require.Less(t, utilization.Volumes, fileSize(t, volumePath)) diff --git a/lib/guest/client.go b/lib/guest/client.go index 61376bf02..ecc56283c 100644 --- a/lib/guest/client.go +++ b/lib/guest/client.go @@ -148,6 +148,9 @@ type ExecOptions struct { Rows uint32 // Initial terminal rows (0 = default 24) Cols uint32 // Initial terminal cols (0 = default 80) ResizeChan <-chan *WindowSize // Optional: channel to receive resize events (pointer to avoid copying mutex) + // KeepPooledConn stops a retryable error from evicting the shared pooled connection. + // Use it for probes that run concurrently with other callers on the same guest. + KeepPooledConn bool } type ReconfigureNetworkOptions struct { @@ -266,7 +269,7 @@ func ExecIntoInstance(ctx context.Context, dialer hypervisor.VsockDialer, opts E // we are waiting for the guest-agent to become reachable. if opts.WaitForAgent == 0 { exit, err := execIntoInstanceOnce(ctx, dialer, opts) - if err != nil && isRetryableConnectionError(err) { + if err != nil && isRetryableConnectionError(err) && !opts.KeepPooledConn { CloseConn(dialer.Key()) } return exit, err @@ -306,7 +309,9 @@ func ExecIntoInstance(ctx context.Context, dialer hypervisor.VsockDialer, opts E firstRetryableErrorType = errType } lastRetryableErrorType = errType - CloseConn(dialer.Key()) + if !opts.KeepPooledConn { + CloseConn(dialer.Key()) + } // Connection error - check if we should retry if time.Now().After(deadline) { diff --git a/lib/guest/keep_pooled_conn_test.go b/lib/guest/keep_pooled_conn_test.go new file mode 100644 index 000000000..6088087c3 --- /dev/null +++ b/lib/guest/keep_pooled_conn_test.go @@ -0,0 +1,48 @@ +package guest + +import ( + "context" + "errors" + "net" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// unreachableDialer fails every dial the way a guest whose agent is not listening yet does. +type unreachableDialer struct{ key string } + +func (d unreachableDialer) Key() string { return d.key } +func (d unreachableDialer) DialVsock(context.Context, int) (net.Conn, error) { + return nil, &AgentVSockDialError{Err: errors.New("connect refused")} +} + +func pooledKeyExists(key string) bool { + connPool.RLock() + defer connPool.RUnlock() + _, ok := connPool.conns[key] + return ok +} + +// A probe that keeps the pooled connection must not evict it for concurrent callers. +func TestKeepPooledConnPreservesSharedConnectionOnRetry(t *testing.T) { + const key = "keep-pooled-conn-test" + t.Cleanup(func() { CloseConn(key) }) + shared := unreachableDialer{key: key} + + // Seed the pool with a live client connection for this key. + _, err := GetOrCreateConn(context.Background(), shared) + require.NoError(t, err) + require.True(t, pooledKeyExists(key)) + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + _, err = ExecIntoInstance(ctx, shared, ExecOptions{Command: []string{"true"}, WaitForAgent: 50 * time.Millisecond, KeepPooledConn: true}) + require.Error(t, err) + require.True(t, pooledKeyExists(key), "a probe that keeps the pooled connection must leave it in place") + + _, err = ExecIntoInstance(ctx, shared, ExecOptions{Command: []string{"true"}, WaitForAgent: 50 * time.Millisecond}) + require.Error(t, err) + require.False(t, pooledKeyExists(key), "default behavior still evicts after retryable failures") +} diff --git a/lib/images/types.go b/lib/images/types.go index b2c8ac272..f49437ef9 100644 --- a/lib/images/types.go +++ b/lib/images/types.go @@ -36,6 +36,9 @@ type MacOSImage struct { MAC string `json:"mac"` CPUs uint `json:"cpus"` Memory uint64 `json:"memory"` + // GuestAgent declares a provisioned system GuestService on vsock 2222. + // Readiness is probed separately; old templates remain unmanaged. + GuestAgent bool `json:"guest_agent,omitempty"` } // Validate checks the platform fields every macOS bundle must carry, whether it diff --git a/lib/instances/create.go b/lib/instances/create.go index ee14d15fd..df67c348b 100644 --- a/lib/instances/create.go +++ b/lib/instances/create.go @@ -466,8 +466,8 @@ func (m *manager) createInstance( }) } - if stored.MacOS != nil && stored.NetworkEnabled { - netConfig = applyMacOSNetwork(stored) + if macOSNetwork := applyMacOSNetwork(stored); macOSNetwork != nil { + netConfig = macOSNetwork } // 15. Validate and attach volumes @@ -808,16 +808,11 @@ func (m *manager) startAndBootVM( refreshHostVersion bool, ) error { log := logger.FromContext(ctx) - if stored.MacOS != nil { - if err := checkMacOSSocketPaths(stored); err != nil { - return err - } - m.macOSBootMu.Lock() - defer m.macOSBootMu.Unlock() - if err := m.checkMacOSIdentityAvailable(ctx, stored); err != nil { - return err - } + release, err := m.admitMacOSBoot(ctx, stored) + if err != nil { + return err } + defer release() // Get VM starter for this hypervisor type starter, err := m.getVMStarter(stored.HypervisorType) diff --git a/lib/instances/delete.go b/lib/instances/delete.go index 92824755c..e33bf8ac7 100644 --- a/lib/instances/delete.go +++ b/lib/instances/delete.go @@ -76,7 +76,7 @@ func (m *manager) deleteInstanceWithOptions( // 2. Get network allocation BEFORE killing VMM (while we can still query it) var networkAlloc *network.Allocation - if inst.NetworkEnabled && inst.MacOS == nil { + if inst.usesLinuxNetwork() { log.DebugContext(ctx, "getting network allocation", "instance_id", id) networkAlloc, err = m.networkManager.GetAllocation(ctx, id) if err != nil { @@ -154,7 +154,7 @@ func (m *manager) deleteInstanceWithOptions( } // 6. Release network allocation - if inst.NetworkEnabled && inst.MacOS == nil { + if inst.usesLinuxNetwork() { m.unregisterEgressProxyInstance(ctx, id) log.DebugContext(ctx, "releasing network", "instance_id", id, "network", "default") releaseNetworkCtx, releaseNetworkSpanEnd := m.startLifecycleStep(ctx, "release_network", diff --git a/lib/instances/guest_agent_ready_command_test.go b/lib/instances/guest_agent_ready_command_test.go new file mode 100644 index 000000000..ef9ad91e1 --- /dev/null +++ b/lib/instances/guest_agent_ready_command_test.go @@ -0,0 +1,13 @@ +package instances + +import ( + "testing" + + "github.com/kernel/hypeman/lib/images" + "github.com/stretchr/testify/require" +) + +func TestGuestAgentReadyExecutableMatchesGuestOS(t *testing.T) { + require.Equal(t, "/bin/true", guestAgentReadyExecutable(&StoredMetadata{})) + require.Equal(t, "/usr/bin/true", guestAgentReadyExecutable(&StoredMetadata{MacOS: &images.MacOSImage{}})) +} diff --git a/lib/instances/guest_os.go b/lib/instances/guest_os.go new file mode 100644 index 000000000..7e93963cc --- /dev/null +++ b/lib/instances/guest_os.go @@ -0,0 +1,23 @@ +package instances + +import "fmt" + +// GuestOperation names a host API operation whose availability depends on the +// guest OS and on the shared guest agent. +type GuestOperation string + +const ( + GuestOpExec GuestOperation = "exec" + GuestOpFileCopy GuestOperation = "file copy" + GuestOpStat GuestOperation = "stat" +) + +// GuestOperationUnsupportedReason returns an empty string when op is available for +// this instance, or the user-facing reason it is not. It is the single rule for +// which operations need the shared system agent on each OS. +func (s *StoredMetadata) GuestOperationUnsupportedReason(op GuestOperation) string { + if s.MacOS != nil && !s.GuestAgentEnabled() { + return fmt.Sprintf("%s requires the shared guest agent on macOS instances", op) + } + return "" +} diff --git a/lib/instances/guest_os_test.go b/lib/instances/guest_os_test.go new file mode 100644 index 000000000..054fa0f69 --- /dev/null +++ b/lib/instances/guest_os_test.go @@ -0,0 +1,23 @@ +package instances + +import ( + "testing" + + "github.com/kernel/hypeman/lib/images" + "github.com/stretchr/testify/require" +) + +func TestGuestOperationCapabilityTable(t *testing.T) { + linux := &StoredMetadata{} + for _, op := range []GuestOperation{GuestOpExec, GuestOpFileCopy, GuestOpStat} { + require.Empty(t, linux.GuestOperationUnsupportedReason(op), "Linux guests keep their historical behavior") + } + + withoutAgent := &StoredMetadata{MacOS: &images.MacOSImage{GuestAgent: false}, SkipGuestAgent: true} + require.Contains(t, withoutAgent.GuestOperationUnsupportedReason(GuestOpExec), "requires the shared guest agent") + + withAgent := &StoredMetadata{MacOS: &images.MacOSImage{GuestAgent: true}} + for _, op := range []GuestOperation{GuestOpExec, GuestOpFileCopy, GuestOpStat} { + require.Empty(t, withAgent.GuestOperationUnsupportedReason(op)) + } +} diff --git a/lib/instances/macos.go b/lib/instances/macos.go index 020f60ee1..cc25613a3 100644 --- a/lib/instances/macos.go +++ b/lib/instances/macos.go @@ -13,10 +13,12 @@ import ( "syscall" "time" + "github.com/kernel/hypeman/lib/healthcheck" "github.com/kernel/hypeman/lib/hypervisor" "github.com/kernel/hypeman/lib/images" "github.com/kernel/hypeman/lib/instances/phasetracking" "github.com/kernel/hypeman/lib/network" + "github.com/kernel/hypeman/lib/paths" restartpolicy "github.com/kernel/hypeman/lib/restart-policy" ) @@ -81,12 +83,15 @@ func prepareMacOSCreate(req *CreateInstanceRequest, img *images.Image, caps hype if img.Platform != "darwin/arm64" || img.SizeBytes == nil || *img.SizeBytes <= 0 { return fmt.Errorf("%w: incomplete macOS image", ErrImageNotReady) } - if req.HotplugSize != 0 || req.OverlaySize != 0 || len(req.Volumes) != 0 || len(req.Devices) != 0 || req.GPU != nil || len(req.Env) != 0 || len(req.Entrypoint) != 0 || len(req.Cmd) != 0 || req.NetworkEgress != nil || len(req.Credentials) != 0 || req.AutoStandby != nil || req.HealthCheck != nil || req.SnapshotPolicy != nil || req.DiskIOBps != 0 || req.NetworkBandwidthDownload != 0 || req.NetworkBandwidthUpload != 0 { - return fmt.Errorf("%w: experimental macOS supports local disk clone, CPU/RAM, tags, expiration and NAT only; Linux commands/env/volumes, overlays, agents, auto-standby/snapshot policies and I/O shaping are unsupported", ErrInvalidRequest) + if req.HotplugSize != 0 || req.OverlaySize != 0 || len(req.Volumes) != 0 || len(req.Devices) != 0 || req.GPU != nil || len(req.Env) != 0 || len(req.Entrypoint) != 0 || len(req.Cmd) != 0 || req.NetworkEgress != nil || len(req.Credentials) != 0 || req.AutoStandby != nil || req.SnapshotPolicy != nil || req.DiskIOBps != 0 || req.NetworkBandwidthDownload != 0 || req.NetworkBandwidthUpload != 0 { + return fmt.Errorf("%w: experimental macOS supports local disk clone, CPU/RAM, tags, expiration and NAT only; Linux commands/env/volumes, overlays, auto-standby/snapshot policies and I/O shaping are unsupported", ErrInvalidRequest) } if err := validateMacOSRestartPolicy(req.RestartPolicy); err != nil { return err } + if err := validateMacOSHealthCheck(req.HealthCheck, img.MacOS.GuestAgent && !req.SkipGuestAgent); err != nil { + return err + } size, vcpus := req.Size, req.Vcpus if size == 0 { size = int64(img.MacOS.Memory) @@ -99,7 +104,7 @@ func prepareMacOSCreate(req *CreateInstanceRequest, img *images.Image, caps hype } req.Size, req.Vcpus = size, vcpus req.OverlaySize = *img.SizeBytes // Reserve the writable boot disk, not a Linux overlay. - req.SkipGuestAgent = true + req.SkipGuestAgent = req.SkipGuestAgent || !img.MacOS.GuestAgent req.SkipKernelHeaders = true return nil } @@ -124,12 +129,29 @@ func checkMacOSUpdate(stored *StoredMetadata, req UpdateInstanceRequest) error { if stored.MacOS == nil { return nil } - if len(req.Env) != 0 || req.AutoStandby != nil || req.HealthCheck != nil { - return fmt.Errorf("%w: macOS updates support expiration and VM restart policy only", ErrInvalidRequest) + if len(req.Env) != 0 || req.AutoStandby != nil { + return fmt.Errorf("%w: macOS updates support expiration, exec health checks and VM restart policy only", ErrInvalidRequest) + } + if err := validateMacOSHealthCheck(req.HealthCheck, stored.GuestAgentEnabled()); err != nil { + return err } return validateMacOSRestartPolicy(req.RestartPolicy) } +func validateMacOSHealthCheck(policy *healthcheck.Policy, agentEnabled bool) error { + normalized, err := normalizeHealthCheckPolicy(policy) + if err != nil { + return err + } + if !healthcheck.Enabled(normalized) { + return nil + } + if normalized.Type != healthcheck.TypeExec || !agentEnabled { + return fmt.Errorf("%w: macOS supports exec health checks with an enabled guest agent only", ErrInvalidRequest) + } + return nil +} + // checkMacOSStart refuses starts a macOS-only server or a macOS guest cannot honor. func (m *manager) checkMacOSStart(stored *StoredMetadata, req StartInstanceRequest) error { if err := m.requireMacOSGuest(stored); err != nil { @@ -152,21 +174,49 @@ func (m *manager) createBootDisk(stored *StoredMetadata, img *images.Image) erro return err } aux := filepath.Join(filepath.Dir(disk), "aux.img") - return cloneMacOSStorage(disk, aux, m.paths.InstanceOverlay(stored.Id), filepath.Join(stored.DataDir, macOSAuxStorageFile)) + return cloneMacOSStorage(disk, aux, m.paths.InstanceMacOSBootDisk(stored.Id), filepath.Join(stored.DataDir, macOSAuxStorageFile)) } -// applyMacOSNetwork sets the template MAC on stored. vmnet assigns the address, which is -// observed after boot, so no Linux TAP or static IP is allocated. +// usesLinuxNetwork reports whether the instance's network is a hypeman TAP +// allocation. Networked macOS guests use vmnet NAT instead. +func (s *StoredMetadata) usesLinuxNetwork() bool { + return s.NetworkEnabled && s.MacOS == nil +} + +// applyMacOSNetwork sets the template MAC on a networked macOS guest and returns its +// network config. vmnet assigns the address, which is observed after boot, so no +// Linux TAP or static IP is allocated. Other instances get nil. func applyMacOSNetwork(stored *StoredMetadata) *network.NetworkConfig { + if stored.MacOS == nil || !stored.NetworkEnabled { + return nil + } stored.MAC = stored.MacOS.MAC stored.IP = "" return &network.NetworkConfig{MAC: stored.MAC} } +// admitMacOSBoot holds the macOS boot lock and checks socket paths and identity +// admission before a macOS guest boots. The returned release ends the hold; for +// other guests it does nothing. +func (m *manager) admitMacOSBoot(ctx context.Context, stored *StoredMetadata) (func(), error) { + if stored.MacOS == nil { + return func() {}, nil + } + if err := checkMacOSSocketPaths(stored); err != nil { + return nil, err + } + m.macOSBootMu.Lock() + if err := m.checkMacOSIdentityAvailable(ctx, stored); err != nil { + m.macOSBootMu.Unlock() + return nil, err + } + return m.macOSBootMu.Unlock, nil +} + func (m *manager) macOSVMConfig(inst *Instance) hypervisor.VMConfig { c := hypervisor.VMConfig{BootMode: hypervisor.BootModeMacOS, VCPUs: inst.Vcpus, MemoryBytes: inst.Size, VsockCID: inst.VsockCID, VsockSocket: inst.VsockSocket, MacOS: &hypervisor.MacOSPlatform{HardwareModelData: inst.MacOS.HardwareModel, MachineIdentifierData: inst.MacOS.MachineIdentifier, AuxStoragePath: filepath.Join(inst.DataDir, macOSAuxStorageFile)}, - Disks: []hypervisor.DiskConfig{{Path: m.paths.InstanceOverlay(inst.Id)}}} + Disks: []hypervisor.DiskConfig{{Path: m.paths.InstanceMacOSBootDisk(inst.Id)}}} if inst.NetworkEnabled { c.Networks = []hypervisor.NetworkConfig{{MAC: inst.MAC}} } @@ -174,7 +224,8 @@ func (m *manager) macOSVMConfig(inst *Instance) hypervisor.VMConfig { } func initialBootPhase(stored *StoredMetadata) phasetracking.Phase { if stored.MacOS != nil { - return phasetracking.PhaseRunning + phase, _ := macOSRunningPhase(stored) + return phase } return phasetracking.PhaseInitializing } @@ -243,6 +294,32 @@ func checkMacOSSocketPaths(stored *StoredMetadata) error { return nil } +// GuestAgentEnabled reports whether exec, copy, readiness and shutdown may use the +// shared guest agent. A macOS image must declare it; hand-written metadata that +// leaves SkipGuestAgent unset does not enable it. +func (s *StoredMetadata) GuestAgentEnabled() bool { + return !s.SkipGuestAgent && (s.MacOS == nil || s.MacOS.GuestAgent) +} + +// programMarkerSettled reports that no workload-start marker is awaited. macOS +// guests have no Linux workload marker, so only agent readiness is probed. +func programMarkerSettled(stored *StoredMetadata) bool { + return stored.MacOS != nil || stored.ProgramStartedAt != nil +} + +// guestAgentReadyExecutable is the readiness probe command. Darwin provides true +// in /usr/bin, not /bin. +func guestAgentReadyExecutable(stored *StoredMetadata) string { + if stored.MacOS != nil { + return "/usr/bin/true" + } + return "/bin/true" +} + +// macOSMinStopTimeout is the shortest graceful stop window, in seconds, given to a +// macOS guest, so a normal stop can record a graceful receipt and strict export waits. +const macOSMinStopTimeout = 30 + // admitGuestOperation refuses an operation this server cannot run for the guest: // any Linux operation on a macOS-only server, and operations macOS guests do not // implement. Callers check it right after loading the record under the lock. @@ -312,7 +389,7 @@ func (s *StoredMetadata) VMStoragePath() string { if s.MacOS == nil || s.DataDir == "" { return "" } - return filepath.Join(s.DataDir, "overlay.raw") + return filepath.Join(s.DataDir, paths.MacOSBootDiskFile) } // requireMacOSGuest refuses Linux guests on a macOS-only server. @@ -409,3 +486,16 @@ func macOSShimRefusing(stored *StoredMetadata) bool { conn.Close() return false } + +// macOSRunningPhase matches deriveRunningState: an agent-enabled guest runs once +// the agent is ready; otherwise it runs from boot. A zero time is clamped to the +// phase start by the caller. +func macOSRunningPhase(stored *StoredMetadata) (phasetracking.Phase, time.Time) { + if !stored.GuestAgentEnabled() { + return phasetracking.PhaseRunning, time.Time{} + } + if stored.GuestAgentReadyAt == nil { + return phasetracking.PhaseInitializing, time.Time{} + } + return phasetracking.PhaseRunning, *stored.GuestAgentReadyAt +} diff --git a/lib/instances/macos_health_check_test.go b/lib/instances/macos_health_check_test.go new file mode 100644 index 000000000..08c9ff4f3 --- /dev/null +++ b/lib/instances/macos_health_check_test.go @@ -0,0 +1,66 @@ +package instances + +import ( + "context" + "testing" + + "github.com/kernel/hypeman/lib/healthcheck" + "github.com/kernel/hypeman/lib/paths" + "github.com/stretchr/testify/require" +) + +func TestMacOSHealthCheckAdmission(t *testing.T) { + for _, tc := range []struct { + name string + policy *healthcheck.Policy + declared, skipped, allowed bool + }{ + {"exec", &healthcheck.Policy{Type: healthcheck.TypeExec, Exec: &healthcheck.ExecCheck{Command: []string{"/usr/bin/true"}}}, true, false, true}, + {"inferred exec", &healthcheck.Policy{Exec: &healthcheck.ExecCheck{Command: []string{"/usr/bin/true"}}}, true, false, true}, + {"undeclared", &healthcheck.Policy{Type: healthcheck.TypeExec, Exec: &healthcheck.ExecCheck{Command: []string{"/usr/bin/true"}}}, false, false, false}, + {"skipped", &healthcheck.Policy{Type: healthcheck.TypeExec, Exec: &healthcheck.ExecCheck{Command: []string{"/usr/bin/true"}}}, true, true, false}, + {"tcp", &healthcheck.Policy{Type: healthcheck.TypeTCP, TCP: &healthcheck.TCPCheck{Port: 80}}, true, false, false}, + {"http", &healthcheck.Policy{Type: healthcheck.TypeHTTP, HTTP: &healthcheck.HTTPCheck{Port: 80}}, true, false, false}, + {"none", &healthcheck.Policy{Type: healthcheck.TypeNone}, false, true, true}, + } { + t.Run(tc.name, func(t *testing.T) { + image := testMacImage() + image.MacOS.GuestAgent = tc.declared + req := CreateInstanceRequest{HealthCheck: tc.policy, SkipGuestAgent: tc.skipped, NetworkEnabled: true} + createErr := prepareMacOSCreate(&req, image, testMacOSCaps()) + stored := StoredMetadata{MacOS: image.MacOS, SkipGuestAgent: tc.skipped} + updateErr := checkMacOSUpdate(&stored, UpdateInstanceRequest{HealthCheck: tc.policy}) + if tc.allowed { + require.NoError(t, createErr) + require.NoError(t, updateErr) + } else { + require.ErrorIs(t, createErr, ErrInvalidRequest) + require.ErrorIs(t, updateErr, ErrInvalidRequest) + } + }) + } +} + +func TestMacOSUpdateHealthCheckPersistsAndDisables(t *testing.T) { + p := paths.New(t.TempDir()) + m := &manager{paths: p, macOSOnly: true, lifecycleEvents: newLifecycleSubscribers()} + id := "mac-health" + image := testMacImage() + image.MacOS.GuestAgent = true + require.NoError(t, m.ensureDirectories(id)) + require.NoError(t, m.saveMetadata(&metadata{StoredMetadata: StoredMetadata{Id: id, DataDir: p.InstanceDir(id), MacOS: image.MacOS}})) + + policy := &healthcheck.Policy{Type: healthcheck.TypeExec, Exec: &healthcheck.ExecCheck{Command: []string{"/usr/bin/true"}}} + updated, err := m.UpdateInstance(context.Background(), id, UpdateInstanceRequest{HealthCheck: policy}) + require.NoError(t, err) + require.Equal(t, healthcheck.TypeExec, updated.HealthCheck.Type) + require.NotEmpty(t, updated.HealthCheck.Interval) + persisted, err := m.loadMetadata(id) + require.NoError(t, err) + require.Equal(t, updated.HealthCheck, persisted.HealthCheck) + + updated, err = m.UpdateInstance(context.Background(), id, UpdateInstanceRequest{HealthCheck: &healthcheck.Policy{Type: healthcheck.TypeNone}}) + require.NoError(t, err) + require.False(t, healthcheck.Enabled(updated.HealthCheck)) + require.Nil(t, updated.HealthCheckRuntime) +} diff --git a/lib/instances/macos_test.go b/lib/instances/macos_test.go index 10c10295f..2684dbbdb 100644 --- a/lib/instances/macos_test.go +++ b/lib/instances/macos_test.go @@ -11,6 +11,7 @@ import ( "github.com/kernel/hypeman/lib/hypervisor" "github.com/kernel/hypeman/lib/images" + "github.com/kernel/hypeman/lib/instances/phasetracking" "github.com/kernel/hypeman/lib/paths" "github.com/stretchr/testify/require" ) @@ -36,6 +37,103 @@ func TestMacOSRequestDefaultsAndRejections(t *testing.T) { } require.ErrorIs(t, prepareMacOSCreate(&CreateInstanceRequest{}, testMacImage(), hypervisor.Capabilities{}), ErrInvalidRequest) } +func TestMacOSGuestAgentOptIn(t *testing.T) { + caps := hypervisor.Capabilities{SupportsMacOSBoot: true} + image := testMacImage() + image.MacOS.GuestAgent = true + request := CreateInstanceRequest{} + require.NoError(t, prepareMacOSCreate(&request, image, caps)) + require.False(t, request.SkipGuestAgent) + request = CreateInstanceRequest{SkipGuestAgent: true} + require.NoError(t, prepareMacOSCreate(&request, image, caps)) + require.True(t, request.SkipGuestAgent) +} + +func TestMacOSAgentReadinessGatesRunning(t *testing.T) { + for _, tc := range []struct { + name string + declared, skipped, ready bool + }{ + {"legacy image", false, true, false}, + {"disabled", true, true, false}, + {"not ready", true, false, false}, + {"ready", true, false, true}, + } { + t.Run(tc.name, func(t *testing.T) { + image := testMacImage() + image.MacOS.GuestAgent = tc.declared + stored := StoredMetadata{Id: "mac", MacOS: image.MacOS, SkipGuestAgent: tc.skipped} + now := time.Now().UTC() + calls := 0 + m := &manager{paths: paths.New(t.TempDir()), now: func() time.Time { return now }, guestAgentReadyProbe: func(context.Context, *StoredMetadata) bool { + calls++ + return tc.ready + }} + agent := tc.declared && !tc.skipped + initial := StateRunning + if agent { + initial = StateInitializing + } + require.Equal(t, initial, deriveRunningState(&stored)) + hydrated := m.hydrateBootMarkersFromLogs(context.Background(), &stored) + require.Equal(t, tc.ready, hydrated) + final := StateRunning + if agent && !tc.ready { + final = StateInitializing + } + require.Equal(t, final, deriveRunningState(&stored), "an agent-enabled guest is Running once the agent is ready") + require.Nil(t, stored.ProgramStartedAt, "macOS does not invent a Linux workload marker") + if tc.ready { + require.Equal(t, &now, stored.GuestAgentReadyAt) + } else { + require.Nil(t, stored.GuestAgentReadyAt) + } + if tc.declared && !tc.skipped { + require.Equal(t, 1, calls) + } else { + require.Zero(t, calls) + } + }) + } +} + +func TestMacOSAgentReadyMarkerPersisted(t *testing.T) { + p := paths.New(t.TempDir()) + now := time.Now().UTC() + m := &manager{paths: p, now: func() time.Time { return now }, guestAgentReadyProbe: func(context.Context, *StoredMetadata) bool { return true }} + require.NoError(t, m.ensureDirectories("mac")) + image := testMacImage() + image.MacOS.GuestAgent = true + require.NoError(t, m.saveMetadata(&metadata{StoredMetadata: StoredMetadata{Id: "mac", DataDir: p.InstanceDir("mac"), MacOS: image.MacOS}})) + m.persistBootMarkers(context.Background(), "mac") + meta, err := m.loadMetadata("mac") + require.NoError(t, err) + require.Equal(t, &now, meta.GuestAgentReadyAt) + require.Nil(t, meta.ProgramStartedAt) + require.True(t, meta.MacOS.GuestAgent) +} + +func TestMacOSAgentReadyMarkerPersistedByPublicRead(t *testing.T) { + p := paths.New(t.TempDir()) + now := time.Now().UTC() + m := &manager{paths: p, now: func() time.Time { return now }, guestAgentReadyProbe: func(context.Context, *StoredMetadata) bool { return true }} + require.NoError(t, m.ensureDirectories("mac")) + socket := filepath.Join(p.InstanceDir("mac"), "vz.sock") + require.NoError(t, os.WriteFile(socket, nil, 0600)) + image := testMacImage() + image.MacOS.GuestAgent = true + stored := StoredMetadata{Id: "mac", DataDir: p.InstanceDir("mac"), SocketPath: socket, MacOS: image.MacOS, HypervisorType: hypervisor.TypeVZ, CreatedAt: now} + require.NoError(t, m.saveMetadata(&metadata{StoredMetadata: stored})) + m.storeCachedHypervisorState("mac", hypervisor.StateRunning) + + inst, err := m.GetInstance(context.Background(), "mac") + require.NoError(t, err) + require.NotNil(t, inst.GuestAgentReadyAt) + meta, err := m.loadMetadata("mac") + require.NoError(t, err) + require.NotNil(t, meta.GuestAgentReadyAt, "readiness must reach metadata through the normal read path") +} + func TestMacOSBootConfigNoLinuxDevices(t *testing.T) { p := paths.New(t.TempDir()) m := &manager{paths: p} @@ -118,3 +216,21 @@ func TestMacOSSocketPathLimit(t *testing.T) { stored.HypervisorType = "test-starter" require.NoError(t, checkMacOSSocketPaths(stored), "only the VZ shim binds these sockets") } + +func TestMacOSRunningPhaseFollowsReadiness(t *testing.T) { + image := testMacImage() + image.MacOS.GuestAgent = true + stored := StoredMetadata{MacOS: image.MacOS} + phase, _ := runningPhaseFromMarkers(&stored) + require.Equal(t, phasetracking.PhaseInitializing, phase) + + ready := time.Date(2026, 10, 14, 1, 0, 0, 0, time.UTC) + stored.GuestAgentReadyAt = &ready + phase, at := runningPhaseFromMarkers(&stored) + require.Equal(t, phasetracking.PhaseRunning, phase) + require.Equal(t, ready, at) + + stored = StoredMetadata{MacOS: image.MacOS, SkipGuestAgent: true} + phase, _ = runningPhaseFromMarkers(&stored) + require.Equal(t, phasetracking.PhaseRunning, phase, "a guest without the agent runs from boot") +} diff --git a/lib/instances/manager.go b/lib/instances/manager.go index 0b39814f0..13beea04d 100644 --- a/lib/instances/manager.go +++ b/lib/instances/manager.go @@ -926,7 +926,7 @@ func (m *manager) ListRunningInstancesInfo(ctx context.Context) ([]resources.Ins } // Derive TAP device name if networking is enabled - if inst.NetworkEnabled && inst.MacOS == nil { + if inst.usesLinuxNetwork() { info.TAPDevice = network.GenerateTAPName(inst.Id) } info.StoragePath = inst.VMStoragePath() diff --git a/lib/instances/query.go b/lib/instances/query.go index e5ebd348d..3741c6f0b 100644 --- a/lib/instances/query.go +++ b/lib/instances/query.go @@ -146,7 +146,7 @@ func (m *manager) deriveStateWithOptions(ctx context.Context, stored *StoredMeta return stateResult{State: StateCreated} case hypervisor.StateRunning: hydrated := false - if hydrateBootMarkers && stored.MacOS == nil { + if hydrateBootMarkers { hydrated = m.hydrateBootMarkersFromLogs(ctx, stored) } return stateResult{ @@ -227,21 +227,13 @@ func (m *manager) updateCachedHypervisorStateFromInstance(inst *Instance) { } } +// deriveRunningState maps the boot markers to Running or Initializing, by the same +// rule as the phase tracker. func deriveRunningState(stored *StoredMetadata) State { - // For unmanaged macOS desktops, Running means VMM running, not app/agent ready. - if stored.MacOS != nil { - return StateRunning - } - if stored.ProgramStartedAt == nil { - return StateInitializing - } - if stored.SkipGuestAgent { + if phase, _ := runningPhaseFromMarkers(stored); phase == phasetracking.PhaseRunning { return StateRunning } - if stored.GuestAgentReadyAt == nil { - return StateInitializing - } - return StateRunning + return StateInitializing } // runningPhaseFromMarkers returns PhaseRunning with the wall-clock timestamp at @@ -250,8 +242,11 @@ func deriveRunningState(stored *StoredMetadata) State { // PhaseInitializing and the zero time. The returned transition time is the // later of ProgramStartedAt / GuestAgentReadyAt (or ProgramStartedAt alone // when the guest agent is skipped), which is the moment the guest actually -// became Running per deriveRunningState's rule. +// became Running. deriveRunningState follows the same rule. func runningPhaseFromMarkers(stored *StoredMetadata) (phasetracking.Phase, time.Time) { + if stored.MacOS != nil { + return macOSRunningPhase(stored) + } if stored.ProgramStartedAt == nil { return phasetracking.PhaseInitializing, time.Time{} } @@ -302,7 +297,7 @@ func advancePhaseIfRunning(stored *StoredMetadata) { // services do not need to forward stdout/stderr to the serial console. // Returns true when at least one missing marker was found and populated. func (m *manager) hydrateBootMarkersFromLogs(ctx context.Context, stored *StoredMetadata) bool { - needProgram := stored.ProgramStartedAt == nil + needProgram := !programMarkerSettled(stored) needAgent := !stored.SkipGuestAgent && stored.GuestAgentReadyAt == nil if !needProgram && !needAgent { m.clearBootMarkerRescan(stored.Id) @@ -327,7 +322,7 @@ func (m *manager) hydrateBootMarkersFromLogs(ctx context.Context, stored *Stored stored.GuestAgentReadyAt = guestAgentReadyAt hydrated = true } - if needAgent && stored.GuestAgentReadyAt == nil && stored.ProgramStartedAt != nil && m.hydrateGuestAgentReadyFromProbe(ctx, stored) { + if needAgent && stored.GuestAgentReadyAt == nil && programMarkerSettled(stored) && m.hydrateGuestAgentReadyFromProbe(ctx, stored) { hydrated = true } if hydrated { @@ -458,9 +453,12 @@ func probeGuestAgentReady(ctx context.Context, stored *StoredMetadata) bool { defer cancel() exit, err := guest.ExecIntoInstance(probeCtx, dialer, guest.ExecOptions{ - Command: []string{"/bin/true"}, + Command: []string{guestAgentReadyExecutable(stored)}, Timeout: int32(guestAgentReadyProbeTimeout / time.Second), WaitForAgent: guestAgentReadyProbeWait, + // Listings probe every instance concurrently. A macOS probe that evicted the + // shared connection cancelled other callers' in-flight RPCs (restore readiness). + KeepPooledConn: stored.MacOS != nil, }) return err == nil && exit != nil && exit.Code == 0 } @@ -652,7 +650,7 @@ func (m *manager) persistBootMarkers(ctx context.Context, id string) { return } - needProgram := meta.ProgramStartedAt == nil + needProgram := !programMarkerSettled(&meta.StoredMetadata) needAgent := !meta.SkipGuestAgent && meta.GuestAgentReadyAt == nil if !needProgram && !needAgent { return @@ -668,7 +666,7 @@ func (m *manager) persistBootMarkers(ctx context.Context, id string) { meta.GuestAgentReadyAt = guestAgentReadyAt updated = true } - if needAgent && meta.GuestAgentReadyAt == nil && meta.ProgramStartedAt != nil && m.hydrateGuestAgentReadyFromProbe(ctx, &meta.StoredMetadata) { + if needAgent && meta.GuestAgentReadyAt == nil && programMarkerSettled(&meta.StoredMetadata) && m.hydrateGuestAgentReadyFromProbe(ctx, &meta.StoredMetadata) { updated = true } if !updated { diff --git a/lib/instances/start.go b/lib/instances/start.go index 763ed44c1..59e8a19c5 100644 --- a/lib/instances/start.go +++ b/lib/instances/start.go @@ -96,25 +96,10 @@ func (m *manager) startInstance( }() } - // 3. Get image info (needed for buildHypervisorConfig). Resolve by the - // digest-pinned boot reference so a moved tag can't drift the rootfs/arch. - // macOS boots the disk and aux storage cloned at create time, so it does not - // depend on the template image after create. - var imageInfo *images.Image - if stored.MacOS == nil { - bootImage := bootImageRef(stored) - log.DebugContext(ctx, "getting image info", "instance_id", id, "image", bootImage) - imageCtx, imageSpanEnd := m.startLifecycleStep(ctx, "resolve_image", - attribute.String("instance_id", id), - attribute.String("hypervisor", string(stored.HypervisorType)), - attribute.String("operation", "resolve_image"), - ) - imageInfo, err = m.imageManager.GetImage(imageCtx, bootImage) - imageSpanEnd(err) - if err != nil { - log.ErrorContext(ctx, "failed to get image", "instance_id", id, "image", bootImage, "error", err) - return nil, fmt.Errorf("get image: %w", err) - } + // 3. Get image info (needed for buildHypervisorConfig). + imageInfo, err := m.resolveBootImage(ctx, stored) + if err != nil { + return nil, err } // Setup cleanup stack for automatic rollback on errors @@ -122,10 +107,8 @@ func (m *manager) startInstance( defer cu.Clean() // 4. Allocate fresh network if network enabled - var netConfig *network.NetworkConfig - if stored.MacOS != nil && stored.NetworkEnabled { - netConfig = applyMacOSNetwork(stored) - } else if stored.NetworkEnabled { + netConfig := applyMacOSNetwork(stored) + if stored.usesLinuxNetwork() { log.DebugContext(ctx, "allocating network for start", "instance_id", id, "network", "default") networkCtx, networkSpanEnd := m.startLifecycleStep(ctx, "allocate_network", attribute.String("instance_id", id), @@ -259,3 +242,27 @@ func (m *manager) startInstance( log.InfoContext(ctx, "instance started successfully", "instance_id", id, "state", finalInst.State) return &finalInst, nil } + +// resolveBootImage resolves the image a guest boots from by its digest-pinned +// reference, so a moved tag can't drift the rootfs/arch. macOS boots the disk and +// aux storage cloned at create time, so it has no boot image and gets nil. +func (m *manager) resolveBootImage(ctx context.Context, stored *StoredMetadata) (*images.Image, error) { + if stored.MacOS != nil { + return nil, nil + } + log := logger.FromContext(ctx) + bootImage := bootImageRef(stored) + log.DebugContext(ctx, "getting image info", "instance_id", stored.Id, "image", bootImage) + imageCtx, imageSpanEnd := m.startLifecycleStep(ctx, "resolve_image", + attribute.String("instance_id", stored.Id), + attribute.String("hypervisor", string(stored.HypervisorType)), + attribute.String("operation", "resolve_image"), + ) + imageInfo, err := m.imageManager.GetImage(imageCtx, bootImage) + imageSpanEnd(err) + if err != nil { + log.ErrorContext(ctx, "failed to get image", "instance_id", stored.Id, "image", bootImage, "error", err) + return nil, fmt.Errorf("get image: %w", err) + } + return imageInfo, nil +} diff --git a/lib/instances/stop.go b/lib/instances/stop.go index 603b9a75e..6024114c9 100644 --- a/lib/instances/stop.go +++ b/lib/instances/stop.go @@ -93,7 +93,9 @@ func (m *manager) tryGracefulGuestShutdown(ctx context.Context, inst *Instance, } waitTimeout := time.Duration(stopTimeout) * time.Second - if !shutdownSent && waitTimeout > shutdownFailureFallbackWait { + // Darwin may close vsock before the reply while power-off is in progress, so a + // macOS guest keeps its full grace period. Success still requires VMM exit. + if !shutdownSent && inst.MacOS == nil && waitTimeout > shutdownFailureFallbackWait { // If we couldn't signal the guest, don't burn the full graceful timeout. waitTimeout = shutdownFailureFallbackWait } @@ -146,7 +148,7 @@ func (m *manager) stopInstance( // 3. Get network allocation BEFORE killing VMM (while we can still query it) var networkAlloc *network.Allocation var networkAllocErr error - if inst.NetworkEnabled && inst.MacOS == nil { + if inst.usesLinuxNetwork() { log.DebugContext(ctx, "getting network allocation", "instance_id", id) networkAlloc, networkAllocErr = m.networkManager.GetAllocation(ctx, id) if networkAllocErr != nil { @@ -157,6 +159,10 @@ func (m *manager) stopInstance( // 4. Graceful shutdown: send signal to guest init via Shutdown RPC, // then wait for VM to power off cleanly. Fall back to hypervisor shutdown on timeout. stopTimeout := resolveStopTimeout(stored) + if inst.MacOS != nil { + // macOS power-off can outlast the ordinary short stop default. + stopTimeout = max(stopTimeout, macOSMinStopTimeout) + } gracefulCtx, gracefulSpanEnd := m.startLifecycleStep(ctx, "graceful_guest_shutdown", attribute.String("instance_id", id), attribute.String("hypervisor", string(stored.HypervisorType)), diff --git a/lib/instances/storage.go b/lib/instances/storage.go index eee2c4a4f..1ee166fc2 100644 --- a/lib/instances/storage.go +++ b/lib/instances/storage.go @@ -26,6 +26,7 @@ const ( // {dataDir}/guests/{instance-id}/ // metadata.json # Instance metadata // overlay.raw # Configurable sparse overlay disk (default 10GB) +// boot.raw # macOS guests: complete boot disk, instead of overlay.raw // config.ext4 # Read-only config disk (generated) // ch.sock # Hypervisor API socket (abbreviated name for SUN_LEN limit) // logs/ diff --git a/lib/instances/vm_storage_path_test.go b/lib/instances/vm_storage_path_test.go index 6b68cdbdb..62fd284e5 100644 --- a/lib/instances/vm_storage_path_test.go +++ b/lib/instances/vm_storage_path_test.go @@ -10,6 +10,6 @@ import ( func TestVMStoragePathOnlyForMacOSGuests(t *testing.T) { require.Empty(t, (&StoredMetadata{DataDir: "/data/guests/i1"}).VMStoragePath()) mac := &StoredMetadata{DataDir: "/data/guests/i1", MacOS: &images.MacOSImage{}} - require.Equal(t, "/data/guests/i1/overlay.raw", mac.VMStoragePath()) + require.Equal(t, "/data/guests/i1/boot.raw", mac.VMStoragePath()) require.Empty(t, (&StoredMetadata{MacOS: &images.MacOSImage{}}).VMStoragePath(), "no data dir, no path") } diff --git a/lib/instances/vsock.go b/lib/instances/vsock.go index f4060950e..8cbefe4e0 100644 --- a/lib/instances/vsock.go +++ b/lib/instances/vsock.go @@ -14,8 +14,8 @@ func (m *manager) GetVsockDialer(ctx context.Context, instanceID string) (hyperv return nil, err } - if inst.MacOS != nil { - return nil, fmt.Errorf("%w: macOS guest-agent exec/stat is not implemented", ErrInvalidRequest) + if inst.MacOS != nil && !inst.StoredMetadata.GuestAgentEnabled() { + return nil, fmt.Errorf("%w: macOS image does not enable the shared guest agent", ErrInvalidRequest) } return hypervisor.NewVsockDialer(hypervisor.Type(inst.HypervisorType), inst.VsockSocket, inst.VsockCID) } diff --git a/lib/paths/paths.go b/lib/paths/paths.go index 314151ce4..ae615c61d 100644 --- a/lib/paths/paths.go +++ b/lib/paths/paths.go @@ -251,6 +251,14 @@ func (p *Paths) InstanceOverlay(id string) string { return filepath.Join(p.InstanceDir(id), "overlay.raw") } +// MacOSBootDiskFile is a macOS guest's complete raw boot disk, cloned from its image. +const MacOSBootDiskFile = "boot.raw" + +// InstanceMacOSBootDisk returns the path to a macOS guest's boot disk. +func (p *Paths) InstanceMacOSBootDisk(id string) string { + return filepath.Join(p.InstanceDir(id), MacOSBootDiskFile) +} + // InstanceConfigDisk returns the path to instance config disk. func (p *Paths) InstanceConfigDisk(id string) string { return filepath.Join(p.InstanceDir(id), "config.ext4") diff --git a/lib/system/guest_agent/darwin_test.go b/lib/system/guest_agent/darwin_test.go new file mode 100644 index 000000000..d7582ea53 --- /dev/null +++ b/lib/system/guest_agent/darwin_test.go @@ -0,0 +1,19 @@ +//go:build darwin + +package main + +import ( + "context" + "testing" + + pb "github.com/kernel/hypeman/lib/guest" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +func TestDarwinNetworkReconfigurationUnsupported(t *testing.T) { + response, err := (&guestServer{}).ReconfigureNetwork(context.Background(), &pb.ReconfigureNetworkRequest{}) + require.Nil(t, response) + require.Equal(t, codes.Unimplemented, status.Code(err)) +} diff --git a/lib/system/guest_agent/exec.go b/lib/system/guest_agent/exec.go index 409754c65..1b8adb323 100644 --- a/lib/system/guest_agent/exec.go +++ b/lib/system/guest_agent/exec.go @@ -29,6 +29,9 @@ func (s *guestServer) Exec(stream pb.GuestService_ExecServer) error { if start == nil { return fmt.Errorf("first message must be ExecStart") } + if s.execLifecycle { + return s.execWithLifecycle(stream, start) + } command := start.Command if len(command) == 0 { diff --git a/lib/system/guest_agent/exec_background_child_test.go b/lib/system/guest_agent/exec_background_child_test.go new file mode 100644 index 000000000..712c37197 --- /dev/null +++ b/lib/system/guest_agent/exec_background_child_test.go @@ -0,0 +1,52 @@ +package main + +import ( + "context" + "os" + "path/filepath" + "strconv" + "strings" + "syscall" + "testing" + "time" + + pb "github.com/kernel/hypeman/lib/guest" + "github.com/stretchr/testify/require" +) + +type exitCapturingExecStream struct { + stalledExecStream + exitCode *int32 +} + +func (s *exitCapturingExecStream) Send(response *pb.ExecResponse) error { + if _, ok := response.Response.(*pb.ExecResponse_ExitCode); ok { + value := response.GetExitCode() + s.exitCode = &value + } + return nil +} + +// Regression guard: a clean TTY exit with a background child still reports status +// and returns within the bounded drain. This did not reproduce the reviewer's hang +// on macOS (the pty hangs up when the shell exits), so it does not prove that claim. +func TestTTYExecExitStatusWithBackgroundChildIsBounded(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + release := make(chan struct{}) + defer close(release) + pidFile := filepath.Join(t.TempDir(), "child") + stream := &exitCapturingExecStream{stalledExecStream: stalledExecStream{ctx: ctx, release: release, start: &pb.ExecRequest{Request: &pb.ExecRequest_Start{Start: &pb.ExecStart{Tty: true, Command: []string{"/bin/sh", "-c", `trap '' HUP; sleep 600 & echo $! > "$1"`, "sh", pidFile}}}}}} + t.Cleanup(func() { + if data, err := os.ReadFile(pidFile); err == nil { + if pid, err := strconv.Atoi(strings.TrimSpace(string(data))); err == nil { + _ = syscall.Kill(pid, syscall.SIGKILL) + } + } + }) + started := time.Now() + require.NoError(t, (&guestServer{execLifecycle: true, drainGrace: 300 * time.Millisecond}).Exec(stream)) + require.Less(t, time.Since(started), 10*time.Second, "exit must not wait for the background child") + require.NotNil(t, stream.exitCode, "exit status must be reported") + require.EqualValues(t, 0, *stream.exitCode) +} diff --git a/lib/system/guest_agent/exec_lifecycle.go b/lib/system/guest_agent/exec_lifecycle.go new file mode 100644 index 000000000..0e5688c3f --- /dev/null +++ b/lib/system/guest_agent/exec_lifecycle.go @@ -0,0 +1,347 @@ +package main + +import ( + "context" + "errors" + "fmt" + "io" + "log" + "os/exec" + "sync" + "sync/atomic" + "syscall" + "time" + + "github.com/creack/pty" + pb "github.com/kernel/hypeman/lib/guest" +) + +// execWithLifecycle runs a command for a macOS guest. A client disconnect cancels +// it, cancellation kills its whole process group, and output drains are bounded, +// so an abandoned or stuck command cannot hold the handler. +func (s *guestServer) execWithLifecycle(stream pb.GuestService_ExecServer, start *pb.ExecStart) error { + if len(start.Command) == 0 { + start.Command = []string{"/bin/sh"} + } + log.Printf("[guest-agent] exec: command=%v tty=%v cwd=%s timeout=%d", + start.Command, start.Tty, start.Cwd, start.TimeoutSeconds) + + ctx := stream.Context() + if start.TimeoutSeconds > 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, time.Duration(start.TimeoutSeconds)*time.Second) + defer cancel() + } + + ctx, cancel := context.WithCancel(ctx) + defer cancel() + cmd, err := s.execCommand(ctx, start) + if err != nil { + return err + } + sender := &execSender{stream: stream} + defer sender.close() + if start.Tty { + return s.executeLifecycleTTY(ctx, cancel, sender, start, cmd) + } + return s.executeLifecycleNoTTY(ctx, cancel, sender, cmd) +} + +// errExecStreamClosed is returned for output written after the handler returned. +var errExecStreamClosed = errors.New("exec stream closed") + +// execSender serializes sends on one exec stream. A gRPC stream allows one sender +// at a time, and nothing may send once the handler has returned, even though a +// bounded drain can leave output writers running. +type execSender struct { + stream pb.GuestService_ExecServer + mu sync.Mutex + closed atomic.Bool +} + +func (s *execSender) send(response *pb.ExecResponse) error { + s.mu.Lock() + defer s.mu.Unlock() + if s.closed.Load() { + return errExecStreamClosed + } + return s.stream.Send(response) +} + +// close refuses later sends. It does not wait for a send already in progress. +func (s *execSender) close() { s.closed.Store(true) } + +// executeLifecycleNoTTY streams output as the command writes it. +func (s *guestServer) executeLifecycleNoTTY(ctx context.Context, cancel context.CancelFunc, sender *execSender, cmd *exec.Cmd) error { + cmd.Stdout = &execStreamWriter{sender: sender, cancel: cancel} + cmd.Stderr = &execStreamWriter{sender: sender, cancel: cancel, stderr: true} + stdin, err := cmd.StdinPipe() + if err != nil { + return fmt.Errorf("open command stdin: %w", err) + } + if err := cmd.Start(); err != nil { + return fmt.Errorf("start command: %w", err) + } + // Cancel is not called once the direct child has exited, so a descendant that + // still holds the pipes would outlive the timeout. Kill the group from the context. + waited := make(chan struct{}) + go killGroupOnDone(ctx, cmd, waited) + + // Handle stdin in background + go func() { + defer stdin.Close() + for { + req, err := sender.stream.Recv() + if err != nil { + return + } + if data := req.GetStdin(); data != nil { + stdin.Write(data) + } + } + }() + + return s.finishExec(ctx, sender, cmd, waited, nil) +} + +// execCommand owns setup and pre-start cancellation for both I/O modes. +func (s *guestServer) execCommand(ctx context.Context, start *pb.ExecStart) (*exec.Cmd, error) { + if err := ctx.Err(); err != nil { + return nil, fmt.Errorf("start command: %w", err) + } + cmd := exec.CommandContext(ctx, start.Command[0], start.Command[1:]...) + cmd.Cancel = func() error { return killProcessGroup(cmd) } + cmd.WaitDelay = 2 * time.Second + cmd.Env = s.buildEnv(start.Env, start.Tty) + cmd.Dir = start.Cwd + if !start.Tty { + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + } + return cmd, nil +} + +// finishExec owns bounded wait, output drain and final status for both I/O modes. +// It closes waited once cmd.Wait returns. +func (s *guestServer) finishExec(ctx context.Context, sender *execSender, cmd *exec.Cmd, waited chan<- struct{}, outputDone <-chan struct{}) error { + // Non-TTY cmd.Wait also waits for copies, which can block in stream.Send. + var waitErr error + if err := s.runBounded(ctx, func() { + waitErr = cmd.Wait() + close(waited) + }); err != nil { + return err + } + // ErrWaitDelay means the command exited but a descendant still held its output. + // The status is already known, so it is reported as a normal exit. + if waitErr != nil { + _, exited := waitErr.(*exec.ExitError) + if !exited && !errors.Is(waitErr, exec.ErrWaitDelay) { + return fmt.Errorf("stream command output: %w", waitErr) + } + } + + if outputDone != nil { + if err := drainAfterExit(ctx, outputDone, s.drainBound()); err != nil { + return err + } + } + exitCode := exitCodeOf(ctx, cmd, waitErr) + + log.Printf("[guest-agent] command finished with exit code: %d", exitCode) + + return s.sendExitCode(ctx, sender, exitCode) +} + +// defaultDrainGrace bounds how long a cancelled command may take to finish. A descendant that +// holds the terminal or output, or a client that stopped reading, must not block the handler. +const defaultDrainGrace = 5 * time.Second + +func (s *guestServer) drainBound() time.Duration { + if s.drainGrace > 0 { + return s.drainGrace + } + return defaultDrainGrace +} + +// drainAfterExit runs after the command status is known. A background child can +// keep a terminal open forever, so a normal exit gets only a bounded output drain +// and still reports its status. Cancellation keeps the error-returning drain. +func drainAfterExit(ctx context.Context, done <-chan struct{}, grace time.Duration) error { + if ctx.Err() != nil { + return awaitBounded(ctx, done, grace) + } + timer := time.NewTimer(grace) + defer timer.Stop() + select { + case <-done: + case <-timer.C: + log.Printf("[guest-agent] output still open after command exit; reporting status") + } + return nil +} + +// awaitBounded waits for done. After ctx ends it allows grace more, then returns an +// error so the handler returns, ending the RPC and releasing any send still blocked on it. +func awaitBounded(ctx context.Context, done <-chan struct{}, grace time.Duration) error { + select { + case <-done: + return nil + case <-ctx.Done(): + } + timer := time.NewTimer(grace) + defer timer.Stop() + select { + case <-done: + return nil + case <-timer.C: + return fmt.Errorf("command did not finish after cancellation: %w", ctx.Err()) + } +} + +// killGroupOnDone kills cmd's process group once ctx ends, unless cmd.Wait has +// returned. Wait reaps the group leader, after which its ID may belong to another +// process group, so a cancellation that arrives later kills nothing. +func killGroupOnDone(ctx context.Context, cmd *exec.Cmd, waited <-chan struct{}) { + select { + case <-ctx.Done(): + case <-waited: + return + } + select { + case <-waited: + default: + _ = killProcessGroup(cmd) + } +} + +// sendExitCode delivers the final status under the same bound as output, so a client +// that stopped reading cannot hold the handler past cancellation. +func (s *guestServer) sendExitCode(ctx context.Context, sender *execSender, exitCode int32) error { + var err error + if boundErr := s.runBounded(ctx, func() { + err = sender.send(&pb.ExecResponse{ + Response: &pb.ExecResponse_ExitCode{ExitCode: exitCode}, + }) + }); boundErr != nil { + return boundErr + } + return err +} + +// runBounded runs fn on its own goroutine and waits for it under the drain bound. +// If the bound expires, fn keeps running and must not rely on the handler still waiting. +func (s *guestServer) runBounded(ctx context.Context, fn func()) error { + done := make(chan struct{}) + go func() { + fn() + close(done) + }() + return awaitBounded(ctx, done, s.drainBound()) +} + +// exitCodeOf reports how a command ended. A command that hit its deadline exits 124 +// (GNU timeout convention) even though the group kill leaves a process state behind. +func exitCodeOf(ctx context.Context, cmd *exec.Cmd, waitErr error) int32 { + if errors.Is(ctx.Err(), context.DeadlineExceeded) { + return 124 + } + if cmd.ProcessState != nil { + return int32(cmd.ProcessState.ExitCode()) + } + if waitErr != nil { + return 124 + } + return 0 +} + +// killProcessGroup kills the command and its descendants. Both exec paths start the command +// as a group leader (Setpgid for no-TTY, Setsid for TTY), so the group id is its pid. +func killProcessGroup(cmd *exec.Cmd) error { + if err := syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL); err != nil && !errors.Is(err, syscall.ESRCH) { + return err + } + return nil +} + +type execStreamWriter struct { + sender *execSender + cancel context.CancelFunc + stderr bool +} + +func (w *execStreamWriter) Write(data []byte) (int, error) { + written := 0 + for len(data) > 0 { + n := min(len(data), 32*1024) + var response *pb.ExecResponse + if w.stderr { + response = &pb.ExecResponse{Response: &pb.ExecResponse_Stderr{Stderr: data[:n]}} + } else { + response = &pb.ExecResponse{Response: &pb.ExecResponse_Stdout{Stdout: data[:n]}} + } + if err := w.sender.send(response); err != nil { + w.cancel() + return written, err + } + written += n + data = data[n:] + } + return written, nil +} + +// executeLifecycleTTY runs the command on a PTY. +func (s *guestServer) executeLifecycleTTY(ctx context.Context, cancel context.CancelFunc, sender *execSender, start *pb.ExecStart, cmd *exec.Cmd) error { + // Set up initial window size (use defaults if not specified) + ws := &pty.Winsize{ + Rows: uint16(start.Rows), + Cols: uint16(start.Cols), + } + if ws.Rows == 0 { + ws.Rows = 24 + } + if ws.Cols == 0 { + ws.Cols = 80 + } + + // Start with PTY and initial window size + ptmx, err := pty.StartWithSize(cmd, ws) + if err != nil { + return fmt.Errorf("start pty: %w", err) + } + defer ptmx.Close() + waited := make(chan struct{}) + go killGroupOnDone(ctx, cmd, waited) + + outputDone := make(chan struct{}) + output := &execStreamWriter{sender: sender, cancel: cancel} + + // Handle stdin and resize in background + go func() { + for { + req, err := sender.stream.Recv() + if err != nil { + return + } + + if data := req.GetStdin(); data != nil { + ptmx.Write(data) + } + + // Handle window resize + if resize := req.GetResize(); resize != nil { + pty.Setsize(ptmx, &pty.Winsize{ + Rows: uint16(resize.Rows), + Cols: uint16(resize.Cols), + }) + } + } + }() + + // Stream output + go func() { + defer close(outputDone) + _, _ = io.Copy(output, ptmx) + }() + + return s.finishExec(ctx, sender, cmd, waited, outputDone) +} diff --git a/lib/system/guest_agent/exec_lifecycle_disconnect_test.go b/lib/system/guest_agent/exec_lifecycle_disconnect_test.go new file mode 100644 index 000000000..8b9afa257 --- /dev/null +++ b/lib/system/guest_agent/exec_lifecycle_disconnect_test.go @@ -0,0 +1,95 @@ +package main + +import ( + "context" + "fmt" + "os/exec" + "path/filepath" + "syscall" + "testing" + "time" + + pb "github.com/kernel/hypeman/lib/guest" + "github.com/stretchr/testify/require" +) + +// On the macOS exec path, a client disconnect cancels the command and its descendants. + +func TestCancelledExecDoesNotAttemptStart(t *testing.T) { + for _, tty := range []bool{false, true} { + t.Run(fmt.Sprintf("tty=%t", tty), func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + stream := &stalledExecStream{ctx: ctx, start: &pb.ExecRequest{Request: &pb.ExecRequest_Start{Start: &pb.ExecStart{Command: []string{filepath.Join(t.TempDir(), "nonexistent-executable")}, Tty: tty}}}} + err := (&guestServer{execLifecycle: true}).Exec(stream) + require.ErrorIs(t, err, context.Canceled, "a canceled request must be rejected before trying to start the command") + }) + } +} + +func TestGuestServiceExecCancellation(t *testing.T) { + client := testGuestClient(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + pidPath := filepath.Join(t.TempDir(), "pid") + stream, err := client.Exec(ctx) + require.NoError(t, err) + require.NoError(t, stream.Send(&pb.ExecRequest{Request: &pb.ExecRequest_Start{Start: &pb.ExecStart{ + Command: []string{"/bin/sh", "-c", "printf '%s' \"$$\" > \"$HYPEMAN_PID_FILE\"; exec /bin/sleep 30"}, + Env: map[string]string{"HYPEMAN_PID_FILE": pidPath}, + }}})) + require.NoError(t, stream.CloseSend()) + pid := waitForPIDFile(t, pidPath) + defer syscall.Kill(pid, syscall.SIGKILL) + cancel() + require.Eventually(t, func() bool { + return syscall.Kill(pid, 0) == syscall.ESRCH + }, 5*time.Second, 10*time.Millisecond, "disconnect must cancel the command, not leave it running") +} + +func TestGuestServiceExecCancellationKillsDescendants(t *testing.T) { + for _, tty := range []bool{false, true} { + t.Run(map[bool]string{false: "pipes", true: "tty"}[tty], func(t *testing.T) { + client := testGuestClient(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + pidPath := filepath.Join(t.TempDir(), "pid") + stream, err := client.Exec(ctx) + require.NoError(t, err) + require.NoError(t, stream.Send(&pb.ExecRequest{Request: &pb.ExecRequest_Start{Start: &pb.ExecStart{ + Command: []string{"/bin/sh", "-c", "sleep 30 & printf '%s' \"$!\" > \"$HYPEMAN_PID_FILE\"; wait"}, + Env: map[string]string{"HYPEMAN_PID_FILE": pidPath}, + Tty: tty, + }}})) + require.NoError(t, stream.CloseSend()) + pid := waitForPIDFile(t, pidPath) + defer syscall.Kill(pid, syscall.SIGKILL) + cancel() + require.Eventually(t, func() bool { + return processGone(pid) + }, 5*time.Second, 10*time.Millisecond, "cancellation must reach descendants, not only the shell") + }) + } +} + +// Once cmd.Wait has reaped the group leader, its group ID may belong to another +// process group, so a cancellation that arrives afterwards must not signal it. +func TestCancellationAfterWaitDoesNotKillGroup(t *testing.T) { + group := exec.Command("sleep", "30") + group.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + require.NoError(t, group.Start()) + t.Cleanup(func() { _ = group.Process.Kill() }) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + waited := make(chan struct{}) + close(waited) + killGroupOnDone(ctx, group, waited) + require.NoError(t, syscall.Kill(group.Process.Pid, 0), "the group was signalled after Wait returned") + + killGroupOnDone(ctx, group, make(chan struct{})) + err := group.Wait() + var exitErr *exec.ExitError + require.ErrorAs(t, err, &exitErr) + require.Equal(t, syscall.SIGKILL, exitErr.Sys().(syscall.WaitStatus).Signal(), "cancellation before Wait kills the group") +} diff --git a/lib/system/guest_agent/exec_send_cancel_test.go b/lib/system/guest_agent/exec_send_cancel_test.go new file mode 100644 index 000000000..76a01087b --- /dev/null +++ b/lib/system/guest_agent/exec_send_cancel_test.go @@ -0,0 +1,48 @@ +package main + +import ( + "context" + "errors" + "os" + "path/filepath" + "strconv" + "strings" + "syscall" + "testing" + "time" + + pb "github.com/kernel/hypeman/lib/guest" + "github.com/stretchr/testify/require" +) + +type failedOutputExecStream struct { + stalledExecStream + failure error +} + +func (s *failedOutputExecStream) Send(*pb.ExecResponse) error { return s.failure } + +func TestExecSendFailureCancelsProcessInBothModes(t *testing.T) { + for _, tty := range []bool{false, true} { + name := "stream" + if tty { + name = "tty" + } + t.Run(name, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + release := make(chan struct{}) + defer close(release) + pidFile := filepath.Join(t.TempDir(), "pid") + failure := errors.New("synthetic send failure") + stream := &failedOutputExecStream{stalledExecStream: stalledExecStream{ctx: ctx, release: release, start: &pb.ExecRequest{Request: &pb.ExecRequest_Start{Start: &pb.ExecStart{Tty: tty, Command: []string{"/bin/sh", "-c", `echo $$ > "$1"; printf ready; exec /bin/sleep 30`, "sh", pidFile}}}}}, failure: failure} + require.Error(t, (&guestServer{execLifecycle: true}).Exec(stream)) + require.NoError(t, ctx.Err(), "send failure must cancel without waiting for the caller deadline") + data, err := os.ReadFile(pidFile) + require.NoError(t, err) + pid, err := strconv.Atoi(strings.TrimSpace(string(data))) + require.NoError(t, err) + require.Eventually(t, func() bool { return errors.Is(syscall.Kill(pid, 0), syscall.ESRCH) }, time.Second, 10*time.Millisecond, "failed output must not leave the command alive") + }) + } +} diff --git a/lib/system/guest_agent/exec_sender_test.go b/lib/system/guest_agent/exec_sender_test.go new file mode 100644 index 000000000..32db63463 --- /dev/null +++ b/lib/system/guest_agent/exec_sender_test.go @@ -0,0 +1,59 @@ +package main + +import ( + "sync" + "sync/atomic" + "testing" + "time" + + pb "github.com/kernel/hypeman/lib/guest" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" +) + +// overlapDetectingStream fails the test if two goroutines are inside Send at once. +type overlapDetectingStream struct { + grpc.ServerStream + active atomic.Int32 + overlap atomic.Bool + sent atomic.Int32 +} + +func (s *overlapDetectingStream) Send(*pb.ExecResponse) error { + if s.active.Add(1) > 1 { + s.overlap.Store(true) + } + time.Sleep(time.Millisecond) + s.active.Add(-1) + s.sent.Add(1) + return nil +} + +func (s *overlapDetectingStream) Recv() (*pb.ExecRequest, error) { select {} } + +func TestExecSenderSerializesAndStopsAfterClose(t *testing.T) { + stream := &overlapDetectingStream{} + sender := &execSender{stream: stream} + output := &execStreamWriter{sender: sender, cancel: func() {}} + + var wg sync.WaitGroup + for range 8 { + wg.Add(2) + go func() { + defer wg.Done() + _, _ = output.Write([]byte("out")) + }() + go func() { + defer wg.Done() + _ = sender.send(&pb.ExecResponse{Response: &pb.ExecResponse_ExitCode{ExitCode: 0}}) + }() + } + wg.Wait() + require.False(t, stream.overlap.Load(), "output and exit status must never send concurrently") + require.EqualValues(t, 16, stream.sent.Load()) + + sender.close() + _, err := output.Write([]byte("late")) + require.ErrorIs(t, err, errExecStreamClosed) + require.EqualValues(t, 16, stream.sent.Load(), "nothing is sent after the handler returns") +} diff --git a/lib/system/guest_agent/main.go b/lib/system/guest_agent/main.go index cc8f5d049..7d2633538 100644 --- a/lib/system/guest_agent/main.go +++ b/lib/system/guest_agent/main.go @@ -3,35 +3,40 @@ package main import ( "fmt" "log" + "net" "os" "path/filepath" + "runtime" "strconv" "time" pb "github.com/kernel/hypeman/lib/guest" - "github.com/mdlayher/vsock" "google.golang.org/grpc" ) const ( - readySentinelPrefix = "HYPEMAN-AGENT-READY" - defaultReadyFilePath = "/run/hypeman/guest-agent-ready" - readyFDEnv = "HYPEMAN_AGENT_READY_FD" + readySentinelPrefix = "HYPEMAN-AGENT-READY" + readyFDEnv = "HYPEMAN_AGENT_READY_FD" ) // guestServer implements the gRPC GuestService type guestServer struct { pb.UnimplementedGuestServiceServer gpuReporter *gpuInitReporter + // execLifecycle selects the macOS exec path: client disconnect cancels the command, + // cancellation kills its process group, and output drains are bounded. + execLifecycle bool + // drainGrace bounds how long a cancelled exec may take to finish; zero means defaultDrainGrace. + drainGrace time.Duration } func main() { // Listen on vsock port 2222 with retries - var l *vsock.Listener + var l net.Listener var err error for i := 0; i < 10; i++ { - l, err = vsock.Listen(2222, nil) + l, err = listenVsock(2222) if err == nil { break } @@ -64,7 +69,7 @@ func main() { // Create gRPC server grpcServer := grpc.NewServer() - pb.RegisterGuestServiceServer(grpcServer, &guestServer{gpuReporter: reporter}) + pb.RegisterGuestServiceServer(grpcServer, &guestServer{gpuReporter: reporter, execLifecycle: runtime.GOOS == "darwin"}) // Serve gRPC over vsock if err := grpcServer.Serve(l); err != nil { diff --git a/lib/system/guest_agent/network.go b/lib/system/guest_agent/network.go index 66b71bad8..3b01f5e53 100644 --- a/lib/system/guest_agent/network.go +++ b/lib/system/guest_agent/network.go @@ -1,3 +1,5 @@ +//go:build linux + package main import ( diff --git a/lib/system/guest_agent/network_darwin.go b/lib/system/guest_agent/network_darwin.go new file mode 100644 index 000000000..38a2370ce --- /dev/null +++ b/lib/system/guest_agent/network_darwin.go @@ -0,0 +1,15 @@ +//go:build darwin + +package main + +import ( + "context" + + pb "github.com/kernel/hypeman/lib/guest" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +func (s *guestServer) ReconfigureNetwork(context.Context, *pb.ReconfigureNetworkRequest) (*pb.ReconfigureNetworkResponse, error) { + return nil, status.Error(codes.Unimplemented, "Darwin network identity reconfiguration is not supported; VZ NAT uses guest DHCP") +} diff --git a/lib/system/guest_agent/ready_darwin.go b/lib/system/guest_agent/ready_darwin.go new file mode 100644 index 000000000..5504a5ae8 --- /dev/null +++ b/lib/system/guest_agent/ready_darwin.go @@ -0,0 +1,7 @@ +//go:build darwin + +package main + +// defaultReadyFilePath is where the guest agent records readiness when +// HYPEMAN_AGENT_READY_FILE is unset. +const defaultReadyFilePath = "/var/run/hypeman/guest-agent-ready" diff --git a/lib/system/guest_agent/ready_linux.go b/lib/system/guest_agent/ready_linux.go new file mode 100644 index 000000000..724c7c0fc --- /dev/null +++ b/lib/system/guest_agent/ready_linux.go @@ -0,0 +1,7 @@ +//go:build linux + +package main + +// defaultReadyFilePath is where the guest agent records readiness when +// HYPEMAN_AGENT_READY_FILE is unset. +const defaultReadyFilePath = "/run/hypeman/guest-agent-ready" diff --git a/lib/system/guest_agent/service_test.go b/lib/system/guest_agent/service_test.go new file mode 100644 index 000000000..6405ce83a --- /dev/null +++ b/lib/system/guest_agent/service_test.go @@ -0,0 +1,295 @@ +package main + +import ( + "context" + "errors" + "io" + "net" + "os" + "os/exec" + "path/filepath" + "strconv" + "strings" + "syscall" + "testing" + "time" + + pb "github.com/kernel/hypeman/lib/guest" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/test/bufconn" +) + +// Exercise the shared RPC contract without starting a VM or listening on vsock. +func testGuestClient(t *testing.T) pb.GuestServiceClient { + t.Helper() + listener := bufconn.Listen(64 << 10) + server := grpc.NewServer() + pb.RegisterGuestServiceServer(server, &guestServer{execLifecycle: true}) + go server.Serve(listener) + t.Cleanup(server.Stop) + t.Cleanup(func() { listener.Close() }) + conn, err := grpc.NewClient("passthrough:///guest-test", grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithContextDialer(func(context.Context, string) (net.Conn, error) { + return listener.Dial() + })) + require.NoError(t, err) + t.Cleanup(func() { conn.Close() }) + return pb.NewGuestServiceClient(conn) +} + +func TestGuestServiceExecRoundTrip(t *testing.T) { + client := testGuestClient(t) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + stream, err := client.Exec(ctx) + require.NoError(t, err) + require.NoError(t, stream.Send(&pb.ExecRequest{Request: &pb.ExecRequest_Start{Start: &pb.ExecStart{ + Command: []string{"/bin/sh", "-c", "printf '%s' \"$HYPEMAN_EXEC_TEST\"; printf 'stderr' >&2; exit 7"}, + Env: map[string]string{"HYPEMAN_EXEC_TEST": "stdout"}, + }}})) + require.NoError(t, stream.CloseSend()) + result := collectExec(t, stream) + require.Equal(t, "stdout", result.stdout) + require.Equal(t, "stderr", result.stderr) + require.NotNil(t, result.exit) + require.Equal(t, int32(7), *result.exit) +} + +func TestGuestServiceExecStreamsBeforeExit(t *testing.T) { + client := testGuestClient(t) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + stream, err := client.Exec(ctx) + require.NoError(t, err) + require.NoError(t, stream.Send(&pb.ExecRequest{Request: &pb.ExecRequest_Start{Start: &pb.ExecStart{ + Command: []string{"/bin/sh", "-c", "printf ready; read input; printf '%s' \"$input\""}, + }}})) + first, err := stream.Recv() + require.NoError(t, err, "output must arrive while the command is still waiting for stdin") + require.Equal(t, "ready", string(first.GetStdout())) + require.NoError(t, stream.Send(&pb.ExecRequest{Request: &pb.ExecRequest_Stdin{Stdin: []byte("finish\n")}})) + require.NoError(t, stream.CloseSend()) + result := collectExec(t, stream) + require.NotNil(t, result.exit) + require.Equal(t, int32(0), *result.exit) + require.Equal(t, "finish", result.stdout) +} + +// processGone reports whether pid no longer runs. A zombie counts as gone. +func processGone(pid int) bool { + if errors.Is(syscall.Kill(pid, 0), syscall.ESRCH) { + return true + } + out, err := exec.Command("ps", "-o", "stat=", "-p", strconv.Itoa(pid)).Output() + var exitErr *exec.ExitError + if errors.As(err, &exitErr) { + return true // ps exits non-zero when no such process exists. + } + // Any other failure (for example, no ps) must not count as proof the process is gone. + return err == nil && strings.HasPrefix(strings.TrimSpace(string(out)), "Z") +} + +// stalledExecStream models a client that stopped reading: Send blocks until release closes. +// With stallExitOnly, only the final exit-code send blocks. +type stalledExecStream struct { + grpc.ServerStream + ctx context.Context + start *pb.ExecRequest + release chan struct{} + stallExitOnly bool +} + +func (s *stalledExecStream) Context() context.Context { return s.ctx } +func (s *stalledExecStream) Recv() (*pb.ExecRequest, error) { + if s.start != nil { + req := s.start + s.start = nil + return req, nil + } + <-s.ctx.Done() + return nil, s.ctx.Err() +} +func (s *stalledExecStream) Send(resp *pb.ExecResponse) error { + if _, exit := resp.Response.(*pb.ExecResponse_ExitCode); s.stallExitOnly && !exit { + return nil + } + <-s.release + return nil +} + +// runStalledExec runs a timed-out command against a client that stopped reading and +// requires the handler to return the bounded-drain error instead of hanging. +func runStalledExec(t *testing.T, command string, exitOnly bool) { + t.Helper() + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + release := make(chan struct{}) + t.Cleanup(func() { close(release) }) + stream := &stalledExecStream{ctx: ctx, release: release, stallExitOnly: exitOnly, start: &pb.ExecRequest{Request: &pb.ExecRequest_Start{Start: &pb.ExecStart{ + Command: []string{"/bin/sh", "-c", command}, + TimeoutSeconds: 1, + }}}} + done := make(chan error, 1) + go func() { done <- (&guestServer{execLifecycle: true, drainGrace: 100 * time.Millisecond}).Exec(stream) }() + select { + case err := <-done: + require.ErrorContains(t, err, "did not finish") + case <-time.After(10 * time.Second): + t.Fatal("exec stayed blocked behind a stalled client after its timeout") + } +} + +func TestGuestServiceExecTimeoutFinishesWhenClientStopsReading(t *testing.T) { + runStalledExec(t, "yes", false) +} + +func TestGuestServiceExecTimeoutBoundsExitCodeSend(t *testing.T) { + runStalledExec(t, "echo started; sleep 30", true) +} + +func TestGuestServiceExecTimeoutKillsDescendantAfterShellExits(t *testing.T) { + for _, tty := range []bool{false, true} { + t.Run(map[bool]string{false: "pipes", true: "tty"}[tty], func(t *testing.T) { + client := testGuestClient(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + pidPath := filepath.Join(t.TempDir(), "pid") + stream, err := client.Exec(ctx) + require.NoError(t, err) + require.NoError(t, stream.Send(&pb.ExecRequest{Request: &pb.ExecRequest_Start{Start: &pb.ExecStart{ + Command: []string{"/bin/sh", "-c", "sleep 30 & printf '%s' \"$!\" > \"$HYPEMAN_PID_FILE\"; exit 0"}, + Env: map[string]string{"HYPEMAN_PID_FILE": pidPath}, + Tty: tty, + TimeoutSeconds: 1, + }}})) + require.NoError(t, stream.CloseSend()) + pid := waitForPIDFile(t, pidPath) + defer syscall.Kill(pid, syscall.SIGKILL) + finished := make(chan error, 1) + go func() { + for { + if _, err := stream.Recv(); err != nil { + finished <- err + return + } + } + }() + select { + case <-finished: + case <-time.After(10 * time.Second): + t.Fatal("RPC stayed open while a descendant held the command's output") + } + require.True(t, processGone(pid), "timeout must kill the descendant even after the shell exited") + }) + } +} + +// runExec drives one exec to completion and returns its stdout and exit code. +func runExec(t *testing.T, start *pb.ExecStart) (string, int32) { + t.Helper() + client := testGuestClient(t) + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + stream, err := client.Exec(ctx) + require.NoError(t, err) + require.NoError(t, stream.Send(&pb.ExecRequest{Request: &pb.ExecRequest_Start{Start: start}})) + require.NoError(t, stream.CloseSend()) + result := collectExec(t, stream) + if result.exit == nil { + return result.stdout, -1 + } + return result.stdout, *result.exit +} + +type execResult struct { + stdout, stderr string + exit *int32 +} + +// collectExec reads an exec stream until EOF. +func collectExec(t *testing.T, stream pb.GuestService_ExecClient) execResult { + t.Helper() + var stdout, stderr strings.Builder + var result execResult + for { + response, err := stream.Recv() + if errors.Is(err, io.EOF) { + break + } + require.NoError(t, err) + stdout.Write(response.GetStdout()) + stderr.Write(response.GetStderr()) + if code, ok := response.Response.(*pb.ExecResponse_ExitCode); ok { + result.exit = &code.ExitCode + } + } + result.stdout, result.stderr = stdout.String(), stderr.String() + return result +} + +// waitForPIDFile returns the PID a test command wrote to path. +func waitForPIDFile(t *testing.T, path string) int { + t.Helper() + var pid int + require.Eventually(t, func() bool { + data, err := os.ReadFile(path) + if err != nil { + return false + } + pid, err = strconv.Atoi(string(data)) + return err == nil && pid > 0 + }, 5*time.Second, 10*time.Millisecond) + return pid +} + +func TestGuestServiceExecExitZeroWhenDescendantHoldsOutput(t *testing.T) { + stdout, exit := runExec(t, &pb.ExecStart{Command: []string{"/bin/sh", "-c", "sleep 5 & echo hi"}}) + require.Equal(t, "hi\n", stdout) + require.Equal(t, int32(0), exit, "a command that exits 0 must report 0, not a stream error") +} + +func TestGuestServiceExecTimeoutReports124(t *testing.T) { + _, exit := runExec(t, &pb.ExecStart{Command: []string{"/bin/sleep", "30"}, TimeoutSeconds: 1}) + require.Equal(t, int32(124), exit) +} + +func TestGuestServiceFileRoundTrip(t *testing.T) { + client := testGuestClient(t) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + path := filepath.Join(t.TempDir(), "copied") + payload := "shared guest service file payload" + stream, err := client.CopyToGuest(ctx) + require.NoError(t, err) + require.NoError(t, stream.Send(&pb.CopyToGuestRequest{Request: &pb.CopyToGuestRequest_Start{Start: &pb.CopyToGuestStart{Path: path, Mode: 0600, Size: int64(len(payload))}}})) + require.NoError(t, stream.Send(&pb.CopyToGuestRequest{Request: &pb.CopyToGuestRequest_Data{Data: []byte(payload)}})) + require.NoError(t, stream.Send(&pb.CopyToGuestRequest{Request: &pb.CopyToGuestRequest_End{End: &pb.CopyToGuestEnd{}}})) + result, err := stream.CloseAndRecv() + require.NoError(t, err) + require.True(t, result.Success, result.Error) + require.Equal(t, int64(len(payload)), result.BytesWritten) + info, err := client.StatPath(ctx, &pb.StatPathRequest{Path: path}) + require.NoError(t, err) + require.True(t, info.Exists && info.IsFile) + require.Equal(t, uint32(0600), info.Mode&0777) + require.Equal(t, int64(len(payload)), info.Size) + read, err := client.CopyFromGuest(ctx, &pb.CopyFromGuestRequest{Path: path}) + require.NoError(t, err) + var content strings.Builder + final := false + for { + response, err := read.Recv() + if err == io.EOF { + break + } + require.NoError(t, err) + require.Nil(t, response.GetError()) + content.Write(response.GetData()) + if response.GetEnd() != nil && response.GetEnd().Final { + final = true + } + } + require.True(t, final) + require.Equal(t, payload, content.String()) +} diff --git a/lib/system/guest_agent/shutdown.go b/lib/system/guest_agent/shutdown.go index e29690aef..5eed220cb 100644 --- a/lib/system/guest_agent/shutdown.go +++ b/lib/system/guest_agent/shutdown.go @@ -1,3 +1,5 @@ +//go:build linux + package main import ( diff --git a/lib/system/guest_agent/shutdown_darwin.go b/lib/system/guest_agent/shutdown_darwin.go new file mode 100644 index 000000000..fcca21846 --- /dev/null +++ b/lib/system/guest_agent/shutdown_darwin.go @@ -0,0 +1,41 @@ +//go:build darwin + +package main + +import ( + "context" + "log" + "os" + "os/exec" + "syscall" + + pb "github.com/kernel/hypeman/lib/guest" +) + +// Darwin's launchd is not Linux init: sending it SIGTERM is not a shutdown API. +func (s *guestServer) Shutdown(ctx context.Context, req *pb.ShutdownRequest) (*pb.ShutdownResponse, error) { + return requestDarwinShutdown(ctx, req, os.Geteuid(), func(ctx context.Context) error { + if err := ctx.Err(); err != nil { + return err + } + command := darwinShutdownCommand() + if err := command.Start(); err != nil { + return err + } + go func() { + if err := command.Wait(); err != nil { + log.Printf("scheduled Darwin shutdown failed: %v", err) + } + }() + return nil + }) +} + +func darwinShutdownCommand() *exec.Cmd { + // A short detached delay lets gRPC send the acceptance reply before macOS + // tears down vsock. The host must still independently confirm owned VMM exit. + // Once accepted, shutdown is not cancelled by the RPC connection disappearing. + command := exec.Command("/bin/sh", "-c", "/bin/sleep 1; exec /sbin/shutdown -h now") + command.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + return command +} diff --git a/lib/system/guest_agent/shutdown_policy.go b/lib/system/guest_agent/shutdown_policy.go new file mode 100644 index 000000000..710a430a0 --- /dev/null +++ b/lib/system/guest_agent/shutdown_policy.go @@ -0,0 +1,28 @@ +package main + +import ( + "context" + "syscall" + + pb "github.com/kernel/hypeman/lib/guest" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +// requestDarwinShutdown applies the Darwin shutdown policy. It takes the effective +// uid and the shutdown command as arguments, so the policy runs on every host. +func requestDarwinShutdown(ctx context.Context, req *pb.ShutdownRequest, euid int, run func(context.Context) error) (*pb.ShutdownResponse, error) { + if req.Signal != 0 && req.Signal != int32(syscall.SIGTERM) { + return nil, status.Error(codes.InvalidArgument, "Darwin supports only an orderly shutdown, not arbitrary init signals") + } + if euid != 0 { + return nil, status.Error(codes.PermissionDenied, "Darwin shutdown requires the system guest agent running as root") + } + if err := ctx.Err(); err != nil { + return nil, status.FromContextError(err).Err() + } + if err := run(ctx); err != nil { + return nil, status.Errorf(codes.Internal, "Darwin shutdown failed: %v", err) + } + return &pb.ShutdownResponse{}, nil +} diff --git a/lib/system/guest_agent/shutdown_policy_test.go b/lib/system/guest_agent/shutdown_policy_test.go new file mode 100644 index 000000000..c2c5873cd --- /dev/null +++ b/lib/system/guest_agent/shutdown_policy_test.go @@ -0,0 +1,52 @@ +package main + +import ( + "context" + "errors" + "syscall" + "testing" + + pb "github.com/kernel/hypeman/lib/guest" + "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +func TestDarwinShutdownPolicy(t *testing.T) { + for _, tc := range []struct { + name string + signal int32 + euid int + canceled bool + commandError error + code codes.Code + called bool + }{ + {name: "default orderly shutdown", called: true}, + {name: "explicit orderly shutdown", signal: int32(syscall.SIGTERM), called: true}, + {name: "reject arbitrary signal", signal: int32(syscall.SIGKILL), code: codes.InvalidArgument}, + {name: "reject desktop agent", euid: 501, code: codes.PermissionDenied}, + {name: "canceled before command", canceled: true, code: codes.Canceled}, + {name: "command failure", commandError: errors.New("shutdown refused"), code: codes.Internal, called: true}, + } { + t.Run(tc.name, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + if tc.canceled { + cancel() + } + called := false + response, err := requestDarwinShutdown(ctx, &pb.ShutdownRequest{Signal: tc.signal}, tc.euid, func(context.Context) error { + called = true + return tc.commandError + }) + require.Equal(t, tc.called, called) + require.Equal(t, tc.code, status.Code(err)) + if tc.code == codes.OK { + require.NotNil(t, response) + } else { + require.Nil(t, response) + } + }) + } +} diff --git a/lib/system/guest_agent/shutdown_schedule_darwin_test.go b/lib/system/guest_agent/shutdown_schedule_darwin_test.go new file mode 100644 index 000000000..017c5d352 --- /dev/null +++ b/lib/system/guest_agent/shutdown_schedule_darwin_test.go @@ -0,0 +1,19 @@ +//go:build darwin + +package main + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestDarwinShutdownDefersPowerOffUntilReply(t *testing.T) { + // Inspect only: never execute a host shutdown command in a test. + command := darwinShutdownCommand() + require.Equal(t, "/bin/sh", command.Path) + require.Equal(t, []string{"/bin/sh", "-c", "/bin/sleep 1; exec /sbin/shutdown -h now"}, command.Args) + require.NotNil(t, command.SysProcAttr) + require.True(t, command.SysProcAttr.Setpgid) + require.Nil(t, command.Cancel, "accepted shutdown must survive RPC cancellation") +} diff --git a/lib/system/guest_agent/vsock_darwin.go b/lib/system/guest_agent/vsock_darwin.go new file mode 100644 index 000000000..3d73642cb --- /dev/null +++ b/lib/system/guest_agent/vsock_darwin.go @@ -0,0 +1,125 @@ +//go:build darwin && cgo + +package main + +/* +#include +#include +#include +#include +#include +static int listen_vm(unsigned port) { + int fd=socket(AF_VSOCK,SOCK_STREAM,0);if(fd<0)return -1; + struct sockaddr_vm a={0};a.svm_len=sizeof(a);a.svm_family=AF_VSOCK;a.svm_cid=VMADDR_CID_ANY;a.svm_port=port; + if(bind(fd,(struct sockaddr*)&a,sizeof(a))<0 || listen(fd,16)<0 || fcntl(fd,F_SETFL,O_NONBLOCK)<0){int e=errno;close(fd);errno=e;return -1;} + if(fcntl(fd,F_SETFD,FD_CLOEXEC)<0){int e=errno;close(fd);errno=e;return -1;}return fd; +} +static int accept_vm(int fd) { + struct sockaddr_vm a={0};socklen_t n=sizeof(a);int c=accept(fd,(struct sockaddr*)&a,&n);if(c<0)return -1; + if(a.svm_cid!=VMADDR_CID_HOST){close(c);errno=EAGAIN;return -1;} + if(fcntl(c,F_SETFL,O_NONBLOCK)<0){int e=errno;close(c);errno=e;return -1;} + if(fcntl(c,F_SETFD,FD_CLOEXEC)<0){int e=errno;close(c);errno=e;return -1;}return c; +} +*/ +import "C" +import ( + "errors" + "fmt" + "io" + "net" + "os" + "sync" + "syscall" + "time" +) + +type vmAddr string + +func (a vmAddr) Network() string { return "vsock" } +func (a vmAddr) String() string { return string(a) } + +// Do not promote os.File's zero-copy methods: AF_VSOCK is not a TCP/file +// descriptor and must use ordinary reads/writes for gRPC transport. +type vmConn struct { + file *os.File + local net.Addr +} + +// Read returns io.EOF unwrapped, as net.Conn implementations do: gRPC and HTTP/2 +// compare against it to recognize an orderly close. +func (c *vmConn) Read(p []byte) (int, error) { + n, err := c.file.Read(p) + if err != nil && err != io.EOF { + return n, &net.OpError{Op: "read", Net: "vsock", Err: err} + } + return n, err +} +func (c *vmConn) Write(p []byte) (int, error) { + n, err := c.file.Write(p) + if err != nil { + return n, &net.OpError{Op: "write", Net: "vsock", Err: err} + } + return n, err +} +func (c *vmConn) Close() error { return c.file.Close() } +func (c *vmConn) SetDeadline(t time.Time) error { return c.file.SetDeadline(t) } +func (c *vmConn) SetReadDeadline(t time.Time) error { return c.file.SetReadDeadline(t) } +func (c *vmConn) SetWriteDeadline(t time.Time) error { return c.file.SetWriteDeadline(t) } + +func (c *vmConn) LocalAddr() net.Addr { return c.local } +func (c *vmConn) RemoteAddr() net.Addr { return vmAddr("host:2") } + +type vmListener struct { + mu sync.Mutex + fd C.int + port uint32 + closed bool +} + +func listenVsock(port uint32) (net.Listener, error) { + fd, err := C.listen_vm(C.uint(port)) + if fd < 0 { + return nil, fmt.Errorf("listen vsock: %w", err) + } + return &vmListener{fd: fd, port: port}, nil +} +func (l *vmListener) Addr() net.Addr { return vmAddr(fmt.Sprintf("guest:%d", l.port)) } +func (l *vmListener) Close() error { + l.mu.Lock() + defer l.mu.Unlock() + if l.closed { + return net.ErrClosed + } + l.closed = true + C.close(l.fd) + return nil +} +func (l *vmListener) Accept() (net.Conn, error) { + for { + l.mu.Lock() + if l.closed { + l.mu.Unlock() + return nil, net.ErrClosed + } + // Darwin has no accept4: the accepted fd is inheritable until accept_vm sets FD_CLOEXEC. + // Holding the fork lock across that window keeps a concurrent fork from copying it. + syscall.ForkLock.RLock() + fd, err := C.accept_vm(l.fd) + syscall.ForkLock.RUnlock() + l.mu.Unlock() + if fd >= 0 { + return &vmConn{file: os.NewFile(uintptr(fd), "guest-vsock"), local: l.Addr()}, nil + } + if !errors.Is(err, syscall.EAGAIN) && !errors.Is(err, syscall.EINTR) { + return nil, acceptError(err) + } + time.Sleep(10 * time.Millisecond) + } +} + +// acceptError keeps the net.OpError Temporary contract. grpc-go retries only a +// direct Temporary() assertion, so wrapping with fmt.Errorf would stop the server +// on transient EMFILE/ENFILE/ECONNABORTED. +func acceptError(err error) error { + return &net.OpError{Op: "accept", Net: "vsock", Err: err} +} diff --git a/lib/system/guest_agent/vsock_darwin_nocgo.go b/lib/system/guest_agent/vsock_darwin_nocgo.go new file mode 100644 index 000000000..7fae1f3b7 --- /dev/null +++ b/lib/system/guest_agent/vsock_darwin_nocgo.go @@ -0,0 +1,12 @@ +//go:build darwin && !cgo + +package main + +import ( + "fmt" + "net" +) + +func listenVsock(uint32) (net.Listener, error) { + return nil, fmt.Errorf("Darwin guest vsock requires a cgo-enabled build") +} diff --git a/lib/system/guest_agent/vsock_darwin_nocgo_test.go b/lib/system/guest_agent/vsock_darwin_nocgo_test.go new file mode 100644 index 000000000..630f8b5d8 --- /dev/null +++ b/lib/system/guest_agent/vsock_darwin_nocgo_test.go @@ -0,0 +1,15 @@ +//go:build darwin && !cgo + +package main + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestDarwinVsockRequiresCGO(t *testing.T) { + listener, err := listenVsock(2222) + require.Nil(t, listener) + require.ErrorContains(t, err, "cgo-enabled build") +} diff --git a/lib/system/guest_agent/vsock_darwin_test.go b/lib/system/guest_agent/vsock_darwin_test.go new file mode 100644 index 000000000..e6a66fad2 --- /dev/null +++ b/lib/system/guest_agent/vsock_darwin_test.go @@ -0,0 +1,62 @@ +//go:build darwin && cgo + +package main + +import ( + "errors" + "io" + "net" + "os" + "syscall" + "testing" + "time" + + "golang.org/x/sys/unix" +) + +func TestVsockReadTimeoutImplementsNetError(t *testing.T) { + fds, err := unix.Socketpair(unix.AF_UNIX, unix.SOCK_STREAM, 0) + if err != nil { + t.Fatal(err) + } + defer unix.Close(fds[1]) + if err = unix.SetNonblock(fds[0], true); err != nil { + unix.Close(fds[0]) + t.Fatal(err) + } + conn := &vmConn{file: os.NewFile(uintptr(fds[0]), "test-vsock"), local: vmAddr("guest:2222")} + defer conn.Close() + if err = conn.SetReadDeadline(time.Now().Add(-time.Second)); err != nil { + t.Fatal(err) + } + _, err = conn.Read(make([]byte, 1)) + var ne net.Error + if !errors.As(err, &ne) || !ne.Timeout() { + t.Fatalf("transport timeout must implement net.Error: %v", err) + } +} + +func TestVsockReadReturnsBareEOF(t *testing.T) { + fds, err := unix.Socketpair(unix.AF_UNIX, unix.SOCK_STREAM, 0) + if err != nil { + t.Fatal(err) + } + conn := &vmConn{file: os.NewFile(uintptr(fds[0]), "test-vsock"), local: vmAddr("guest:2222")} + defer conn.Close() + unix.Close(fds[1]) + if _, err = conn.Read(make([]byte, 1)); err != io.EOF { + t.Fatalf("an orderly close must read as io.EOF, got %#v", err) + } +} + +func TestVsockAcceptErrorsRemainGRPCTemporary(t *testing.T) { + for _, errno := range []syscall.Errno{syscall.EMFILE, syscall.ENFILE, syscall.ECONNABORTED} { + err := acceptError(errno) + if ne, ok := err.(interface{ Temporary() bool }); !ok || !ne.Temporary() { + t.Fatalf("%v: grpc direct Temporary assertion failed for %T", errno, err) + } + if !errors.Is(err, errno) { + t.Fatalf("%v: lost errno", errno) + } + } +} diff --git a/lib/system/guest_agent/vsock_linux.go b/lib/system/guest_agent/vsock_linux.go new file mode 100644 index 000000000..3de52ddc5 --- /dev/null +++ b/lib/system/guest_agent/vsock_linux.go @@ -0,0 +1,13 @@ +//go:build linux + +package main + +import ( + "net" + + "github.com/mdlayher/vsock" +) + +func listenVsock(port uint32) (net.Listener, error) { + return vsock.Listen(port, nil) +} From ebfa5831d06ab5a291dd2c17e862d556ba65f0a5 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Tue, 20 Oct 2026 20:10:53 +0000 Subject: [PATCH 02/11] Give the macOS guest display a 2x size The guest display was fixed at 1280x800 at 80 ppi. Use 3024x1964 at 160 ppi, which is 1512x982 points at 2x. Changes the display only on the macOS guest path; the Linux path is untouched. --- cmd/vz-shim/macos_arm64.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/vz-shim/macos_arm64.go b/cmd/vz-shim/macos_arm64.go index a8b5c761c..62962e082 100644 --- a/cmd/vz-shim/macos_arm64.go +++ b/cmd/vz-shim/macos_arm64.go @@ -49,7 +49,8 @@ func configureMacPlatform(vc *vz.VirtualMachineConfiguration, c *shimconfig.Shim if e != nil { return e } - display, e := vz.NewMacGraphicsDisplayConfiguration(1280, 800, 80) + // 3024x1964 pixels at 2x: 1512x982 points, the logical size of a 14-inch MacBook Pro. + display, e := vz.NewMacGraphicsDisplayConfiguration(3024, 1964, 160) if e != nil { return e } From 7989497f94beb98a9bdf37dfeef7c30ed7c66d5c Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Wed, 21 Oct 2026 14:51:33 +0000 Subject: [PATCH 03/11] Keep the exec timeout killing its group after the leader exits The group watcher stopped when cmd.Wait returned. On a TTY, Wait returns as soon as the shell exits, so a timeout during the output drain never killed a background child that held the terminal open. The watcher now runs until the exec lifecycle finishes. The exit status was bounded only after cancellation. A client that stopped reading could hold the handler with no cancellation at all. The status send now has its own deadline, counted from the send. A timed-out exec whose status cannot be sent still reports the cancellation error. Tests: a TTY timeout during the drain kills the background child (failed before this change), and a stalled client cannot block the exit status. --- lib/system/guest_agent/exec_lifecycle.go | 67 +++++++++------- .../exec_lifecycle_remediation_test.go | 79 +++++++++++++++++++ 2 files changed, 119 insertions(+), 27 deletions(-) create mode 100644 lib/system/guest_agent/exec_lifecycle_remediation_test.go diff --git a/lib/system/guest_agent/exec_lifecycle.go b/lib/system/guest_agent/exec_lifecycle.go index 0e5688c3f..0abb1c3ec 100644 --- a/lib/system/guest_agent/exec_lifecycle.go +++ b/lib/system/guest_agent/exec_lifecycle.go @@ -84,8 +84,9 @@ func (s *guestServer) executeLifecycleNoTTY(ctx context.Context, cancel context. } // Cancel is not called once the direct child has exited, so a descendant that // still holds the pipes would outlive the timeout. Kill the group from the context. - waited := make(chan struct{}) - go killGroupOnDone(ctx, cmd, waited) + lifecycleDone := make(chan struct{}) + defer close(lifecycleDone) + go killGroupOnDone(ctx, cmd, lifecycleDone) // Handle stdin in background go func() { @@ -101,7 +102,7 @@ func (s *guestServer) executeLifecycleNoTTY(ctx context.Context, cancel context. } }() - return s.finishExec(ctx, sender, cmd, waited, nil) + return s.finishExec(ctx, sender, cmd, nil) } // execCommand owns setup and pre-start cancellation for both I/O modes. @@ -121,13 +122,11 @@ func (s *guestServer) execCommand(ctx context.Context, start *pb.ExecStart) (*ex } // finishExec owns bounded wait, output drain and final status for both I/O modes. -// It closes waited once cmd.Wait returns. -func (s *guestServer) finishExec(ctx context.Context, sender *execSender, cmd *exec.Cmd, waited chan<- struct{}, outputDone <-chan struct{}) error { +func (s *guestServer) finishExec(ctx context.Context, sender *execSender, cmd *exec.Cmd, outputDone <-chan struct{}) error { // Non-TTY cmd.Wait also waits for copies, which can block in stream.Send. var waitErr error if err := s.runBounded(ctx, func() { waitErr = cmd.Wait() - close(waited) }); err != nil { return err } @@ -198,34 +197,47 @@ func awaitBounded(ctx context.Context, done <-chan struct{}, grace time.Duration } } -// killGroupOnDone kills cmd's process group once ctx ends, unless cmd.Wait has -// returned. Wait reaps the group leader, after which its ID may belong to another -// process group, so a cancellation that arrives later kills nothing. -func killGroupOnDone(ctx context.Context, cmd *exec.Cmd, waited <-chan struct{}) { +// killGroupOnDone kills cmd's process group if ctx ends before the exec lifecycle +// finishes (done closes). It watches the lifecycle, not cmd.Wait, so a timeout that +// fires during the output drain can still kill background children that hold the +// terminal open. Wait reaps the leader first, so the group id is used only while +// members remain. A later kill can reach a group whose id a new leader has reused, +// which is a residual risk. +func killGroupOnDone(ctx context.Context, cmd *exec.Cmd, done <-chan struct{}) { select { - case <-ctx.Done(): - case <-waited: + case <-done: return + case <-ctx.Done(): } select { - case <-waited: + case <-done: + // The command finished before the cancellation. Background children are left + // running on a normal exit, as the exec contract says. + return default: - _ = killProcessGroup(cmd) } + _ = killProcessGroup(cmd) } -// sendExitCode delivers the final status under the same bound as output, so a client -// that stopped reading cannot hold the handler past cancellation. +// sendExitCode delivers the final status within the drain bound, counted from the +// send itself, so a client that stopped reading cannot hold the handler even when +// nothing is cancelled. func (s *guestServer) sendExitCode(ctx context.Context, sender *execSender, exitCode int32) error { - var err error - if boundErr := s.runBounded(ctx, func() { - err = sender.send(&pb.ExecResponse{ - Response: &pb.ExecResponse_ExitCode{ExitCode: exitCode}, - }) - }); boundErr != nil { - return boundErr + sent := make(chan error, 1) + go func() { + sent <- sender.send(&pb.ExecResponse{Response: &pb.ExecResponse_ExitCode{ExitCode: exitCode}}) + }() + timer := time.NewTimer(s.drainBound()) + defer timer.Stop() + select { + case err := <-sent: + return err + case <-timer.C: + if ctx.Err() != nil { + return fmt.Errorf("command did not finish after cancellation: %w", ctx.Err()) + } + return fmt.Errorf("exit status not delivered within %s: client stopped reading", s.drainBound()) } - return err } // runBounded runs fn on its own goroutine and waits for it under the drain bound. @@ -309,8 +321,9 @@ func (s *guestServer) executeLifecycleTTY(ctx context.Context, cancel context.Ca return fmt.Errorf("start pty: %w", err) } defer ptmx.Close() - waited := make(chan struct{}) - go killGroupOnDone(ctx, cmd, waited) + lifecycleDone := make(chan struct{}) + defer close(lifecycleDone) + go killGroupOnDone(ctx, cmd, lifecycleDone) outputDone := make(chan struct{}) output := &execStreamWriter{sender: sender, cancel: cancel} @@ -343,5 +356,5 @@ func (s *guestServer) executeLifecycleTTY(ctx context.Context, cancel context.Ca _, _ = io.Copy(output, ptmx) }() - return s.finishExec(ctx, sender, cmd, waited, outputDone) + return s.finishExec(ctx, sender, cmd, outputDone) } diff --git a/lib/system/guest_agent/exec_lifecycle_remediation_test.go b/lib/system/guest_agent/exec_lifecycle_remediation_test.go new file mode 100644 index 000000000..11a575362 --- /dev/null +++ b/lib/system/guest_agent/exec_lifecycle_remediation_test.go @@ -0,0 +1,79 @@ +//go:build linux + +package main + +import ( + "context" + "os" + "path/filepath" + "strconv" + "strings" + "syscall" + "testing" + "time" + + pb "github.com/kernel/hypeman/lib/guest" + "github.com/stretchr/testify/require" +) + +// execProcessGone reports whether pid has exited or is a zombie. A zombie is not +// running, but kill(pid, 0) still succeeds for it, so /proc is the check. +func execProcessGone(pid int) bool { + data, err := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/stat") + if err != nil { + return true + } + // The state is the field after the parenthesised command name. + fields := strings.Fields(string(data[strings.LastIndexByte(string(data), ')')+1:])) + return len(fields) > 0 && fields[0] == "Z" +} + +// On a TTY, a timeout that fires while the exec drains output after the shell exited +// must still kill the background child that holds the terminal open. +func TestExecTimeoutDuringDrainKillsBackgroundChild(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + release := make(chan struct{}) + defer close(release) + pidFile := filepath.Join(t.TempDir(), "child") + stream := &exitCapturingExecStream{stalledExecStream: stalledExecStream{ctx: ctx, release: release, start: &pb.ExecRequest{Request: &pb.ExecRequest_Start{Start: &pb.ExecStart{ + Tty: true, + TimeoutSeconds: 3, + Command: []string{"/bin/sh", "-c", `trap '' HUP; sleep 60 & echo $! > "$1"; exit 0`, "sh", pidFile}, + }}}}} + + started := time.Now() + require.NoError(t, (&guestServer{execLifecycle: true, drainGrace: 5 * time.Second}).Exec(stream)) + require.Less(t, time.Since(started), 7*time.Second, "the timeout must end the exec, not the drain grace") + require.NotNil(t, stream.exitCode) + require.EqualValues(t, 124, *stream.exitCode, "a timed-out exec reports 124") + + data, err := os.ReadFile(pidFile) + require.NoError(t, err, "background child pid was not written") + pid, err := strconv.Atoi(strings.TrimSpace(string(data))) + require.NoError(t, err) + t.Cleanup(func() { _ = syscall.Kill(pid, syscall.SIGKILL) }) + require.Eventually(t, func() bool { return execProcessGone(pid) }, 2*time.Second, 50*time.Millisecond, + "background child %d survived the timeout", pid) +} + +// The final status is delivered within the drain bound even when no cancellation +// ever happens and the client never reads. +func TestSendExitCodeBoundedWhenClientStopsReading(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + release := make(chan struct{}) + defer close(release) + stream := &stalledExecStream{ctx: ctx, release: release} + sender := &execSender{stream: stream} + + server := &guestServer{drainGrace: 200 * time.Millisecond} + done := make(chan error, 1) + go func() { done <- server.sendExitCode(ctx, sender, 0) }() + select { + case err := <-done: + require.Error(t, err, "a stalled client must fail the status send, not succeed") + case <-time.After(3 * time.Second): + t.Fatal("sendExitCode blocked on a client that stopped reading") + } +} From b8a74ce9f9b8ebaa9344f69847d684cea755132c Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Wed, 21 Oct 2026 14:51:33 +0000 Subject: [PATCH 04/11] Use one agent predicate and keep legacy macOS boot disks readable Readiness probes, the readiness state and shutdown checked SkipGuestAgent alone. They now use GuestAgentEnabled, which also requires the image to declare the agent. For Linux guests the two are equal. macOS instances created before the boot.raw name keep their disk at overlay.raw. Start and VM-process lookup use overlay.raw when boot.raw is absent. New instances still use boot.raw. The guest-operation helper's comment no longer claims to be the only rule. --- lib/instances/guest_os.go | 5 +-- lib/instances/macos.go | 24 +++++++++++-- lib/instances/macos_boot_disk_test.go | 52 +++++++++++++++++++++++++++ lib/instances/query.go | 8 ++--- lib/instances/stop.go | 2 +- 5 files changed, 82 insertions(+), 9 deletions(-) create mode 100644 lib/instances/macos_boot_disk_test.go diff --git a/lib/instances/guest_os.go b/lib/instances/guest_os.go index 7e93963cc..647d4e303 100644 --- a/lib/instances/guest_os.go +++ b/lib/instances/guest_os.go @@ -13,8 +13,9 @@ const ( ) // GuestOperationUnsupportedReason returns an empty string when op is available for -// this instance, or the user-facing reason it is not. It is the single rule for -// which operations need the shared system agent on each OS. +// this instance, or the user-facing reason it is not. The host API uses it to decide +// which operations need the shared system agent. Readiness, probes and shutdown check +// GuestAgentEnabled directly, which is the same predicate. func (s *StoredMetadata) GuestOperationUnsupportedReason(op GuestOperation) string { if s.MacOS != nil && !s.GuestAgentEnabled() { return fmt.Sprintf("%s requires the shared guest agent on macOS instances", op) diff --git a/lib/instances/macos.go b/lib/instances/macos.go index cc25613a3..e2ddcc151 100644 --- a/lib/instances/macos.go +++ b/lib/instances/macos.go @@ -216,7 +216,7 @@ func (m *manager) admitMacOSBoot(ctx context.Context, stored *StoredMetadata) (f func (m *manager) macOSVMConfig(inst *Instance) hypervisor.VMConfig { c := hypervisor.VMConfig{BootMode: hypervisor.BootModeMacOS, VCPUs: inst.Vcpus, MemoryBytes: inst.Size, VsockCID: inst.VsockCID, VsockSocket: inst.VsockSocket, MacOS: &hypervisor.MacOSPlatform{HardwareModelData: inst.MacOS.HardwareModel, MachineIdentifierData: inst.MacOS.MachineIdentifier, AuxStoragePath: filepath.Join(inst.DataDir, macOSAuxStorageFile)}, - Disks: []hypervisor.DiskConfig{{Path: m.paths.InstanceMacOSBootDisk(inst.Id)}}} + Disks: []hypervisor.DiskConfig{{Path: macOSBootDiskPath(m.paths, inst.Id)}}} if inst.NetworkEnabled { c.Networks = []hypervisor.NetworkConfig{{MAC: inst.MAC}} } @@ -389,7 +389,27 @@ func (s *StoredMetadata) VMStoragePath() string { if s.MacOS == nil || s.DataDir == "" { return "" } - return filepath.Join(s.DataDir, paths.MacOSBootDiskFile) + boot := filepath.Join(s.DataDir, paths.MacOSBootDiskFile) + if legacy := filepath.Join(s.DataDir, "overlay.raw"); !fileExists(boot) && fileExists(legacy) { + return legacy + } + return boot +} + +// macOSBootDiskPath is the boot disk a macOS instance runs from. Instances created +// before the boot.raw name keep their disk at overlay.raw, so that file is used when +// boot.raw is absent. New instances always use boot.raw. +func macOSBootDiskPath(p *paths.Paths, id string) string { + boot := p.InstanceMacOSBootDisk(id) + if legacy := p.InstanceOverlay(id); !fileExists(boot) && fileExists(legacy) { + return legacy + } + return boot +} + +func fileExists(path string) bool { + _, err := os.Stat(path) + return err == nil } // requireMacOSGuest refuses Linux guests on a macOS-only server. diff --git a/lib/instances/macos_boot_disk_test.go b/lib/instances/macos_boot_disk_test.go new file mode 100644 index 000000000..3fa8c7b2e --- /dev/null +++ b/lib/instances/macos_boot_disk_test.go @@ -0,0 +1,52 @@ +package instances + +import ( + "os" + "path/filepath" + "testing" + + "github.com/kernel/hypeman/lib/images" + "github.com/kernel/hypeman/lib/paths" + "github.com/stretchr/testify/require" +) + +// Instances created before the boot.raw name keep their disk at overlay.raw. Start +// and VM-process lookup must find that disk, and new instances must use boot.raw. +func TestMacOSBootDiskPathFallsBackToLegacyOverlay(t *testing.T) { + t.Parallel() + cases := []struct { + name string + create []string // files to create, relative to the instance dir + want string // expected disk, relative to the instance dir + }{ + {name: "new instance without a disk yet", want: paths.MacOSBootDiskFile}, + {name: "new instance", create: []string{paths.MacOSBootDiskFile}, want: paths.MacOSBootDiskFile}, + {name: "legacy instance", create: []string{"overlay.raw"}, want: "overlay.raw"}, + {name: "boot.raw wins when both exist", create: []string{paths.MacOSBootDiskFile, "overlay.raw"}, want: paths.MacOSBootDiskFile}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + dataDir := t.TempDir() + p := paths.New(dataDir) + const id = "inst1" + instDir := p.InstanceDir(id) + require.NoError(t, os.MkdirAll(instDir, 0o755)) + for _, f := range tc.create { + require.NoError(t, os.WriteFile(filepath.Join(instDir, f), []byte("disk"), 0o600)) + } + want := filepath.Join(instDir, tc.want) + + require.Equal(t, want, macOSBootDiskPath(p, id)) + + stored := &StoredMetadata{MacOS: &images.MacOSImage{}, DataDir: instDir} + require.Equal(t, want, stored.VMStoragePath()) + }) + } +} + +func TestVMStoragePathIsEmptyForLinuxGuests(t *testing.T) { + t.Parallel() + stored := &StoredMetadata{DataDir: t.TempDir()} + require.Empty(t, stored.VMStoragePath()) +} diff --git a/lib/instances/query.go b/lib/instances/query.go index 3741c6f0b..2b69cabdf 100644 --- a/lib/instances/query.go +++ b/lib/instances/query.go @@ -298,7 +298,7 @@ func advancePhaseIfRunning(stored *StoredMetadata) { // Returns true when at least one missing marker was found and populated. func (m *manager) hydrateBootMarkersFromLogs(ctx context.Context, stored *StoredMetadata) bool { needProgram := !programMarkerSettled(stored) - needAgent := !stored.SkipGuestAgent && stored.GuestAgentReadyAt == nil + needAgent := stored.GuestAgentEnabled() && stored.GuestAgentReadyAt == nil if !needProgram && !needAgent { m.clearBootMarkerRescan(stored.Id) return false @@ -425,7 +425,7 @@ func (m *manager) nowUTC() time.Time { } func (m *manager) hydrateGuestAgentReadyFromProbe(ctx context.Context, stored *StoredMetadata) bool { - if stored == nil || stored.SkipGuestAgent || stored.GuestAgentReadyAt != nil { + if stored == nil || !stored.GuestAgentEnabled() || stored.GuestAgentReadyAt != nil { return false } probe := m.guestAgentReadyProbe @@ -441,7 +441,7 @@ func (m *manager) hydrateGuestAgentReadyFromProbe(ctx context.Context, stored *S } func probeGuestAgentReady(ctx context.Context, stored *StoredMetadata) bool { - if stored == nil || stored.SkipGuestAgent { + if stored == nil || !stored.GuestAgentEnabled() { return false } dialer, err := hypervisor.NewVsockDialer(stored.HypervisorType, stored.VsockSocket, stored.VsockCID) @@ -651,7 +651,7 @@ func (m *manager) persistBootMarkers(ctx context.Context, id string) { } needProgram := !programMarkerSettled(&meta.StoredMetadata) - needAgent := !meta.SkipGuestAgent && meta.GuestAgentReadyAt == nil + needAgent := meta.GuestAgentEnabled() && meta.GuestAgentReadyAt == nil if !needProgram && !needAgent { return } diff --git a/lib/instances/stop.go b/lib/instances/stop.go index 6024114c9..fd8648fcc 100644 --- a/lib/instances/stop.go +++ b/lib/instances/stop.go @@ -44,7 +44,7 @@ func hasVFIODevices(stored *StoredMetadata) bool { func (m *manager) tryGracefulGuestShutdown(ctx context.Context, inst *Instance, stopTimeout int) bool { log := logger.FromContext(ctx) - if inst.SkipGuestAgent { + if !inst.GuestAgentEnabled() { log.DebugContext(ctx, "guest-agent disabled, skipping graceful guest shutdown", "instance_id", inst.Id) return false } From af406e326ccbe4db13b645ca45d0552c161f1717 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Wed, 21 Oct 2026 14:58:59 +0000 Subject: [PATCH 05/11] Share exec setup between the direct and macOS paths The direct exec path and the macOS lifecycle each built the command, started the PTY, ran the stdin and resize pump, and computed the exit status. Those now live in exec_common.go. The policy stays in each path: the direct path buffers stdout and waits without a bound, and the lifecycle streams output and bounds its drain. A deadline now reports 124 on the direct path too, which matches its existing comment. Before, a timed-out direct exec reported the signal exit code (-1). --- lib/system/guest_agent/exec.go | 93 ++++-------------------- lib/system/guest_agent/exec_common.go | 79 ++++++++++++++++++++ lib/system/guest_agent/exec_lifecycle.go | 77 +++----------------- 3 files changed, 102 insertions(+), 147 deletions(-) create mode 100644 lib/system/guest_agent/exec_common.go diff --git a/lib/system/guest_agent/exec.go b/lib/system/guest_agent/exec.go index 1b8adb323..75aa44fa2 100644 --- a/lib/system/guest_agent/exec.go +++ b/lib/system/guest_agent/exec.go @@ -2,11 +2,11 @@ package main import ( "context" + "errors" "fmt" "io" "log" "os" - "os/exec" "strings" "sync" "time" @@ -62,15 +62,7 @@ func (s *guestServer) executeNoTTY(ctx context.Context, stream pb.GuestService_E return fmt.Errorf("empty command") } - cmd := exec.CommandContext(ctx, start.Command[0], start.Command[1:]...) - - // Set up environment (no TTY defaults for non-TTY mode) - cmd.Env = s.buildEnv(start.Env, false) - - // Set up working directory - if start.Cwd != "" { - cmd.Dir = start.Cwd - } + cmd := s.newExecCmd(ctx, start) stdin, _ := cmd.StdinPipe() stdout, _ := cmd.StdoutPipe() @@ -87,19 +79,7 @@ func (s *guestServer) executeNoTTY(ctx context.Context, stream pb.GuestService_E var wg sync.WaitGroup var stdoutData, stderrData []byte - // Handle stdin in background - go func() { - defer stdin.Close() - for { - req, err := stream.Recv() - if err != nil { - return - } - if data := req.GetStdin(); data != nil { - stdin.Write(data) - } - } - }() + pumpExecInput(stream, func(data []byte) { stdin.Write(data) }, nil, func() { stdin.Close() }) // Read all stdout/stderr BEFORE calling Wait() - Wait() closes the pipes! wg.Add(1) @@ -147,13 +127,7 @@ func (s *guestServer) executeNoTTY(ctx context.Context, stream pb.GuestService_E sendMu.Unlock() } - exitCode := int32(0) - if cmd.ProcessState != nil { - exitCode = int32(cmd.ProcessState.ExitCode()) - } else if waitErr != nil { - // If killed by timeout, exit with 124 (GNU timeout convention) - exitCode = 124 - } + exitCode := execExitCode(cmd, waitErr, errors.Is(ctx.Err(), context.DeadlineExceeded)) log.Printf("[guest-agent] command finished with exit code: %d", exitCode) @@ -171,30 +145,10 @@ func (s *guestServer) executeTTY(ctx context.Context, stream pb.GuestService_Exe return fmt.Errorf("empty command") } - cmd := exec.CommandContext(ctx, start.Command[0], start.Command[1:]...) - - // Set up environment (TTY mode adds TERM default) - cmd.Env = s.buildEnv(start.Env, true) - - // Set up working directory - if start.Cwd != "" { - cmd.Dir = start.Cwd - } - - // Set up initial window size (use defaults if not specified) - ws := &pty.Winsize{ - Rows: uint16(start.Rows), - Cols: uint16(start.Cols), - } - if ws.Rows == 0 { - ws.Rows = 24 - } - if ws.Cols == 0 { - ws.Cols = 80 - } + cmd := s.newExecCmd(ctx, start) // Start with PTY and initial window size - ptmx, err := pty.StartWithSize(cmd, ws) + ptmx, err := startExecPTY(cmd, start) if err != nil { return fmt.Errorf("start pty: %w", err) } @@ -206,27 +160,12 @@ func (s *guestServer) executeTTY(ctx context.Context, stream pb.GuestService_Exe // Use WaitGroup to ensure all output is sent before exit code var wg sync.WaitGroup - // Handle stdin and resize in background - go func() { - for { - req, err := stream.Recv() - if err != nil { - return - } - - if data := req.GetStdin(); data != nil { - ptmx.Write(data) - } - - // Handle window resize - if resize := req.GetResize(); resize != nil { - pty.Setsize(ptmx, &pty.Winsize{ - Rows: uint16(resize.Rows), - Cols: uint16(resize.Cols), - }) - } - } - }() + pumpExecInput(stream, func(data []byte) { ptmx.Write(data) }, func(resize *pb.WindowSize) { + pty.Setsize(ptmx, &pty.Winsize{ + Rows: uint16(resize.Rows), + Cols: uint16(resize.Cols), + }) + }, nil) // Stream output wg.Add(1) @@ -254,13 +193,7 @@ func (s *guestServer) executeTTY(ctx context.Context, stream pb.GuestService_Exe // Wait for all output to be sent wg.Wait() - exitCode := int32(0) - if cmd.ProcessState != nil { - exitCode = int32(cmd.ProcessState.ExitCode()) - } else if waitErr != nil { - // If killed by timeout, exit with 124 (GNU timeout convention) - exitCode = 124 - } + exitCode := execExitCode(cmd, waitErr, errors.Is(ctx.Err(), context.DeadlineExceeded)) log.Printf("[guest-agent] TTY command finished with exit code: %d", exitCode) diff --git a/lib/system/guest_agent/exec_common.go b/lib/system/guest_agent/exec_common.go new file mode 100644 index 000000000..ba081c87d --- /dev/null +++ b/lib/system/guest_agent/exec_common.go @@ -0,0 +1,79 @@ +package main + +import ( + "context" + "os" + "os/exec" + + "github.com/creack/pty" + pb "github.com/kernel/hypeman/lib/guest" +) + +// Both exec paths, the direct path and the macOS lifecycle in exec_lifecycle.go, +// share command setup, PTY start, input handling and exit status. They differ in +// policy, which stays in their own files: the direct path buffers stdout and waits +// without a bound, the lifecycle streams output and bounds its drain, and a timeout +// kills the direct child or the whole process group. + +// newExecCmd builds the command both paths run. Only a TTY session gets TERM defaults. +func (s *guestServer) newExecCmd(ctx context.Context, start *pb.ExecStart) *exec.Cmd { + cmd := exec.CommandContext(ctx, start.Command[0], start.Command[1:]...) + cmd.Env = s.buildEnv(start.Env, start.Tty) + if start.Cwd != "" { + cmd.Dir = start.Cwd + } + return cmd +} + +// startExecPTY starts cmd on a PTY sized from start, with 24x80 defaults. +func startExecPTY(cmd *exec.Cmd, start *pb.ExecStart) (*os.File, error) { + ws := &pty.Winsize{ + Rows: uint16(start.Rows), + Cols: uint16(start.Cols), + } + if ws.Rows == 0 { + ws.Rows = 24 + } + if ws.Cols == 0 { + ws.Cols = 80 + } + return pty.StartWithSize(cmd, ws) +} + +// pumpExecInput forwards client stdin and resize messages until the stream ends. +// onResize may be nil. onClose, if set, runs when the stream ends. +func pumpExecInput(stream pb.GuestService_ExecServer, onStdin func([]byte), onResize func(*pb.WindowSize), onClose func()) { + go func() { + if onClose != nil { + defer onClose() + } + for { + req, err := stream.Recv() + if err != nil { + return + } + if data := req.GetStdin(); data != nil { + onStdin(data) + } + if resize := req.GetResize(); resize != nil && onResize != nil { + onResize(resize) + } + } + }() +} + +// execExitCode is the status a client sees. A deadline always reports 124, the GNU +// timeout convention, even when the command exited during the drain. Otherwise the +// process exit code is used, and a command that could not be waited for reports 124. +func execExitCode(cmd *exec.Cmd, waitErr error, timedOut bool) int32 { + if timedOut { + return 124 + } + if cmd.ProcessState != nil { + return int32(cmd.ProcessState.ExitCode()) + } + if waitErr != nil { + return 124 + } + return 0 +} diff --git a/lib/system/guest_agent/exec_lifecycle.go b/lib/system/guest_agent/exec_lifecycle.go index 0abb1c3ec..1fcb0cfc3 100644 --- a/lib/system/guest_agent/exec_lifecycle.go +++ b/lib/system/guest_agent/exec_lifecycle.go @@ -88,19 +88,7 @@ func (s *guestServer) executeLifecycleNoTTY(ctx context.Context, cancel context. defer close(lifecycleDone) go killGroupOnDone(ctx, cmd, lifecycleDone) - // Handle stdin in background - go func() { - defer stdin.Close() - for { - req, err := sender.stream.Recv() - if err != nil { - return - } - if data := req.GetStdin(); data != nil { - stdin.Write(data) - } - } - }() + pumpExecInput(sender.stream, func(data []byte) { stdin.Write(data) }, nil, func() { stdin.Close() }) return s.finishExec(ctx, sender, cmd, nil) } @@ -110,11 +98,9 @@ func (s *guestServer) execCommand(ctx context.Context, start *pb.ExecStart) (*ex if err := ctx.Err(); err != nil { return nil, fmt.Errorf("start command: %w", err) } - cmd := exec.CommandContext(ctx, start.Command[0], start.Command[1:]...) + cmd := s.newExecCmd(ctx, start) cmd.Cancel = func() error { return killProcessGroup(cmd) } cmd.WaitDelay = 2 * time.Second - cmd.Env = s.buildEnv(start.Env, start.Tty) - cmd.Dir = start.Cwd if !start.Tty { cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} } @@ -144,7 +130,7 @@ func (s *guestServer) finishExec(ctx context.Context, sender *execSender, cmd *e return err } } - exitCode := exitCodeOf(ctx, cmd, waitErr) + exitCode := execExitCode(cmd, waitErr, errors.Is(ctx.Err(), context.DeadlineExceeded)) log.Printf("[guest-agent] command finished with exit code: %d", exitCode) @@ -251,21 +237,6 @@ func (s *guestServer) runBounded(ctx context.Context, fn func()) error { return awaitBounded(ctx, done, s.drainBound()) } -// exitCodeOf reports how a command ended. A command that hit its deadline exits 124 -// (GNU timeout convention) even though the group kill leaves a process state behind. -func exitCodeOf(ctx context.Context, cmd *exec.Cmd, waitErr error) int32 { - if errors.Is(ctx.Err(), context.DeadlineExceeded) { - return 124 - } - if cmd.ProcessState != nil { - return int32(cmd.ProcessState.ExitCode()) - } - if waitErr != nil { - return 124 - } - return 0 -} - // killProcessGroup kills the command and its descendants. Both exec paths start the command // as a group leader (Setpgid for no-TTY, Setsid for TTY), so the group id is its pid. func killProcessGroup(cmd *exec.Cmd) error { @@ -303,20 +274,7 @@ func (w *execStreamWriter) Write(data []byte) (int, error) { // executeLifecycleTTY runs the command on a PTY. func (s *guestServer) executeLifecycleTTY(ctx context.Context, cancel context.CancelFunc, sender *execSender, start *pb.ExecStart, cmd *exec.Cmd) error { - // Set up initial window size (use defaults if not specified) - ws := &pty.Winsize{ - Rows: uint16(start.Rows), - Cols: uint16(start.Cols), - } - if ws.Rows == 0 { - ws.Rows = 24 - } - if ws.Cols == 0 { - ws.Cols = 80 - } - - // Start with PTY and initial window size - ptmx, err := pty.StartWithSize(cmd, ws) + ptmx, err := startExecPTY(cmd, start) if err != nil { return fmt.Errorf("start pty: %w", err) } @@ -328,27 +286,12 @@ func (s *guestServer) executeLifecycleTTY(ctx context.Context, cancel context.Ca outputDone := make(chan struct{}) output := &execStreamWriter{sender: sender, cancel: cancel} - // Handle stdin and resize in background - go func() { - for { - req, err := sender.stream.Recv() - if err != nil { - return - } - - if data := req.GetStdin(); data != nil { - ptmx.Write(data) - } - - // Handle window resize - if resize := req.GetResize(); resize != nil { - pty.Setsize(ptmx, &pty.Winsize{ - Rows: uint16(resize.Rows), - Cols: uint16(resize.Cols), - }) - } - } - }() + pumpExecInput(sender.stream, func(data []byte) { ptmx.Write(data) }, func(resize *pb.WindowSize) { + pty.Setsize(ptmx, &pty.Winsize{ + Rows: uint16(resize.Rows), + Cols: uint16(resize.Cols), + }) + }, nil) // Stream output go func() { From e9f77447f403f57d08cd5fdaabda1a807b5150c1 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Wed, 21 Oct 2026 14:58:59 +0000 Subject: [PATCH 06/11] Give readiness probes their own connection Probes run concurrently with other callers on the same guest, so a failed probe evicted the pooled connection and cancelled their RPCs. KeepPooledConn only stopped that eviction. ProbeExec dials a connection for each attempt and closes it. The pooled connection is never touched by a probe. The readiness probe uses ProbeExec, and KeepPooledConn is removed. --- lib/guest/client.go | 64 +++++++++++++++---- ...pooled_conn_test.go => probe_exec_test.go} | 14 ++-- lib/instances/query.go | 5 +- 3 files changed, 58 insertions(+), 25 deletions(-) rename lib/guest/{keep_pooled_conn_test.go => probe_exec_test.go} (63%) diff --git a/lib/guest/client.go b/lib/guest/client.go index ecc56283c..c22a69374 100644 --- a/lib/guest/client.go +++ b/lib/guest/client.go @@ -81,7 +81,18 @@ func GetOrCreateConn(ctx context.Context, dialer hypervisor.VsockDialer) (*grpc. return conn, nil } - // Create new connection using the VsockDialer + conn, err := dialGuestConn(ctx, dialer) + if err != nil { + return nil, err + } + connPool.conns[key] = conn + slog.Debug("created new gRPC connection", "key", key) + return conn, nil +} + +// dialGuestConn creates a gRPC client connection to the guest agent. The connection is +// not registered in the pool, so the caller owns it. +func dialGuestConn(ctx context.Context, dialer hypervisor.VsockDialer) (*grpc.ClientConn, error) { traceCtx := ctx conn, err := grpc.Dial("passthrough:///vsock", grpc.WithContextDialer(func(ctx context.Context, addr string) (net.Conn, error) { @@ -99,9 +110,6 @@ func GetOrCreateConn(ctx context.Context, dialer hypervisor.VsockDialer) (*grpc. if err != nil { return nil, fmt.Errorf("create grpc connection: %w", err) } - - connPool.conns[key] = conn - slog.Debug("created new gRPC connection", "key", key) return conn, nil } @@ -148,9 +156,6 @@ type ExecOptions struct { Rows uint32 // Initial terminal rows (0 = default 24) Cols uint32 // Initial terminal cols (0 = default 80) ResizeChan <-chan *WindowSize // Optional: channel to receive resize events (pointer to avoid copying mutex) - // KeepPooledConn stops a retryable error from evicting the shared pooled connection. - // Use it for probes that run concurrently with other callers on the same guest. - KeepPooledConn bool } type ReconfigureNetworkOptions struct { @@ -269,7 +274,7 @@ func ExecIntoInstance(ctx context.Context, dialer hypervisor.VsockDialer, opts E // we are waiting for the guest-agent to become reachable. if opts.WaitForAgent == 0 { exit, err := execIntoInstanceOnce(ctx, dialer, opts) - if err != nil && isRetryableConnectionError(err) && !opts.KeepPooledConn { + if err != nil && isRetryableConnectionError(err) { CloseConn(dialer.Key()) } return exit, err @@ -309,9 +314,7 @@ func ExecIntoInstance(ctx context.Context, dialer hypervisor.VsockDialer, opts E firstRetryableErrorType = errType } lastRetryableErrorType = errType - if !opts.KeepPooledConn { - CloseConn(dialer.Key()) - } + CloseConn(dialer.Key()) // Connection error - check if we should retry if time.Now().After(deadline) { @@ -333,6 +336,35 @@ func ExecIntoInstance(ctx context.Context, dialer hypervisor.VsockDialer, opts E } } +// ProbeExec runs a readiness probe. Probes run concurrently with other callers on the +// same guest, so each attempt dials its own connection and closes it. A failed probe +// therefore cannot evict or cancel the pooled connection other callers use. Retryable +// failures are retried until WaitForAgent passes, as in ExecIntoInstance. +func ProbeExec(ctx context.Context, dialer hypervisor.VsockDialer, opts ExecOptions) (*ExitStatus, error) { + start := time.Now() + deadline := start.Add(opts.WaitForAgent) + for { + exit, err := probeAttempt(ctx, dialer, opts) + if err == nil || !isRetryableConnectionError(err) || !time.Now().Before(deadline) { + return exit, err + } + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-time.After(guestExecRetryInterval(time.Since(start))): + } + } +} + +func probeAttempt(ctx context.Context, dialer hypervisor.VsockDialer, opts ExecOptions) (*ExitStatus, error) { + conn, err := dialGuestConn(ctx, dialer) + if err != nil { + return nil, err + } + defer conn.Close() + return execOnConn(ctx, conn, opts) +} + func guestExecRetryInterval(elapsed time.Duration) time.Duration { if elapsed < guestExecFastRetryWindow { return guestExecFastRetryInterval @@ -447,9 +479,6 @@ func retryableConnectionErrorType(err error) string { // execIntoInstanceOnce executes command in instance via vsock using gRPC (single attempt). func execIntoInstanceOnce(ctx context.Context, dialer hypervisor.VsockDialer, opts ExecOptions) (*ExitStatus, error) { - start := time.Now() - var bytesSent int64 - // Get or create a reusable gRPC connection for this vsock dialer _, finishGetConn := startGuestExecStep(ctx, opts, "guest.exec.get_conn") grpcConn, err := GetOrCreateConn(ctx, dialer) @@ -458,6 +487,13 @@ func execIntoInstanceOnce(ctx context.Context, dialer hypervisor.VsockDialer, op return nil, fmt.Errorf("get grpc connection: %w", err) } // Note: Don't close the connection - it's pooled and reused + return execOnConn(ctx, grpcConn, opts) +} + +// execOnConn runs one command over grpcConn and returns its exit status. +func execOnConn(ctx context.Context, grpcConn *grpc.ClientConn, opts ExecOptions) (*ExitStatus, error) { + start := time.Now() + var bytesSent int64 // Create guest client client := NewGuestServiceClient(grpcConn) diff --git a/lib/guest/keep_pooled_conn_test.go b/lib/guest/probe_exec_test.go similarity index 63% rename from lib/guest/keep_pooled_conn_test.go rename to lib/guest/probe_exec_test.go index 6088087c3..1e2a0a4e7 100644 --- a/lib/guest/keep_pooled_conn_test.go +++ b/lib/guest/probe_exec_test.go @@ -25,24 +25,24 @@ func pooledKeyExists(key string) bool { return ok } -// A probe that keeps the pooled connection must not evict it for concurrent callers. -func TestKeepPooledConnPreservesSharedConnectionOnRetry(t *testing.T) { - const key = "keep-pooled-conn-test" +// A probe owns its connection, so a failed probe leaves the pooled connection that +// other callers use in place. Ordinary exec still evicts after retryable failures. +func TestProbeExecLeavesPooledConnectionInPlace(t *testing.T) { + const key = "probe-exec-test" t.Cleanup(func() { CloseConn(key) }) shared := unreachableDialer{key: key} - // Seed the pool with a live client connection for this key. _, err := GetOrCreateConn(context.Background(), shared) require.NoError(t, err) require.True(t, pooledKeyExists(key)) ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) defer cancel() - _, err = ExecIntoInstance(ctx, shared, ExecOptions{Command: []string{"true"}, WaitForAgent: 50 * time.Millisecond, KeepPooledConn: true}) + _, err = ProbeExec(ctx, shared, ExecOptions{Command: []string{"true"}, WaitForAgent: 50 * time.Millisecond}) require.Error(t, err) - require.True(t, pooledKeyExists(key), "a probe that keeps the pooled connection must leave it in place") + require.True(t, pooledKeyExists(key), "a failed probe must not evict the pooled connection") _, err = ExecIntoInstance(ctx, shared, ExecOptions{Command: []string{"true"}, WaitForAgent: 50 * time.Millisecond}) require.Error(t, err) - require.False(t, pooledKeyExists(key), "default behavior still evicts after retryable failures") + require.False(t, pooledKeyExists(key), "ordinary exec still evicts after retryable failures") } diff --git a/lib/instances/query.go b/lib/instances/query.go index 2b69cabdf..07dc18f01 100644 --- a/lib/instances/query.go +++ b/lib/instances/query.go @@ -452,13 +452,10 @@ func probeGuestAgentReady(ctx context.Context, stored *StoredMetadata) bool { probeCtx, cancel := context.WithTimeout(ctx, guestAgentReadyProbeTimeout) defer cancel() - exit, err := guest.ExecIntoInstance(probeCtx, dialer, guest.ExecOptions{ + exit, err := guest.ProbeExec(probeCtx, dialer, guest.ExecOptions{ Command: []string{guestAgentReadyExecutable(stored)}, Timeout: int32(guestAgentReadyProbeTimeout / time.Second), WaitForAgent: guestAgentReadyProbeWait, - // Listings probe every instance concurrently. A macOS probe that evicted the - // shared connection cancelled other callers' in-flight RPCs (restore readiness). - KeepPooledConn: stored.MacOS != nil, }) return err == nil && exit != nil && exit.Code == 0 } From ba5e03016f6e77fff0295b91d9b65307ffa39d0a Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Wed, 21 Oct 2026 14:58:59 +0000 Subject: [PATCH 07/11] Keep one macOS boot disk path Instances created before this change kept their disk at overlay.raw. macOS is experimental and breaking changes there are acceptable, so there is no fallback. New instances and existing ones both use boot.raw. --- lib/instances/macos.go | 24 ++--------------- lib/instances/macos_boot_disk_test.go | 37 +++------------------------ 2 files changed, 6 insertions(+), 55 deletions(-) diff --git a/lib/instances/macos.go b/lib/instances/macos.go index e2ddcc151..cc25613a3 100644 --- a/lib/instances/macos.go +++ b/lib/instances/macos.go @@ -216,7 +216,7 @@ func (m *manager) admitMacOSBoot(ctx context.Context, stored *StoredMetadata) (f func (m *manager) macOSVMConfig(inst *Instance) hypervisor.VMConfig { c := hypervisor.VMConfig{BootMode: hypervisor.BootModeMacOS, VCPUs: inst.Vcpus, MemoryBytes: inst.Size, VsockCID: inst.VsockCID, VsockSocket: inst.VsockSocket, MacOS: &hypervisor.MacOSPlatform{HardwareModelData: inst.MacOS.HardwareModel, MachineIdentifierData: inst.MacOS.MachineIdentifier, AuxStoragePath: filepath.Join(inst.DataDir, macOSAuxStorageFile)}, - Disks: []hypervisor.DiskConfig{{Path: macOSBootDiskPath(m.paths, inst.Id)}}} + Disks: []hypervisor.DiskConfig{{Path: m.paths.InstanceMacOSBootDisk(inst.Id)}}} if inst.NetworkEnabled { c.Networks = []hypervisor.NetworkConfig{{MAC: inst.MAC}} } @@ -389,27 +389,7 @@ func (s *StoredMetadata) VMStoragePath() string { if s.MacOS == nil || s.DataDir == "" { return "" } - boot := filepath.Join(s.DataDir, paths.MacOSBootDiskFile) - if legacy := filepath.Join(s.DataDir, "overlay.raw"); !fileExists(boot) && fileExists(legacy) { - return legacy - } - return boot -} - -// macOSBootDiskPath is the boot disk a macOS instance runs from. Instances created -// before the boot.raw name keep their disk at overlay.raw, so that file is used when -// boot.raw is absent. New instances always use boot.raw. -func macOSBootDiskPath(p *paths.Paths, id string) string { - boot := p.InstanceMacOSBootDisk(id) - if legacy := p.InstanceOverlay(id); !fileExists(boot) && fileExists(legacy) { - return legacy - } - return boot -} - -func fileExists(path string) bool { - _, err := os.Stat(path) - return err == nil + return filepath.Join(s.DataDir, paths.MacOSBootDiskFile) } // requireMacOSGuest refuses Linux guests on a macOS-only server. diff --git a/lib/instances/macos_boot_disk_test.go b/lib/instances/macos_boot_disk_test.go index 3fa8c7b2e..acd64e9fb 100644 --- a/lib/instances/macos_boot_disk_test.go +++ b/lib/instances/macos_boot_disk_test.go @@ -1,7 +1,6 @@ package instances import ( - "os" "path/filepath" "testing" @@ -10,39 +9,11 @@ import ( "github.com/stretchr/testify/require" ) -// Instances created before the boot.raw name keep their disk at overlay.raw. Start -// and VM-process lookup must find that disk, and new instances must use boot.raw. -func TestMacOSBootDiskPathFallsBackToLegacyOverlay(t *testing.T) { +func TestVMStoragePathIsTheMacOSBootDisk(t *testing.T) { t.Parallel() - cases := []struct { - name string - create []string // files to create, relative to the instance dir - want string // expected disk, relative to the instance dir - }{ - {name: "new instance without a disk yet", want: paths.MacOSBootDiskFile}, - {name: "new instance", create: []string{paths.MacOSBootDiskFile}, want: paths.MacOSBootDiskFile}, - {name: "legacy instance", create: []string{"overlay.raw"}, want: "overlay.raw"}, - {name: "boot.raw wins when both exist", create: []string{paths.MacOSBootDiskFile, "overlay.raw"}, want: paths.MacOSBootDiskFile}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - dataDir := t.TempDir() - p := paths.New(dataDir) - const id = "inst1" - instDir := p.InstanceDir(id) - require.NoError(t, os.MkdirAll(instDir, 0o755)) - for _, f := range tc.create { - require.NoError(t, os.WriteFile(filepath.Join(instDir, f), []byte("disk"), 0o600)) - } - want := filepath.Join(instDir, tc.want) - - require.Equal(t, want, macOSBootDiskPath(p, id)) - - stored := &StoredMetadata{MacOS: &images.MacOSImage{}, DataDir: instDir} - require.Equal(t, want, stored.VMStoragePath()) - }) - } + dataDir := t.TempDir() + stored := &StoredMetadata{MacOS: &images.MacOSImage{}, DataDir: dataDir} + require.Equal(t, filepath.Join(dataDir, paths.MacOSBootDiskFile), stored.VMStoragePath()) } func TestVMStoragePathIsEmptyForLinuxGuests(t *testing.T) { From e848c2d7fc7503c82730a237db342f4bf7de1ec0 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Wed, 21 Oct 2026 15:07:41 +0000 Subject: [PATCH 08/11] Keep the exec group leader unreaped until its output drain ends A kill after the leader was reaped could reach a reused process group id. Go's cmd.Wait reaps the leader, and the lifecycle needed that to read the pipes. The leader now waits for its exit without being reaped: waitid with WNOWAIT on Linux, and kqueue NOTE_EXIT on Darwin. Darwin checked on a macOS guest: the exit event fires before the reap, the group is still signalable, and a watch on an already-exited child still fires. The lifecycle reads its own pipes, drains within the bound, and reaps last. processGroup allows a kill only before reap. cmd.Cancel is now a no-op, so the group watcher is the only killer. Tests: the reuse guard now uses processGroup. A pipe-drain timeout test covers the drain on every platform. The TTY drain test is skipped on macOS, which revokes the terminal when the shell exits. --- lib/system/guest_agent/exec_group.go | 54 +++++++++ lib/system/guest_agent/exec_lifecycle.go | 110 +++++++++--------- .../exec_lifecycle_disconnect_test.go | 17 +-- .../exec_lifecycle_remediation_test.go | 45 +++++-- lib/system/guest_agent/exec_reap_darwin.go | 42 +++++++ lib/system/guest_agent/exec_reap_linux.go | 17 +++ 6 files changed, 214 insertions(+), 71 deletions(-) create mode 100644 lib/system/guest_agent/exec_group.go create mode 100644 lib/system/guest_agent/exec_reap_darwin.go create mode 100644 lib/system/guest_agent/exec_reap_linux.go diff --git a/lib/system/guest_agent/exec_group.go b/lib/system/guest_agent/exec_group.go new file mode 100644 index 000000000..2c00fe559 --- /dev/null +++ b/lib/system/guest_agent/exec_group.go @@ -0,0 +1,54 @@ +package main + +import ( + "context" + "errors" + "log" + "os/exec" + "sync" + "syscall" +) + +// processGroup owns the kill of an exec's process group. The command's pid is the +// group id. While the leader is unreaped, the kernel keeps its pid reserved, so a kill +// reaches only this group. After the leader is reaped, the id can belong to another +// process group, so no kill is sent. +type processGroup struct { + mu sync.Mutex + pid int + reaped bool +} + +func newProcessGroup(cmd *exec.Cmd) *processGroup { + return &processGroup{pid: cmd.Process.Pid} +} + +// kill sends SIGKILL to the group unless the leader was already reaped. +func (g *processGroup) kill() { + g.mu.Lock() + defer g.mu.Unlock() + if g.reaped { + return + } + if err := syscall.Kill(-g.pid, syscall.SIGKILL); err != nil && !errors.Is(err, syscall.ESRCH) { + log.Printf("[guest-agent] kill process group %d: %v", g.pid, err) + } +} + +// reap reaps the leader and returns cmd.Wait's error. No kill is sent after it. +func (g *processGroup) reap(cmd *exec.Cmd) error { + g.mu.Lock() + g.reaped = true + g.mu.Unlock() + return cmd.Wait() +} + +// killGroupOnDone kills the group if ctx ends before done closes. done closes when +// the exec lifecycle finishes, so a normal exit leaves background children alone. +func killGroupOnDone(ctx context.Context, grp *processGroup, done <-chan struct{}) { + select { + case <-done: + case <-ctx.Done(): + grp.kill() + } +} diff --git a/lib/system/guest_agent/exec_lifecycle.go b/lib/system/guest_agent/exec_lifecycle.go index 1fcb0cfc3..adc73da02 100644 --- a/lib/system/guest_agent/exec_lifecycle.go +++ b/lib/system/guest_agent/exec_lifecycle.go @@ -72,25 +72,50 @@ func (s *execSender) send(response *pb.ExecResponse) error { func (s *execSender) close() { s.closed.Store(true) } // executeLifecycleNoTTY streams output as the command writes it. +// executeLifecycleNoTTY streams output as the command writes it. The pipes are read +// here, not by cmd.Wait, which would reap the leader before the output drain. func (s *guestServer) executeLifecycleNoTTY(ctx context.Context, cancel context.CancelFunc, sender *execSender, cmd *exec.Cmd) error { - cmd.Stdout = &execStreamWriter{sender: sender, cancel: cancel} - cmd.Stderr = &execStreamWriter{sender: sender, cancel: cancel, stderr: true} stdin, err := cmd.StdinPipe() if err != nil { return fmt.Errorf("open command stdin: %w", err) } + stdout, err := cmd.StdoutPipe() + if err != nil { + return fmt.Errorf("open command stdout: %w", err) + } + stderr, err := cmd.StderrPipe() + if err != nil { + return fmt.Errorf("open command stderr: %w", err) + } if err := cmd.Start(); err != nil { return fmt.Errorf("start command: %w", err) } - // Cancel is not called once the direct child has exited, so a descendant that - // still holds the pipes would outlive the timeout. Kill the group from the context. + grp := newProcessGroup(cmd) + // A descendant that holds the pipes outlives the leader. The group is killed from + // the context, which is the only killer. lifecycleDone := make(chan struct{}) defer close(lifecycleDone) - go killGroupOnDone(ctx, cmd, lifecycleDone) + go killGroupOnDone(ctx, grp, lifecycleDone) + + var copies sync.WaitGroup + copies.Add(2) + go func() { + defer copies.Done() + _, _ = io.Copy(&execStreamWriter{sender: sender, cancel: cancel}, stdout) + }() + go func() { + defer copies.Done() + _, _ = io.Copy(&execStreamWriter{sender: sender, cancel: cancel, stderr: true}, stderr) + }() + outputDone := make(chan struct{}) + go func() { + copies.Wait() + close(outputDone) + }() pumpExecInput(sender.stream, func(data []byte) { stdin.Write(data) }, nil, func() { stdin.Close() }) - return s.finishExec(ctx, sender, cmd, nil) + return s.finishExec(ctx, sender, cmd, grp, outputDone) } // execCommand owns setup and pre-start cancellation for both I/O modes. @@ -99,7 +124,8 @@ func (s *guestServer) execCommand(ctx context.Context, start *pb.ExecStart) (*ex return nil, fmt.Errorf("start command: %w", err) } cmd := s.newExecCmd(ctx, start) - cmd.Cancel = func() error { return killProcessGroup(cmd) } + // The group watcher kills the group, so it can check whether the leader was reaped. + cmd.Cancel = func() error { return nil } cmd.WaitDelay = 2 * time.Second if !start.Tty { cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} @@ -108,28 +134,30 @@ func (s *guestServer) execCommand(ctx context.Context, start *pb.ExecStart) (*ex } // finishExec owns bounded wait, output drain and final status for both I/O modes. -func (s *guestServer) finishExec(ctx context.Context, sender *execSender, cmd *exec.Cmd, outputDone <-chan struct{}) error { - // Non-TTY cmd.Wait also waits for copies, which can block in stream.Send. - var waitErr error - if err := s.runBounded(ctx, func() { - waitErr = cmd.Wait() - }); err != nil { - return err +func (s *guestServer) finishExec(ctx context.Context, sender *execSender, cmd *exec.Cmd, grp *processGroup, outputDone <-chan struct{}) error { + // The leader exits, or the context kills the group, and the leader stays unreaped + // until reap below. Its group id stays reserved, so a deadline during the output + // drain can still kill background children that hold the output open. + if err := waitLeaderExit(grp.pid); err != nil { + log.Printf("[guest-agent] wait for command exit: %v", err) + } + var drainErr error + if outputDone != nil { + drainErr = drainAfterExit(ctx, outputDone, s.drainBound()) + } + waitErr := grp.reap(cmd) + if drainErr != nil { + return drainErr } - // ErrWaitDelay means the command exited but a descendant still held its output. - // The status is already known, so it is reported as a normal exit. if waitErr != nil { + // exec.CommandContext reports the context error from Wait once the context ends. + // That is a cancellation, not a failed stream, and execExitCode maps it. _, exited := waitErr.(*exec.ExitError) - if !exited && !errors.Is(waitErr, exec.ErrWaitDelay) { + cancelled := ctx.Err() != nil && errors.Is(waitErr, ctx.Err()) + if !exited && !cancelled { return fmt.Errorf("stream command output: %w", waitErr) } } - - if outputDone != nil { - if err := drainAfterExit(ctx, outputDone, s.drainBound()); err != nil { - return err - } - } exitCode := execExitCode(cmd, waitErr, errors.Is(ctx.Err(), context.DeadlineExceeded)) log.Printf("[guest-agent] command finished with exit code: %d", exitCode) @@ -183,28 +211,6 @@ func awaitBounded(ctx context.Context, done <-chan struct{}, grace time.Duration } } -// killGroupOnDone kills cmd's process group if ctx ends before the exec lifecycle -// finishes (done closes). It watches the lifecycle, not cmd.Wait, so a timeout that -// fires during the output drain can still kill background children that hold the -// terminal open. Wait reaps the leader first, so the group id is used only while -// members remain. A later kill can reach a group whose id a new leader has reused, -// which is a residual risk. -func killGroupOnDone(ctx context.Context, cmd *exec.Cmd, done <-chan struct{}) { - select { - case <-done: - return - case <-ctx.Done(): - } - select { - case <-done: - // The command finished before the cancellation. Background children are left - // running on a normal exit, as the exec contract says. - return - default: - } - _ = killProcessGroup(cmd) -} - // sendExitCode delivers the final status within the drain bound, counted from the // send itself, so a client that stopped reading cannot hold the handler even when // nothing is cancelled. @@ -237,15 +243,6 @@ func (s *guestServer) runBounded(ctx context.Context, fn func()) error { return awaitBounded(ctx, done, s.drainBound()) } -// killProcessGroup kills the command and its descendants. Both exec paths start the command -// as a group leader (Setpgid for no-TTY, Setsid for TTY), so the group id is its pid. -func killProcessGroup(cmd *exec.Cmd) error { - if err := syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL); err != nil && !errors.Is(err, syscall.ESRCH) { - return err - } - return nil -} - type execStreamWriter struct { sender *execSender cancel context.CancelFunc @@ -279,9 +276,10 @@ func (s *guestServer) executeLifecycleTTY(ctx context.Context, cancel context.Ca return fmt.Errorf("start pty: %w", err) } defer ptmx.Close() + grp := newProcessGroup(cmd) lifecycleDone := make(chan struct{}) defer close(lifecycleDone) - go killGroupOnDone(ctx, cmd, lifecycleDone) + go killGroupOnDone(ctx, grp, lifecycleDone) outputDone := make(chan struct{}) output := &execStreamWriter{sender: sender, cancel: cancel} @@ -299,5 +297,5 @@ func (s *guestServer) executeLifecycleTTY(ctx context.Context, cancel context.Ca _, _ = io.Copy(output, ptmx) }() - return s.finishExec(ctx, sender, cmd, outputDone) + return s.finishExec(ctx, sender, cmd, grp, outputDone) } diff --git a/lib/system/guest_agent/exec_lifecycle_disconnect_test.go b/lib/system/guest_agent/exec_lifecycle_disconnect_test.go index 8b9afa257..8a59edef0 100644 --- a/lib/system/guest_agent/exec_lifecycle_disconnect_test.go +++ b/lib/system/guest_agent/exec_lifecycle_disconnect_test.go @@ -72,8 +72,9 @@ func TestGuestServiceExecCancellationKillsDescendants(t *testing.T) { } } -// Once cmd.Wait has reaped the group leader, its group ID may belong to another -// process group, so a cancellation that arrives afterwards must not signal it. +// Once the group leader is reaped, its group ID may belong to another process group, +// so a cancellation that arrives afterwards must not signal it. Before reap, the same +// cancellation kills the group. func TestCancellationAfterWaitDoesNotKillGroup(t *testing.T) { group := exec.Command("sleep", "30") group.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} @@ -82,14 +83,14 @@ func TestCancellationAfterWaitDoesNotKillGroup(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() - waited := make(chan struct{}) - close(waited) - killGroupOnDone(ctx, group, waited) - require.NoError(t, syscall.Kill(group.Process.Pid, 0), "the group was signalled after Wait returned") + reaped := newProcessGroup(group) + reaped.reaped = true // stands in for reap, which would have waited for the process + killGroupOnDone(ctx, reaped, make(chan struct{})) + require.NoError(t, syscall.Kill(group.Process.Pid, 0), "the group was signalled after reap") - killGroupOnDone(ctx, group, make(chan struct{})) + killGroupOnDone(ctx, newProcessGroup(group), make(chan struct{})) err := group.Wait() var exitErr *exec.ExitError require.ErrorAs(t, err, &exitErr) - require.Equal(t, syscall.SIGKILL, exitErr.Sys().(syscall.WaitStatus).Signal(), "cancellation before Wait kills the group") + require.Equal(t, syscall.SIGKILL, exitErr.Sys().(syscall.WaitStatus).Signal(), "cancellation before reap kills the group") } diff --git a/lib/system/guest_agent/exec_lifecycle_remediation_test.go b/lib/system/guest_agent/exec_lifecycle_remediation_test.go index 11a575362..c386f8ea3 100644 --- a/lib/system/guest_agent/exec_lifecycle_remediation_test.go +++ b/lib/system/guest_agent/exec_lifecycle_remediation_test.go @@ -1,11 +1,11 @@ -//go:build linux - package main import ( "context" "os" + "os/exec" "path/filepath" + "runtime" "strconv" "strings" "syscall" @@ -17,20 +17,24 @@ import ( ) // execProcessGone reports whether pid has exited or is a zombie. A zombie is not -// running, but kill(pid, 0) still succeeds for it, so /proc is the check. +// running, but kill(pid, 0) still succeeds for it, so ps reports its state instead. func execProcessGone(pid int) bool { - data, err := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/stat") + out, err := exec.Command("ps", "-o", "stat=", "-p", strconv.Itoa(pid)).Output() if err != nil { return true } - // The state is the field after the parenthesised command name. - fields := strings.Fields(string(data[strings.LastIndexByte(string(data), ')')+1:])) - return len(fields) > 0 && fields[0] == "Z" + state := strings.TrimSpace(string(out)) + return state == "" || strings.HasPrefix(state, "Z") } // On a TTY, a timeout that fires while the exec drains output after the shell exited // must still kill the background child that holds the terminal open. func TestExecTimeoutDuringDrainKillsBackgroundChild(t *testing.T) { + if runtime.GOOS == "darwin" { + // macOS revokes the terminal when the session leader exits, so a background + // child cannot hold the drain open there. The pipe variant covers the drain. + t.Skip("terminal drain is revoked on macOS when the shell exits") + } ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() release := make(chan struct{}) @@ -77,3 +81,30 @@ func TestSendExitCodeBoundedWhenClientStopsReading(t *testing.T) { t.Fatal("sendExitCode blocked on a client that stopped reading") } } + +// The same drain on pipes, which every platform keeps open for a background child. +func TestExecTimeoutDuringPipeDrainKillsBackgroundChild(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + release := make(chan struct{}) + defer close(release) + pidFile := filepath.Join(t.TempDir(), "child") + stream := &exitCapturingExecStream{stalledExecStream: stalledExecStream{ctx: ctx, release: release, start: &pb.ExecRequest{Request: &pb.ExecRequest_Start{Start: &pb.ExecStart{ + TimeoutSeconds: 3, + Command: []string{"/bin/sh", "-c", `sleep 60 & echo $! > "$1"; exit 0`, "sh", pidFile}, + }}}}} + + started := time.Now() + require.NoError(t, (&guestServer{execLifecycle: true, drainGrace: 5 * time.Second}).Exec(stream)) + require.Less(t, time.Since(started), 7*time.Second, "the timeout must end the exec, not the drain grace") + require.NotNil(t, stream.exitCode) + require.EqualValues(t, 124, *stream.exitCode, "a timed-out exec reports 124") + + data, err := os.ReadFile(pidFile) + require.NoError(t, err, "background child pid was not written") + pid, err := strconv.Atoi(strings.TrimSpace(string(data))) + require.NoError(t, err) + t.Cleanup(func() { _ = syscall.Kill(pid, syscall.SIGKILL) }) + require.Eventually(t, func() bool { return execProcessGone(pid) }, 2*time.Second, 50*time.Millisecond, + "background child %d survived the timeout", pid) +} diff --git a/lib/system/guest_agent/exec_reap_darwin.go b/lib/system/guest_agent/exec_reap_darwin.go new file mode 100644 index 000000000..6537611f1 --- /dev/null +++ b/lib/system/guest_agent/exec_reap_darwin.go @@ -0,0 +1,42 @@ +//go:build darwin + +package main + +import ( + "errors" + + "golang.org/x/sys/unix" +) + +// waitLeaderExit blocks until pid exits, without reaping it. kqueue reports NOTE_EXIT +// when the process exits, before it is reaped, so the pid stays reserved until the +// caller reaps it. A process that has already gone is reported as exited. +func waitLeaderExit(pid int) error { + kq, err := unix.Kqueue() + if err != nil { + return err + } + defer unix.Close(kq) + change := unix.Kevent_t{ + Ident: uint64(pid), + Filter: unix.EVFILT_PROC, + Flags: unix.EV_ADD | unix.EV_ONESHOT, + Fflags: unix.NOTE_EXIT, + } + events := make([]unix.Kevent_t, 1) + for { + n, err := unix.Kevent(kq, []unix.Kevent_t{change}, events, nil) + if errors.Is(err, unix.EINTR) { + continue + } + if err != nil { + return err + } + if n > 0 && events[0].Flags&unix.EV_ERROR != 0 { + if errno := unix.Errno(events[0].Data); errno != unix.ESRCH { + return errno + } + } + return nil + } +} diff --git a/lib/system/guest_agent/exec_reap_linux.go b/lib/system/guest_agent/exec_reap_linux.go new file mode 100644 index 000000000..6563a893c --- /dev/null +++ b/lib/system/guest_agent/exec_reap_linux.go @@ -0,0 +1,17 @@ +//go:build linux + +package main + +import "golang.org/x/sys/unix" + +// waitLeaderExit blocks until pid exits, without reaping it. The pid stays reserved +// until the caller reaps it, so a process group keyed by that pid stays addressable. +func waitLeaderExit(pid int) error { + for { + var info unix.Siginfo + err := unix.Waitid(unix.P_PID, pid, &info, unix.WEXITED|unix.WNOWAIT, nil) + if err != unix.EINTR { + return err + } + } +} From e2090c3b34e2700fe1daa423a2179c9742be0613 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Wed, 21 Oct 2026 20:05:10 +0000 Subject: [PATCH 09/11] Report the process exit status on the direct exec path The shared exit-code helper reported 124 whenever the exec deadline had passed when the status was computed. On the direct (Linux) path that happens after the buffered output is sent, so a command that exited in time could report 124 if delivering its output outlasted the deadline. A timed-out command also changed from its signal status (-1) to 124. The direct path now reports the process status, as before the macOS work. The macOS lifecycle keeps reporting 124 on a deadline. --- lib/system/guest_agent/exec.go | 5 +- lib/system/guest_agent/exec_common.go | 14 ++++-- .../guest_agent/exec_direct_exit_test.go | 48 +++++++++++++++++++ 3 files changed, 61 insertions(+), 6 deletions(-) create mode 100644 lib/system/guest_agent/exec_direct_exit_test.go diff --git a/lib/system/guest_agent/exec.go b/lib/system/guest_agent/exec.go index 75aa44fa2..987618c1a 100644 --- a/lib/system/guest_agent/exec.go +++ b/lib/system/guest_agent/exec.go @@ -2,7 +2,6 @@ package main import ( "context" - "errors" "fmt" "io" "log" @@ -127,7 +126,7 @@ func (s *guestServer) executeNoTTY(ctx context.Context, stream pb.GuestService_E sendMu.Unlock() } - exitCode := execExitCode(cmd, waitErr, errors.Is(ctx.Err(), context.DeadlineExceeded)) + exitCode := processExitCode(cmd, waitErr) log.Printf("[guest-agent] command finished with exit code: %d", exitCode) @@ -193,7 +192,7 @@ func (s *guestServer) executeTTY(ctx context.Context, stream pb.GuestService_Exe // Wait for all output to be sent wg.Wait() - exitCode := execExitCode(cmd, waitErr, errors.Is(ctx.Err(), context.DeadlineExceeded)) + exitCode := processExitCode(cmd, waitErr) log.Printf("[guest-agent] TTY command finished with exit code: %d", exitCode) diff --git a/lib/system/guest_agent/exec_common.go b/lib/system/guest_agent/exec_common.go index ba081c87d..00cc73c74 100644 --- a/lib/system/guest_agent/exec_common.go +++ b/lib/system/guest_agent/exec_common.go @@ -62,13 +62,21 @@ func pumpExecInput(stream pb.GuestService_ExecServer, onStdin func([]byte), onRe }() } -// execExitCode is the status a client sees. A deadline always reports 124, the GNU -// timeout convention, even when the command exited during the drain. Otherwise the -// process exit code is used, and a command that could not be waited for reports 124. +// execExitCode is the status the macOS lifecycle reports. A deadline always reports +// 124, the GNU timeout convention, even when the command exited during the drain. +// Otherwise it is the process exit status. func execExitCode(cmd *exec.Cmd, waitErr error, timedOut bool) int32 { if timedOut { return 124 } + return processExitCode(cmd, waitErr) +} + +// processExitCode is the status the direct (Linux) path reports: the process exit +// code, which is -1 for a command a timeout killed, and 124 only for a command that +// could not be waited for. The deadline is not consulted, so a command that finished +// in time keeps its code even if sending its output outlasts the deadline. +func processExitCode(cmd *exec.Cmd, waitErr error) int32 { if cmd.ProcessState != nil { return int32(cmd.ProcessState.ExitCode()) } diff --git a/lib/system/guest_agent/exec_direct_exit_test.go b/lib/system/guest_agent/exec_direct_exit_test.go new file mode 100644 index 000000000..59bb41d6f --- /dev/null +++ b/lib/system/guest_agent/exec_direct_exit_test.go @@ -0,0 +1,48 @@ +package main + +import ( + "context" + "testing" + "time" + + pb "github.com/kernel/hypeman/lib/guest" + "github.com/stretchr/testify/require" +) + +// slowOutputExecStream delays each output send, so delivering output can outlast an +// exec deadline after the command has already exited. It records the exit code. +type slowOutputExecStream struct { + stalledExecStream + delay time.Duration + exitCode *int32 +} + +func (s *slowOutputExecStream) Send(response *pb.ExecResponse) error { + if _, ok := response.Response.(*pb.ExecResponse_ExitCode); ok { + value := response.GetExitCode() + s.exitCode = &value + return nil + } + time.Sleep(s.delay) + return nil +} + +// The direct (Linux) exec path reports the process status. A command that exits in +// time keeps its code even if delivering its output outlasts the deadline, and a +// command a timeout kills reports -1, as before the macOS work. +func TestDirectExecReportsProcessExitCode(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + for _, tty := range []bool{false, true} { + run := func(command string, timeout int32, delay time.Duration) int32 { + stream := &slowOutputExecStream{delay: delay, stalledExecStream: stalledExecStream{ctx: ctx, start: &pb.ExecRequest{Request: &pb.ExecRequest_Start{Start: &pb.ExecStart{ + Tty: tty, TimeoutSeconds: timeout, Command: []string{"/bin/sh", "-c", command}, + }}}}} + require.NoError(t, (&guestServer{}).Exec(stream)) + require.NotNil(t, stream.exitCode, "tty=%v: exit status must be reported", tty) + return *stream.exitCode + } + require.EqualValues(t, 3, run("echo done; exit 3", 1, 1500*time.Millisecond), "tty=%v: slow output delivery must not turn an exit into 124", tty) + require.EqualValues(t, -1, run("exec sleep 30", 1, 0), "tty=%v: a timed-out command reports its signal status", tty) + } +} From 23a38e6efdb23dd98d40980804a8ff66833bce83 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Wed, 21 Oct 2026 20:12:10 +0000 Subject: [PATCH 10/11] Report guest agent version information to the host The macOS agent ships inside its image, so it can be older or newer than the host. A new GetAgentInfo RPC reports the agent's protocol version, build version and optional capabilities. The host calls it when a macOS agent first answers and records the reply on the instance. An agent that predates the RPC answers Unimplemented and is recorded as protocol 0. An agent newer than the host's protocol is refused, so the guest stays Initializing with that reason. Linux guests run the agent embedded in hypeman's initrd, so the host does not call GetAgentInfo for them. --- docs/macos-guest-agent.md | 24 +++ lib/guest/agent_info_test.go | 55 +++++ lib/guest/client.go | 19 ++ lib/guest/guest.pb.go | 272 ++++++++++++++++++++----- lib/guest/guest.proto | 23 +++ lib/guest/guest_grpc.pb.go | 42 ++++ lib/instances/macos_agent_info.go | 59 ++++++ lib/instances/macos_test.go | 48 ++++- lib/instances/manager.go | 5 + lib/instances/query.go | 9 + lib/instances/start.go | 1 + lib/instances/stop.go | 1 + lib/instances/types.go | 3 + lib/system/guest_agent/agent_info.go | 26 +++ lib/system/guest_agent/service_test.go | 9 + 15 files changed, 537 insertions(+), 59 deletions(-) create mode 100644 lib/guest/agent_info_test.go create mode 100644 lib/instances/macos_agent_info.go create mode 100644 lib/system/guest_agent/agent_info.go diff --git a/docs/macos-guest-agent.md b/docs/macos-guest-agent.md index 219b14b9a..de98a9fcf 100644 --- a/docs/macos-guest-agent.md +++ b/docs/macos-guest-agent.md @@ -13,6 +13,7 @@ Build on a macOS development machine with the macOS SDK and cgo enabled: ```sh CGO_ENABLED=1 GOOS=darwin GOARCH=arm64 go build \ + -ldflags "-X main.agentVersion=$(git describe --always --dirty)" \ -o guest-agent-darwin-arm64 ./lib/system/guest_agent ``` @@ -61,6 +62,29 @@ agent shutdown/probes. The declaration is an image capability claim, not a verified live guest handshake or a security credential. Runtime connectivity and privileged provisioning still require the validation below. +## Versioning + +The macOS agent ships inside the image, so an instance can run an agent older or +newer than the host. When the agent first answers, the host calls `GetAgentInfo` +and records the reply on the instance: + +- **Protocol version** changes only for a change an older host cannot handle. The + host refuses an agent with a newer protocol than it supports; the guest stays + `Initializing` with that reason until the startup deadline stops it. +- **Capabilities** list optional features. An operation that needs one returns + 501 naming the missing capability, rather than failing at the transport. +- **Version** is the build string set with `-X main.agentVersion`, for diagnostics. + +An agent that predates `GetAgentInfo` answers `Unimplemented` and is recorded as +protocol 0 with no optional capabilities. It keeps exec, copy, stat and shutdown. + +To upgrade the agent, build a new image with the new agent and create instances +from it; existing instances keep the agent their disk holds. To roll back, create +instances from the previous image. The host checks the agent on every boot, so a +guest whose agent was replaced in place is rechecked at its next start. Linux +guests get the agent embedded in hypeman's initrd, so it always matches the host +at boot; the host does not call `GetAgentInfo` for them. + ## OS-specific operations - **Shutdown:** root-only `/sbin/shutdown -h now`, not a signal to launchd/PID 1. diff --git a/lib/guest/agent_info_test.go b/lib/guest/agent_info_test.go new file mode 100644 index 000000000..49a2a1734 --- /dev/null +++ b/lib/guest/agent_info_test.go @@ -0,0 +1,55 @@ +package guest + +import ( + "context" + "net" + "testing" + "time" + + "github.com/kernel/hypeman/lib/hypervisor" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/test/bufconn" +) + +// bufconnDialer reaches an in-process agent. +type bufconnDialer struct{ listener *bufconn.Listener } + +func (d bufconnDialer) Key() string { return "agent-info-test" } +func (d bufconnDialer) DialVsock(ctx context.Context, _ int) (net.Conn, error) { + return d.listener.DialContext(ctx) +} + +type infoAgent struct { + UnimplementedGuestServiceServer + info *GetAgentInfoResponse +} + +func (a infoAgent) GetAgentInfo(context.Context, *GetAgentInfoRequest) (*GetAgentInfoResponse, error) { + return a.info, nil +} + +func serveAgent(t *testing.T, agent GuestServiceServer) hypervisor.VsockDialer { + t.Helper() + listener := bufconn.Listen(1 << 16) + server := grpc.NewServer() + RegisterGuestServiceServer(server, agent) + go server.Serve(listener) + t.Cleanup(server.Stop) + return bufconnDialer{listener: listener} +} + +func TestGetAgentInfo(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + info, err := GetAgentInfo(ctx, serveAgent(t, infoAgent{info: &GetAgentInfoResponse{ProtocolVersion: 1, Version: "v1.2.3"}})) + require.NoError(t, err) + require.EqualValues(t, 1, info.ProtocolVersion) + require.Equal(t, "v1.2.3", info.Version) + + info, err = GetAgentInfo(ctx, serveAgent(t, &UnimplementedGuestServiceServer{})) + require.NoError(t, err, "an agent that predates the RPC is not an error") + require.Zero(t, info.ProtocolVersion, "it reports protocol 0") + require.Empty(t, info.Capabilities) +} diff --git a/lib/guest/client.go b/lib/guest/client.go index c22a69374..3241be6af 100644 --- a/lib/guest/client.go +++ b/lib/guest/client.go @@ -1003,6 +1003,25 @@ func GetGPUInitStatus(ctx context.Context, dialer hypervisor.VsockDialer) (GPUIn return resp.State, resp.FailureMessage, nil } +// GetAgentInfo asks the agent for its protocol version and capabilities. It dials its +// own connection, as readiness probes do. An agent that predates the RPC is reported +// as protocol 0 with no optional capabilities. +func GetAgentInfo(ctx context.Context, dialer hypervisor.VsockDialer) (*GetAgentInfoResponse, error) { + conn, err := dialGuestConn(ctx, dialer) + if err != nil { + return nil, err + } + defer conn.Close() + info, err := NewGuestServiceClient(conn).GetAgentInfo(ctx, &GetAgentInfoRequest{}) + if status.Code(err) == codes.Unimplemented { + return &GetAgentInfoResponse{}, nil + } + if err != nil { + return nil, fmt.Errorf("agent info RPC: %w", err) + } + return info, nil +} + // ShutdownInstance sends a shutdown signal to the guest VM's init process (PID 1). // The guest-agent forwards the signal to init, which forwards it to the entrypoint. // sig is the signal number to send (0 = SIGTERM default). diff --git a/lib/guest/guest.pb.go b/lib/guest/guest.pb.go index f1b1156a4..a5970eb34 100644 --- a/lib/guest/guest.pb.go +++ b/lib/guest/guest.pb.go @@ -71,6 +71,51 @@ func (GPUInitState) EnumDescriptor() ([]byte, []int) { return file_lib_guest_guest_proto_rawDescGZIP(), []int{0} } +// AgentCapability names an optional agent feature. Operations every agent supports, +// such as exec, copy, stat and shutdown, are not listed. +type AgentCapability int32 + +const ( + AgentCapability_AGENT_CAPABILITY_UNSPECIFIED AgentCapability = 0 +) + +// Enum value maps for AgentCapability. +var ( + AgentCapability_name = map[int32]string{ + 0: "AGENT_CAPABILITY_UNSPECIFIED", + } + AgentCapability_value = map[string]int32{ + "AGENT_CAPABILITY_UNSPECIFIED": 0, + } +) + +func (x AgentCapability) Enum() *AgentCapability { + p := new(AgentCapability) + *p = x + return p +} + +func (x AgentCapability) String() string { + return protoimpl.X.EnumStringOf(x.Descriptor(), protoreflect.EnumNumber(x)) +} + +func (AgentCapability) Descriptor() protoreflect.EnumDescriptor { + return file_lib_guest_guest_proto_enumTypes[1].Descriptor() +} + +func (AgentCapability) Type() protoreflect.EnumType { + return &file_lib_guest_guest_proto_enumTypes[1] +} + +func (x AgentCapability) Number() protoreflect.EnumNumber { + return protoreflect.EnumNumber(x) +} + +// Deprecated: Use AgentCapability.Descriptor instead. +func (AgentCapability) EnumDescriptor() ([]byte, []int) { + return file_lib_guest_guest_proto_rawDescGZIP(), []int{1} +} + // ExecRequest represents messages from client to server type ExecRequest struct { state protoimpl.MessageState `protogen:"open.v1"` @@ -1521,6 +1566,107 @@ func (x *GetGPUInitStatusResponse) GetFailureMessage() string { return "" } +// GetAgentInfoRequest requests the agent's version information +type GetAgentInfoRequest struct { + state protoimpl.MessageState `protogen:"open.v1"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *GetAgentInfoRequest) Reset() { + *x = GetAgentInfoRequest{} + mi := &file_lib_guest_guest_proto_msgTypes[21] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *GetAgentInfoRequest) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*GetAgentInfoRequest) ProtoMessage() {} + +func (x *GetAgentInfoRequest) ProtoReflect() protoreflect.Message { + mi := &file_lib_guest_guest_proto_msgTypes[21] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use GetAgentInfoRequest.ProtoReflect.Descriptor instead. +func (*GetAgentInfoRequest) Descriptor() ([]byte, []int) { + return file_lib_guest_guest_proto_rawDescGZIP(), []int{21} +} + +// GetAgentInfoResponse describes the running agent +type GetAgentInfoResponse struct { + state protoimpl.MessageState `protogen:"open.v1"` + // Incremented only for changes an older host cannot handle; additive changes + // are reported as capabilities instead + ProtocolVersion uint32 `protobuf:"varint,1,opt,name=protocol_version,json=protocolVersion,proto3" json:"protocol_version,omitempty"` + // Build version, for diagnostics only + Version string `protobuf:"bytes,2,opt,name=version,proto3" json:"version,omitempty"` + Capabilities []AgentCapability `protobuf:"varint,3,rep,packed,name=capabilities,proto3,enum=guest.AgentCapability" json:"capabilities,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *GetAgentInfoResponse) Reset() { + *x = GetAgentInfoResponse{} + mi := &file_lib_guest_guest_proto_msgTypes[22] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *GetAgentInfoResponse) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*GetAgentInfoResponse) ProtoMessage() {} + +func (x *GetAgentInfoResponse) ProtoReflect() protoreflect.Message { + mi := &file_lib_guest_guest_proto_msgTypes[22] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use GetAgentInfoResponse.ProtoReflect.Descriptor instead. +func (*GetAgentInfoResponse) Descriptor() ([]byte, []int) { + return file_lib_guest_guest_proto_rawDescGZIP(), []int{22} +} + +func (x *GetAgentInfoResponse) GetProtocolVersion() uint32 { + if x != nil { + return x.ProtocolVersion + } + return 0 +} + +func (x *GetAgentInfoResponse) GetVersion() string { + if x != nil { + return x.Version + } + return "" +} + +func (x *GetAgentInfoResponse) GetCapabilities() []AgentCapability { + if x != nil { + return x.Capabilities + } + return nil +} + var File_lib_guest_guest_proto protoreflect.FileDescriptor const file_lib_guest_guest_proto_rawDesc = "" + @@ -1624,11 +1770,18 @@ const file_lib_guest_guest_proto_rawDesc = "" + "\x17GetGPUInitStatusRequest\"n\n" + "\x18GetGPUInitStatusResponse\x12)\n" + "\x05state\x18\x01 \x01(\x0e2\x13.guest.GPUInitStateR\x05state\x12'\n" + - "\x0ffailure_message\x18\x02 \x01(\tR\x0efailureMessage*\\\n" + + "\x0ffailure_message\x18\x02 \x01(\tR\x0efailureMessage\"\x15\n" + + "\x13GetAgentInfoRequest\"\x97\x01\n" + + "\x14GetAgentInfoResponse\x12)\n" + + "\x10protocol_version\x18\x01 \x01(\rR\x0fprotocolVersion\x12\x18\n" + + "\aversion\x18\x02 \x01(\tR\aversion\x12:\n" + + "\fcapabilities\x18\x03 \x03(\x0e2\x16.guest.AgentCapabilityR\fcapabilities*\\\n" + "\fGPUInitState\x12\x1a\n" + "\x16GPU_INIT_STATE_UNKNOWN\x10\x00\x12\x15\n" + "\x11GPU_INIT_STATE_OK\x10\x01\x12\x19\n" + - "\x15GPU_INIT_STATE_FAILED\x10\x022\x83\x04\n" + + "\x15GPU_INIT_STATE_FAILED\x10\x02*3\n" + + "\x0fAgentCapability\x12 \n" + + "\x1cAGENT_CAPABILITY_UNSPECIFIED\x10\x002\xcc\x04\n" + "\fGuestService\x123\n" + "\x04Exec\x12\x12.guest.ExecRequest\x1a\x13.guest.ExecResponse(\x010\x01\x12F\n" + "\vCopyToGuest\x12\x19.guest.CopyToGuestRequest\x1a\x1a.guest.CopyToGuestResponse(\x01\x12L\n" + @@ -1636,7 +1789,8 @@ const file_lib_guest_guest_proto_rawDesc = "" + "\bStatPath\x12\x16.guest.StatPathRequest\x1a\x17.guest.StatPathResponse\x12;\n" + "\bShutdown\x12\x16.guest.ShutdownRequest\x1a\x17.guest.ShutdownResponse\x12Y\n" + "\x12ReconfigureNetwork\x12 .guest.ReconfigureNetworkRequest\x1a!.guest.ReconfigureNetworkResponse\x12S\n" + - "\x10GetGPUInitStatus\x12\x1e.guest.GetGPUInitStatusRequest\x1a\x1f.guest.GetGPUInitStatusResponseB'Z%github.com/onkernel/hypeman/lib/guestb\x06proto3" + "\x10GetGPUInitStatus\x12\x1e.guest.GetGPUInitStatusRequest\x1a\x1f.guest.GetGPUInitStatusResponse\x12G\n" + + "\fGetAgentInfo\x12\x1a.guest.GetAgentInfoRequest\x1a\x1b.guest.GetAgentInfoResponseB'Z%github.com/onkernel/hypeman/lib/guestb\x06proto3" var ( file_lib_guest_guest_proto_rawDescOnce sync.Once @@ -1650,62 +1804,68 @@ func file_lib_guest_guest_proto_rawDescGZIP() []byte { return file_lib_guest_guest_proto_rawDescData } -var file_lib_guest_guest_proto_enumTypes = make([]protoimpl.EnumInfo, 1) -var file_lib_guest_guest_proto_msgTypes = make([]protoimpl.MessageInfo, 22) +var file_lib_guest_guest_proto_enumTypes = make([]protoimpl.EnumInfo, 2) +var file_lib_guest_guest_proto_msgTypes = make([]protoimpl.MessageInfo, 24) var file_lib_guest_guest_proto_goTypes = []any{ (GPUInitState)(0), // 0: guest.GPUInitState - (*ExecRequest)(nil), // 1: guest.ExecRequest - (*ExecStart)(nil), // 2: guest.ExecStart - (*WindowSize)(nil), // 3: guest.WindowSize - (*ExecResponse)(nil), // 4: guest.ExecResponse - (*CopyToGuestRequest)(nil), // 5: guest.CopyToGuestRequest - (*CopyToGuestStart)(nil), // 6: guest.CopyToGuestStart - (*CopyToGuestEnd)(nil), // 7: guest.CopyToGuestEnd - (*CopyToGuestResponse)(nil), // 8: guest.CopyToGuestResponse - (*CopyFromGuestRequest)(nil), // 9: guest.CopyFromGuestRequest - (*CopyFromGuestResponse)(nil), // 10: guest.CopyFromGuestResponse - (*CopyFromGuestHeader)(nil), // 11: guest.CopyFromGuestHeader - (*CopyFromGuestEnd)(nil), // 12: guest.CopyFromGuestEnd - (*CopyFromGuestError)(nil), // 13: guest.CopyFromGuestError - (*StatPathRequest)(nil), // 14: guest.StatPathRequest - (*StatPathResponse)(nil), // 15: guest.StatPathResponse - (*ShutdownRequest)(nil), // 16: guest.ShutdownRequest - (*ShutdownResponse)(nil), // 17: guest.ShutdownResponse - (*ReconfigureNetworkRequest)(nil), // 18: guest.ReconfigureNetworkRequest - (*ReconfigureNetworkResponse)(nil), // 19: guest.ReconfigureNetworkResponse - (*GetGPUInitStatusRequest)(nil), // 20: guest.GetGPUInitStatusRequest - (*GetGPUInitStatusResponse)(nil), // 21: guest.GetGPUInitStatusResponse - nil, // 22: guest.ExecStart.EnvEntry + (AgentCapability)(0), // 1: guest.AgentCapability + (*ExecRequest)(nil), // 2: guest.ExecRequest + (*ExecStart)(nil), // 3: guest.ExecStart + (*WindowSize)(nil), // 4: guest.WindowSize + (*ExecResponse)(nil), // 5: guest.ExecResponse + (*CopyToGuestRequest)(nil), // 6: guest.CopyToGuestRequest + (*CopyToGuestStart)(nil), // 7: guest.CopyToGuestStart + (*CopyToGuestEnd)(nil), // 8: guest.CopyToGuestEnd + (*CopyToGuestResponse)(nil), // 9: guest.CopyToGuestResponse + (*CopyFromGuestRequest)(nil), // 10: guest.CopyFromGuestRequest + (*CopyFromGuestResponse)(nil), // 11: guest.CopyFromGuestResponse + (*CopyFromGuestHeader)(nil), // 12: guest.CopyFromGuestHeader + (*CopyFromGuestEnd)(nil), // 13: guest.CopyFromGuestEnd + (*CopyFromGuestError)(nil), // 14: guest.CopyFromGuestError + (*StatPathRequest)(nil), // 15: guest.StatPathRequest + (*StatPathResponse)(nil), // 16: guest.StatPathResponse + (*ShutdownRequest)(nil), // 17: guest.ShutdownRequest + (*ShutdownResponse)(nil), // 18: guest.ShutdownResponse + (*ReconfigureNetworkRequest)(nil), // 19: guest.ReconfigureNetworkRequest + (*ReconfigureNetworkResponse)(nil), // 20: guest.ReconfigureNetworkResponse + (*GetGPUInitStatusRequest)(nil), // 21: guest.GetGPUInitStatusRequest + (*GetGPUInitStatusResponse)(nil), // 22: guest.GetGPUInitStatusResponse + (*GetAgentInfoRequest)(nil), // 23: guest.GetAgentInfoRequest + (*GetAgentInfoResponse)(nil), // 24: guest.GetAgentInfoResponse + nil, // 25: guest.ExecStart.EnvEntry } var file_lib_guest_guest_proto_depIdxs = []int32{ - 2, // 0: guest.ExecRequest.start:type_name -> guest.ExecStart - 3, // 1: guest.ExecRequest.resize:type_name -> guest.WindowSize - 22, // 2: guest.ExecStart.env:type_name -> guest.ExecStart.EnvEntry - 6, // 3: guest.CopyToGuestRequest.start:type_name -> guest.CopyToGuestStart - 7, // 4: guest.CopyToGuestRequest.end:type_name -> guest.CopyToGuestEnd - 11, // 5: guest.CopyFromGuestResponse.header:type_name -> guest.CopyFromGuestHeader - 12, // 6: guest.CopyFromGuestResponse.end:type_name -> guest.CopyFromGuestEnd - 13, // 7: guest.CopyFromGuestResponse.error:type_name -> guest.CopyFromGuestError + 3, // 0: guest.ExecRequest.start:type_name -> guest.ExecStart + 4, // 1: guest.ExecRequest.resize:type_name -> guest.WindowSize + 25, // 2: guest.ExecStart.env:type_name -> guest.ExecStart.EnvEntry + 7, // 3: guest.CopyToGuestRequest.start:type_name -> guest.CopyToGuestStart + 8, // 4: guest.CopyToGuestRequest.end:type_name -> guest.CopyToGuestEnd + 12, // 5: guest.CopyFromGuestResponse.header:type_name -> guest.CopyFromGuestHeader + 13, // 6: guest.CopyFromGuestResponse.end:type_name -> guest.CopyFromGuestEnd + 14, // 7: guest.CopyFromGuestResponse.error:type_name -> guest.CopyFromGuestError 0, // 8: guest.GetGPUInitStatusResponse.state:type_name -> guest.GPUInitState - 1, // 9: guest.GuestService.Exec:input_type -> guest.ExecRequest - 5, // 10: guest.GuestService.CopyToGuest:input_type -> guest.CopyToGuestRequest - 9, // 11: guest.GuestService.CopyFromGuest:input_type -> guest.CopyFromGuestRequest - 14, // 12: guest.GuestService.StatPath:input_type -> guest.StatPathRequest - 16, // 13: guest.GuestService.Shutdown:input_type -> guest.ShutdownRequest - 18, // 14: guest.GuestService.ReconfigureNetwork:input_type -> guest.ReconfigureNetworkRequest - 20, // 15: guest.GuestService.GetGPUInitStatus:input_type -> guest.GetGPUInitStatusRequest - 4, // 16: guest.GuestService.Exec:output_type -> guest.ExecResponse - 8, // 17: guest.GuestService.CopyToGuest:output_type -> guest.CopyToGuestResponse - 10, // 18: guest.GuestService.CopyFromGuest:output_type -> guest.CopyFromGuestResponse - 15, // 19: guest.GuestService.StatPath:output_type -> guest.StatPathResponse - 17, // 20: guest.GuestService.Shutdown:output_type -> guest.ShutdownResponse - 19, // 21: guest.GuestService.ReconfigureNetwork:output_type -> guest.ReconfigureNetworkResponse - 21, // 22: guest.GuestService.GetGPUInitStatus:output_type -> guest.GetGPUInitStatusResponse - 16, // [16:23] is the sub-list for method output_type - 9, // [9:16] is the sub-list for method input_type - 9, // [9:9] is the sub-list for extension type_name - 9, // [9:9] is the sub-list for extension extendee - 0, // [0:9] is the sub-list for field type_name + 1, // 9: guest.GetAgentInfoResponse.capabilities:type_name -> guest.AgentCapability + 2, // 10: guest.GuestService.Exec:input_type -> guest.ExecRequest + 6, // 11: guest.GuestService.CopyToGuest:input_type -> guest.CopyToGuestRequest + 10, // 12: guest.GuestService.CopyFromGuest:input_type -> guest.CopyFromGuestRequest + 15, // 13: guest.GuestService.StatPath:input_type -> guest.StatPathRequest + 17, // 14: guest.GuestService.Shutdown:input_type -> guest.ShutdownRequest + 19, // 15: guest.GuestService.ReconfigureNetwork:input_type -> guest.ReconfigureNetworkRequest + 21, // 16: guest.GuestService.GetGPUInitStatus:input_type -> guest.GetGPUInitStatusRequest + 23, // 17: guest.GuestService.GetAgentInfo:input_type -> guest.GetAgentInfoRequest + 5, // 18: guest.GuestService.Exec:output_type -> guest.ExecResponse + 9, // 19: guest.GuestService.CopyToGuest:output_type -> guest.CopyToGuestResponse + 11, // 20: guest.GuestService.CopyFromGuest:output_type -> guest.CopyFromGuestResponse + 16, // 21: guest.GuestService.StatPath:output_type -> guest.StatPathResponse + 18, // 22: guest.GuestService.Shutdown:output_type -> guest.ShutdownResponse + 20, // 23: guest.GuestService.ReconfigureNetwork:output_type -> guest.ReconfigureNetworkResponse + 22, // 24: guest.GuestService.GetGPUInitStatus:output_type -> guest.GetGPUInitStatusResponse + 24, // 25: guest.GuestService.GetAgentInfo:output_type -> guest.GetAgentInfoResponse + 18, // [18:26] is the sub-list for method output_type + 10, // [10:18] is the sub-list for method input_type + 10, // [10:10] is the sub-list for extension type_name + 10, // [10:10] is the sub-list for extension extendee + 0, // [0:10] is the sub-list for field type_name } func init() { file_lib_guest_guest_proto_init() } @@ -1739,8 +1899,8 @@ func file_lib_guest_guest_proto_init() { File: protoimpl.DescBuilder{ GoPackagePath: reflect.TypeOf(x{}).PkgPath(), RawDescriptor: unsafe.Slice(unsafe.StringData(file_lib_guest_guest_proto_rawDesc), len(file_lib_guest_guest_proto_rawDesc)), - NumEnums: 1, - NumMessages: 22, + NumEnums: 2, + NumMessages: 24, NumExtensions: 0, NumServices: 1, }, diff --git a/lib/guest/guest.proto b/lib/guest/guest.proto index a450665d1..9b59e4a4a 100644 --- a/lib/guest/guest.proto +++ b/lib/guest/guest.proto @@ -26,6 +26,10 @@ service GuestService { // GetGPUInitStatus reports the GPU driver init state observed by the guest agent rpc GetGPUInitStatus(GetGPUInitStatusRequest) returns (GetGPUInitStatusResponse); + + // GetAgentInfo reports the agent's protocol version and optional capabilities. + // Agents that predate it answer Unimplemented, which the host treats as protocol 0. + rpc GetAgentInfo(GetAgentInfoRequest) returns (GetAgentInfoResponse); } // ExecRequest represents messages from client to server @@ -189,3 +193,22 @@ message GetGPUInitStatusResponse { // Most recent NVRM init-failure line observed in kmsg; set when state is FAILED string failure_message = 2; } + +// AgentCapability names an optional agent feature. Operations every agent supports, +// such as exec, copy, stat and shutdown, are not listed. +enum AgentCapability { + AGENT_CAPABILITY_UNSPECIFIED = 0; +} + +// GetAgentInfoRequest requests the agent's version information +message GetAgentInfoRequest {} + +// GetAgentInfoResponse describes the running agent +message GetAgentInfoResponse { + // Incremented only for changes an older host cannot handle; additive changes + // are reported as capabilities instead + uint32 protocol_version = 1; + // Build version, for diagnostics only + string version = 2; + repeated AgentCapability capabilities = 3; +} diff --git a/lib/guest/guest_grpc.pb.go b/lib/guest/guest_grpc.pb.go index 630a0bd36..4db469a32 100644 --- a/lib/guest/guest_grpc.pb.go +++ b/lib/guest/guest_grpc.pb.go @@ -26,6 +26,7 @@ const ( GuestService_Shutdown_FullMethodName = "/guest.GuestService/Shutdown" GuestService_ReconfigureNetwork_FullMethodName = "/guest.GuestService/ReconfigureNetwork" GuestService_GetGPUInitStatus_FullMethodName = "/guest.GuestService/GetGPUInitStatus" + GuestService_GetAgentInfo_FullMethodName = "/guest.GuestService/GetAgentInfo" ) // GuestServiceClient is the client API for GuestService service. @@ -48,6 +49,9 @@ type GuestServiceClient interface { ReconfigureNetwork(ctx context.Context, in *ReconfigureNetworkRequest, opts ...grpc.CallOption) (*ReconfigureNetworkResponse, error) // GetGPUInitStatus reports the GPU driver init state observed by the guest agent GetGPUInitStatus(ctx context.Context, in *GetGPUInitStatusRequest, opts ...grpc.CallOption) (*GetGPUInitStatusResponse, error) + // GetAgentInfo reports the agent's protocol version and optional capabilities. + // Agents that predate it answer Unimplemented, which the host treats as protocol 0. + GetAgentInfo(ctx context.Context, in *GetAgentInfoRequest, opts ...grpc.CallOption) (*GetAgentInfoResponse, error) } type guestServiceClient struct { @@ -143,6 +147,16 @@ func (c *guestServiceClient) GetGPUInitStatus(ctx context.Context, in *GetGPUIni return out, nil } +func (c *guestServiceClient) GetAgentInfo(ctx context.Context, in *GetAgentInfoRequest, opts ...grpc.CallOption) (*GetAgentInfoResponse, error) { + cOpts := append([]grpc.CallOption{grpc.StaticMethod()}, opts...) + out := new(GetAgentInfoResponse) + err := c.cc.Invoke(ctx, GuestService_GetAgentInfo_FullMethodName, in, out, cOpts...) + if err != nil { + return nil, err + } + return out, nil +} + // GuestServiceServer is the server API for GuestService service. // All implementations must embed UnimplementedGuestServiceServer // for forward compatibility. @@ -163,6 +177,9 @@ type GuestServiceServer interface { ReconfigureNetwork(context.Context, *ReconfigureNetworkRequest) (*ReconfigureNetworkResponse, error) // GetGPUInitStatus reports the GPU driver init state observed by the guest agent GetGPUInitStatus(context.Context, *GetGPUInitStatusRequest) (*GetGPUInitStatusResponse, error) + // GetAgentInfo reports the agent's protocol version and optional capabilities. + // Agents that predate it answer Unimplemented, which the host treats as protocol 0. + GetAgentInfo(context.Context, *GetAgentInfoRequest) (*GetAgentInfoResponse, error) mustEmbedUnimplementedGuestServiceServer() } @@ -194,6 +211,9 @@ func (UnimplementedGuestServiceServer) ReconfigureNetwork(context.Context, *Reco func (UnimplementedGuestServiceServer) GetGPUInitStatus(context.Context, *GetGPUInitStatusRequest) (*GetGPUInitStatusResponse, error) { return nil, status.Error(codes.Unimplemented, "method GetGPUInitStatus not implemented") } +func (UnimplementedGuestServiceServer) GetAgentInfo(context.Context, *GetAgentInfoRequest) (*GetAgentInfoResponse, error) { + return nil, status.Error(codes.Unimplemented, "method GetAgentInfo not implemented") +} func (UnimplementedGuestServiceServer) mustEmbedUnimplementedGuestServiceServer() {} func (UnimplementedGuestServiceServer) testEmbeddedByValue() {} @@ -312,6 +332,24 @@ func _GuestService_GetGPUInitStatus_Handler(srv interface{}, ctx context.Context return interceptor(ctx, in, info, handler) } +func _GuestService_GetAgentInfo_Handler(srv interface{}, ctx context.Context, dec func(interface{}) error, interceptor grpc.UnaryServerInterceptor) (interface{}, error) { + in := new(GetAgentInfoRequest) + if err := dec(in); err != nil { + return nil, err + } + if interceptor == nil { + return srv.(GuestServiceServer).GetAgentInfo(ctx, in) + } + info := &grpc.UnaryServerInfo{ + Server: srv, + FullMethod: GuestService_GetAgentInfo_FullMethodName, + } + handler := func(ctx context.Context, req interface{}) (interface{}, error) { + return srv.(GuestServiceServer).GetAgentInfo(ctx, req.(*GetAgentInfoRequest)) + } + return interceptor(ctx, in, info, handler) +} + // GuestService_ServiceDesc is the grpc.ServiceDesc for GuestService service. // It's only intended for direct use with grpc.RegisterService, // and not to be introspected or modified (even as a copy) @@ -335,6 +373,10 @@ var GuestService_ServiceDesc = grpc.ServiceDesc{ MethodName: "GetGPUInitStatus", Handler: _GuestService_GetGPUInitStatus_Handler, }, + { + MethodName: "GetAgentInfo", + Handler: _GuestService_GetAgentInfo_Handler, + }, }, Streams: []grpc.StreamDesc{ { diff --git a/lib/instances/macos_agent_info.go b/lib/instances/macos_agent_info.go new file mode 100644 index 000000000..b9ce60659 --- /dev/null +++ b/lib/instances/macos_agent_info.go @@ -0,0 +1,59 @@ +package instances + +import ( + "context" + "fmt" + "slices" + + "github.com/kernel/hypeman/lib/guest" + "github.com/kernel/hypeman/lib/hypervisor" +) + +// maxGuestAgentProtocol is the newest agent protocol this host understands. A macOS +// agent ships inside its image, so it can be older or newer than the host. +const maxGuestAgentProtocol = 1 + +// GuestAgentInfo is what a macOS guest's agent reported when it became ready. +// Protocol 0 is an agent that predates GetAgentInfo. +type GuestAgentInfo struct { + ProtocolVersion uint32 + Version string `json:",omitempty"` + Capabilities []string `json:",omitempty"` +} + +// HasCapability reports whether the agent advertised an optional capability. +func (i *GuestAgentInfo) HasCapability(c guest.AgentCapability) bool { + return i != nil && slices.Contains(i.Capabilities, c.String()) +} + +// macOSGuestAgentInfo asks a ready macOS agent what it supports. An agent speaking a +// newer protocol than this host is refused, so the guest stays Initializing with that +// reason instead of being driven with requests it may not honor. +func (m *manager) macOSGuestAgentInfo(ctx context.Context, stored *StoredMetadata) (*GuestAgentInfo, error) { + fetch := m.guestAgentInfoProbe + if fetch == nil { + fetch = fetchGuestAgentInfo + } + resp, err := fetch(ctx, stored) + if err != nil { + return nil, fmt.Errorf("read guest agent info: %w", err) + } + if resp.ProtocolVersion > maxGuestAgentProtocol { + return nil, fmt.Errorf("guest agent %q speaks protocol %d; this host supports up to %d", resp.Version, resp.ProtocolVersion, maxGuestAgentProtocol) + } + info := &GuestAgentInfo{ProtocolVersion: resp.ProtocolVersion, Version: resp.Version} + for _, c := range resp.Capabilities { + info.Capabilities = append(info.Capabilities, c.String()) + } + return info, nil +} + +func fetchGuestAgentInfo(ctx context.Context, stored *StoredMetadata) (*guest.GetAgentInfoResponse, error) { + dialer, err := hypervisor.NewVsockDialer(stored.HypervisorType, stored.VsockSocket, stored.VsockCID) + if err != nil { + return nil, err + } + ctx, cancel := context.WithTimeout(ctx, guestAgentReadyProbeTimeout) + defer cancel() + return guest.GetAgentInfo(ctx, dialer) +} diff --git a/lib/instances/macos_test.go b/lib/instances/macos_test.go index 2684dbbdb..5c3479871 100644 --- a/lib/instances/macos_test.go +++ b/lib/instances/macos_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/kernel/hypeman/lib/guest" "github.com/kernel/hypeman/lib/hypervisor" "github.com/kernel/hypeman/lib/images" "github.com/kernel/hypeman/lib/instances/phasetracking" @@ -65,7 +66,7 @@ func TestMacOSAgentReadinessGatesRunning(t *testing.T) { stored := StoredMetadata{Id: "mac", MacOS: image.MacOS, SkipGuestAgent: tc.skipped} now := time.Now().UTC() calls := 0 - m := &manager{paths: paths.New(t.TempDir()), now: func() time.Time { return now }, guestAgentReadyProbe: func(context.Context, *StoredMetadata) bool { + m := &manager{paths: paths.New(t.TempDir()), now: func() time.Time { return now }, guestAgentInfoProbe: currentAgentInfo, guestAgentReadyProbe: func(context.Context, *StoredMetadata) bool { calls++ return tc.ready }} @@ -100,7 +101,7 @@ func TestMacOSAgentReadinessGatesRunning(t *testing.T) { func TestMacOSAgentReadyMarkerPersisted(t *testing.T) { p := paths.New(t.TempDir()) now := time.Now().UTC() - m := &manager{paths: p, now: func() time.Time { return now }, guestAgentReadyProbe: func(context.Context, *StoredMetadata) bool { return true }} + m := &manager{paths: p, now: func() time.Time { return now }, guestAgentReadyProbe: func(context.Context, *StoredMetadata) bool { return true }, guestAgentInfoProbe: currentAgentInfo} require.NoError(t, m.ensureDirectories("mac")) image := testMacImage() image.MacOS.GuestAgent = true @@ -116,7 +117,7 @@ func TestMacOSAgentReadyMarkerPersisted(t *testing.T) { func TestMacOSAgentReadyMarkerPersistedByPublicRead(t *testing.T) { p := paths.New(t.TempDir()) now := time.Now().UTC() - m := &manager{paths: p, now: func() time.Time { return now }, guestAgentReadyProbe: func(context.Context, *StoredMetadata) bool { return true }} + m := &manager{paths: p, now: func() time.Time { return now }, guestAgentReadyProbe: func(context.Context, *StoredMetadata) bool { return true }, guestAgentInfoProbe: currentAgentInfo} require.NoError(t, m.ensureDirectories("mac")) socket := filepath.Join(p.InstanceDir("mac"), "vz.sock") require.NoError(t, os.WriteFile(socket, nil, 0600)) @@ -234,3 +235,44 @@ func TestMacOSRunningPhaseFollowsReadiness(t *testing.T) { phase, _ = runningPhaseFromMarkers(&stored) require.Equal(t, phasetracking.PhaseRunning, phase, "a guest without the agent runs from boot") } + +// currentAgentInfo answers GetAgentInfo as an agent at this host's protocol does. +func currentAgentInfo(context.Context, *StoredMetadata) (*guest.GetAgentInfoResponse, error) { + return &guest.GetAgentInfoResponse{ProtocolVersion: maxGuestAgentProtocol, Version: "test"}, nil +} + +func TestMacOSAgentInfoRecordedAtReadiness(t *testing.T) { + image := testMacImage() + image.MacOS.GuestAgent = true + for _, tc := range []struct { + name string + info *guest.GetAgentInfoResponse + err error + ready bool + reason string + }{ + {name: "current agent", info: &guest.GetAgentInfoResponse{ProtocolVersion: 1, Version: "v1"}, ready: true}, + {name: "agent that predates the RPC", info: &guest.GetAgentInfoResponse{}, ready: true}, + {name: "agent newer than the host", info: &guest.GetAgentInfoResponse{ProtocolVersion: maxGuestAgentProtocol + 1, Version: "v9"}, reason: "this host supports up to"}, + {name: "info RPC fails", err: errors.New("connection reset"), reason: "connection reset"}, + } { + t.Run(tc.name, func(t *testing.T) { + m := &manager{paths: paths.New(t.TempDir()), now: time.Now, + guestAgentReadyProbe: func(context.Context, *StoredMetadata) bool { return true }, + guestAgentInfoProbe: func(context.Context, *StoredMetadata) (*guest.GetAgentInfoResponse, error) { return tc.info, tc.err }, + } + stored := StoredMetadata{Id: "mac", MacOS: image.MacOS} + require.Equal(t, tc.ready, m.hydrateGuestAgentReadyFromProbe(context.Background(), &stored)) + reason, failed := m.macOSAgentNotReady.Load("mac") + if !tc.ready { + require.Nil(t, stored.GuestAgentReadyAt) + require.True(t, failed) + require.Contains(t, reason, tc.reason) + return + } + require.False(t, failed) + require.NotNil(t, stored.GuestAgentReadyAt) + require.Equal(t, &GuestAgentInfo{ProtocolVersion: tc.info.ProtocolVersion, Version: tc.info.Version}, stored.GuestAgent) + }) + } +} diff --git a/lib/instances/manager.go b/lib/instances/manager.go index 13beea04d..859361565 100644 --- a/lib/instances/manager.go +++ b/lib/instances/manager.go @@ -12,6 +12,7 @@ import ( "github.com/kernel/hypeman/lib/devices" "github.com/kernel/hypeman/lib/egressproxy" + "github.com/kernel/hypeman/lib/guest" "github.com/kernel/hypeman/lib/guestmemory" "github.com/kernel/hypeman/lib/hypervisor" "github.com/kernel/hypeman/lib/images" @@ -209,8 +210,12 @@ type manager struct { nativeCodecPaths map[string]string imageUsageRecorder ImageUsageRecorder guestAgentReadyProbe func(context.Context, *StoredMetadata) bool + guestAgentInfoProbe func(context.Context, *StoredMetadata) (*guest.GetAgentInfoResponse, error) shutdownGuestFn func(context.Context, hypervisor.VsockDialer, int32) error + // macOSAgentNotReady holds, per instance id, why a macOS agent probe last failed. + macOSAgentNotReady sync.Map + // Shared lifecycle event subscriptions for internal consumers. lifecycleEvents *lifecycleSubscribers diff --git a/lib/instances/query.go b/lib/instances/query.go index 07dc18f01..9cf8ffe84 100644 --- a/lib/instances/query.go +++ b/lib/instances/query.go @@ -435,6 +435,15 @@ func (m *manager) hydrateGuestAgentReadyFromProbe(ctx context.Context, stored *S if !probe(ctx, stored) { return false } + if stored.MacOS != nil { + info, err := m.macOSGuestAgentInfo(ctx, stored) + if err != nil { + m.macOSAgentNotReady.Store(stored.Id, err.Error()) + return false + } + m.macOSAgentNotReady.Delete(stored.Id) + stored.GuestAgent = info + } readyAt := m.nowUTC() stored.GuestAgentReadyAt = &readyAt return true diff --git a/lib/instances/start.go b/lib/instances/start.go index 59e8a19c5..e4287bde8 100644 --- a/lib/instances/start.go +++ b/lib/instances/start.go @@ -71,6 +71,7 @@ func (m *manager) startInstance( stored.ExitMessage = "" stored.ProgramStartedAt = nil stored.GuestAgentReadyAt = nil + stored.GuestAgent = nil if len(req.Entrypoint) > 0 { stored.Entrypoint = req.Entrypoint } diff --git a/lib/instances/stop.go b/lib/instances/stop.go index fd8648fcc..8cc602f07 100644 --- a/lib/instances/stop.go +++ b/lib/instances/stop.go @@ -289,6 +289,7 @@ func (m *manager) stopInstance( // Boot markers are per-boot-run and must not carry across stop/restore/start. stored.ProgramStartedAt = nil stored.GuestAgentReadyAt = nil + stored.GuestAgent = nil stored.FirecrackerSnapshotCacheKey = "" clearFirecrackerUFFDRestoreState(stored) stored.Phases.Record(phasetracking.PhaseStopped, now) diff --git a/lib/instances/types.go b/lib/instances/types.go index 638dbf2b9..d280e3e1c 100644 --- a/lib/instances/types.go +++ b/lib/instances/types.go @@ -126,6 +126,9 @@ type StoredMetadata struct { ProgramStartedAt *time.Time // Set when guest program handoff/start boundary is reached GuestAgentReadyAt *time.Time // Set when guest-agent is ready (unless skip_guest_agent=true) + // macOS: what the image's agent reported when it became ready + GuestAgent *GuestAgentInfo `json:",omitempty"` + // Versions KernelVersion string // Kernel version (e.g., "ch-v6.12.9") diff --git a/lib/system/guest_agent/agent_info.go b/lib/system/guest_agent/agent_info.go new file mode 100644 index 000000000..2b87a2c25 --- /dev/null +++ b/lib/system/guest_agent/agent_info.go @@ -0,0 +1,26 @@ +package main + +import ( + "context" + + pb "github.com/kernel/hypeman/lib/guest" +) + +// agentProtocolVersion is reported to the host. Bump it only for a change an older +// host cannot handle; report additive features as capabilities instead. +const agentProtocolVersion = 1 + +// agentVersion identifies the build in diagnostics. Builds set it with +// -ldflags "-X main.agentVersion=...". +var agentVersion = "dev" + +// agentCapabilities lists optional features this agent supports. +var agentCapabilities []pb.AgentCapability + +func (s *guestServer) GetAgentInfo(context.Context, *pb.GetAgentInfoRequest) (*pb.GetAgentInfoResponse, error) { + return &pb.GetAgentInfoResponse{ + ProtocolVersion: agentProtocolVersion, + Version: agentVersion, + Capabilities: agentCapabilities, + }, nil +} diff --git a/lib/system/guest_agent/service_test.go b/lib/system/guest_agent/service_test.go index 6405ce83a..7c9bf4215 100644 --- a/lib/system/guest_agent/service_test.go +++ b/lib/system/guest_agent/service_test.go @@ -293,3 +293,12 @@ func TestGuestServiceFileRoundTrip(t *testing.T) { require.True(t, final) require.Equal(t, payload, content.String()) } + +func TestGuestServiceReportsAgentInfo(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + info, err := testGuestClient(t).GetAgentInfo(ctx, &pb.GetAgentInfoRequest{}) + require.NoError(t, err) + require.EqualValues(t, agentProtocolVersion, info.ProtocolVersion) + require.Equal(t, agentVersion, info.Version) +} From e85a8c7adbcdc0f2a37db372671fd3d4934ea311 Mon Sep 17 00:00:00 2001 From: chruffins <23645059+chruffins@users.noreply.github.com> Date: Wed, 21 Oct 2026 20:15:34 +0000 Subject: [PATCH 11/11] Stop macOS guests whose agent does not become ready An image that declares the guest agent but ships a missing, broken or incompatible one left the guest Initializing indefinitely. On macOS hosts a controller now stops a guest that has been Initializing for 10 minutes, after one last probe under the instance lock. The reason, with the last probe failure, is recorded in the exit message. Disks and logs are kept, start clears the message, and the restart policy applies as for any other stop. Linux guests are not checked. --- cmd/api/main.go | 8 ++ docs/macos-guest-agent.md | 6 ++ lib/instances/macos_agent_startup.go | 101 ++++++++++++++++++++++ lib/instances/macos_agent_startup_test.go | 84 ++++++++++++++++++ 4 files changed, 199 insertions(+) create mode 100644 lib/instances/macos_agent_startup.go create mode 100644 lib/instances/macos_agent_startup_test.go diff --git a/cmd/api/main.go b/cmd/api/main.go index 8ff95031e..47bc41a3a 100644 --- a/cmd/api/main.go +++ b/cmd/api/main.go @@ -684,6 +684,14 @@ func run() error { return restartController.StartRestartPolicyController(gctx) }) } + if startupController, ok := app.InstanceManager.(interface { + StartMacOSAgentStartupController(context.Context) error + }); ok && runtime.GOOS == "darwin" { + grp.Go(func() error { + logger.Info("starting macOS agent startup controller") + return startupController.StartMacOSAgentStartupController(gctx) + }) + } if ttlReaper, ok := app.InstanceManager.(interface { StartTTLReaper(context.Context) error }); ok { diff --git a/docs/macos-guest-agent.md b/docs/macos-guest-agent.md index de98a9fcf..95ebe9b5b 100644 --- a/docs/macos-guest-agent.md +++ b/docs/macos-guest-agent.md @@ -57,6 +57,12 @@ start marker. An enabled instance is `Initializing` until that marker is recorde then `Running`; this does not certify desktop or browser readiness. This readiness timestamp is a boot observation, not continuous agent health. Start clears the prior boot's readiness marker. +An enabled instance that is still `Initializing` 10 minutes after it booted (or +resumed) is stopped, because its agent is missing, broken or incompatible. The +reason, including the last probe failure, is in the instance's `exit_message`. +Its disk and logs are kept, start clears the message, and the restart policy +applies as for any other stop. + Undeclared/disabled images reject exec/copy before WebSocket upgrade and skip agent shutdown/probes. The declaration is an image capability claim, not a verified live guest handshake or a security credential. Runtime connectivity diff --git a/lib/instances/macos_agent_startup.go b/lib/instances/macos_agent_startup.go new file mode 100644 index 000000000..05451f57f --- /dev/null +++ b/lib/instances/macos_agent_startup.go @@ -0,0 +1,101 @@ +package instances + +import ( + "context" + "fmt" + "log/slog" + "path/filepath" + "time" + + "github.com/kernel/hypeman/lib/instances/phasetracking" + "github.com/kernel/hypeman/lib/logger" +) + +// macOSAgentStartupDeadline bounds how long an agent-enabled macOS guest may stay +// Initializing. A missing or broken agent in the image would otherwise leave it +// there indefinitely. +const macOSAgentStartupDeadline = 10 * time.Minute + +const macOSAgentStartupCheckInterval = 30 * time.Second + +// StartMacOSAgentStartupController stops agent-enabled macOS guests whose agent did +// not become ready by the deadline, and records why in the exit message. Disks and +// logs are kept, and the restart policy applies as for any other stop. +func (m *manager) StartMacOSAgentStartupController(ctx context.Context) error { + log := logger.FromContext(ctx).With("controller", "macos_agent_startup") + log.InfoContext(ctx, "macOS agent startup controller started", "deadline", macOSAgentStartupDeadline) + ticker := time.NewTicker(macOSAgentStartupCheckInterval) + defer ticker.Stop() + for { + select { + case <-ctx.Done(): + return nil + case <-ticker.C: + m.enforceMacOSAgentStartupDeadline(ctx, log) + } + } +} + +func (m *manager) enforceMacOSAgentStartupDeadline(ctx context.Context, log *slog.Logger) { + files, err := m.listMetadataFiles() + if err != nil { + log.WarnContext(ctx, "list instances for macOS agent startup check", "error", err) + return + } + for _, file := range files { + id := filepath.Base(filepath.Dir(file)) + meta, err := m.loadMetadata(id) + if err != nil || !macOSAgentStartupOverdue(&meta.StoredMetadata, m.nowUTC()) { + continue + } + if err := m.stopOverdueMacOSAgentStartup(ctx, id, log); err != nil { + log.WarnContext(ctx, "stop macOS guest whose agent did not start", "instance_id", id, "error", err) + } + } +} + +// macOSAgentStartupOverdue reports whether an agent-enabled macOS guest has been +// Initializing for longer than the deadline. The clock is the current phase, so a +// guest resumed from a memory checkpoint gets a full deadline. +func macOSAgentStartupOverdue(stored *StoredMetadata, now time.Time) bool { + return stored.MacOS != nil && stored.GuestAgentEnabled() && stored.GuestAgentReadyAt == nil && + stored.Phases.Current == phasetracking.PhaseInitializing && now.Sub(stored.Phases.Since) > macOSAgentStartupDeadline +} + +func (m *manager) stopOverdueMacOSAgentStartup(ctx context.Context, id string, log *slog.Logger) error { + lock := m.getInstanceLock(id) + lock.Lock() + defer lock.Unlock() + + // Probe once more under the lock; a ready agent is persisted and spared. + m.persistBootMarkers(ctx, id) + meta, err := m.loadMetadata(id) + if err != nil || !macOSAgentStartupOverdue(&meta.StoredMetadata, m.nowUTC()) { + return err + } + current := m.toInstanceWithoutHydration(ctx, meta) + if current.State != StateInitializing { + return nil + } + + reason := fmt.Sprintf("guest agent did not become ready within %s; the image declares guest_agent", macOSAgentStartupDeadline) + if last, ok := m.macOSAgentNotReady.Load(id); ok { + reason += ": " + last.(string) + } + log.WarnContext(ctx, "stopping macOS guest whose agent did not start", "instance_id", id, "reason", reason) + if _, err := m.stopInstance(ctx, id); err != nil { + return err + } + meta, err = m.loadMetadata(id) + if err != nil { + return err + } + meta.ExitMessage = reason + if err := m.saveMetadata(meta); err != nil { + return err + } + m.macOSAgentNotReady.Delete(id) + stopped := m.toInstanceWithoutHydration(ctx, meta) + m.notifyLifecycleEvent(ctx, LifecycleEventStop, &stopped) + return nil +} diff --git a/lib/instances/macos_agent_startup_test.go b/lib/instances/macos_agent_startup_test.go new file mode 100644 index 000000000..dbe980b01 --- /dev/null +++ b/lib/instances/macos_agent_startup_test.go @@ -0,0 +1,84 @@ +package instances + +import ( + "context" + "log/slog" + "os" + "path/filepath" + "testing" + "time" + + "github.com/kernel/hypeman/lib/hypervisor" + "github.com/kernel/hypeman/lib/instances/phasetracking" + "github.com/kernel/hypeman/lib/paths" + "github.com/stretchr/testify/require" +) + +func TestMacOSAgentStartupDeadlineStopsGuest(t *testing.T) { + p := paths.New(t.TempDir()) + booted := time.Now().UTC().Add(-time.Hour) + now := booted + m := &manager{paths: p, now: func() time.Time { return now }, lifecycleEvents: newLifecycleSubscribers(), + guestAgentReadyProbe: func(context.Context, *StoredMetadata) bool { return false }, + guestAgentInfoProbe: currentAgentInfo, + } + image := testMacImage() + image.MacOS.GuestAgent = true + for _, id := range []string{"overdue", "booting"} { + require.NoError(t, m.ensureDirectories(id)) + socket := filepath.Join(p.InstanceDir(id), "vz.sock") + require.NoError(t, os.WriteFile(socket, nil, 0600)) + lifecycleNoopHypervisorStates.Store(socket, hypervisor.StateRunning) + t.Cleanup(func() { lifecycleNoopHypervisorStates.Delete(socket) }) + stored := StoredMetadata{Id: id, Name: id, DataDir: p.InstanceDir(id), SocketPath: socket, HypervisorType: lifecycleNoopHypervisorType, MacOS: image.MacOS, CreatedAt: booted} + since := booted + if id == "booting" { + since = booted.Add(time.Hour - time.Minute) + } + stored.Phases.Record(phasetracking.PhaseInitializing, since) + require.NoError(t, m.saveMetadata(&metadata{StoredMetadata: stored})) + } + m.macOSAgentNotReady.Store("overdue", "read guest agent info: connection reset") + now = booted.Add(time.Hour) + + m.enforceMacOSAgentStartupDeadline(context.Background(), slog.Default()) + + overdue, err := m.GetInstance(context.Background(), "overdue") + require.NoError(t, err) + require.Equal(t, StateStopped, overdue.State) + require.Contains(t, overdue.ExitMessage, "did not become ready within 10m0s") + require.Contains(t, overdue.ExitMessage, "connection reset", "the last probe failure is kept") + require.Nil(t, overdue.ExitCode) + require.FileExists(t, filepath.Join(p.InstanceDir("overdue"), "metadata.json"), "the instance and its files are kept") + + booting, err := m.GetInstance(context.Background(), "booting") + require.NoError(t, err) + require.Equal(t, StateInitializing, booting.State, "a guest within the deadline is left alone") + require.Empty(t, booting.ExitMessage) +} + +func TestMacOSAgentStartupOverdueRules(t *testing.T) { + image := testMacImage() + image.MacOS.GuestAgent = true + now := time.Now().UTC() + initializing := func(since time.Time) phasetracking.Tracker { + var tr phasetracking.Tracker + tr.Record(phasetracking.PhaseInitializing, since) + return tr + } + old := now.Add(-macOSAgentStartupDeadline - time.Second) + ready := now + for name, tc := range map[string]struct { + stored StoredMetadata + want bool + }{ + "overdue": {StoredMetadata{MacOS: image.MacOS, Phases: initializing(old)}, true}, + "within deadline": {StoredMetadata{MacOS: image.MacOS, Phases: initializing(now.Add(-time.Minute))}, false}, + "agent ready": {StoredMetadata{MacOS: image.MacOS, Phases: initializing(old), GuestAgentReadyAt: &ready}, false}, + "agent skipped": {StoredMetadata{MacOS: image.MacOS, Phases: initializing(old), SkipGuestAgent: true}, false}, + "image without an agent": {StoredMetadata{MacOS: testMacImage().MacOS, Phases: initializing(old)}, false}, + "linux guest": {StoredMetadata{Phases: initializing(old)}, false}, + } { + require.Equal(t, tc.want, macOSAgentStartupOverdue(&tc.stored, now), name) + } +}