diff --git a/docs/getting-started/installation.md b/docs/getting-started/installation.md index d61ccc1c..1b160d5b 100644 --- a/docs/getting-started/installation.md +++ b/docs/getting-started/installation.md @@ -187,9 +187,10 @@ install -Dm755 stoat ~/.local/bin/stoat ``` A release also carries `darwin_amd64` and `darwin_arm64` tarballs, and -`windows_amd64` and `windows_arm64` zip archives. On those hosts Stoat runs -`doctor` and `capabilities` only. Every other command reports the host as -unqualified, because macOS needs the QEMU HVF accelerator +`windows_amd64` and `windows_arm64` zip archives. On those hosts Stoat cannot +start or stop a local VM; it reports the host as unqualified. Commands that +only read or edit VM records still run. This is because macOS needs the +QEMU HVF accelerator ([#82](https://github.com/NovusEdge/stoat/issues/82)) and Windows needs WHPX ([#83](https://github.com/NovusEdge/stoat/issues/83)). Linux with KVM is the supported host today. diff --git a/docs/reference/samples/vm.toml b/docs/reference/samples/vm.toml index 39bd56a9..7c91fed9 100644 --- a/docs/reference/samples/vm.toml +++ b/docs/reference/samples/vm.toml @@ -16,6 +16,7 @@ sshport = 2200 # int; default an allocated free port; stoat recipes = ["docker"] # string[]; default []; user/TUI and stoat create/update write the selection. display = "auto" # string; default "auto"; user/TUI writes: auto, window, or vnc. backend = "apkovl" # string; default inferred from image; stoat writes: apkovl, cloudinit, or ssh. +provider = "qemu" # string; default qemu when empty; stoat writes the execution surface. base = "" # string path; default empty; stoat writes the absolute shared base-image path. sshuser = "root" # string; default guest-defined user (empty means root); stoat writes it. console_password = "" # string; default "stoat" for cloud VMs, empty otherwise; stoat writes, never ssh. diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 67d3225d..b8ab2956 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -7,20 +7,19 @@ Find the reported symptom or error text below, then follow its recovery steps. ``` stoat: up: native VM operations are not qualified on darwin/arm64. darwin/arm64 needs a qualified runtime with the QEMU HVF accelerator (tracked in stoat#82). -doctor and capabilities still work here; every other command needs a qualified host. +starting and stopping a local VM needs a qualified host; commands that only read or edit VM records still work here. Linux with KVM is the supported configuration today. ``` -Stoat's VM lifecycle (create, start, stop, ssh, recipes, and every other -command except `doctor` and `capabilities`) is qualified on Linux with KVM -only. macOS arm64 and Windows amd64 builds exist and compile, but their -native runtimes are not qualified yet; see +Starting and stopping a local VM is qualified on Linux with KVM only. macOS +arm64 and Windows amd64 builds exist and compile, but their native runtimes +are not qualified yet; see [stoat#82](https://github.com/novusedge/stoat/issues/82) and [stoat#83](https://github.com/novusedge/stoat/issues/83). `--json` and MCP report this as the `host_unsupported` error code. **Fix:** run Stoat on Linux with KVM, or track the issue above for your -platform. `stoat doctor` and `stoat capabilities` still run on any host. +platform. Commands that only read or edit VM records still run on any host. ## No QEMU window appears @@ -111,10 +110,10 @@ every command and for the TUI: `STOAT_GRAPHICAL=0` is also the answer to the OpenGL error above, where a session exists but QEMU cannot draw on it. -## `ssh not reachable on port N after 1m30s` +## `ssh not reachable at HOST:PORT after 1m30s` ``` -: ssh not reachable on port after 1m30s +: ssh not reachable at : after 1m30s ``` **If this happens while applying recipes to a disk VM:** the VM is still diff --git a/internal/capabilities/build.go b/internal/capabilities/build.go index 88931821..db5f393f 100644 --- a/internal/capabilities/build.go +++ b/internal/capabilities/build.go @@ -3,7 +3,7 @@ package capabilities import ( "runtime" - "github.com/novusedge/stoat/internal/core" + "github.com/novusedge/stoat/internal/hostcheck" ) // Build evaluates the supplied host and metadata observations without I/O. @@ -131,14 +131,14 @@ func currentEntry(name, status, scope string, requirements []Requirement, limits return Capability{Name: name, Status: status, Scope: scope, Requirements: requirements, Limits: limits, Reason: reason, Evidence: []Evidence{implementationEvidence(name)}} } -func qemuProfile(checks []core.HostCheck) Profile { +func qemuProfile(checks []hostcheck.Check) Profile { requirements := []Requirement{ requirement("host_tool", "qemu-system-x86_64", ""), requirement("host_tool", "qemu-img", ""), requirement("host_device", "/dev/kvm", ""), } p := Profile{Name: "qemu-x86_64", Status: StatusUnknown, Scope: ScopeHost, Requirements: requirements, Limits: []Limit{}, Evidence: []Evidence{}} - byName := make(map[string]core.HostCheck, len(checks)) + byName := make(map[string]hostcheck.Check, len(checks)) for _, c := range checks { if _, exists := byName[c.Name]; !exists { byName[c.Name] = c diff --git a/internal/capabilities/build_test.go b/internal/capabilities/build_test.go index d079766e..cc0c55ff 100644 --- a/internal/capabilities/build_test.go +++ b/internal/capabilities/build_test.go @@ -8,11 +8,12 @@ import ( "strconv" "testing" - "github.com/novusedge/stoat/internal/core" + "github.com/novusedge/stoat/internal/coreerr" + "github.com/novusedge/stoat/internal/hostcheck" ) -func passingChecks() []core.HostCheck { - return []core.HostCheck{ +func passingChecks() []hostcheck.Check { + return []hostcheck.Check{ {Name: "qemu-system-x86_64", OK: true}, {Name: "qemu-img", OK: true}, {Name: "/dev/kvm", OK: true}, @@ -174,7 +175,7 @@ func TestCapabilitiesBoundaries(t *testing.T) { } t.Run("partial required host observations are unknown", func(t *testing.T) { - report := Build(Input{HostChecks: []core.HostCheck{{Name: "qemu-system-x86_64", OK: true}}}) + report := Build(Input{HostChecks: []hostcheck.Check{{Name: "qemu-system-x86_64", OK: true}}}) profile := profileEntry(t, report.Profiles, "qemu-x86_64") if profile.Status != "unknown" || profile.Reason == nil || profile.Reason.Code != "host_probe_unavailable" { t.Errorf("qemu-x86_64 = %+v, want unknown/host_probe_unavailable", profile) @@ -182,7 +183,7 @@ func TestCapabilitiesBoundaries(t *testing.T) { }) t.Run("failed fully observed host requirement is limited", func(t *testing.T) { - report := Build(Input{HostChecks: []core.HostCheck{ + report := Build(Input{HostChecks: []hostcheck.Check{ {Name: "qemu-system-x86_64", OK: true}, {Name: "qemu-img", OK: false}, {Name: "/dev/kvm", OK: true}, @@ -197,7 +198,7 @@ func TestCapabilitiesBoundaries(t *testing.T) { }) t.Run("empty host observations are unknown", func(t *testing.T) { - report := Build(Input{HostChecks: []core.HostCheck{}}) + report := Build(Input{HostChecks: []hostcheck.Check{}}) profile := profileEntry(t, report.Profiles, "qemu-x86_64") if profile.Status != "unknown" || profile.Reason == nil || profile.Reason.Code != "host_probe_unavailable" { t.Errorf("qemu-x86_64 = %+v, want unknown/host_probe_unavailable", profile) @@ -248,11 +249,11 @@ func TestCapabilitiesBoundaries(t *testing.T) { } for _, name := range []string{"", "../escape", "bad/name", "-bad", "_bad", "bad name"} { - if _, err := LoadTarget(name); !errors.Is(err, core.ErrInvalidSpec) { + if _, err := LoadTarget(name); !errors.Is(err, coreerr.ErrInvalidSpec) { t.Errorf("LoadTarget(%q) error = %v, want ErrInvalidSpec", name, err) } } - if _, err := LoadTarget("missing"); !errors.Is(err, core.ErrNotFound) { + if _, err := LoadTarget("missing"); !errors.Is(err, coreerr.ErrNotFound) { t.Errorf("LoadTarget(missing) error = %v, want ErrNotFound", err) } brokenDir := filepath.Join(root, "broken") @@ -262,7 +263,7 @@ func TestCapabilitiesBoundaries(t *testing.T) { if err := os.WriteFile(filepath.Join(brokenDir, "vm.toml"), []byte("name = \"broken\"\nmode = \"cloud\n"), 0o644); err != nil { t.Fatal(err) } - if _, err := LoadTarget("broken"); !errors.Is(err, core.ErrBroken) { + if _, err := LoadTarget("broken"); !errors.Is(err, coreerr.ErrBroken) { t.Errorf("LoadTarget(broken) error = %v, want ErrBroken", err) } }) diff --git a/internal/capabilities/load.go b/internal/capabilities/load.go index 8b46fe47..f0e33c66 100644 --- a/internal/capabilities/load.go +++ b/internal/capabilities/load.go @@ -7,7 +7,7 @@ import ( "regexp" "github.com/novusedge/stoat/internal/config" - "github.com/novusedge/stoat/internal/core" + "github.com/novusedge/stoat/internal/coreerr" ) var targetNameRE = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]*$`) @@ -15,18 +15,18 @@ var targetNameRE = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]*$`) // LoadTarget reads one VM's stored metadata without inspecting runtime state. func LoadTarget(name string) (Target, error) { if !targetNameRE.MatchString(name) { - return Target{}, fmt.Errorf("%w: VM name %q", core.ErrInvalidSpec, name) + return Target{}, fmt.Errorf("%w: VM name %q", coreerr.ErrInvalidSpec, name) } path := filepath.Join(config.Root(), name, "vm.toml") if _, err := os.Stat(path); err != nil { if os.IsNotExist(err) { - return Target{}, fmt.Errorf("%w: %s", core.ErrNotFound, name) + return Target{}, fmt.Errorf("%w: %s", coreerr.ErrNotFound, name) } - return Target{}, fmt.Errorf("%w: %s: %v", core.ErrBroken, name, err) + return Target{}, fmt.Errorf("%w: %s: %v", coreerr.ErrBroken, name, err) } v, err := config.Load(name) if err != nil { - return Target{}, fmt.Errorf("%w: %s: %v", core.ErrBroken, name, err) + return Target{}, fmt.Errorf("%w: %s: %v", coreerr.ErrBroken, name, err) } return Target{Name: name, Mode: v.Mode, AgentAccess: v.AgentAccess}, nil } diff --git a/internal/capabilities/model.go b/internal/capabilities/model.go index 87241289..a849462c 100644 --- a/internal/capabilities/model.go +++ b/internal/capabilities/model.go @@ -1,6 +1,6 @@ package capabilities -import "github.com/novusedge/stoat/internal/core" +import "github.com/novusedge/stoat/internal/hostcheck" const ( StatusSupported = "supported" @@ -103,6 +103,6 @@ type Evidence struct { type Input struct { Version string ProjectState string - HostChecks []core.HostCheck + HostChecks []hostcheck.Check Target *Target } diff --git a/internal/cli/cli.go b/internal/cli/cli.go index c509db13..d187c99e 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -495,11 +495,15 @@ func Main(args []string, version string, stdin io.Reader, stdout, stderr io.Writ return runDoctor(a, stdout, stderr) } - // Every mutating or process-facing command must reject before resolving - // secrets, reading project scope, creating the data root, or initializing - // logs. The independent capabilities command is dispatched before this - // boundary by its owner and remains metadata-only. - if err := hostops.RequireVM(); err != nil { + // Every command past this point either drives a machine or writes to the + // data root, so an unqualified host refuses here, before secrets resolve, + // project scope is read, or a log is written. + // + // hostops.RequireDataRoot exists for the day a provider runs a VM + // somewhere other than this host. Moving these call sites onto it needs + // that provider first; opening the gate earlier only lets a macOS user + // reach a QEMU path that cannot work. + if err := hostops.RequireLocalHypervisor(); err != nil { return a.fail(stdout, stderr, err) } if len(a.Params) > 0 { diff --git a/internal/cli/run_access.go b/internal/cli/run_access.go index c77b33bf..f84989a7 100644 --- a/internal/cli/run_access.go +++ b/internal/cli/run_access.go @@ -110,7 +110,7 @@ func runSSH(a *Args, stdout, stderr io.Writer) int { fmt.Fprintln(stderr, "stoat: ssh:", err) return ExitFail } - argv := append([]string{"ssh"}, sshx.Args(v)...) + argv := append([]string{"ssh"}, sshx.Args(sshx.LocalEndpoint(v))...) if err := syscall.Exec(path, argv, os.Environ()); err != nil { fmt.Fprintln(stderr, "stoat: ssh:", err) return ExitFail diff --git a/internal/cli/run_capabilities.go b/internal/cli/run_capabilities.go index 64724d1a..72539177 100644 --- a/internal/cli/run_capabilities.go +++ b/internal/cli/run_capabilities.go @@ -7,8 +7,21 @@ import ( "github.com/novusedge/stoat/internal/capabilities" "github.com/novusedge/stoat/internal/cli/wire" "github.com/novusedge/stoat/internal/core" + "github.com/novusedge/stoat/internal/hostcheck" ) +// toHostChecks adapts core.Doctor's result to capabilities.Input.HostChecks. +// The two types have identical fields but capabilities cannot import core: +// core now imports provider, which imports capabilities, so the reverse +// import would close a cycle. +func toHostChecks(cs []core.HostCheck) []hostcheck.Check { + out := make([]hostcheck.Check, len(cs)) + for i, c := range cs { + out[i] = hostcheck.Check{Name: c.Name, OK: c.OK, Detail: c.Detail, Fix: c.Fix, Optional: c.Optional} + } + return out +} + func runCapabilities(a *Args, version string, stdout, stderr io.Writer) int { var target *capabilities.Target if a.VM != "" { @@ -25,7 +38,7 @@ func runCapabilities(a *Args, version string, stdout, stderr io.Writer) int { report := capabilities.Build(capabilities.Input{ Version: version, ProjectState: projectState, - HostChecks: core.Doctor(), + HostChecks: toHostChecks(core.Doctor()), Target: target, }) if a.JSON { diff --git a/internal/config/config.go b/internal/config/config.go index 6fd792a6..6ed6b081 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -94,6 +94,10 @@ type VM struct { // Written by the form at creation time; dispatch elsewhere in stoat // keys off Mode, not this field. Backend string `toml:"backend"` + // Provider is the execution surface: "qemu" for a local hypervisor VM, + // or a cloud provider's name. Empty means "qemu", which is what every + // vm.toml written before this field existed says. + Provider string `toml:"provider,omitempty"` // Base is the absolute path to the shared base image an overlay is // created from. Cloud mode only. Base string `toml:"base"` @@ -202,7 +206,7 @@ func Root() string { // EnsureRoot creates the data root and its fixed subdirectories. func EnsureRoot() error { - if err := hostops.RequireVM(); err != nil { + if err := hostops.RequireLocalHypervisor(); err != nil { return err } for _, d := range []string{"isos", "recipes"} { @@ -276,7 +280,7 @@ func (v *VM) ISOPath() string { // Save writes vm.toml, creating the VM directory if needed. func (v *VM) Save() error { - if err := hostops.RequireVM(); err != nil { + if err := hostops.RequireLocalHypervisor(); err != nil { return err } if v.Dir == "" { @@ -381,7 +385,7 @@ var sshPortLine = regexp.MustCompile(`(?m)^\s*sshport\s*=\s*(\d+)\s*$`) // Delete removes the VM directory. It never touches isos/. func (v *VM) Delete() error { - if err := hostops.RequireVM(); err != nil { + if err := hostops.RequireLocalHypervisor(); err != nil { return err } if v.Dir == "" || filepath.Dir(v.Dir) != Root() { diff --git a/internal/core/access.go b/internal/core/access.go index 9a51390c..8ebafff8 100644 --- a/internal/core/access.go +++ b/internal/core/access.go @@ -2,6 +2,7 @@ package core import ( "bytes" + "context" "errors" "fmt" "io" @@ -30,7 +31,15 @@ func SSHCommand(name string) ([]string, error) { if err != nil { return nil, err } - return append([]string{"ssh"}, sshx.Args(v)...), nil + p, err := providerFor(v) + if err != nil { + return nil, err + } + ep, err := p.Endpoint(context.Background(), v) + if err != nil { + return nil, err + } + return append([]string{"ssh"}, sshx.Args(ep)...), nil } // Which selects one of a VM's two log files. diff --git a/internal/core/apply.go b/internal/core/apply.go index 7d856d0b..e59f62cd 100644 --- a/internal/core/apply.go +++ b/internal/core/apply.go @@ -13,7 +13,6 @@ import ( "github.com/novusedge/stoat/internal/backend" "github.com/novusedge/stoat/internal/cloudinit" "github.com/novusedge/stoat/internal/config" - "github.com/novusedge/stoat/internal/qemu" "github.com/novusedge/stoat/internal/recipes" "github.com/novusedge/stoat/internal/sshx" ) @@ -142,7 +141,11 @@ func resolveTargets(v *config.VM, only []string) ([]string, error) { // applyLocked is Apply's body, run while Apply holds name's provision lock. func applyLocked(ctx context.Context, v *config.VM, opts ApplyOpts) error { - if !qemu.Running(v) { + state, err := StateOf(ctx, v) + if err != nil { + return err + } + if state != StateRunning { return fmt.Errorf("%w: %s", ErrNotRunning, v.Name) } @@ -283,10 +286,19 @@ func applyLocked(ctx context.Context, v *config.VM, opts ApplyOpts) error { func rebootAndWait(ctx context.Context, v *config.VM, recipe string) error { appendProvisionLog(v, fmt.Sprintf("rebooting %s to finish %s...\n", v.Name, recipe)) + p, err := providerFor(v) + if err != nil { + return err + } + ep, err := p.Endpoint(ctx, v) + if err != nil { + return err + } + // `reboot` tears down the ssh session before the process can report an // exit status back to this host, so cmd.Run() returning an error here is // expected and not a failure signal; only the wait below is. - cmd := exec.CommandContext(ctx, "ssh", sshx.Args(v, "reboot")...) + cmd := exec.CommandContext(ctx, "ssh", sshx.Args(ep, "reboot")...) _ = cmd.Run() // The pre-reboot sshd can keep answering for a moment after the reboot @@ -328,7 +340,7 @@ func appendProvisionWarnings(v *config.VM, warnings []string) { // discoverCloudInitApplied rebuilds v.Applied for a cloudinit VM from the // marker files cloud-init left after first boot. It runs over ssh, so the VM -// must be reachable; applyLocked calls it only after the qemu.Running check. +// must be reachable; applyLocked calls it only after the running check. // // It no-ops unless the backend is cloudinit and v.Applied is still empty: once // a post-boot Apply has recorded state, that state is authoritative and this @@ -344,7 +356,15 @@ func discoverCloudInitApplied(ctx context.Context, v *config.VM) ([]string, erro return nil, nil } script := fmt.Sprintf("for marker in %s/*; do case \"$marker\" in *.out) continue;; esac; [ -f \"$marker\" ] || continue; name=$(basename \"$marker\"); printf '===%%s\\n' \"$name\"; cat \"$marker.out\" 2>/dev/null; done", cloudinit.MarkerDir) - out, err := exec.CommandContext(ctx, "ssh", sshx.Args(v, script)...).Output() + p, err := providerFor(v) + if err != nil { + return nil, err + } + ep, err := p.Endpoint(ctx, v) + if err != nil { + return nil, err + } + out, err := exec.CommandContext(ctx, "ssh", sshx.Args(ep, script)...).Output() if err != nil { return nil, nil // marker dir missing or a transient ssh error; discover nothing } diff --git a/internal/core/autorestart.go b/internal/core/autorestart.go index 0658563e..82fad691 100644 --- a/internal/core/autorestart.go +++ b/internal/core/autorestart.go @@ -3,8 +3,6 @@ package core import ( "context" "errors" - - "github.com/novusedge/stoat/internal/qemu" ) // AutoRestartAfterInstall waits for an uninstalled disk VM's unattended @@ -19,14 +17,21 @@ import ( // apkovlBackend.Args adds -no-reboot for this exact boot, so a successful // install's own "poweroff" (internal/apkovl's installScript) exits QEMU // instead of re-entering the installer. A failed install leaves the -// installer's shell running, so qemu.Running never turns false and this -// call rides out ctx's deadline instead of restarting. +// installer's shell running, so the VM never stops and this call rides out +// ctx's deadline instead of restarting. func AutoRestartAfterInstall(ctx context.Context, name string) (bool, error) { v, err := load(name) if err != nil { return false, err } - if v.Mode != "disk" || v.Installed || !qemu.Running(v) { + if v.Mode != "disk" || v.Installed { + return false, nil + } + state, err := StateOf(ctx, v) + if err != nil { + return false, err + } + if state != StateRunning { return false, nil } diff --git a/internal/core/autorestart_test.go b/internal/core/autorestart_test.go index cdeaf494..c7b22414 100644 --- a/internal/core/autorestart_test.go +++ b/internal/core/autorestart_test.go @@ -107,11 +107,11 @@ func TestAutoRestartAfterInstallGivesUpSilentlyOnTimeout(t *testing.T) { // TestAutoRestartAfterInstallAttemptsStartOnceInstallerStops proves the // installer-stop signal actually drives a restart attempt, not just the -// guard. qemu.Start has no fake seam, so the restart attempt fails here for +// guard. The fake provider's Start always succeeds, so this test keeps the +// real qemu provider (realQemuProcess) and lets the restart attempt fail for // lack of a real install ISO (v.ISOPath() names nothing on disk); the test // asserts on that failure's shape rather than a real boot, distinguishing a -// restart ATTEMPT (the failure names v's ISO path) from the guard's silent -// no-op (nil error, near-instant return). +// restart ATTEMPT from the guard's silent no-op. func TestAutoRestartAfterInstallAttemptsStartOnceInstallerStops(t *testing.T) { dir := root(t) v := &config.VM{Name: "work", Mode: "disk", Installed: false, OS: "alpine", Backend: "apkovl", RAM: 1024, CPUs: 1, SSHPort: 2404} @@ -119,7 +119,7 @@ func TestAutoRestartAfterInstallAttemptsStartOnceInstallerStops(t *testing.T) { t.Fatal(err) } v.Dir = dir + "/work" - stop := fakeRunning(t, v) + stop := realQemuProcess(t, v) go func() { time.Sleep(2 * pollInterval) diff --git a/internal/core/clone.go b/internal/core/clone.go index 5c99e3f3..168f7229 100644 --- a/internal/core/clone.go +++ b/internal/core/clone.go @@ -1,6 +1,7 @@ package core import ( + "context" "fmt" "os" "os/exec" @@ -11,7 +12,6 @@ import ( "github.com/novusedge/stoat/internal/config" "github.com/novusedge/stoat/internal/guest" "github.com/novusedge/stoat/internal/keys" - "github.com/novusedge/stoat/internal/qemu" "github.com/novusedge/stoat/internal/recipes" ) @@ -45,7 +45,11 @@ func Clone(name, newName string) (VM, error) { if err != nil { return VM{}, err } - if qemu.Running(src) { + state, err := StateOf(context.Background(), src) + if err != nil { + return VM{}, err + } + if state == StateRunning { return VM{}, fmt.Errorf("%w: %s: stop it before cloning", ErrAlreadyRunning, name) } diff --git a/internal/core/clone_test.go b/internal/core/clone_test.go index 404492d4..5422d3d5 100644 --- a/internal/core/clone_test.go +++ b/internal/core/clone_test.go @@ -52,17 +52,14 @@ func TestCloneRefusesInvalidName(t *testing.T) { // A running source must be refused outright: an overlay's backing file must // never change after the overlay is made, and a running qemu process is -// writing to its disk continuously. fakeRunning (vm_test.go) fakes liveness -// without a real qemu process. +// writing to its disk continuously. func TestCloneRefusesRunningSource(t *testing.T) { - dir := root(t) + root(t) v := &config.VM{Name: "work", Mode: "live", RAM: 1024, CPUs: 1, SSHPort: 2200} if err := v.Save(); err != nil { t.Fatal(err) } - v.Dir = filepath.Join(dir, "work") - stop := fakeRunning(t, v) - defer stop() + defer fakeRunning(t, v)() if _, err := Clone("work", "clone1"); !errors.Is(err, ErrAlreadyRunning) { t.Fatalf("err = %v, want ErrAlreadyRunning", err) diff --git a/internal/core/copy.go b/internal/core/copy.go index 714e862c..c999d48c 100644 --- a/internal/core/copy.go +++ b/internal/core/copy.go @@ -8,7 +8,6 @@ import ( "os/exec" "strings" - "github.com/novusedge/stoat/internal/qemu" "github.com/novusedge/stoat/internal/sshx" ) @@ -52,12 +51,24 @@ func doCopy(ctx context.Context, name, localPath, remotePath string, toRemote bo // A stopped VM is the common cause of an scp failure. Report it as // ErrNotRunning rather than let scp's "connection refused" surface as a // bare non-zero exit. Exec checks the same way. - if !qemu.Running(v) { + state, err := StateOf(ctx, v) + if err != nil { + return err + } + if state != StateRunning { return fmt.Errorf("%w: %s", ErrNotRunning, name) } + p, err := providerFor(v) + if err != nil { + return err + } + ep, err := p.Endpoint(ctx, v) + if err != nil { + return err + } var stderr bytes.Buffer - c := exec.CommandContext(ctx, "scp", sshx.CopyArgs(v, localPath, remotePath, toRemote)...) + c := exec.CommandContext(ctx, "scp", sshx.CopyArgs(ep, localPath, remotePath, toRemote)...) c.Stderr = &stderr err = c.Run() diff --git a/internal/core/core.go b/internal/core/core.go index cd48bbe6..7f42ef89 100644 --- a/internal/core/core.go +++ b/internal/core/core.go @@ -19,16 +19,20 @@ import ( "strings" "github.com/novusedge/stoat/internal/config" + "github.com/novusedge/stoat/internal/coreerr" "github.com/novusedge/stoat/internal/recipes" ) // Typed errors, because every caller branches on them and string matching is // how that goes wrong. Each wraps with the specific subject. +// +// ErrNotFound and ErrInvalidSpec are coreerr's values, not new ones: capabilities.LoadTarget +// wraps the same sentinels, and errors.Is matches by identity. var ( - ErrNotFound = errors.New("not found") + ErrNotFound = coreerr.ErrNotFound ErrNameTaken = errors.New("name already taken") ErrImageNotDownloaded = errors.New("image not downloaded") - ErrInvalidSpec = errors.New("invalid spec") + ErrInvalidSpec = coreerr.ErrInvalidSpec ErrInUse = errors.New("in use") // ErrRecipeNotApplicable: a recipe was named that this VM's OS and // backend cannot run. Typed because a caller retrying with a corrected diff --git a/internal/core/exec.go b/internal/core/exec.go index 75167700..234a1579 100644 --- a/internal/core/exec.go +++ b/internal/core/exec.go @@ -6,7 +6,6 @@ import ( "fmt" "strings" - "github.com/novusedge/stoat/internal/qemu" "github.com/novusedge/stoat/internal/sshx" ) @@ -56,7 +55,11 @@ func Exec(ctx context.Context, name string, cmd []string) (ExecResult, error) { // is the most common reason Exec cannot connect. ErrNotRunning is // faster and clearer than a bare 255 with "connection refused" buried // in stderr. - if !qemu.Running(v) { + state, err := StateOf(ctx, v) + if err != nil { + return ExecResult{}, err + } + if state != StateRunning { return ExecResult{}, fmt.Errorf("%w: %s", ErrNotRunning, name) } diff --git a/internal/core/forward.go b/internal/core/forward.go index afca1f08..886e6ae0 100644 --- a/internal/core/forward.go +++ b/internal/core/forward.go @@ -1,13 +1,13 @@ package core import ( + "context" "fmt" "path/filepath" "strconv" "strings" "github.com/novusedge/stoat/internal/config" - "github.com/novusedge/stoat/internal/qemu" ) // PortForward is an alias, not a copy, of config.PortForward. This package @@ -71,7 +71,11 @@ func Forward(name string, fwds []PortForward) (active bool, err error) { if err := v.Save(); err != nil { return false, err } - return !qemu.Running(v), nil + state, err := StateOf(context.Background(), v) + if err != nil { + return false, err + } + return state != StateRunning, nil } // validateForwards checks a proposed forward list against everything that diff --git a/internal/core/prune.go b/internal/core/prune.go index 030b35d0..ac98e467 100644 --- a/internal/core/prune.go +++ b/internal/core/prune.go @@ -1,6 +1,7 @@ package core import ( + "context" "os" "path/filepath" "regexp" @@ -9,7 +10,6 @@ import ( "time" "github.com/novusedge/stoat/internal/config" - "github.com/novusedge/stoat/internal/qemu" ) // partStaleAfter is how long a *.part file under isos/ sits with no mtime @@ -158,12 +158,12 @@ func pruneBroken(dryRun bool) ([]PruneItem, error) { dir := filepath.Join(config.Root(), b.Name) bv := &config.VM{Name: b.Name, Dir: dir} - // qemu.Running only needs v.Dir and v.PidPath(), both derivable - // without a parsed vm.toml, so it works on a broken VM too. The VM - // may have started before the edit that broke vm.toml. Destroy - // refuses to touch a running VM; Prune must refuse the same way, - // even acting in bulk. - if qemu.Running(bv) { + // StateOf only needs v.Dir and v.PidPath(), both derivable without a + // parsed vm.toml, so it works on a broken VM too. The VM may have + // started before the edit that broke vm.toml. Destroy refuses to + // touch a running VM; Prune must refuse the same way, even acting + // in bulk. + if state, err := StateOf(context.Background(), bv); err == nil && state == StateRunning { continue } diff --git a/internal/core/prune_test.go b/internal/core/prune_test.go index 2e425529..fcfe25c8 100644 --- a/internal/core/prune_test.go +++ b/internal/core/prune_test.go @@ -340,16 +340,15 @@ func TestPruneRemovesABrokenVMWithAStalePidfile(t *testing.T) { // disk. A vm.toml can be corrupted after its VM started; that is the state // this test reaches. // -// It uses fakeRunning, which spawns a real process with the VM's directory -// in its argv: that is what qemu.Running matches on. An earlier version -// used an impossible pid instead, so the guard never ran; the mutation -// `if qemu.Running(bv)` -> `if false && ...` still passed every test. +// It spawns a real process, so the guard exercises qemu.Running on the +// reconstructed Dir. An earlier version used an impossible pid instead, so +// the guard never ran. func TestPruneNeverRemovesARunningBrokenVM(t *testing.T) { dir := root(t) writeBroken(t, dir, "busted") vdir := filepath.Join(dir, "busted") - stop := fakeRunning(t, &config.VM{Name: "busted", Dir: vdir}) + stop := realQemuProcess(t, &config.VM{Name: "busted", Dir: vdir}) defer stop() removed, err := Prune(PruneOpts{Broken: true}) diff --git a/internal/core/update.go b/internal/core/update.go index 6e38ebc3..c394b145 100644 --- a/internal/core/update.go +++ b/internal/core/update.go @@ -1,6 +1,7 @@ package core import ( + "context" "errors" "fmt" "os" @@ -9,7 +10,6 @@ import ( "strings" "github.com/novusedge/stoat/internal/config" - "github.com/novusedge/stoat/internal/qemu" "github.com/novusedge/stoat/internal/recipes" ) @@ -559,7 +559,11 @@ func validateDiskGrow(v *config.VM, size string) (string, error) { // because qemu-img resize runs against the live file below. RAM, CPUs and // SSHPort defer to next start instead. ErrAlreadyRunning matches how Destroy // signals the same "needs stopped, isn't" refusal. - if qemu.Running(v) { + state, err := StateOf(context.Background(), v) + if err != nil { + return "", err + } + if state == StateRunning { return "", fmt.Errorf("%w: disk: stop %s before resizing its disk", ErrAlreadyRunning, v.Name) } diff --git a/internal/core/vm.go b/internal/core/vm.go index 41169128..7254e352 100644 --- a/internal/core/vm.go +++ b/internal/core/vm.go @@ -1,6 +1,7 @@ package core import ( + "context" "errors" "fmt" "os" @@ -9,9 +10,11 @@ import ( "time" "github.com/novusedge/stoat/internal/config" + "github.com/novusedge/stoat/internal/coreerr" "github.com/novusedge/stoat/internal/guest" "github.com/novusedge/stoat/internal/iso" - "github.com/novusedge/stoat/internal/qemu" + "github.com/novusedge/stoat/internal/provider" + _ "github.com/novusedge/stoat/internal/provider/qemu" "github.com/novusedge/stoat/internal/recipes" ) @@ -19,10 +22,11 @@ import ( // fresh from the process table and the filesystem each time. // // The state model has six states. Only three are knowable today. StateStopped -// and StateRunning come from qemu.Running, which checks pid liveness and -// matches /proc//cmdline, so a reused pid never reads as running. -// StateBroken comes from a vm.toml that exists but fails to parse -// (config.ListBroken's concept). +// and StateRunning come from the VM's provider (StateOf); qemu's own check +// matches pid liveness against /proc//cmdline, so a reused pid never +// reads as running. StateBroken has two sources: a vm.toml that exists but +// fails to parse (config.ListBroken's concept), and a vm.toml that parsed +// whose provider cannot be resolved or cannot answer (fromConfigUnchecked). // // StateStarting, StateApplying and StateFailed are not declared here. No // code path yet distinguishes "qemu process is up" from "guest is @@ -57,7 +61,10 @@ var ErrAlreadyRunning = errors.New("already running") // destroyed) rather than looking deleted. Start/Stop/Destroy have no VM view // to attach a state to, so a typed error is the only way to say "broken" to // those callers. -var ErrBroken = errors.New("broken vm.toml") +// +// It is coreerr's value: capabilities.LoadTarget wraps the same sentinel, +// and errors.Is matches by identity. +var ErrBroken = coreerr.ErrBroken // Paths are the on-disk locations for one VM, resolved once here so a caller // (an MCP server describing a VM, a TUI detail screen) does not have to @@ -109,7 +116,7 @@ type RecipeState struct { // VM answers "what is this VM doing right now". It is not the on-disk // record. config.VM is vm.toml: what was asked for, valid the instant it was // last saved. It says nothing about "is it running" on its own; that needs -// qemu.Running too. core.VM combines both, computed together, so a caller +// StateOf too. core.VM combines both, computed together, so a caller // never has one without the other. // // The design doc's VM (§1) also lists Progress and Created. Progress is @@ -192,8 +199,9 @@ type VM struct { Paths Paths // Error is populated only when State is StateBroken, and holds - // config.Load's parse error so a caller can show the user why, not just - // that it's broken. + // config.Load's parse error, or providerFor's or Status's error when the + // config parsed but its provider could not be resolved, so a caller can + // show the user why, not just that it's broken. Error string // Project is the absolute directory of the stoat.toml that declared this @@ -261,8 +269,15 @@ func checkGuest(v *config.VM) error { // State and Paths are the two things config.VM cannot answer for itself. func fromConfigUnchecked(v *config.VM) VM { state := StateStopped - if qemu.Running(v) { - state = StateRunning + var startedAt time.Time + var stateErr string + p, err := providerFor(v) + if err != nil { + state, stateErr = StateBroken, err.Error() + } else if s, err := p.Status(context.Background(), v); err != nil { + state, stateErr = StateBroken, err.Error() + } else if s.Running { + state, startedAt = StateRunning, s.StartedAt } osName, backend := inferMissing(v) return VM{ @@ -274,7 +289,8 @@ func fromConfigUnchecked(v *config.VM) VM { Mode: v.Mode, Backend: backend, State: state, - StartedAt: qemu.StartedAt(v), + Error: stateErr, + StartedAt: startedAt, RAM: v.RAM, CPUs: v.CPUs, CPUModel: v.CPUModel, @@ -511,48 +527,92 @@ func Get(name string) (VM, error) { return fromConfigChecked(v) } -// Start launches VM name. It wraps qemu.Start; the actual work (pidfile, -// backend Prepare, marking a disk VM installed) lives there. +// providerFor resolves v's execution surface. Every core call site goes +// through here rather than provider.For, so a per-provider check added +// later has one place to land. +func providerFor(v *config.VM) (provider.Provider, error) { return provider.For(v) } + +// StateOf asks v's provider what the machine is doing. A provider this +// binary does not implement is an error, never a fallback to qemu: a VM +// created by a newer stoat must not be operated as a local one. +func StateOf(ctx context.Context, v *config.VM) (State, error) { + p, err := providerFor(v) + if err != nil { + return StateBroken, err + } + s, err := p.Status(ctx, v) + if err != nil { + return StateBroken, err + } + if s.Running { + return StateRunning, nil + } + return StateStopped, nil +} + +// Start launches VM name through its provider. The actual work (pidfile, +// backend Prepare, marking a disk VM installed) lives in the qemu provider +// for a local VM. // -// qemu.Start already refuses to run twice, but with an untyped error -// ("%s is already running") that a caller can only detect by string -// matching. Start checks qemu.Running first and returns typed -// ErrAlreadyRunning instead; qemu.Start's own check never fires, because +// The provider's own Start already refuses to run twice, but with an +// untyped error ("%s is already running") that a caller can only detect by +// string matching. Start checks StateOf first and returns typed +// ErrAlreadyRunning instead; the provider's own check never fires, because // this function has already returned. func Start(name string) error { v, err := load(name) if err != nil { return err } - if qemu.Running(v) { + state, err := StateOf(context.Background(), v) + if err != nil { + return err + } + if state == StateRunning { return fmt.Errorf("%w: %s", ErrAlreadyRunning, name) } - return qemu.Start(v) + p, err := providerFor(v) + if err != nil { + return err + } + return p.Start(context.Background(), v) } // EnsureRunning refuses with ErrNotRunning when v is not running. It exists // so a caller above core, such as mcpsrv, answers the same error Stop and -// Exec give without importing qemu.Running itself. +// Exec give without calling StateOf itself. func EnsureRunning(v *config.VM) error { - if !qemu.Running(v) { + state, err := StateOf(context.Background(), v) + if err != nil { + return err + } + if state != StateRunning { return fmt.Errorf("%w: %s", ErrNotRunning, v.Name) } return nil } -// Stop powers down VM name. qemu.Stop treats "already stopped" as a -// successful no-op; Stop does not. The CLI's `down` (internal/cli/cli.go's -// runDown) already refuses a stopped VM as a failure, and Stop preserves -// that behavior. +// Stop powers down VM name through its provider. The provider's own Stop +// treats "already stopped" as a successful no-op; Stop does not. The CLI's +// `down` (internal/cli/cli.go's runDown) already refuses a stopped VM as a +// failure, and Stop preserves that behavior. func Stop(name string) error { v, err := load(name) if err != nil { return err } - if !qemu.Running(v) { + state, err := StateOf(context.Background(), v) + if err != nil { + return err + } + if state != StateRunning { return fmt.Errorf("%w: %s", ErrNotRunning, name) } - return qemu.Stop(v) + p, err := providerFor(v) + if err != nil { + return err + } + return p.Stop(context.Background(), v) } // Destroy removes VM name's directory and vm.toml. @@ -585,14 +645,14 @@ func Destroy(name string) error { // why Get/List surface broken VMs instead of hiding them: they can // be cleared instead of sitting forever unparseable. // - // The running check below still applies. qemu.Running only needs - // Dir, which is reconstructed here; it does not need a parsed - // config.VM. Skipping this check let a vm.toml corrupted after its - // VM was started bypass the running-VM refusal, deleting the - // directory, pidfile, monitor socket and disk out from under a live - // qemu process. + // The running check below still applies. StateOf only needs Dir, + // which is reconstructed here; it does not need a parsed config.VM. + // Skipping this check let a vm.toml corrupted after its VM was + // started bypass the running-VM refusal, deleting the directory, + // pidfile, monitor socket and disk out from under a live qemu + // process. bv := &config.VM{Name: name, Dir: filepath.Join(config.Root(), name)} - if qemu.Running(bv) { + if state, err := StateOf(context.Background(), bv); err == nil && state == StateRunning { return fmt.Errorf("%w: %s: stop it first", ErrAlreadyRunning, name) } return bv.Delete() @@ -600,7 +660,11 @@ func Destroy(name string) error { if err != nil { return err } - if qemu.Running(v) { + state, err := StateOf(context.Background(), v) + if err != nil { + return err + } + if state == StateRunning { return fmt.Errorf("%w: %s: stop it first", ErrAlreadyRunning, name) } return v.Delete() diff --git a/internal/core/vm_test.go b/internal/core/vm_test.go index 734fb01d..4899d82a 100644 --- a/internal/core/vm_test.go +++ b/internal/core/vm_test.go @@ -1,6 +1,7 @@ package core import ( + "context" "errors" "os" "path/filepath" @@ -9,6 +10,9 @@ import ( "time" "github.com/novusedge/stoat/internal/config" + "github.com/novusedge/stoat/internal/provider" + "github.com/novusedge/stoat/internal/provider/fake" + "github.com/novusedge/stoat/internal/sshx" "github.com/novusedge/stoat/internal/testutil" ) @@ -27,16 +31,25 @@ func writeRawVMToml(t *testing.T, name, content string) { } } -// fakeRunning marks v as running without a real qemu process. It spawns -// `sleep` with v.Dir in its argv (qemu.Running's cmdlineMatches only checks -// that /proc//cmdline contains dir+"/", not which binary it is), and -// points the VM's pidfile at it. The returned func kills the process; the -// caller must defer it. -// -// This makes Start/Stop/Destroy's "is it running" branches testable without -// qemu-system-x86_64 installed, the same CI constraint existing tests work -// around for qemu-img. +// fakeRunning marks v as running in the fake provider installed for this +// test. The returned func marks it stopped again; the caller must call or +// defer it. func fakeRunning(t *testing.T, v *config.VM) func() { + f := fake.Install(t) + f.SetRunning(v.Name) + return func() { f.SetStopped(v.Name) } +} + +// realQemuProcess spawns a real process and points v's pidfile at it, +// leaving the real "qemu" provider registered. Use this instead of +// fakeRunning for a test that exercises code reading qemu.Running directly +// (clone.go), a provider.Start call that must actually run and fail +// (autorestart_test.go's ISO-missing case: fake.Install swaps in a Start +// that always succeeds, which would pass that test for the wrong reason), +// or a broken-VM path whose running check is fed a hand-reconstructed +// config.VM (TestDestroyRefusesARunningBrokenVM below), where fakeRunning's +// name-only match can't tell whether Dir was reconstructed. +func realQemuProcess(t *testing.T, v *config.VM) func() { return testutil.FakeRunning(t, v.Dir) } @@ -235,8 +248,7 @@ func TestStartAlreadyRunning(t *testing.T) { t.Fatal(err) } v.Dir = filepath.Join(dir, "work") - stop := fakeRunning(t, v) - defer stop() + defer fakeRunning(t, v)() if err := Start("work"); !errors.Is(err, ErrAlreadyRunning) { t.Fatalf("err = %v, want ErrAlreadyRunning", err) @@ -275,15 +287,12 @@ func TestDestroyRefusesWhileRunning(t *testing.T) { stop := fakeRunning(t, v) if err := Destroy("work"); !errors.Is(err, ErrAlreadyRunning) { - stop() t.Fatalf("err = %v, want ErrAlreadyRunning", err) } if _, err := os.Stat(v.Dir); err != nil { - stop() t.Fatalf("VM directory should still exist after a refused destroy: %v", err) } stop() - _ = os.Remove(v.PidPath()) if err := Destroy("work"); err != nil { t.Fatalf("Destroy after stopping: %v", err) @@ -471,30 +480,33 @@ func TestGetDoesNotModifyVMTomlOnDisk(t *testing.T) { // TestStartedAtRunningVsStopped pins that a running VM's StartedAt comes // from the pidfile qemu.Running just read, and a stopped one gets the zero -// time, not some stale value left over from a previous run. +// time, not some stale value left over from a previous run. It uses +// realQemuProcess, not fakeRunning, so this still exercises qemu.StartedAt +// reading the pidfile's mtime rather than a hardcoded fake value. func TestStartedAtRunningVsStopped(t *testing.T) { dir := root(t) - if err := (&config.VM{Name: "work", Mode: "live", RAM: 1024, CPUs: 1, SSHPort: 2200}).Save(); err != nil { + v := &config.VM{Name: "work", Mode: "live", RAM: 1024, CPUs: 1, SSHPort: 2200} + if err := v.Save(); err != nil { t.Fatal(err) } + v.Dir = filepath.Join(dir, "work") - v, err := Get("work") + got, err := Get("work") if err != nil { t.Fatal(err) } - if !v.StartedAt.IsZero() { - t.Errorf("stopped VM: StartedAt = %v, want the zero time", v.StartedAt) + if !got.StartedAt.IsZero() { + t.Errorf("stopped VM: StartedAt = %v, want the zero time", got.StartedAt) } - cv := &config.VM{Name: "work", Dir: filepath.Join(dir, "work")} - stop := fakeRunning(t, cv) + stop := realQemuProcess(t, v) defer stop() - v, err = Get("work") + got, err = Get("work") if err != nil { t.Fatal(err) } - if v.StartedAt.IsZero() { + if got.StartedAt.IsZero() { t.Error("running VM: StartedAt is the zero time, want the pidfile's mtime") } } @@ -550,7 +562,7 @@ func TestAppliedNilWhenNoRecipesApplied(t *testing.T) { } // TestDestroyRefusesARunningBrokenVM pins a real bug: Destroy's broken-VM -// branch once skipped the running check. qemu.Running needs only Dir, which +// branch once skipped the running check. StateOf needs only Dir, which // that branch reconstructs. Skipping the check let a vm.toml corrupted // after its VM was started bypass the refusal that applies to every // healthy VM, deleting the pidfile, monitor socket and disk from under a @@ -559,8 +571,7 @@ func TestDestroyRefusesARunningBrokenVM(t *testing.T) { root(t) // A directory that is running but whose vm.toml no longer parses. writeRawVMToml(t, "hosed", "name = \"hosed\"\nmode = \"disk\n") - stop := fakeRunning(t, &config.VM{Name: "hosed", Dir: filepath.Join(config.Root(), "hosed")}) - defer stop() + defer realQemuProcess(t, &config.VM{Name: "hosed", Dir: filepath.Join(config.Root(), "hosed")})() if err := Destroy("hosed"); !errors.Is(err, ErrAlreadyRunning) { t.Fatalf("Destroy on a running broken VM = %v, want ErrAlreadyRunning", err) @@ -569,3 +580,80 @@ func TestDestroyRefusesARunningBrokenVM(t *testing.T) { t.Fatalf("the directory was deleted from under a running qemu: %v", err) } } + +// The fake is keyed by Name and the real provider by Dir, so this fails if +// StateOf stops consulting the registry. +func TestStateOfFollowsTheProvider(t *testing.T) { + f := fake.Install(t) + v := &config.VM{Name: "dev", Dir: t.TempDir()} + + if got, err := StateOf(context.Background(), v); err != nil || got != StateStopped { + t.Fatalf("StateOf() = %q, %v, want %q, nil", got, err, StateStopped) + } + f.SetRunning("dev") + if got, err := StateOf(context.Background(), v); err != nil || got != StateRunning { + t.Fatalf("StateOf() = %q, %v, want %q, nil", got, err, StateRunning) + } + f.SetStopped("dev") + if got, err := StateOf(context.Background(), v); err != nil || got != StateStopped { + t.Fatalf("StateOf() = %q, %v, want %q, nil", got, err, StateStopped) + } +} + +func TestStateRejectsAnUnknownProvider(t *testing.T) { + v := &config.VM{Name: "dev", Dir: t.TempDir(), Provider: "nope"} + if _, err := StateOf(context.Background(), v); !errors.Is(err, provider.ErrUnknownProvider) { + t.Errorf("StateOf() error = %v, want ErrUnknownProvider", err) + } +} + +// The point of the endpoint seam is that a VM answering somewhere other than +// a loopback forward is reached there. A fake endpoint is the only way to +// prove SSHCommand consults the provider instead of rebuilding +// sshx.LocalEndpoint for itself. +func TestSSHCommandUsesTheProvidersEndpoint(t *testing.T) { + root(t) + f := fake.Install(t) + f.Ep = sshx.Endpoint{Name: "work", Host: "203.0.113.7", Port: 2022, User: "stoat"} + if err := (&config.VM{Name: "work", Mode: "cloud", RAM: 1024, CPUs: 1, SSHPort: 2200}).Save(); err != nil { + t.Fatal(err) + } + + argv, err := SSHCommand("work") + if err != nil { + t.Fatal(err) + } + got := strings.Join(argv, " ") + if !strings.Contains(got, "stoat@203.0.113.7") { + t.Errorf("SSHCommand = %q, want the provider's host", got) + } + if !strings.Contains(got, "-p 2022") { + t.Errorf("SSHCommand = %q, want the provider's port", got) + } + if strings.Contains(got, "127.0.0.1") { + t.Errorf("SSHCommand = %q, must not fall back to loopback", got) + } +} + +func TestStartAndStopPropagateTheProvidersError(t *testing.T) { + root(t) + f := fake.Install(t) + boom := errors.New("provider refused") + f.StartErr = boom + if err := (&config.VM{Name: "work", Mode: "live", RAM: 1024, CPUs: 1, SSHPort: 2200}).Save(); err != nil { + t.Fatal(err) + } + + if err := Start("work"); !errors.Is(err, boom) { + t.Errorf("Start() = %v, want the provider's error", err) + } + + f.StartErr = nil + if err := Start("work"); err != nil { + t.Fatal(err) + } + f.StopErr = boom + if err := Stop("work"); !errors.Is(err, boom) { + t.Errorf("Stop() = %v, want the provider's error", err) + } +} diff --git a/internal/core/wait.go b/internal/core/wait.go index 0dabacb8..cc16c9ef 100644 --- a/internal/core/wait.go +++ b/internal/core/wait.go @@ -14,7 +14,6 @@ import ( "github.com/novusedge/stoat/internal/backend" "github.com/novusedge/stoat/internal/config" - "github.com/novusedge/stoat/internal/qemu" ) // Until is a state Wait can block for. It is a small, closed set distinct @@ -32,7 +31,7 @@ const ( // UntilApplied is the most recent recipe run having finished // successfully. See waitApplied. UntilApplied Until = "applied" - // UntilStopped is qemu.Running turning false. + // UntilStopped is the provider's state turning to not-running. UntilStopped Until = "stopped" // UntilHealthy is every applied recipe's health check passing. UntilHealthy Until = "healthy" @@ -171,22 +170,37 @@ func healthFailure(verdict RecipeHealth) error { // polled. Wait never starts a VM itself; that is Start's job. A VM that is // already not running can never bring sshd up on its own. func waitReachable(ctx context.Context, v *config.VM) error { - if !qemu.Running(v) { + state, err := StateOf(ctx, v) + if err != nil { + return err + } + if state != StateRunning { return fmt.Errorf("%w: %s: not running", ErrCannotReach, v.Name) } return pollUntil(ctx, func() bool { return sshBannerUp(ctx, v) }) } -// sshBannerUp reports whether v's forwarded port answers as a real sshd, not +// sshBannerUp reports whether v's ssh endpoint answers as a real sshd, not // merely accepting TCP. QEMU/libslirp's user-mode networking accepts the // host-side socket before the guest is dialled, so a bare accept() proves // nothing. sshBannerUp requires the "SSH-" identification banner, the same // check sshx.Wait's bannerReady makes. It is a ctx-aware reimplementation, // not a call to bannerReady, because sshx.Wait takes a fixed timeout and // cannot give up early when ctx is cancelled mid-dial. +// +// The address comes from the provider, so a VM that answers somewhere other +// than a loopback forward is probed where it actually lives. func sshBannerUp(ctx context.Context, v *config.VM) bool { + p, err := providerFor(v) + if err != nil { + return false + } + e, err := p.Endpoint(ctx, v) + if err != nil { + return false + } d := net.Dialer{Timeout: time.Second} - c, err := d.DialContext(ctx, "tcp", fmt.Sprintf("127.0.0.1:%d", v.SSHPort)) + c, err := d.DialContext(ctx, "tcp", fmt.Sprintf("%s:%d", e.Host, e.Port)) if err != nil { return false } @@ -276,12 +290,15 @@ func lastProvisionLineIs(v *config.VM, want string) bool { return false } -// waitStopped blocks until qemu.Running(v) turns false. Unlike Reachable and -// Applied, there is no impossible-by-construction case to refuse up front: -// a running VM can always, in principle, be stopped by something else before -// ctx gives up. +// waitStopped blocks until v's provider reports it not running. Unlike +// Reachable and Applied, there is no impossible-by-construction case to +// refuse up front: a running VM can always, in principle, be stopped by +// something else before ctx gives up. func waitStopped(ctx context.Context, v *config.VM) error { - return pollUntil(ctx, func() bool { return !qemu.Running(v) }) + return pollUntil(ctx, func() bool { + state, err := StateOf(ctx, v) + return err == nil && state != StateRunning + }) } // pollUntil calls check immediately, so an already-satisfied condition diff --git a/internal/coreerr/coreerr.go b/internal/coreerr/coreerr.go new file mode 100644 index 00000000..4befddf7 --- /dev/null +++ b/internal/coreerr/coreerr.go @@ -0,0 +1,15 @@ +// Package coreerr holds the sentinel errors internal/core and +// internal/capabilities both need to identify by identity (errors.Is). +// capabilities cannot import core directly: core imports provider, provider +// imports capabilities, and a capabilities->core edge would close that +// cycle. core re-exports these under its own names so every existing +// core.ErrXxx caller compiles unchanged. +package coreerr + +import "errors" + +var ( + ErrNotFound = errors.New("not found") + ErrInvalidSpec = errors.New("invalid spec") + ErrBroken = errors.New("broken vm.toml") +) diff --git a/internal/hostcheck/checks_other.go b/internal/hostcheck/checks_other.go index 72dd5973..584f4126 100644 --- a/internal/hostcheck/checks_other.go +++ b/internal/hostcheck/checks_other.go @@ -7,7 +7,7 @@ import "github.com/novusedge/stoat/internal/hostops" // RunChecks reports the native qualification boundary without probing Linux // binaries or /dev/kvm. Those requirements do not describe this host. func RunChecks(_ Distro) []Check { - err := hostops.RequireVM() + err := hostops.RequireLocalHypervisor() return []Check{{ Name: "native VM operations", Detail: err.Error(), diff --git a/internal/hostops/message_test.go b/internal/hostops/message_test.go index a3aafc8d..bd9835f6 100644 --- a/internal/hostops/message_test.go +++ b/internal/hostops/message_test.go @@ -23,7 +23,7 @@ func TestMessage(t *testing.T) { "darwin/arm64", "QEMU HVF accelerator", "stoat#82", - "doctor and capabilities still work", + "starting and stopping a local VM needs a qualified host", "Linux with KVM is the supported configuration today", }, }, @@ -35,7 +35,7 @@ func TestMessage(t *testing.T) { "windows/amd64", "QEMU WHPX accelerator", "stoat#83", - "doctor and capabilities still work", + "starting and stopping a local VM needs a qualified host", "Linux with KVM is the supported configuration today", }, }, diff --git a/internal/hostops/support.go b/internal/hostops/support.go index 24ca9909..4fa3d654 100644 --- a/internal/hostops/support.go +++ b/internal/hostops/support.go @@ -7,8 +7,9 @@ import ( ) // ErrUnsupported reports a native host whose VM operations have not been -// qualified yet. Only doctor and capabilities run there. The gate in -// internal/cli sits ahead of every other command, so ls and get refuse too. +// qualified yet. Only RequireLocalHypervisor returns it, so a command that +// starts or stops a local VM refuses while one that reads or edits a VM +// record does not. var ErrUnsupported = errors.New("native VM operations are not qualified") // requirement names the accelerator a host needs before stoat qualifies it, @@ -44,7 +45,7 @@ func Message(goos, goarch string) string { lines = append(lines, fmt.Sprintf("%s has no qualified runtime yet.", host)) } lines = append(lines, - "doctor and capabilities still work here; every other command needs a qualified host.", + "starting and stopping a local VM needs a qualified host; commands that only read or edit VM records still work here.", "Linux with KVM is the supported configuration today.", ) out := lines[0] diff --git a/internal/hostops/support_linux.go b/internal/hostops/support_linux.go index 3a341cd7..dbba330a 100644 --- a/internal/hostops/support_linux.go +++ b/internal/hostops/support_linux.go @@ -2,6 +2,11 @@ package hostops -// RequireVM permits native VM operations on Linux, the currently qualified -// host platform. -func RequireVM() error { return nil } +// RequireLocalHypervisor permits starting a VM on this host. Linux is the +// qualified platform. +func RequireLocalHypervisor() error { return nil } + +// RequireDataRoot permits owning a data root. Reading, writing and listing +// VM records needs no hypervisor, so a host that cannot start a local VM can +// still manage one that runs elsewhere. +func RequireDataRoot() error { return nil } diff --git a/internal/hostops/support_other.go b/internal/hostops/support_other.go index f4b6f9b3..6e6e3af5 100644 --- a/internal/hostops/support_other.go +++ b/internal/hostops/support_other.go @@ -4,14 +4,18 @@ package hostops import "runtime" -// RequireVM refuses native VM operations until this host platform has a -// complete runtime qualification. The message names what is missing, what -// still works, and where the work is tracked, so the refusal is actionable -// without a doc lookup. -func RequireVM() error { +// RequireLocalHypervisor refuses native VM operations until this host +// platform has a complete runtime qualification. The message names what is +// missing, what still works, and where the work is tracked, so the refusal +// is actionable without a doc lookup. +func RequireLocalHypervisor() error { return unsupportedError{Message(runtime.GOOS, runtime.GOARCH)} } +// RequireDataRoot permits owning a data root on any host. See the Linux +// implementation for why this is not gated. +func RequireDataRoot() error { return nil } + // unsupportedError carries Message's text as Error() while still satisfying // errors.Is(err, ErrUnsupported): fmt.Errorf("%w: %s", ...) would duplicate // Message's own "not qualified on " opening line. diff --git a/internal/hostops/support_test.go b/internal/hostops/support_test.go index 62df09c5..47621a3e 100644 --- a/internal/hostops/support_test.go +++ b/internal/hostops/support_test.go @@ -1,5 +1,3 @@ -//go:build !linux - package hostops import ( @@ -9,16 +7,18 @@ import ( "testing" ) -func TestRequireVMUnsupportedHost(t *testing.T) { - err := RequireVM() - if err == nil { - t.Fatal("RequireVM() = nil on an unqualified native host") +func TestRequireLocalHypervisorFollowsPlatform(t *testing.T) { + err := RequireLocalHypervisor() + if runtime.GOOS == "linux" { + if err != nil { + t.Errorf("RequireLocalHypervisor() = %v, want nil on linux", err) + } + return } if !errors.Is(err, ErrUnsupported) { - t.Fatalf("RequireVM() = %v, want errors.Is(..., ErrUnsupported)", err) + t.Errorf("RequireLocalHypervisor() = %v, want ErrUnsupported", err) } - wantHost := runtime.GOOS + "/" + runtime.GOARCH - if !strings.Contains(err.Error(), wantHost) { - t.Errorf("RequireVM() = %q, want it to identify %s", err, wantHost) + if !strings.Contains(err.Error(), runtime.GOOS+"/"+runtime.GOARCH) { + t.Errorf("RequireLocalHypervisor() = %v, want message to name %s/%s", err, runtime.GOOS, runtime.GOARCH) } } diff --git a/internal/mcpsrv/tools_read.go b/internal/mcpsrv/tools_read.go index 4ee0d2b1..643b9aff 100644 --- a/internal/mcpsrv/tools_read.go +++ b/internal/mcpsrv/tools_read.go @@ -9,8 +9,21 @@ import ( "github.com/novusedge/stoat/internal/capabilities" "github.com/novusedge/stoat/internal/cli/wire" "github.com/novusedge/stoat/internal/core" + "github.com/novusedge/stoat/internal/hostcheck" ) +// toHostChecks adapts core.Doctor's result to capabilities.Input.HostChecks. +// The two types have identical fields but capabilities cannot import core: +// core now imports provider, which imports capabilities, so the reverse +// import would close a cycle. +func toHostChecks(cs []core.HostCheck) []hostcheck.Check { + out := make([]hostcheck.Check, len(cs)) + for i, c := range cs { + out[i] = hostcheck.Check{Name: c.Name, OK: c.OK, Detail: c.Detail, Fix: c.Fix, Optional: c.Optional} + } + return out +} + type emptyIn struct{} type vmIn struct { @@ -159,7 +172,7 @@ func (s *srv) registerRead(server *mcp.Server) { } return wire.Capabilities(capabilities.Build(capabilities.Input{ Version: s.opts.Version, ProjectState: projectState, - HostChecks: core.Doctor(), Target: target, + HostChecks: toHostChecks(core.Doctor()), Target: target, })), nil }) diff --git a/internal/provider/fake/fake.go b/internal/provider/fake/fake.go new file mode 100644 index 00000000..457f38b0 --- /dev/null +++ b/internal/provider/fake/fake.go @@ -0,0 +1,97 @@ +// Package fake is a provider.Provider for tests. It lets a core test run with +// no QEMU process and no cloud API. +package fake + +import ( + "context" + "sync" + "testing" + "time" + + "github.com/novusedge/stoat/internal/capabilities" + "github.com/novusedge/stoat/internal/config" + "github.com/novusedge/stoat/internal/provider" + _ "github.com/novusedge/stoat/internal/provider/qemu" + "github.com/novusedge/stoat/internal/sshx" +) + +// Provider answers from a liveness map the test controls. core.waitStopped +// polls Status from its own goroutine while the test flips a VM, so every +// read and write of that map takes mu. +type Provider struct { + StartErr error + StopErr error + Ep sshx.Endpoint + + mu sync.Mutex + running map[string]bool +} + +func (*Provider) Name() string { return "qemu" } +func (*Provider) Capabilities(*config.VM) []capabilities.Capability { return nil } + +// SetRunning marks name live. Tests that need a VM to look started without +// calling Start use this. +func (p *Provider) SetRunning(name string) { + p.mu.Lock() + defer p.mu.Unlock() + p.running[name] = true +} + +// SetStopped marks name dead. +func (p *Provider) SetStopped(name string) { + p.mu.Lock() + defer p.mu.Unlock() + delete(p.running, name) +} + +// IsRunning reports the recorded liveness of name. +func (p *Provider) IsRunning(name string) bool { + p.mu.Lock() + defer p.mu.Unlock() + return p.running[name] +} + +func (p *Provider) Start(_ context.Context, v *config.VM) error { + if p.StartErr != nil { + return p.StartErr + } + p.SetRunning(v.Name) + return nil +} + +func (p *Provider) Stop(_ context.Context, v *config.VM) error { + if p.StopErr != nil { + return p.StopErr + } + p.SetStopped(v.Name) + return nil +} + +func (p *Provider) Status(_ context.Context, v *config.VM) (provider.Status, error) { + if !p.IsRunning(v.Name) { + return provider.Status{}, nil + } + return provider.Status{Running: true, StartedAt: time.Unix(1, 0)}, nil +} + +func (p *Provider) Endpoint(_ context.Context, v *config.VM) (sshx.Endpoint, error) { + if p.Ep.Host != "" { + return p.Ep, nil + } + return sshx.LocalEndpoint(v), nil +} + +// Install registers a fresh fake under "qemu" and restores the previous +// provider when the test ends. +func Install(t *testing.T) *Provider { + t.Helper() + prev, err := provider.For(&config.VM{}) + if err != nil { + t.Fatalf("no provider registered as qemu: %v", err) + } + f := &Provider{running: map[string]bool{}} + provider.Register("qemu", f) + t.Cleanup(func() { provider.Register("qemu", prev) }) + return f +} diff --git a/internal/provider/fake/fake_test.go b/internal/provider/fake/fake_test.go new file mode 100644 index 00000000..aa6083fa --- /dev/null +++ b/internal/provider/fake/fake_test.go @@ -0,0 +1,43 @@ +package fake + +import ( + "context" + "testing" + + "github.com/novusedge/stoat/internal/config" + "github.com/novusedge/stoat/internal/provider" +) + +func TestInstallReplacesQemuForOneTest(t *testing.T) { + before, err := provider.For(&config.VM{}) + if err != nil { + t.Fatalf("For() error = %v", err) + } + + t.Run("installed", func(t *testing.T) { + f := Install(t) + f.SetRunning("dev") + p, err := provider.For(&config.VM{Name: "dev"}) + if err != nil { + t.Fatalf("For() error = %v", err) + } + if p != provider.Provider(f) { + t.Fatalf("For() = %T, want the installed fake", p) + } + s, err := p.Status(context.Background(), &config.VM{Name: "dev"}) + if err != nil { + t.Fatalf("Status() error = %v", err) + } + if !s.Running { + t.Error("Running = false, want true: the fake was told this VM runs") + } + }) + + after, err := provider.For(&config.VM{}) + if err != nil { + t.Fatalf("For() error = %v", err) + } + if after != before { + t.Errorf("provider after the subtest = %T, want the qemu provider restored", after) + } +} diff --git a/internal/provider/provider.go b/internal/provider/provider.go new file mode 100644 index 00000000..e40177f2 --- /dev/null +++ b/internal/provider/provider.go @@ -0,0 +1,61 @@ +// Package provider owns the execution surface a VM runs on. internal/qemu is +// one surface; a cloud API is another. It sits below internal/core and must +// never import it: core maps a Status onto core.State, and the reverse +// dependency would close a cycle. +package provider + +import ( + "context" + "errors" + "fmt" + "time" + + "github.com/novusedge/stoat/internal/capabilities" + "github.com/novusedge/stoat/internal/config" + "github.com/novusedge/stoat/internal/sshx" +) + +// ErrUnknownProvider is For's answer for a vm.toml naming a provider this +// binary does not implement. It is never a fallback to qemu: a VM created by +// a newer stoat must not be operated as a local one. +var ErrUnknownProvider = errors.New("unknown provider") + +// Status is what a provider knows about a machine. Raw carries the +// provider's own status word so a frontend can report what the API said; +// it is empty for a provider whose states already match Running. +type Status struct { + Running bool + StartedAt time.Time + Raw string +} + +// Provider is one execution surface. Endpoint is the boundary: everything +// above it reaches the guest over ssh and needs no provider knowledge. +type Provider interface { + Name() string + Capabilities(v *config.VM) []capabilities.Capability + Start(ctx context.Context, v *config.VM) error + Stop(ctx context.Context, v *config.VM) error + Status(ctx context.Context, v *config.VM) (Status, error) + Endpoint(ctx context.Context, v *config.VM) (sshx.Endpoint, error) +} + +var registry = map[string]Provider{} + +// Register adds p under name. Implementations register from their own +// package's init, so importing an implementation is what makes it available. +func Register(name string, p Provider) { registry[name] = p } + +// For resolves v's provider. An empty field means "qemu": every vm.toml +// written before the field existed describes a local QEMU VM. +func For(v *config.VM) (Provider, error) { + name := v.Provider + if name == "" { + name = "qemu" + } + p, ok := registry[name] + if !ok { + return nil, fmt.Errorf("%w: %q", ErrUnknownProvider, name) + } + return p, nil +} diff --git a/internal/provider/provider_test.go b/internal/provider/provider_test.go new file mode 100644 index 00000000..333ea0ac --- /dev/null +++ b/internal/provider/provider_test.go @@ -0,0 +1,43 @@ +package provider + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/novusedge/stoat/internal/capabilities" + "github.com/novusedge/stoat/internal/config" + "github.com/novusedge/stoat/internal/sshx" +) + +type stub struct{ name string } + +func (s stub) Name() string { return s.name } +func (stub) Capabilities(*config.VM) []capabilities.Capability { return nil } +func (stub) Start(context.Context, *config.VM) error { return nil } +func (stub) Stop(context.Context, *config.VM) error { return nil } +func (stub) Status(context.Context, *config.VM) (Status, error) { + return Status{Running: true, StartedAt: time.Unix(1, 0)}, nil +} +func (stub) Endpoint(context.Context, *config.VM) (sshx.Endpoint, error) { + return sshx.Endpoint{}, nil +} + +func TestForDefaultsToQemuWhenUnset(t *testing.T) { + Register("qemu", stub{name: "qemu"}) + p, err := For(&config.VM{Name: "dev"}) + if err != nil { + t.Fatalf("For() error = %v", err) + } + if p.Name() != "qemu" { + t.Errorf("Name() = %q, want qemu for a VM with no provider field", p.Name()) + } +} + +func TestForRejectsUnknownProvider(t *testing.T) { + _, err := For(&config.VM{Name: "dev", Provider: "nope"}) + if !errors.Is(err, ErrUnknownProvider) { + t.Errorf("For() error = %v, want ErrUnknownProvider", err) + } +} diff --git a/internal/provider/qemu/qemu.go b/internal/provider/qemu/qemu.go new file mode 100644 index 00000000..cb1e278f --- /dev/null +++ b/internal/provider/qemu/qemu.go @@ -0,0 +1,35 @@ +// Package qemu implements provider.Provider over internal/qemu, the local +// hypervisor. It holds no logic of its own: every call forwards. +package qemu + +import ( + "context" + + "github.com/novusedge/stoat/internal/capabilities" + "github.com/novusedge/stoat/internal/config" + "github.com/novusedge/stoat/internal/provider" + "github.com/novusedge/stoat/internal/qemu" + "github.com/novusedge/stoat/internal/sshx" +) + +func init() { provider.Register("qemu", Provider{}) } + +type Provider struct{} + +func (Provider) Name() string { return "qemu" } + +// Capabilities returns nothing. The capability set still lives in +// internal/capabilities and moves here with the code that reads it. +func (Provider) Capabilities(*config.VM) []capabilities.Capability { return nil } + +func (Provider) Start(_ context.Context, v *config.VM) error { return qemu.Start(v) } + +func (Provider) Stop(_ context.Context, v *config.VM) error { return qemu.Stop(v) } + +func (Provider) Status(_ context.Context, v *config.VM) (provider.Status, error) { + return provider.Status{Running: qemu.Running(v), StartedAt: qemu.StartedAt(v)}, nil +} + +func (Provider) Endpoint(_ context.Context, v *config.VM) (sshx.Endpoint, error) { + return sshx.LocalEndpoint(v), nil +} diff --git a/internal/provider/qemu/qemu_test.go b/internal/provider/qemu/qemu_test.go new file mode 100644 index 00000000..327df996 --- /dev/null +++ b/internal/provider/qemu/qemu_test.go @@ -0,0 +1,55 @@ +package qemu + +import ( + "context" + "testing" + + "github.com/novusedge/stoat/internal/config" + "github.com/novusedge/stoat/internal/provider" + "github.com/novusedge/stoat/internal/testutil" +) + +func TestRegisteredAsQemu(t *testing.T) { + p, err := provider.For(&config.VM{Name: "dev"}) + if err != nil { + t.Fatalf("For() error = %v", err) + } + if p.Name() != "qemu" { + t.Errorf("Name() = %q, want qemu", p.Name()) + } +} + +func TestEndpointIsLoopback(t *testing.T) { + p := Provider{} + e, err := p.Endpoint(context.Background(), &config.VM{Name: "dev", SSHPort: 2222}) + if err != nil { + t.Fatalf("Endpoint() error = %v", err) + } + if e.Host != "127.0.0.1" || e.Port != 2222 { + t.Errorf("Endpoint() = %s:%d, want 127.0.0.1:2222", e.Host, e.Port) + } + if e.KnownHosts != "" { + t.Errorf("KnownHosts = %q, want empty: a loopback forward is not pinned", e.KnownHosts) + } +} + +func TestStatusReflectsTheQemuProcess(t *testing.T) { + v := &config.VM{Name: "dev", Dir: t.TempDir()} + + s, err := Provider{}.Status(context.Background(), v) + if err != nil || s.Running || !s.StartedAt.IsZero() { + t.Fatalf("Status() = %+v, %v, want a stopped zero status", s, err) + } + + defer testutil.FakeRunning(t, v.Dir)() + s, err = Provider{}.Status(context.Background(), v) + if err != nil { + t.Fatalf("Status() error = %v", err) + } + if !s.Running { + t.Error("Running = false, want true while the pidfile names a live process") + } + if s.StartedAt.IsZero() { + t.Error("StartedAt is zero, want the pidfile's mtime") + } +} diff --git a/internal/qemu/run.go b/internal/qemu/run.go index 52576f46..7827022e 100644 --- a/internal/qemu/run.go +++ b/internal/qemu/run.go @@ -46,7 +46,7 @@ func diskWritten(v *config.VM) bool { // Start launches QEMU. -daemonize means it detaches itself; stoat supervises // nothing and tracks the process by pidfile. func Start(v *config.VM) error { - if err := hostops.RequireVM(); err != nil { + if err := hostops.RequireLocalHypervisor(); err != nil { return err } if Running(v) { @@ -147,7 +147,7 @@ func consoleCredential(v *config.VM, user string) string { // back to SIGTERM. The fallback is a power cut: fine for live VMs, lossy for // disk ones, which is why it is not the first move. func Stop(v *config.VM) error { - if err := hostops.RequireVM(); err != nil { + if err := hostops.RequireLocalHypervisor(); err != nil { return err } if !Running(v) { diff --git a/internal/sshx/endpoint.go b/internal/sshx/endpoint.go new file mode 100644 index 00000000..556a4e01 --- /dev/null +++ b/internal/sshx/endpoint.go @@ -0,0 +1,30 @@ +package sshx + +import "github.com/novusedge/stoat/internal/config" + +// Endpoint is where ssh(1) and scp(1) reach a guest, and under what host key +// policy. A QEMU VM answers on a loopback forward, where an unchecked host key +// costs nothing. A cloud instance answers on a routable address, where the +// same setting is a machine-in-the-middle hole, so that endpoint names a +// per-VM known_hosts file instead. +type Endpoint struct { + Name string + Host string + Port int + User string + + // KnownHosts is the known_hosts file to pin against. Empty keeps the + // loopback policy: no host key checking at all. + KnownHosts string +} + +// LocalEndpoint is the endpoint for a VM reached through QEMU's user-mode +// port forward. +func LocalEndpoint(v *config.VM) Endpoint { + return Endpoint{ + Name: v.Name, + Host: "127.0.0.1", + Port: v.SSHPort, + User: User(v), + } +} diff --git a/internal/sshx/endpoint_test.go b/internal/sshx/endpoint_test.go new file mode 100644 index 00000000..66b67bd4 --- /dev/null +++ b/internal/sshx/endpoint_test.go @@ -0,0 +1,84 @@ +package sshx + +import ( + "strings" + "testing" + + "github.com/novusedge/stoat/internal/config" +) + +func joined(a []string) string { return strings.Join(a, " ") } + +func TestLocalEndpoint(t *testing.T) { + v := &config.VM{Name: "dev", SSHPort: 2222, SSHUser: "stoat"} + e := LocalEndpoint(v) + if e.Host != "127.0.0.1" { + t.Errorf("Host = %q, want 127.0.0.1", e.Host) + } + if e.Port != 2222 { + t.Errorf("Port = %d, want 2222", e.Port) + } + if e.User != "stoat" { + t.Errorf("User = %q, want stoat", e.User) + } + if e.Name != "dev" { + t.Errorf("Name = %q, want dev", e.Name) + } + if e.KnownHosts != "" { + t.Errorf("KnownHosts = %q, want empty for a loopback forward", e.KnownHosts) + } +} + +func TestLocalEndpointDefaultsUserToRoot(t *testing.T) { + e := LocalEndpoint(&config.VM{Name: "live", SSHPort: 2200}) + if e.User != "root" { + t.Errorf("User = %q, want root", e.User) + } +} + +func TestConnOptionsLoopbackKeepsUncheckedPolicy(t *testing.T) { + got := joined(connOptions(Endpoint{Host: "127.0.0.1", Port: 2222})) + if !strings.Contains(got, "StrictHostKeyChecking=no") { + t.Errorf("loopback options = %q, want StrictHostKeyChecking=no", got) + } + if !strings.Contains(got, "UserKnownHostsFile=/dev/null") { + t.Errorf("loopback options = %q, want UserKnownHostsFile=/dev/null", got) + } +} + +func TestConnOptionsPinsWhenKnownHostsSet(t *testing.T) { + got := joined(connOptions(Endpoint{Host: "34.1.2.3", Port: 22, KnownHosts: "/tmp/kh"})) + if !strings.Contains(got, "StrictHostKeyChecking=accept-new") { + t.Errorf("pinned options = %q, want StrictHostKeyChecking=accept-new", got) + } + if !strings.Contains(got, "UserKnownHostsFile=/tmp/kh") { + t.Errorf("pinned options = %q, want the named known_hosts file", got) + } + if strings.Contains(got, "/dev/null") { + t.Errorf("pinned options = %q, must not discard the host key", got) + } +} + +func TestArgsUsesEndpointHostAndPort(t *testing.T) { + got := joined(Args(Endpoint{Host: "34.1.2.3", Port: 22, User: "stoat"}, "uptime")) + if !strings.Contains(got, "-p 22") { + t.Errorf("args = %q, want -p 22", got) + } + if !strings.Contains(got, "stoat@34.1.2.3") { + t.Errorf("args = %q, want stoat@34.1.2.3", got) + } + if !strings.HasSuffix(got, "uptime") { + t.Errorf("args = %q, want the remote command last", got) + } +} + +func TestCopyArgsUsesEndpointHostAndPort(t *testing.T) { + e := Endpoint{Host: "34.1.2.3", Port: 22, User: "stoat"} + got := joined(CopyArgs(e, "/local", "/remote", true)) + if !strings.Contains(got, "-P 22") { + t.Errorf("copy args = %q, want -P 22", got) + } + if !strings.HasSuffix(got, "/local stoat@34.1.2.3:/remote") { + t.Errorf("copy args = %q, want local then remote", got) + } +} diff --git a/internal/sshx/outputs.go b/internal/sshx/outputs.go index 1faa0dc1..1063da23 100644 --- a/internal/sshx/outputs.go +++ b/internal/sshx/outputs.go @@ -48,7 +48,7 @@ func collectOutputs(ctx context.Context, v *config.VM, name string, m recipes.Ma // it, so a multi-word script must travel as one already-quoted argv // element or only its first word ends up under the escalation prefix. remote := []string{"sh -c " + guest.ShQuote(script)} - out, err := exec.CommandContext(ctx, "ssh", Args(v, escalate(v, remote)...)...).Output() + out, err := exec.CommandContext(ctx, "ssh", Args(LocalEndpoint(v), escalate(v, remote)...)...).Output() if err != nil { var ee *exec.ExitError if errors.As(err, &ee) { diff --git a/internal/sshx/run.go b/internal/sshx/run.go index e51b13b0..e3ec838f 100644 --- a/internal/sshx/run.go +++ b/internal/sshx/run.go @@ -44,7 +44,7 @@ func Run(ctx context.Context, v *config.VM, root bool, argv []string, stdin io.R remote = escalate(v, argv) } var out, errb bytes.Buffer - c := exec.CommandContext(ctx, "ssh", Args(v, Quote(remote))...) + c := exec.CommandContext(ctx, "ssh", Args(LocalEndpoint(v), Quote(remote))...) c.Stdin = stdin c.Stdout = &out c.Stderr = &errb diff --git a/internal/sshx/run_test.go b/internal/sshx/run_test.go index 6db10a03..58d629fe 100644 --- a/internal/sshx/run_test.go +++ b/internal/sshx/run_test.go @@ -84,7 +84,7 @@ func TestRunDoesNotEscalateForRoot(t *testing.T) { // strings end in "'id'". Comparing the whole line against Args with the // bare quoted argv is the only check that catches a prefix Run should // not have added. - want := strings.Join(sshx.Args(v, sshx.Quote([]string{"id"})), " ") + want := strings.Join(sshx.Args(sshx.LocalEndpoint(v), sshx.Quote([]string{"id"})), " ") if got := calls.Calls()[0].Remote; got != want { t.Fatalf("ssh argv = %q, want %q (root must not escalate)", got, want) } diff --git a/internal/sshx/sharemount.go b/internal/sshx/sharemount.go index 786ea6f4..5f47f73a 100644 --- a/internal/sshx/sharemount.go +++ b/internal/sshx/sharemount.go @@ -109,7 +109,7 @@ func mountShares(ctx context.Context, v *config.VM, log io.Writer) { return } fmt.Fprintln(log, "\n=== mounting 9p shares ===") - cmd := exec.CommandContext(ctx, "ssh", Args(v, escalate(v, []string{"sh", "-s"})...)...) + cmd := exec.CommandContext(ctx, "ssh", Args(LocalEndpoint(v), escalate(v, []string{"sh", "-s"})...)...) cmd.Cancel = func() error { return cmd.Process.Signal(syscall.SIGTERM) } cmd.WaitDelay = recipeShutdownGrace cmd.Stdin = strings.NewReader(shareMountScript(tags, guestFstabPath)) diff --git a/internal/sshx/sshx.go b/internal/sshx/sshx.go index 1a7b1764..446098d7 100644 --- a/internal/sshx/sshx.go +++ b/internal/sshx/sshx.go @@ -58,23 +58,29 @@ func User(v *config.VM) string { // The port flag is not here. ssh takes "-p" and scp takes "-P" (capital, // since scp's lowercase -p means "preserve file times"), so each caller // supplies it itself. See CopyArgs. -func connOptions() []string { - return []string{ +func connOptions(e Endpoint) []string { + host := []string{ "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", + } + if e.KnownHosts != "" { + host = []string{ + "-o", "StrictHostKeyChecking=accept-new", + "-o", "UserKnownHostsFile=" + e.KnownHosts, + } + } + return append(host, "-o", "LogLevel=ERROR", "-o", "ConnectTimeout=5", "-o", "BatchMode=yes", "-i", keys.PrivatePath(), - } + ) } -// Args returns the argv (excluding argv[0]) for ssh into v. Host key checks -// are off on purpose: this is a loopback forward to a VM stoat just built, -// and live VMs are recreated constantly. -func Args(v *config.VM, extra ...string) []string { - a := append([]string{"-p", fmt.Sprint(v.SSHPort)}, connOptions()...) - a = append(a, User(v)+"@127.0.0.1") +// Args returns the argv (excluding argv[0]) for ssh to e. +func Args(e Endpoint, extra ...string) []string { + a := append([]string{"-p", fmt.Sprint(e.Port)}, connOptions(e)...) + a = append(a, e.User+"@"+e.Host) return append(a, extra...) } @@ -105,21 +111,20 @@ func preludeFor(v *config.VM, runtime string) string { } // CopyArgs returns the argv (excluding argv[0]) for scp between the host and -// v's guest. It shares every connection setting Args does (see connOptions) +// e's guest. It shares every connection setting Args does (see connOptions) // and differs only in the port flag, since scp's is capital -P. // -// toRemote picks the direction. true puts the guest spec -// ("user@127.0.0.1:remotePath") on the right, as scp's destination -// (core.CopyTo). false puts it on the left, as scp's source (core.CopyFrom). -// localPath is always a bare host path, never quoted or rewritten: it is -// scp's own argv element, not something a shell re-parses. +// toRemote picks the direction. true puts the guest spec on the right, as +// scp's destination (core.CopyTo). false puts it on the left, as scp's source +// (core.CopyFrom). localPath is always a bare host path, never quoted or +// rewritten: it is scp's own argv element, not something a shell re-parses. // // -q suppresses scp's interactive progress meter. This argv is built for // exec.CommandContext, never a terminal, so stray meter output would // otherwise get captured as if it were an error. -func CopyArgs(v *config.VM, localPath, remotePath string, toRemote bool) []string { - a := append([]string{"-P", fmt.Sprint(v.SSHPort), "-q"}, connOptions()...) - remoteSpec := User(v) + "@127.0.0.1:" + remotePath +func CopyArgs(e Endpoint, localPath, remotePath string, toRemote bool) []string { + a := append([]string{"-P", fmt.Sprint(e.Port), "-q"}, connOptions(e)...) + remoteSpec := e.User + "@" + e.Host + ":" + remotePath if toRemote { return append(a, localPath, remoteSpec) } @@ -140,9 +145,9 @@ func CopyArgs(v *config.VM, localPath, remotePath string, toRemote bool) []strin // expires elsewhere, sshx.Wait needs a caller-supplied timeout ceiling, and // duplicating the ~10-line dial is cheaper than reconciling those two // different contracts. -func Wait(ctx context.Context, v *config.VM, timeout time.Duration) error { +func Wait(ctx context.Context, e Endpoint, timeout time.Duration) error { deadline := time.Now().Add(timeout) - addr := fmt.Sprintf("127.0.0.1:%d", v.SSHPort) + addr := fmt.Sprintf("%s:%d", e.Host, e.Port) for { if err := ctx.Err(); err != nil { return err @@ -179,7 +184,7 @@ func Wait(ctx context.Context, v *config.VM, timeout time.Duration) error { case <-time.After(sleep): } } - return fmt.Errorf("%s: ssh not reachable on port %d after %s", v.Name, v.SSHPort, timeout) + return fmt.Errorf("%s: ssh not reachable at %s after %s", e.Name, addr, timeout) } // dialCtx dials addr, bounding the attempt by whichever of ctx or @@ -286,7 +291,7 @@ func cloudInitProbe(ctx context.Context, v *config.VM, log io.Writer) (cloudInit defer cancel() var out bytes.Buffer - ci := exec.CommandContext(probeCtx, "ssh", Args(v, argv...)...) + ci := exec.CommandContext(probeCtx, "ssh", Args(LocalEndpoint(v), argv...)...) ci.Cancel = func() error { return ci.Process.Signal(syscall.SIGTERM) } ci.WaitDelay = recipeShutdownGrace ci.Stdout = &out @@ -383,7 +388,7 @@ func RunCheck(ctx context.Context, v *config.VM, command string, timeout time.Du prelude = guest.Prelude(o, "sh") } body := prelude + "\n" + command + "\n" - cmd := exec.CommandContext(ctx, "ssh", Args(v, escalate(v, []string{"sh", "-s"})...)...) + cmd := exec.CommandContext(ctx, "ssh", Args(LocalEndpoint(v), escalate(v, []string{"sh", "-s"})...)...) cmd.Cancel = func() error { return cmd.Process.Signal(syscall.SIGTERM) } cmd.WaitDelay = healthShutdownGrace cmd.Stdin = strings.NewReader(body) @@ -416,7 +421,7 @@ func Provision(ctx context.Context, v *config.VM) (err error) { defer func() { _ = log.Close() }() fmt.Fprintf(log, "waiting for ssh on port %d…\n", v.SSHPort) - if err := Wait(ctx, v, WaitTimeout); err != nil { + if err := Wait(ctx, LocalEndpoint(v), WaitTimeout); err != nil { if ctx.Err() != nil { fmt.Fprintf(log, "CANCELLED: %v\n", err) } else { @@ -446,7 +451,7 @@ func Provision(ctx context.Context, v *config.VM) (err error) { if o, ok := guest.Lookup(v.OS); ok && strings.TrimSpace(o.Pkg.Setup) != "" { fmt.Fprintln(log, "refreshing the package index...") - st := exec.CommandContext(ctx, "ssh", Args(v, escalate(v, []string{"sh", "-s"})...)...) + st := exec.CommandContext(ctx, "ssh", Args(LocalEndpoint(v), escalate(v, []string{"sh", "-s"})...)...) st.Cancel = func() error { return st.Process.Signal(syscall.SIGTERM) } st.WaitDelay = recipeShutdownGrace st.Stdin = strings.NewReader(guest.Prelude(o, "sh") + "stoat_pkg_setup\n") @@ -497,7 +502,7 @@ func Provision(ctx context.Context, v *config.VM) (err error) { if bootstrap := recipes.BootstrapScript(runtime, v.OS); bootstrap != "" { fmt.Fprintf(log, "ensuring %s is installed...\n", runtime) - bs := exec.CommandContext(ctx, "ssh", Args(v, escalate(v, []string{"sh", "-s"})...)...) + bs := exec.CommandContext(ctx, "ssh", Args(LocalEndpoint(v), escalate(v, []string{"sh", "-s"})...)...) bs.Cancel = func() error { return bs.Process.Signal(syscall.SIGTERM) } bs.WaitDelay = recipeShutdownGrace bs.Stdin = strings.NewReader(guest.WithPrelude(bootstrap, preludeFor(v, "sh"))) @@ -513,7 +518,7 @@ func Provision(ctx context.Context, v *config.VM) (err error) { } } - cmd := exec.CommandContext(ctx, "ssh", Args(v, escalate(v, recipes.InterpreterArgs(runtime))...)...) + cmd := exec.CommandContext(ctx, "ssh", Args(LocalEndpoint(v), escalate(v, recipes.InterpreterArgs(runtime))...)...) cmd.Cancel = func() error { return cmd.Process.Signal(syscall.SIGTERM) } cmd.WaitDelay = recipeShutdownGrace cmd.Stdin = strings.NewReader(input) diff --git a/internal/sshx/sshx_test.go b/internal/sshx/sshx_test.go index 24679ced..83914837 100644 --- a/internal/sshx/sshx_test.go +++ b/internal/sshx/sshx_test.go @@ -18,7 +18,7 @@ import ( func TestArgs(t *testing.T) { t.Setenv("STOAT_HOME", "/data") v := &config.VM{Name: "x", SSHPort: 2201, Dir: "/data/x"} - got := strings.Join(Args(v), " ") + got := strings.Join(Args(LocalEndpoint(v)), " ") for _, want := range []string{ "-p 2201", @@ -40,7 +40,7 @@ func TestArgs(t *testing.T) { func TestArgsUsesConfiguredSSHUser(t *testing.T) { t.Setenv("STOAT_HOME", "/data") v := &config.VM{Name: "x", SSHPort: 2201, Dir: "/data/x", SSHUser: "stoat"} - got := strings.Join(Args(v), " ") + got := strings.Join(Args(LocalEndpoint(v)), " ") if !strings.Contains(got, "stoat@127.0.0.1") { t.Errorf("expected stoat@127.0.0.1 in: %s", got) @@ -57,7 +57,7 @@ func TestArgsUsesConfiguredSSHUser(t *testing.T) { func TestCopyArgsUsesScpsPortFlagNotSSHs(t *testing.T) { t.Setenv("STOAT_HOME", "/data") v := &config.VM{Name: "x", SSHPort: 2201, Dir: "/data/x"} - got := CopyArgs(v, "/tmp/local", "/root/remote", true) + got := CopyArgs(LocalEndpoint(v), "/tmp/local", "/root/remote", true) if !containsPair(got, "-P", "2201") { t.Errorf("missing -P 2201 in: %v", got) @@ -75,7 +75,7 @@ func TestCopyArgsUsesScpsPortFlagNotSSHs(t *testing.T) { func TestCopyArgsSharesConnOptionsWithArgs(t *testing.T) { t.Setenv("STOAT_HOME", "/data") v := &config.VM{Name: "x", SSHPort: 2201, Dir: "/data/x"} - got := strings.Join(CopyArgs(v, "/tmp/local", "/root/remote", true), " ") + got := strings.Join(CopyArgs(LocalEndpoint(v), "/tmp/local", "/root/remote", true), " ") for _, want := range []string{ "-o StrictHostKeyChecking=no", @@ -96,12 +96,12 @@ func TestCopyArgsDirection(t *testing.T) { t.Setenv("STOAT_HOME", "/data") v := &config.VM{Name: "x", SSHPort: 2201, Dir: "/data/x"} - up := CopyArgs(v, "/tmp/local", "/root/remote", true) + up := CopyArgs(LocalEndpoint(v), "/tmp/local", "/root/remote", true) if up[len(up)-2] != "/tmp/local" || up[len(up)-1] != "root@127.0.0.1:/root/remote" { t.Errorf("CopyTo argv = %v, want local then remote", up) } - down := CopyArgs(v, "/tmp/local", "/root/remote", false) + down := CopyArgs(LocalEndpoint(v), "/tmp/local", "/root/remote", false) if down[len(down)-2] != "root@127.0.0.1:/root/remote" || down[len(down)-1] != "/tmp/local" { t.Errorf("CopyFrom argv = %v, want remote then local", down) } @@ -118,7 +118,7 @@ func TestCopyArgsDirection(t *testing.T) { func TestCopyArgsRemotePathIsNotShellQuoted(t *testing.T) { t.Setenv("STOAT_HOME", "/data") v := &config.VM{Name: "x", SSHPort: 2201, Dir: "/data/x"} - got := CopyArgs(v, "/tmp/local", "/root/my file.txt", true) + got := CopyArgs(LocalEndpoint(v), "/tmp/local", "/root/my file.txt", true) want := "root@127.0.0.1:/root/my file.txt" if got[len(got)-1] != want { @@ -129,7 +129,7 @@ func TestCopyArgsRemotePathIsNotShellQuoted(t *testing.T) { func TestArgsExtraGoesAfterTarget(t *testing.T) { t.Setenv("STOAT_HOME", "/data") v := &config.VM{Name: "x", SSHPort: 2201, Dir: "/data/x"} - got := Args(v, "sh", "-s") + got := Args(LocalEndpoint(v), "sh", "-s") if got[len(got)-2] != "sh" || got[len(got)-1] != "-s" { t.Errorf("extra args must come last, got %v", got) } @@ -237,7 +237,7 @@ func TestWaitTimesOutWhenAcceptedButNoBanner(t *testing.T) { v := &config.VM{Name: "x", SSHPort: port, Dir: t.TempDir()} start := time.Now() - err := Wait(context.Background(), v, 500*time.Millisecond) + err := Wait(context.Background(), LocalEndpoint(v), 500*time.Millisecond) elapsed := time.Since(start) t.Logf("accept-without-banner: Wait took %s", elapsed) if err == nil { @@ -253,7 +253,7 @@ func TestWaitSucceedsOnceBannerArrives(t *testing.T) { v := &config.VM{Name: "x", SSHPort: port, Dir: t.TempDir()} start := time.Now() - err := Wait(context.Background(), v, 2*time.Second) + err := Wait(context.Background(), LocalEndpoint(v), 2*time.Second) elapsed := time.Since(start) t.Logf("accept-with-banner: Wait took %s", elapsed) if err != nil { @@ -287,7 +287,7 @@ func TestWaitSucceedsOnSlowBanner(t *testing.T) { v := &config.VM{Name: "x", SSHPort: port, Dir: t.TempDir()} start := time.Now() - err = Wait(context.Background(), v, 3*time.Second) + err = Wait(context.Background(), LocalEndpoint(v), 3*time.Second) elapsed := time.Since(start) t.Logf("slow-banner (500ms): Wait took %s", elapsed) if err != nil { @@ -299,7 +299,7 @@ func TestWaitTimesOutOnClosedPort(t *testing.T) { // Port 1 on loopback: reserved, nothing listens. v := &config.VM{Name: "x", SSHPort: 1, Dir: t.TempDir()} start := time.Now() - err := Wait(context.Background(), v, 300*time.Millisecond) + err := Wait(context.Background(), LocalEndpoint(v), 300*time.Millisecond) if err == nil { t.Fatal("Wait returned nil for a closed port") } @@ -352,7 +352,7 @@ func TestWaitCancelDuringRetrySleepReturnsPromptly(t *testing.T) { }() start := time.Now() - err := Wait(ctx, v, 10*time.Second) + err := Wait(ctx, LocalEndpoint(v), 10*time.Second) elapsed := time.Since(start) if !errors.Is(err, context.Canceled) { t.Fatalf("err = %v, want context.Canceled", err) @@ -374,7 +374,7 @@ func TestWaitAlreadyCancelledReturnsImmediately(t *testing.T) { cancel() start := time.Now() - err := Wait(ctx, v, 10*time.Second) + err := Wait(ctx, LocalEndpoint(v), 10*time.Second) elapsed := time.Since(start) if !errors.Is(err, context.Canceled) { t.Fatalf("err = %v, want context.Canceled", err) diff --git a/internal/tui/cloudinit.go b/internal/tui/cloudinit.go index deccd962..6478b47a 100644 --- a/internal/tui/cloudinit.go +++ b/internal/tui/cloudinit.go @@ -104,7 +104,7 @@ func decodeCloudInitStatus(out []byte) string { func checkCloudInit(v core.VM) tea.Cmd { name := v.Name return func() tea.Msg { - out, err := exec.Command("ssh", sshx.Args(cfgVM(v), "cloud-init", "status", "--format", "json")...).Output() + out, err := exec.Command("ssh", sshx.Args(sshx.LocalEndpoint(cfgVM(v)), "cloud-init", "status", "--format", "json")...).Output() if exitErr, ok := err.(*exec.ExitError); err != nil && (!ok || exitErr.ExitCode() == 255) { // Not reachable yet is the normal case for the first ~30 seconds // of a boot, so it is a state, not an error. diff --git a/internal/tui/provstep_test.go b/internal/tui/provstep_test.go index 97c30a67..7336c93d 100644 --- a/internal/tui/provstep_test.go +++ b/internal/tui/provstep_test.go @@ -57,8 +57,8 @@ func TestReadProvStep(t *testing.T) { }, { "failed", - "waiting for ssh on port 2200…\nFAILED: work: ssh not reachable on port 2200 after 1m30s\n", - "waiting for ssh", "FAILED: work: ssh not reachable on port 2200 after 1m30s", + "waiting for ssh on port 2200…\nFAILED: work: ssh not reachable at 127.0.0.1:2200 after 1m30s\n", + "waiting for ssh", "FAILED: work: ssh not reachable at 127.0.0.1:2200 after 1m30s", }, } for _, c := range cases { diff --git a/internal/tui/vmlist.go b/internal/tui/vmlist.go index 69269956..bd18fa5d 100644 --- a/internal/tui/vmlist.go +++ b/internal/tui/vmlist.go @@ -107,10 +107,10 @@ func (d vmDelegate) Render(w io.Writer, m list.Model, index int, item list.Item) // next reload. if v.State == core.StateRunning { dot, dotStyle = glyphRunning, upStyle - // State and StartedAt come from separate qemu.Running checks in - // fromConfig; a pidfile vanishing between them leaves a running row - // with a zero StartedAt, which time.Since renders as a nonsense - // six-figure uptime. Drop the duration in that window. + // fromConfigUnchecked takes State and StartedAt from one provider.Status + // call, but the qemu provider reads pid liveness and pidfile mtime + // separately: a pidfile vanishing between them leaves a running row with a + // zero StartedAt, which time.Since renders as a nonsense six-figure uptime. up := "up ?" if !v.StartedAt.IsZero() { up = "up " + time.Since(v.StartedAt).Truncate(time.Second).String()