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/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/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 } 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..95ebe9b5b --- /dev/null +++ b/docs/macos-guest-agent.md @@ -0,0 +1,145 @@ +# 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 \ + -ldflags "-X main.agentVersion=$(git describe --always --dirty)" \ + -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. + +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 +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. + 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/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 61376bf02..3241be6af 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 } @@ -328,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 @@ -442,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) @@ -453,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) @@ -962,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/guest/probe_exec_test.go b/lib/guest/probe_exec_test.go new file mode 100644 index 000000000..1e2a0a4e7 --- /dev/null +++ b/lib/guest/probe_exec_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 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} + + _, 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 = ProbeExec(ctx, shared, ExecOptions{Command: []string{"true"}, WaitForAgent: 50 * time.Millisecond}) + require.Error(t, err) + 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), "ordinary exec 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..647d4e303 --- /dev/null +++ b/lib/instances/guest_os.go @@ -0,0 +1,24 @@ +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. 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) + } + 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_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_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) + } +} diff --git a/lib/instances/macos_boot_disk_test.go b/lib/instances/macos_boot_disk_test.go new file mode 100644 index 000000000..acd64e9fb --- /dev/null +++ b/lib/instances/macos_boot_disk_test.go @@ -0,0 +1,23 @@ +package instances + +import ( + "path/filepath" + "testing" + + "github.com/kernel/hypeman/lib/images" + "github.com/kernel/hypeman/lib/paths" + "github.com/stretchr/testify/require" +) + +func TestVMStoragePathIsTheMacOSBootDisk(t *testing.T) { + t.Parallel() + 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) { + t.Parallel() + stored := &StoredMetadata{DataDir: t.TempDir()} + require.Empty(t, stored.VMStoragePath()) +} 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..5c3479871 100644 --- a/lib/instances/macos_test.go +++ b/lib/instances/macos_test.go @@ -9,8 +9,10 @@ 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" "github.com/kernel/hypeman/lib/paths" "github.com/stretchr/testify/require" ) @@ -36,6 +38,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 }, guestAgentInfoProbe: currentAgentInfo, 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 }, guestAgentInfoProbe: currentAgentInfo} + 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 }, guestAgentInfoProbe: currentAgentInfo} + 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 +217,62 @@ 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") +} + +// 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 0b39814f0..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 @@ -926,7 +931,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..9cf8ffe84 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,8 +297,8 @@ 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 - needAgent := !stored.SkipGuestAgent && stored.GuestAgentReadyAt == nil + needProgram := !programMarkerSettled(stored) + needAgent := stored.GuestAgentEnabled() && stored.GuestAgentReadyAt == nil if !needProgram && !needAgent { m.clearBootMarkerRescan(stored.Id) return false @@ -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 { @@ -430,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 @@ -440,13 +435,22 @@ 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 } 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) @@ -457,8 +461,8 @@ func probeGuestAgentReady(ctx context.Context, stored *StoredMetadata) bool { probeCtx, cancel := context.WithTimeout(ctx, guestAgentReadyProbeTimeout) defer cancel() - exit, err := guest.ExecIntoInstance(probeCtx, dialer, guest.ExecOptions{ - Command: []string{"/bin/true"}, + exit, err := guest.ProbeExec(probeCtx, dialer, guest.ExecOptions{ + Command: []string{guestAgentReadyExecutable(stored)}, Timeout: int32(guestAgentReadyProbeTimeout / time.Second), WaitForAgent: guestAgentReadyProbeWait, }) @@ -652,8 +656,8 @@ func (m *manager) persistBootMarkers(ctx context.Context, id string) { return } - needProgram := meta.ProgramStartedAt == nil - needAgent := !meta.SkipGuestAgent && meta.GuestAgentReadyAt == nil + needProgram := !programMarkerSettled(&meta.StoredMetadata) + needAgent := meta.GuestAgentEnabled() && meta.GuestAgentReadyAt == nil if !needProgram && !needAgent { return } @@ -668,7 +672,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..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 } @@ -96,25 +97,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 +108,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 +243,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..8cc602f07 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 } @@ -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)), @@ -283,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/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/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/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/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/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..987618c1a 100644 --- a/lib/system/guest_agent/exec.go +++ b/lib/system/guest_agent/exec.go @@ -6,7 +6,6 @@ import ( "io" "log" "os" - "os/exec" "strings" "sync" "time" @@ -29,6 +28,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 { @@ -59,15 +61,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() @@ -84,19 +78,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) @@ -144,13 +126,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 := processExitCode(cmd, waitErr) log.Printf("[guest-agent] command finished with exit code: %d", exitCode) @@ -168,30 +144,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) } @@ -203,27 +159,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) @@ -251,13 +192,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 := processExitCode(cmd, waitErr) log.Printf("[guest-agent] TTY command finished with exit code: %d", exitCode) 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_common.go b/lib/system/guest_agent/exec_common.go new file mode 100644 index 000000000..00cc73c74 --- /dev/null +++ b/lib/system/guest_agent/exec_common.go @@ -0,0 +1,87 @@ +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 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()) + } + if waitErr != nil { + return 124 + } + return 0 +} 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) + } +} 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 new file mode 100644 index 000000000..adc73da02 --- /dev/null +++ b/lib/system/guest_agent/exec_lifecycle.go @@ -0,0 +1,301 @@ +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. +// 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 { + 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) + } + 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, 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, grp, outputDone) +} + +// 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 := s.newExecCmd(ctx, start) + // 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} + } + return cmd, nil +} + +// 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, 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 + } + 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) + cancelled := ctx.Err() != nil && errors.Is(waitErr, ctx.Err()) + if !exited && !cancelled { + return fmt.Errorf("stream command output: %w", waitErr) + } + } + exitCode := execExitCode(cmd, waitErr, errors.Is(ctx.Err(), context.DeadlineExceeded)) + + 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()) + } +} + +// 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 { + 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()) + } +} + +// 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()) +} + +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 { + ptmx, err := startExecPTY(cmd, start) + if err != nil { + return fmt.Errorf("start pty: %w", err) + } + defer ptmx.Close() + grp := newProcessGroup(cmd) + lifecycleDone := make(chan struct{}) + defer close(lifecycleDone) + go killGroupOnDone(ctx, grp, lifecycleDone) + + outputDone := make(chan struct{}) + output := &execStreamWriter{sender: sender, cancel: cancel} + + 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() { + defer close(outputDone) + _, _ = io.Copy(output, ptmx) + }() + + 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 new file mode 100644 index 000000000..8a59edef0 --- /dev/null +++ b/lib/system/guest_agent/exec_lifecycle_disconnect_test.go @@ -0,0 +1,96 @@ +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 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} + require.NoError(t, group.Start()) + t.Cleanup(func() { _ = group.Process.Kill() }) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + 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, 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 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 new file mode 100644 index 000000000..c386f8ea3 --- /dev/null +++ b/lib/system/guest_agent/exec_lifecycle_remediation_test.go @@ -0,0 +1,110 @@ +package main + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "runtime" + "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 ps reports its state instead. +func execProcessGone(pid int) bool { + out, err := exec.Command("ps", "-o", "stat=", "-p", strconv.Itoa(pid)).Output() + if err != nil { + return true + } + 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{}) + 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") + } +} + +// 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 + } + } +} 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..7c9bf4215 --- /dev/null +++ b/lib/system/guest_agent/service_test.go @@ -0,0 +1,304 @@ +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()) +} + +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) +} 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) +}