diff --git a/docs/design/core-api.md b/docs/design/core-api.md index 03acf9a3..4837e705 100644 --- a/docs/design/core-api.md +++ b/docs/design/core-api.md @@ -230,14 +230,34 @@ All of these are wanted. Ordered by effort, so the cheap wins land early and the Typed errors, because every caller branches on them and string matching is how that goes wrong: ``` -ErrNotFound ErrNameTaken ErrImmutableField -ErrImageNotDownloaded ErrRecipeNotApplicable ErrNotRunning -ErrAlreadyRunning ErrTimeout ErrDependencyMissing -ErrBroken ErrDiskShrink +core: ErrNotFound ErrNameTaken ErrImmutableField + ErrImageNotDownloaded ErrRecipeNotApplicable ErrNotRunning + ErrAlreadyRunning ErrTimeout ErrDependencyMissing + ErrBroken ErrDiskShrink ErrCannotReach + ErrNoDisk ErrUnknownWhich + +qemu: ErrBinaryMissing ErrKVMUnusable ErrStartFailed + ErrMonitorUnreachable ErrMonitorRejected ErrNoConsolePassword + ErrShareInvalid ErrNoXattr ErrScreenshotFailed + ErrNotRunning ErrAlreadyRunning + +iso: ErrDownloadFailed ErrDownloadStalled ErrChecksumMismatch + ErrNoSuchImage ``` Each carries the specific subject (which field, which recipe, which dependency) rather than only a message. +`qemu` and `iso` declare their own sentinels beside the code that raises +them. `internal/cli/wire` maps each to a code in `codeTable`; a sentinel with +no row reaches a consumer as `internal` with prose. + +`qemu.ErrNotRunning` and `qemu.ErrAlreadyRunning` duplicate `core`'s two by +necessity: `core` imports `qemu`, so `qemu` cannot name them. They add no +code, and `codeTable` maps each pair to one. + +Wrapping a sentinel prefixes its text to the message: `Preflight` now returns +`qemu binary not found: qemu-system-x86_64 not found in PATH`. + The CLI reports these two ways, and the choice is not a style question. `a.fail` takes an error `core` returned and maps its sentinel to the JSON error code. `a.failMsg` takes a sentinel the CLI picked itself, for a condition `core` never saw: a bad flag combination, an diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 5281d7d5..2e6db57a 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -45,6 +45,7 @@ usage: stoat [flags] | [`guest ls`](#stoat-guest-ls) | List loaded guest OS definitions | 0 | | [`guest show`](#stoat-guest-show-name) | Print one guest's merged definition | 0, 1 | | [`logs`](#stoat-logs-name--n-n) | Tail a VM's log, or stoat's own | 0, 1 | +| [`screenshot`](#stoat-screenshot-name--o-path) | Write the VM's screen to a PNG | 0, 1 | | [`doctor`](#stoat-doctor) | Check host prerequisites | 0, 1 | | [`version`](#stoat-version) | Print the stoat version | 0 | | [`help`](#stoat-help) | Show the usage message | 0 | @@ -540,6 +541,21 @@ $ stoat logs -n 20 **Exit codes:** 0 on success (including an empty log, which prints nothing); 1 if the log can't be opened or read. +## `stoat screenshot [-o path]` + +Writes the VM's screen to a PNG and prints the path, the pixel size and the byte count. qemu dumps its own framebuffer over the monitor socket, so the image is the same whether the display is a GTK window or a VNC socket, and a VM stuck at a boot prompt still answers. + +``` +$ stoat screenshot work +/home/u/.stoat/work/screenshots/2026-09-05T140302Z.png (1280x800, 48213 bytes) +``` + +Without `-o`, the file lands in `/screenshots/`, named for the second it was taken: RFC3339 with the colons stripped, since a colon in a filename breaks scp's `host:path` split. A second shot inside the same second gets `-2`, then `-3`, so a caller polling a boot never overwrites the frame it just took. + +`-o` names the file instead. qemu writes it as its own user, so a relative path resolves against the caller's working directory before qemu sees it. + +**Exit codes:** 0 on success; 1 if the VM does not exist, is not running (`not_running`), or qemu refuses the dump (`screenshot_failed`). + ## `stoat doctor` Checks host prerequisites: qemu/KVM, `qemu-img`, `ssh`, `xorriso` and `/dev/kvm`, the same set the installer's own checklist runs, so `stoat doctor` and `just setup` can't disagree about whether the host is ready. diff --git a/docs/reference/json.md b/docs/reference/json.md index adc83808..3cf0d8b1 100644 --- a/docs/reference/json.md +++ b/docs/reference/json.md @@ -114,6 +114,19 @@ bump the contract version. Do not write code that requires them. | `cannot_reach` | `wait` was asked for a state this VM can never reach | | `applied_at_boot` | reserved; no command emits it. A cloud VM whose recipes ran at boot answers `ok:true` with `applied: []` | | `unknown_log` | bad `--which` | +| `qemu_missing` | `qemu-system-x86_64` is not on `PATH` | +| `kvm_unusable` | `/dev/kvm` cannot be opened; the user is usually not in the `kvm` group | +| `qemu_start_failed` | qemu ran and refused to start the VM | +| `monitor_unreachable` | the VM's qemu monitor socket does not answer | +| `monitor_rejected` | qemu answered the monitor command with an error | +| `no_console_password` | the VM has no console password to type | +| `share_invalid` | the configured share is not a directory | +| `no_xattr` | the share's filesystem cannot store `user.*` extended attributes | +| `screenshot_failed` | qemu refused the screendump | +| `download_failed` | a mirror or a checksum file did not answer with the image | +| `download_stalled` | the download stopped producing bytes | +| `checksum_mismatch` | the downloaded bytes do not match the published digest | +| `no_such_image` | the requested image is not in the index | | `timeout` | the deadline expired | | `canceled` | the context was cancelled | | `usage` | a bad flag, a missing argument, an unknown subcommand | @@ -123,6 +136,7 @@ bump the contract version. Do not write code that requires them. **Codes are only ever added.** Never renamed, never repurposed, never removed. A consumer MUST treat an unrecognized code as a generic failure rather than crashing on it, because that is what makes adding one a non-breaking change. +The list above is `wire.Codes()`, which returns every declared code, sorted. ## Exit codes @@ -324,6 +338,7 @@ so a leak fails the build rather than shipping. | `guest show` | `{"guest":Guest}` | | `recipe list` | `{"dir":"...","recipes":["xfce"]}`, see note below | | `recipe new` | `{"path":"/home/u/.stoat/recipes/foo.alpine.sh"}` | +| `screenshot` | `{"vm":"work","path":"/home/u/.stoat/work/screenshots/2026-09-05T140302Z.png","bytes":48213,"width":1280,"height":800}` | Both `recipe` subcommands report `"cmd":"recipe"`, not `"cmd":"recipe list"`, and both `guest` subcommands report `"cmd":"guest"`. Distinguish them by which diff --git a/internal/apkovl/apkovl.go b/internal/apkovl/apkovl.go index 15ade204..b526a195 100644 --- a/internal/apkovl/apkovl.go +++ b/internal/apkovl/apkovl.go @@ -77,7 +77,7 @@ echo "stoat: installing Alpine unattended, this takes a few minutes..." # here so the child inherits it and repartitions /dev/vda unattended. export ERASE_DISKS=/dev/vda if setup-alpine -e -f /etc/stoat/answerfile; then - echo "$(date -Iseconds)" > /mnt/work/.installed +` + postInstall + ` echo "$(date -Iseconds)" > /mnt/work/.installed echo "stoat: install complete, powering off; run 'stoat up' to boot the disk" poweroff else diff --git a/internal/apkovl/postinstall.go b/internal/apkovl/postinstall.go new file mode 100644 index 00000000..d722cd42 --- /dev/null +++ b/internal/apkovl/postinstall.go @@ -0,0 +1,87 @@ +package apkovl + +// postInstall runs on the installed disk after setup-alpine succeeds and +// before the installer powers off. It is sh, and it runs in the live +// installer environment, so every path it touches is under $target. +// +// setup-disk leaves nothing mounted for this to reuse, and the partition +// layout is whatever setup-disk chose, so mountTarget finds the root by +// looking for /etc/inittab rather than by assuming a partition number. The +// installer environment has no /etc/mtab worth reading either: the target +// was mounted and unmounted by a child process. +const postInstall = mountTarget + extlinuxTimeoutFix + workMountFix + issueFix + umountTarget + +const mountTarget = `target=/mnt/target +mkdir -p "$target" +root= +for p in /dev/vda3 /dev/vda2 /dev/vda1; do + [ -b "$p" ] || continue + mount "$p" "$target" 2>/dev/null || continue + if [ -f "$target/etc/inittab" ]; then root=$p; break; fi + umount "$target" +done +if [ -z "$root" ]; then + echo "stoat: no installed root found on /dev/vda; skipping the post-install fixes" +else + # /boot is inside the root on a single-partition install and its own + # partition otherwise. Either way extlinux.conf must be writable. + [ -f "$target/boot/extlinux.conf" ] || mount /dev/vda1 "$target/boot" 2>/dev/null || true +` + +const umountTarget = ` umount "$target/boot" 2>/dev/null || true + umount "$target" +fi +` + +// extlinuxTimeoutFix appends TOTALTIMEOUT to the installed extlinux.conf +// (issue #59). The installed config carries DEFAULT menu.c32, PROMPT 0, +// MENU HIDDEN and TIMEOUT 10, one second. syslinux cancels TIMEOUT and draws +// the hidden menu when input arrives during that second, and then waits with +// no countdown left. TOTALTIMEOUT fires whatever the user typed. +// +// update-extlinux.conf has no key for TOTALTIMEOUT, so the line is appended +// to the generated file. A kernel upgrade that reruns update-extlinux drops +// it again. +const extlinuxTimeoutFix = ` conf="$target/boot/extlinux.conf" + if [ -f "$conf" ] && ! grep -q '^TOTALTIMEOUT ' "$conf"; then + echo 'TOTALTIMEOUT 100' >> "$conf" + echo "stoat: added TOTALTIMEOUT to extlinux.conf" + fi +` + +// workMountFix repairs the installed fstab's 9p lines (issue #60). +// +// setup-disk writes the target's fstab from the live system's mounts, and +// stoat's shares sit under /mnt, the same directory setup-disk mounts the +// target on, so the work share lands as "work /work 9p ... 0 2". busybox +// fsck then has no fsck.9p helper, and /work does not exist on the target, +// so the guest prints a failed mount on every boot. +// +// awk rewrites only lines whose type field is 9p; every other line prints +// unchanged, comments included. +const workMountFix = ` fstab="$target/etc/fstab" + if [ -f "$fstab" ]; then + awk '$3 == "9p" { $2 = "/mnt/" $1; $5 = "0"; $6 = "0" } { print }' "$fstab" > "$fstab.stoat" && + mv "$fstab.stoat" "$fstab" + awk '$3 == "9p" { print $2 }' "$fstab" | while read -r d; do + mkdir -p "$target$d" + done + fi +` + +// issueFix restores the stock /etc/issue on the target (issue #61). +// +// setup-disk -m sys copies the live system's /etc, so the installed login +// screen still reads "Installing Alpine. Unattended. Do not log in." The +// guard on the banner text means a rerun never overwrites an issue the user +// has since edited. +// +// printf's format is single-quoted, so \r, \m and \l reach the file as the +// getty escapes \r, \m and \l rather than as control characters. +const issueFix = ` issue="$target/etc/issue" + if [ -f "$issue" ] && grep -q 'Installing Alpine' "$issue"; then + printf 'Welcome to Alpine Linux %s\nKernel \\r on an \\m (\\l)\n\n' \ + "$(cut -d. -f1,2 "$target/etc/alpine-release")" > "$issue" + echo "stoat: restored the stock /etc/issue" + fi +` diff --git a/internal/apkovl/postinstall_test.go b/internal/apkovl/postinstall_test.go new file mode 100644 index 00000000..2dc407ab --- /dev/null +++ b/internal/apkovl/postinstall_test.go @@ -0,0 +1,132 @@ +package apkovl + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "testing" +) + +// runFix runs one post-install fragment against a fake target root, the way +// the installer runs it, and returns nothing: a failing shell fails the test. +func runFix(t *testing.T, target, fragment string) { + t.Helper() + if _, err := exec.LookPath("sh"); err != nil { + t.Skip("sh not installed") + } + cmd := exec.Command("sh", "-c", "set -e\ntarget="+target+"\n"+fragment) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("fragment failed: %v\n%s", err, out) + } +} + +// syslinux cancels TIMEOUT when a key arrives during the countdown and then +// waits forever. TOTALTIMEOUT fires whatever the user typed. The install runs +// the append on a disk that may already carry the line, so it must not stack. +func TestExtlinuxTimeoutFixIsIdempotent(t *testing.T) { + target := t.TempDir() + if err := os.MkdirAll(filepath.Join(target, "boot"), 0o755); err != nil { + t.Fatal(err) + } + conf := filepath.Join(target, "boot", "extlinux.conf") + if err := os.WriteFile(conf, []byte("DEFAULT menu.c32\nPROMPT 0\nMENU HIDDEN\nTIMEOUT 10\n"), 0o644); err != nil { + t.Fatal(err) + } + + runFix(t, target, extlinuxTimeoutFix) + runFix(t, target, extlinuxTimeoutFix) + + b, err := os.ReadFile(conf) + if err != nil { + t.Fatal(err) + } + if n := strings.Count(string(b), "TOTALTIMEOUT"); n != 1 { + t.Errorf("TOTALTIMEOUT appears %d times, want 1:\n%s", n, b) + } +} + +// The installed fstab carries stoat's work share as "work /work 9p ... 0 2": +// setup-disk writes the target's fstab from the live system's mounts, and +// stoat mounts its shares under /mnt, the directory setup-disk mounts the +// target on. fsck then looks for fsck.9p and /work does not exist, so the +// guest reports a failed mount on every boot. +func TestWorkMountFixRepairsTheInstalledFstab(t *testing.T) { + target := t.TempDir() + if err := os.MkdirAll(filepath.Join(target, "etc"), 0o755); err != nil { + t.Fatal(err) + } + fstab := filepath.Join(target, "etc", "fstab") + body := "/dev/vda3 / ext4 rw,relatime 0 1\n" + + "work /work 9p trans=virtio,version=9p2000.L,rw,nofail 0 2\n" + if err := os.WriteFile(fstab, []byte(body), 0o644); err != nil { + t.Fatal(err) + } + + runFix(t, target, workMountFix) + runFix(t, target, workMountFix) + + b, err := os.ReadFile(fstab) + if err != nil { + t.Fatal(err) + } + got := string(b) + if !strings.Contains(got, "work /mnt/work 9p") { + t.Errorf("fstab does not point work at /mnt/work:\n%s", got) + } + for _, line := range strings.Split(strings.TrimRight(got, "\n"), "\n") { + f := strings.Fields(line) + if len(f) >= 6 && f[2] == "9p" && f[5] != "0" { + t.Errorf("9p line has fsck pass %q, want 0: %s", f[5], line) + } + } + if !strings.Contains(got, "/dev/vda3 / ext4 rw,relatime 0 1") { + t.Errorf("the root line was rewritten:\n%s", got) + } + if _, err := os.Stat(filepath.Join(target, "mnt", "work")); err != nil { + t.Errorf("mountpoint was not created: %v", err) + } +} + +// setup-disk -m sys copies the live /etc onto the target, so the installer +// banner is still what the installed system's login screen shows. +func TestIssueFixRestoresTheStockBanner(t *testing.T) { + target := t.TempDir() + if err := os.MkdirAll(filepath.Join(target, "etc"), 0o755); err != nil { + t.Fatal(err) + } + issue := filepath.Join(target, "etc", "issue") + if err := os.WriteFile(issue, []byte(installIssue), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(target, "etc", "alpine-release"), []byte("3.22.1\n"), 0o644); err != nil { + t.Fatal(err) + } + + runFix(t, target, issueFix) + + b, err := os.ReadFile(issue) + if err != nil { + t.Fatal(err) + } + got := string(b) + if strings.Contains(got, "Installing Alpine") { + t.Errorf("the installer banner survived:\n%s", got) + } + if !strings.Contains(got, "Welcome to Alpine Linux 3.22") { + t.Errorf("issue = %q, want the stock banner", got) + } + + // A second run must leave a banner the user may have edited alone. + if err := os.WriteFile(issue, []byte("my own banner\n"), 0o644); err != nil { + t.Fatal(err) + } + runFix(t, target, issueFix) + b, err = os.ReadFile(issue) + if err != nil { + t.Fatal(err) + } + if string(b) != "my own banner\n" { + t.Errorf("issue = %q, want the user's own banner untouched", b) + } +} diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 7e2a1334..d1d6748d 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -49,6 +49,9 @@ type Args struct { Yes bool N int // logs -n + // Out belongs to "screenshot": the -o path, empty for the default. + Out string + // NoApply belongs to "up": it skips the automatic post-boot apply, // leaving `up` returning as soon as the VM starts, as it did before that // behavior existed. @@ -404,6 +407,8 @@ func Main(args []string, version string, stdin io.Reader, stdout, stderr io.Writ return runGuest(a, stdout, stderr) case "logs": return runLogs(a, stdout, stderr) + case "screenshot": + return runScreenshot(a, stdout, stderr) case "get": return runGet(a, stdout, stderr) case "ssh-command": diff --git a/internal/cli/grammar.go b/internal/cli/grammar.go index 8e550384..59a9db5c 100644 --- a/internal/cli/grammar.go +++ b/internal/cli/grammar.go @@ -57,10 +57,11 @@ type grammar struct { Recipe recipeCmd `cmd:"" help:"author recipes"` Guest recipeGuestCmd `cmd:"" help:"list or show guest OS definitions"` - Logs logsCmd `cmd:"" help:"tail a VM's log, or stoat's own"` - Doctor doctorCmd `cmd:"" help:"check host prerequisites"` - Version versionCmd `cmd:"" help:"print the stoat version"` - Help helpCmd `cmd:"" help:"show this message"` + Logs logsCmd `cmd:"" help:"tail a VM's log, or stoat's own"` + Screenshot screenshotCmd `cmd:"" help:"write the VM's screen to a PNG"` + Doctor doctorCmd `cmd:"" help:"check host prerequisites"` + Version versionCmd `cmd:"" help:"print the stoat version"` + Help helpCmd `cmd:"" help:"show this message"` } type helpCmd struct{} @@ -244,6 +245,11 @@ type logsCmd struct { Which string `enum:"console,apply" default:"console" help:"which log, with a vm name"` } +type screenshotCmd struct { + VM string `arg:"" help:"vm name"` + Out string `short:"o" help:"where to write the png; default /screenshots/.png"` +} + // toArgs flattens the selected command into the Args every runX consumes. // Args stays the interface between parsing and running rather than // threading the kong structs through 18 run functions directly. @@ -425,6 +431,9 @@ func (g *grammar) toArgs(path string) (*Args, error) { l := g.Logs a.VM, a.N, a.Which = l.VM, l.N, core.Which(l.Which) + case "screenshot": + a.VM, a.Out = g.Screenshot.VM, g.Screenshot.Out + default: return nil, usageError("unknown subcommand " + path) } diff --git a/internal/cli/json_test.go b/internal/cli/json_test.go index 93e93e35..ddfed2a0 100644 --- a/internal/cli/json_test.go +++ b/internal/cli/json_test.go @@ -3,6 +3,7 @@ package cli import ( "bytes" "encoding/json" + "slices" "strings" "testing" @@ -63,7 +64,7 @@ func TestJSONEnvelopeEveryCommand(t *testing.T) { name string argv []string ok bool - code string // error.code when ok is false + code wire.Code // error.code when ok is false exit int }{ {name: "ls", argv: []string{"ls"}, ok: true, exit: ExitOK}, @@ -85,6 +86,9 @@ func TestJSONEnvelopeEveryCommand(t *testing.T) { {name: "doctor", argv: []string{"doctor"}, ok: true, exit: ExitOK}, // The plan is computed host-side, so a stopped fixture VM answers it. {name: "apply dry-run", argv: []string{"apply", "work", "--dry-run"}, ok: true, exit: ExitOK}, + // The fixture VM is stopped, so the honest answer is not_running; the + // point here is that the command answers in the envelope at all. + {name: "screenshot stopped", argv: []string{"screenshot", "work"}, code: wire.CodeNotRunning, exit: ExitFail}, {name: "up unknown", argv: []string{"up", "nope"}, code: wire.CodeNotFound, exit: ExitFail}, {name: "down stopped", argv: []string{"down", "work"}, code: wire.CodeNotRunning, exit: ExitFail}, @@ -139,9 +143,15 @@ func TestJSONEnvelopeEveryCommand(t *testing.T) { if errObj == nil { t.Fatalf("failed result has no error object: %v", res) } - if errObj["code"] != tt.code { + got, _ := errObj["code"].(string) + if got != string(tt.code) { t.Errorf("error.code = %v, want %q", errObj["code"], tt.code) } + // A code absent from Codes() is one no consumer can generate a + // switch for, whatever the message beside it says. + if !slices.Contains(wire.Codes(), wire.Code(got)) { + t.Errorf("error.code = %q, which wire.Codes() does not declare", got) + } if msg, _ := errObj["message"].(string); msg == "" { t.Errorf("error.message is empty: %v", errObj) } diff --git a/internal/cli/run_screenshot.go b/internal/cli/run_screenshot.go new file mode 100644 index 00000000..e87de872 --- /dev/null +++ b/internal/cli/run_screenshot.go @@ -0,0 +1,21 @@ +package cli + +import ( + "fmt" + "io" + + "github.com/novusedge/stoat/internal/cli/wire" + "github.com/novusedge/stoat/internal/core" +) + +func runScreenshot(a *Args, stdout, stderr io.Writer) int { + shot, err := core.Screenshot(a.VM, a.Out) + if err != nil { + return a.fail(stdout, stderr, err) + } + if a.JSON { + return a.ok(stdout, wire.FromShot(a.VM, shot)) + } + fmt.Fprintf(stdout, "%s (%dx%d, %d bytes)\n", shot.Path, shot.Width, shot.Height, shot.Bytes) + return ExitOK +} diff --git a/internal/cli/subcommands_test.go b/internal/cli/subcommands_test.go index ab75835e..569968d7 100644 --- a/internal/cli/subcommands_test.go +++ b/internal/cli/subcommands_test.go @@ -70,7 +70,7 @@ func TestGetMissingVMIsNotFound(t *testing.T) { t.Fatalf("exit = %d, want %d", code, ExitFail) } errObj, _ := result(t, objs)["error"].(map[string]any) - if errObj["code"] != wire.CodeNotFound { + if errObj["code"] != string(wire.CodeNotFound) { t.Errorf("code = %v, want %q", errObj["code"], wire.CodeNotFound) } } @@ -277,7 +277,7 @@ func TestApplyMissingVMIsNotFound(t *testing.T) { t.Fatalf("exit = %d, want %d", code, ExitFail) } errObj, _ := result(t, objs)["error"].(map[string]any) - if errObj["code"] != wire.CodeNotFound { + if errObj["code"] != string(wire.CodeNotFound) { t.Errorf("code = %v, want %q", errObj["code"], wire.CodeNotFound) } } diff --git a/internal/cli/wait_test.go b/internal/cli/wait_test.go index 97fa87f4..274052d0 100644 --- a/internal/cli/wait_test.go +++ b/internal/cli/wait_test.go @@ -17,7 +17,7 @@ func TestWaitMissingVM(t *testing.T) { } res := result(t, objs) errObj, _ := res["error"].(map[string]any) - if errObj["code"] != wire.CodeNotFound { + if errObj["code"] != string(wire.CodeNotFound) { t.Errorf("error.code = %v, want %q", errObj["code"], wire.CodeNotFound) } } @@ -37,7 +37,7 @@ func TestWaitCannotReach(t *testing.T) { } res := result(t, objs) errObj, _ := res["error"].(map[string]any) - if errObj["code"] != wire.CodeCannotReach { + if errObj["code"] != string(wire.CodeCannotReach) { t.Errorf("error.code = %v, want %q", errObj["code"], wire.CodeCannotReach) } } @@ -63,7 +63,7 @@ func TestWaitTimeout(t *testing.T) { } res := result(t, objs) errObj, _ := res["error"].(map[string]any) - if errObj["code"] != wire.CodeTimeout { + if errObj["code"] != string(wire.CodeTimeout) { t.Errorf("error.code = %v, want %q", errObj["code"], wire.CodeTimeout) } } @@ -94,7 +94,7 @@ func TestWaitUsageErrorsUnderJSON(t *testing.T) { } res := result(t, objs) errObj, _ := res["error"].(map[string]any) - if errObj["code"] != wire.CodeUsage { + if errObj["code"] != string(wire.CodeUsage) { t.Errorf("Main(%v) error.code = %v, want %q", args, errObj["code"], wire.CodeUsage) } } diff --git a/internal/cli/wire/dto.go b/internal/cli/wire/dto.go index 71e85601..b37a57fc 100644 --- a/internal/cli/wire/dto.go +++ b/internal/cli/wire/dto.go @@ -422,6 +422,21 @@ func FromGuests(os []guest.OS) []Guest { return nonNil(out) } +// Screenshot is `screenshot` data. Path is the host path qemu wrote, always +// absolute, so a caller that passed a relative -o can read back what +// actually happened. +type Screenshot struct { + VM string `json:"vm"` + Path string `json:"path"` + Bytes int64 `json:"bytes"` + Width int `json:"width"` + Height int `json:"height"` +} + +func FromShot(vm string, s core.Shot) Screenshot { + return Screenshot{VM: vm, Path: s.Path, Bytes: s.Bytes, Width: s.Width, Height: s.Height} +} + // GuestList is `guest ls` data. type GuestList struct { Guests []Guest `json:"guests"` diff --git a/internal/cli/wire/envelope.go b/internal/cli/wire/envelope.go index 3f108fb6..a4a95d49 100644 --- a/internal/cli/wire/envelope.go +++ b/internal/cli/wire/envelope.go @@ -42,7 +42,7 @@ const ( // site supplies them explicitly rather than this package trying to parse // them back out. type ErrorInfo struct { - Code string `json:"code"` + Code Code `json:"code"` Message string `json:"message"` // Subject names what the error is about ("work", "os"); absent when // there is nothing more specific than Message to give. diff --git a/internal/cli/wire/envelope_test.go b/internal/cli/wire/envelope_test.go index db154a9f..3afc2e16 100644 --- a/internal/cli/wire/envelope_test.go +++ b/internal/cli/wire/envelope_test.go @@ -63,7 +63,7 @@ func TestEmitterErrorResultOmitsData(t *testing.T) { if !ok { t.Fatalf("expected an error object, got %v", got["error"]) } - if errObj["code"] != CodeConfirmationRequired { + if errObj["code"] != string(CodeConfirmationRequired) { t.Errorf("code = %v, want %s", errObj["code"], CodeConfirmationRequired) } } diff --git a/internal/cli/wire/errors.go b/internal/cli/wire/errors.go index 992ee98c..d3853ece 100644 --- a/internal/cli/wire/errors.go +++ b/internal/cli/wire/errors.go @@ -3,36 +3,81 @@ package wire import ( "context" "errors" + "slices" "github.com/novusedge/stoat/internal/core" + "github.com/novusedge/stoat/internal/iso" + "github.com/novusedge/stoat/internal/qemu" ) +// Code is a stable, machine-readable error code. Codes are only ever ADDED: +// never renamed, never repurposed, never removed (§2's compatibility +// promise). A consumer MUST treat an unrecognized code as a generic failure. +type Code string + // Stable snake_case error codes (§2), one per typed error in internal/core // plus the three the CLI layer owns (usage, confirmation_required, // internal). Codes are only ever ADDED: never renamed, never repurposed, // never removed (§2's compatibility promise). A consumer MUST treat an // unrecognized code as a generic failure, never crash on it. const ( - CodeNotFound = "not_found" - CodeBroken = "broken" - CodeNameTaken = "name_taken" - CodeInvalidSpec = "invalid_spec" - CodeImageNotDownloaded = "image_not_downloaded" - CodeRecipeNotApplicable = "recipe_not_applicable" - CodeNotRunning = "not_running" - CodeAlreadyRunning = "already_running" - CodeNoDisk = "no_disk" - CodeImmutableField = "immutable_field" - CodeDiskShrink = "disk_shrink" - CodeCannotReach = "cannot_reach" - CodeUnknownLog = "unknown_log" - CodeTimeout = "timeout" - CodeCanceled = "canceled" - CodeUsage = "usage" - CodeConfirmationRequired = "confirmation_required" - CodeInternal = "internal" + CodeNotFound Code = "not_found" + CodeBroken Code = "broken" + CodeNameTaken Code = "name_taken" + CodeInvalidSpec Code = "invalid_spec" + CodeImageNotDownloaded Code = "image_not_downloaded" + CodeRecipeNotApplicable Code = "recipe_not_applicable" + CodeNotRunning Code = "not_running" + CodeAlreadyRunning Code = "already_running" + CodeNoDisk Code = "no_disk" + CodeImmutableField Code = "immutable_field" + CodeDiskShrink Code = "disk_shrink" + CodeCannotReach Code = "cannot_reach" + CodeUnknownLog Code = "unknown_log" + CodeTimeout Code = "timeout" + CodeCanceled Code = "canceled" + CodeUsage Code = "usage" + CodeConfirmationRequired Code = "confirmation_required" + CodeInternal Code = "internal" + + CodeQemuMissing Code = "qemu_missing" + CodeKVMUnusable Code = "kvm_unusable" + CodeQemuStartFailed Code = "qemu_start_failed" + CodeMonitorUnreachable Code = "monitor_unreachable" + CodeMonitorRejected Code = "monitor_rejected" + CodeNoConsolePassword Code = "no_console_password" + CodeShareInvalid Code = "share_invalid" + CodeNoXattr Code = "no_xattr" + + CodeDownloadFailed Code = "download_failed" + CodeDownloadStalled Code = "download_stalled" + CodeChecksumMismatch Code = "checksum_mismatch" + CodeNoSuchImage Code = "no_such_image" + + CodeScreenshotFailed Code = "screenshot_failed" ) +// Codes returns every declared code, sorted. Built from the same string +// constants codeTable and ErrorInfo.Code use, so a code added to one and +// forgotten here fails TestCodesCoversTheTable. +func Codes() []Code { + out := []Code{ + CodeNotFound, CodeBroken, CodeNameTaken, CodeInvalidSpec, + CodeImageNotDownloaded, CodeRecipeNotApplicable, CodeNotRunning, + CodeAlreadyRunning, CodeNoDisk, CodeImmutableField, CodeDiskShrink, + CodeCannotReach, CodeUnknownLog, CodeTimeout, CodeCanceled, + CodeUsage, CodeConfirmationRequired, CodeInternal, + CodeQemuMissing, CodeKVMUnusable, CodeQemuStartFailed, + CodeMonitorUnreachable, CodeMonitorRejected, CodeNoConsolePassword, + CodeShareInvalid, CodeNoXattr, + CodeDownloadFailed, CodeDownloadStalled, CodeChecksumMismatch, + CodeNoSuchImage, + CodeScreenshotFailed, + } + slices.Sort(out) + return out +} + // ErrConfirmationRequired is confirmation_required's sentinel (§2: "new, // CLI-only"). core has no equivalent; --json never prompts (§1), so `rm` // without -y wraps this instead of reading stdin. A future cli.go caller @@ -49,7 +94,7 @@ var ErrConfirmationRequired = errors.New("confirmation required; pass -y under - // ErrAlreadyRunning directly, never ErrDiskShrink, so there is nothing to // arbitrate there. Rows keep the design doc's order in case that changes. var codeTable = []struct { - code string + code Code err error }{ {CodeNotFound, core.ErrNotFound}, @@ -68,6 +113,21 @@ var codeTable = []struct { {CodeTimeout, context.DeadlineExceeded}, {CodeCanceled, context.Canceled}, {CodeConfirmationRequired, ErrConfirmationRequired}, + {CodeQemuMissing, qemu.ErrBinaryMissing}, + {CodeKVMUnusable, qemu.ErrKVMUnusable}, + {CodeQemuStartFailed, qemu.ErrStartFailed}, + {CodeMonitorUnreachable, qemu.ErrMonitorUnreachable}, + {CodeMonitorRejected, qemu.ErrMonitorRejected}, + {CodeNoConsolePassword, qemu.ErrNoConsolePassword}, + {CodeShareInvalid, qemu.ErrShareInvalid}, + {CodeNoXattr, qemu.ErrNoXattr}, + {CodeAlreadyRunning, qemu.ErrAlreadyRunning}, + {CodeDownloadFailed, iso.ErrDownloadFailed}, + {CodeDownloadStalled, iso.ErrDownloadStalled}, + {CodeChecksumMismatch, iso.ErrChecksumMismatch}, + {CodeNoSuchImage, iso.ErrNoSuchImage}, + {CodeScreenshotFailed, qemu.ErrScreenshotFailed}, + {CodeNotRunning, qemu.ErrNotRunning}, } // MapError converts a core (or context) error into an ErrorInfo, walking diff --git a/internal/cli/wire/errors_test.go b/internal/cli/wire/errors_test.go index 4d1b49a7..3dd6e6d0 100644 --- a/internal/cli/wire/errors_test.go +++ b/internal/cli/wire/errors_test.go @@ -4,15 +4,19 @@ import ( "context" "errors" "fmt" + "regexp" + "slices" "testing" "github.com/novusedge/stoat/internal/core" + "github.com/novusedge/stoat/internal/iso" + "github.com/novusedge/stoat/internal/qemu" ) func TestMapErrorEveryCoreSentinel(t *testing.T) { tests := []struct { err error - code string + code Code }{ {fmt.Errorf("%w: work", core.ErrNotFound), CodeNotFound}, {fmt.Errorf("%w: work: parse fail", core.ErrBroken), CodeBroken}, @@ -32,7 +36,7 @@ func TestMapErrorEveryCoreSentinel(t *testing.T) { {ErrConfirmationRequired, CodeConfirmationRequired}, } for _, tt := range tests { - t.Run(tt.code, func(t *testing.T) { + t.Run(string(tt.code), func(t *testing.T) { got := MapError(tt.err) if got == nil { t.Fatal("MapError returned nil for a non-nil error") @@ -98,3 +102,84 @@ func TestErrorInfoWithSubject(t *testing.T) { t.Errorf("got %+v", got) } } + +// Every code in codeTable is declared by Codes(). A code that reaches a +// consumer but is absent from the published list is a code no consumer can +// generate a switch for. +func TestCodesCoversTheTable(t *testing.T) { + declared := make(map[Code]bool, len(Codes())) + for _, c := range Codes() { + declared[c] = true + } + for _, row := range codeTable { + if !declared[row.code] { + t.Errorf("codeTable has %q, Codes() does not", row.code) + } + } + for _, c := range []Code{CodeUsage, CodeInternal} { + if !declared[c] { + t.Errorf("Codes() is missing the CLI-owned code %q", c) + } + } +} + +// Codes are the machine-readable half of the contract: snake_case, unique, +// and sorted, so a generated switch is stable between builds. +func TestCodesAreSnakeCaseUniqueAndSorted(t *testing.T) { + valid := regexp.MustCompile(`^[a-z][a-z0-9_]*$`) + seen := make(map[Code]bool) + codes := Codes() + for _, c := range codes { + if !valid.MatchString(string(c)) { + t.Errorf("code %q is not snake_case", c) + } + if seen[c] { + t.Errorf("code %q is declared twice", c) + } + seen[c] = true + } + if !slices.IsSorted(codes) { + t.Errorf("Codes() = %v, want sorted", codes) + } +} + +// Each qemu sentinel has exactly one row in codeTable. A sentinel with no row +// reaches a consumer as "internal" with prose, which is the shape this whole +// table exists to remove. +func TestEveryQemuSentinelHasOneRow(t *testing.T) { + want := map[error]Code{ + qemu.ErrBinaryMissing: CodeQemuMissing, + qemu.ErrKVMUnusable: CodeKVMUnusable, + qemu.ErrStartFailed: CodeQemuStartFailed, + qemu.ErrMonitorUnreachable: CodeMonitorUnreachable, + qemu.ErrMonitorRejected: CodeMonitorRejected, + qemu.ErrNoConsolePassword: CodeNoConsolePassword, + qemu.ErrShareInvalid: CodeShareInvalid, + qemu.ErrNoXattr: CodeNoXattr, + // No new code: core.ErrAlreadyRunning already names this condition, + // and qemu cannot import core to reuse the sentinel itself. + qemu.ErrAlreadyRunning: CodeAlreadyRunning, + } + for sentinel, code := range want { + got := MapError(fmt.Errorf("%w: subject", sentinel)) + if got.Code != code { + t.Errorf("MapError(%v).Code = %q, want %q", sentinel, got.Code, code) + } + } +} + +// Each iso sentinel has exactly one row in codeTable. +func TestEveryISOSentinelHasOneRow(t *testing.T) { + want := map[error]Code{ + iso.ErrDownloadFailed: CodeDownloadFailed, + iso.ErrDownloadStalled: CodeDownloadStalled, + iso.ErrChecksumMismatch: CodeChecksumMismatch, + iso.ErrNoSuchImage: CodeNoSuchImage, + } + for sentinel, code := range want { + got := MapError(fmt.Errorf("%w: subject", sentinel)) + if got.Code != code { + t.Errorf("MapError(%v).Code = %q, want %q", sentinel, got.Code, code) + } + } +} diff --git a/internal/cli/wire/status.go b/internal/cli/wire/status.go new file mode 100644 index 00000000..138d293d --- /dev/null +++ b/internal/cli/wire/status.go @@ -0,0 +1,14 @@ +package wire + +// The status values a DTO carries, so no constructor writes a raw literal. +// They are plain strings, not a named type: wire.VM.State is a string field +// and a consumer reads the JSON, not the Go type. +const ( + StateStopped = "stopped" + StateRunning = "running" + StateBroken = "broken" + + HealthOK = "ok" + HealthFailed = "failed" + HealthUnknown = "unknown" +) diff --git a/internal/core/access.go b/internal/core/access.go index fedd4ae4..8e6ff76b 100644 --- a/internal/core/access.go +++ b/internal/core/access.go @@ -7,6 +7,7 @@ import ( "io" "os" "path/filepath" + "slices" "github.com/novusedge/stoat/internal/config" "github.com/novusedge/stoat/internal/sshx" @@ -48,6 +49,12 @@ const ( // reading the wrong file. var ErrUnknownWhich = fmt.Errorf("unknown log selector") +// Whichs returns every log a caller can ask for. +func Whichs() []Which { return []Which{WhichConsole, WhichApply} } + +// Valid reports whether w is one of Whichs(). +func (w Which) Valid() bool { return slices.Contains(Whichs(), w) } + // Logs opens the requested log file for VM name. // // A missing file is normal, not an error. A VM that never started has no @@ -60,6 +67,9 @@ var ErrUnknownWhich = fmt.Errorf("unknown log selector") // field, and a broken VM is exactly the one a user wants console output from, // so Logs builds a directory-only VM, like Destroy does. func Logs(name string, which Which) (io.ReadCloser, error) { + if !which.Valid() { + return nil, fmt.Errorf("%w: %q", ErrUnknownWhich, which) + } v, err := load(name) switch { case errors.Is(err, ErrBroken): @@ -68,14 +78,9 @@ func Logs(name string, which Which) (io.ReadCloser, error) { return nil, err } - var path string - switch which { - case WhichConsole: + path := v.ProvisionLogPath() + if which == WhichConsole { path = v.ConsoleLogPath() - case WhichApply: - path = v.ProvisionLogPath() - default: - return nil, fmt.Errorf("%w: %q", ErrUnknownWhich, which) } f, err := os.Open(path) diff --git a/internal/core/enums_test.go b/internal/core/enums_test.go new file mode 100644 index 00000000..22a6cf21 --- /dev/null +++ b/internal/core/enums_test.go @@ -0,0 +1,59 @@ +package core + +import ( + "context" + "errors" + "testing" +) + +// Wait refuses an Until it does not know, rather than falling into a default +// branch that silently means one of the three real ones. +func TestWaitRefusesAnUnknownUntil(t *testing.T) { + root(t) + err := Wait(context.Background(), "work", Until("nope")) + if !errors.Is(err, ErrInvalidSpec) { + t.Errorf("Wait(nope) = %v, want ErrInvalidSpec", err) + } +} + +// Logs refuses a Which it does not know. Before Valid(), the default branch +// read the apply log for any unrecognised selector. +func TestLogsRefusesAnUnknownWhich(t *testing.T) { + root(t) + if _, err := Logs("work", Which("nope")); !errors.Is(err, ErrUnknownWhich) { + t.Errorf("Logs(nope) = %v, want ErrUnknownWhich", err) + } +} + +// The member lists are what a consumer generates a switch from, so each one +// holds exactly the declared values and calls them valid. +func TestMemberListsMatchTheDeclaredValues(t *testing.T) { + if got, want := States(), []State{StateStopped, StateRunning, StateBroken}; !equalStates(got, want) { + t.Errorf("States() = %v, want %v", got, want) + } + for _, u := range Untils() { + if !u.Valid() { + t.Errorf("Untils() lists %q, which Valid() rejects", u) + } + } + for _, w := range Whichs() { + if !w.Valid() { + t.Errorf("Whichs() lists %q, which Valid() rejects", w) + } + } + if got, want := Healths(), []Health{HealthOK, HealthFailed, HealthUnknown}; len(got) != len(want) { + t.Errorf("Healths() = %v, want %v", got, want) + } +} + +func equalStates(a, b []State) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} diff --git a/internal/core/health.go b/internal/core/health.go new file mode 100644 index 00000000..5913e8f5 --- /dev/null +++ b/internal/core/health.go @@ -0,0 +1,14 @@ +package core + +// Health is a recipe's health-check result. The recipe contract writes the +// checks that report it; this declares the values they may report. +type Health string + +const ( + HealthOK Health = "ok" + HealthFailed Health = "failed" + HealthUnknown Health = "unknown" +) + +// Healths returns every declared health value. +func Healths() []Health { return []Health{HealthOK, HealthFailed, HealthUnknown} } diff --git a/internal/core/screenshot.go b/internal/core/screenshot.go new file mode 100644 index 00000000..705cfc6f --- /dev/null +++ b/internal/core/screenshot.go @@ -0,0 +1,90 @@ +package core + +import ( + "fmt" + "image/png" + "os" + "path/filepath" + "strconv" + "strings" + "time" + + "github.com/novusedge/stoat/internal/qemu" +) + +// Shot is one screenshot: where it landed and what is in it. +type Shot struct { + Path string + Bytes int64 + Width int + Height int +} + +// Screenshot writes VM name's screen to dest, or to +// /screenshots/.png when dest is +// empty. See shotPath. +// +// The running check is here rather than in qemu.Screenshot so a stopped VM +// answers ErrNotRunning, the same error Stop gives, instead of the +// monitor-socket error a caller cannot act on differently. +func Screenshot(name, dest string) (Shot, error) { + v, err := load(name) + if err != nil { + return Shot{}, err + } + if !qemu.Running(v) { + return Shot{}, fmt.Errorf("%w: %s", ErrNotRunning, name) + } + + if dest == "" { + dir := filepath.Join(v.Dir, "screenshots") + if err := os.MkdirAll(dir, 0o755); err != nil { + return Shot{}, err + } + dest = shotPath(dir, time.Now().UTC()) + } + dest, err = filepath.Abs(dest) + if err != nil { + return Shot{}, err + } + if err := qemu.Screenshot(v, dest); err != nil { + return Shot{}, err + } + + fi, err := os.Stat(dest) + if err != nil { + return Shot{}, err + } + f, err := os.Open(dest) + if err != nil { + return Shot{}, err + } + defer func() { _ = f.Close() }() + // The header alone gives the size, so a 4K frame is never decoded into + // memory to answer two integers. + cfg, err := png.DecodeConfig(f) + if err != nil { + return Shot{}, err + } + return Shot{Path: dest, Bytes: fi.Size(), Width: cfg.Width, Height: cfg.Height}, nil +} + +// shotPath names a screenshot after the second it was taken: RFC3339 with +// the colons stripped, since a colon in a filename breaks scp's host:path +// split and Windows refuses it outright. +// +// A second shot inside the same second gets -2, then -3. A caller polling a +// boot takes frames faster than the name's resolution, and overwriting the +// frame that showed the stall is the failure this avoids. +func shotPath(dir string, at time.Time) string { + stamp := strings.ReplaceAll(at.Format(time.RFC3339), ":", "") + path := filepath.Join(dir, stamp+".png") + for n := 2; ; n++ { + // Any stat failure other than "gone" is answered the same way: the + // name is used, and qemu reports the write failure with the reason. + if _, err := os.Stat(path); err != nil { + return path + } + path = filepath.Join(dir, stamp+"-"+strconv.Itoa(n)+".png") + } +} diff --git a/internal/core/screenshot_test.go b/internal/core/screenshot_test.go new file mode 100644 index 00000000..7826936f --- /dev/null +++ b/internal/core/screenshot_test.go @@ -0,0 +1,47 @@ +package core + +import ( + "errors" + "os" + "path/filepath" + "testing" + "time" + + "github.com/novusedge/stoat/internal/config" +) + +// A stopped VM has no framebuffer, so Screenshot refuses up front with the +// same typed error Stop uses, rather than failing at the monitor socket. +func TestScreenshotRefusesAStoppedVM(t *testing.T) { + root(t) + if err := (&config.VM{Name: "work", Mode: "live", RAM: 1024, CPUs: 1, SSHPort: 2200}).Save(); err != nil { + t.Fatal(err) + } + if _, err := Screenshot("work", ""); !errors.Is(err, ErrNotRunning) { + t.Errorf("Screenshot() = %v, want ErrNotRunning", err) + } +} + +// Polling a boot is the use this command exists for, and such a caller takes +// several frames a second. The name resolves to the second, so two frames in +// one second must not become one file. shotPath takes the time as a parameter +// because the collision cannot be reached through Screenshot without a +// running VM and a clock the test controls. +func TestScreenshotNamesDoNotCollideWithinASecond(t *testing.T) { + dir := t.TempDir() + at := time.Date(2026, 9, 5, 14, 3, 2, 0, time.UTC) + + for _, want := range []string{ + "2026-09-05T140302Z.png", + "2026-09-05T140302Z-2.png", + "2026-09-05T140302Z-3.png", + } { + got := shotPath(dir, at) + if filepath.Base(got) != want { + t.Fatalf("shotPath() = %q, want %q", filepath.Base(got), want) + } + if err := os.WriteFile(got, nil, 0o644); err != nil { + t.Fatal(err) + } + } +} diff --git a/internal/core/vm.go b/internal/core/vm.go index a747f050..bc43f57c 100644 --- a/internal/core/vm.go +++ b/internal/core/vm.go @@ -36,6 +36,10 @@ const ( StateBroken State = "broken" ) +// States returns every State this package produces. Three of the design +// doc's six; see State's own comment for why the other three are absent. +func States() []State { return []State{StateStopped, StateRunning, StateBroken} } + // ErrNotRunning is returned by Stop (and by Destroy, transitively) for a VM // that is not running. qemu.Stop itself treats "already stopped" as a // no-op; Stop checks first so the caller gets a typed error instead. diff --git a/internal/core/wait.go b/internal/core/wait.go index b16b2205..79d2a04e 100644 --- a/internal/core/wait.go +++ b/internal/core/wait.go @@ -7,6 +7,7 @@ import ( "io" "net" "os" + "slices" "strings" "time" @@ -33,6 +34,13 @@ const ( UntilStopped Until = "stopped" ) +// Untils returns every state Wait can block for. +func Untils() []Until { return []Until{UntilReachable, UntilApplied, UntilStopped} } + +// Valid reports whether u is one of Untils(). Wait calls it before it loads +// the VM, so a typo fails with the reason rather than with "not found". +func (u Until) Valid() bool { return slices.Contains(Untils(), u) } + // ErrCannotReach is returned by Wait for a VM that cannot, by construction, // ever reach the requested state: a stopped VM asked to become Reachable, or // a VM with no recipes asked to become Applied. Wait checks for these cases @@ -65,19 +73,20 @@ const InstallTimeout = 15 * time.Minute // already mid-boot by List. Wait lets that caller block on the outcome // instead of re-polling Get in a loop. func Wait(ctx context.Context, name string, until Until) error { + if !until.Valid() { + return fmt.Errorf("%w: unknown Until %q", ErrInvalidSpec, until) + } v, err := load(name) if err != nil { return err } switch until { - case UntilReachable: - return waitReachable(ctx, v) case UntilApplied: return waitApplied(ctx, v) case UntilStopped: return waitStopped(ctx, v) default: - return fmt.Errorf("%w: unknown Until %q", ErrInvalidSpec, until) + return waitReachable(ctx, v) } } diff --git a/internal/iso/errors.go b/internal/iso/errors.go new file mode 100644 index 00000000..331dcb92 --- /dev/null +++ b/internal/iso/errors.go @@ -0,0 +1,13 @@ +package iso + +import "errors" + +// Sentinels for the failures a caller branches on. A stalled download and a +// checksum mismatch are both "the image is not here", but the first is worth +// retrying on the same mirror and the second is not. +var ( + ErrDownloadFailed = errors.New("download failed") + ErrDownloadStalled = errors.New("download stalled") + ErrChecksumMismatch = errors.New("checksum mismatch") + ErrNoSuchImage = errors.New("no such image") +) diff --git a/internal/iso/iso.go b/internal/iso/iso.go index 70216915..434ceab1 100644 --- a/internal/iso/iso.go +++ b/internal/iso/iso.go @@ -348,7 +348,7 @@ func Latest(flavor string) (*Release, error) { } defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("index: %s", resp.Status) + return nil, fmt.Errorf("%w: index: %s", ErrDownloadFailed, resp.Status) } var releases []Release if err := yaml.NewDecoder(resp.Body).Decode(&releases); err != nil { @@ -359,7 +359,7 @@ func Latest(flavor string) (*Release, error) { return &releases[i], nil } } - return nil, fmt.Errorf("no %s iso in index", flavor) + return nil, fmt.Errorf("%w: no %s iso in index", ErrNoSuchImage, flavor) } // Resolve turns a catalog Entry into a concrete Release ready for Download. @@ -412,7 +412,7 @@ func fetchChecksum(checksumURL, filename string) (string, error) { } defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { - return "", fmt.Errorf("checksum: %s", resp.Status) + return "", fmt.Errorf("%w: checksum: %s", ErrDownloadFailed, resp.Status) } body, err := io.ReadAll(resp.Body) if err != nil { @@ -450,7 +450,7 @@ func parseChecksum(body []byte, filename string) (string, error) { return strings.ToLower(fields[0]), nil } } - return "", fmt.Errorf("no checksum for %s", filename) + return "", fmt.Errorf("%w: no checksum for %s", ErrDownloadFailed, filename) } // newDigest picks the hash algorithm by the expected digest's hex length: @@ -530,7 +530,7 @@ func Download(ctx context.Context, r *Release, progress func(done, total int64)) } defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { - return "", fmt.Errorf("download: %s", resp.Status) + return "", fmt.Errorf("%w: download: %s", ErrDownloadFailed, resp.Status) } // Cancelling the request unblocks a Read that would otherwise never @@ -592,7 +592,7 @@ func Download(ctx context.Context, r *Release, progress func(done, total int64)) _ = f.Close() _ = os.Remove(part) if stalled.Load() { - return "", fmt.Errorf("download stalled: no data for %s", stallTimeout) + return "", fmt.Errorf("%w: download stalled: no data for %s", ErrDownloadStalled, stallTimeout) } return "", rerr } @@ -609,7 +609,7 @@ func Download(ctx context.Context, r *Release, progress func(done, total int64)) if r.SHA256 != "" { if got := hex.EncodeToString(h.Sum(nil)); got != r.SHA256 { _ = os.Remove(part) - return "", fmt.Errorf("checksum mismatch: got %s, want %s", got, r.SHA256) + return "", fmt.Errorf("%w: checksum mismatch: got %s, want %s", ErrChecksumMismatch, got, r.SHA256) } r.Verified = true } diff --git a/internal/iso/iso_test.go b/internal/iso/iso_test.go index bfbb46d3..57a80d1a 100644 --- a/internal/iso/iso_test.go +++ b/internal/iso/iso_test.go @@ -6,6 +6,7 @@ import ( "crypto/sha256" "crypto/sha512" "encoding/hex" + "errors" "fmt" "net/http" "net/http/httptest" @@ -728,3 +729,37 @@ func TestDownloadCancelStopsTheTransfer(t *testing.T) { t.Error("a cancelled download was renamed into place") } } + +// A mirror that answers with a status other than 200 is a download failure a +// caller retries; it is not a broken stoat. +func TestDownloadReportsAFailedFetch(t *testing.T) { + t.Setenv("STOAT_HOME", t.TempDir()) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer srv.Close() + + r := &Release{File: "broken.iso", URL: srv.URL + "/broken.iso"} + if _, err := Download(context.Background(), r, nil); !errors.Is(err, ErrDownloadFailed) { + t.Errorf("Download() = %v, want ErrDownloadFailed", err) + } +} + +// Bytes that do not match the published digest are a mismatch, not a +// transport failure: the fetch succeeded and the content is wrong. +func TestDownloadReportsAChecksumMismatch(t *testing.T) { + t.Setenv("STOAT_HOME", t.TempDir()) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + _, _ = w.Write([]byte("not the expected bytes")) + })) + defer srv.Close() + + r := &Release{ + File: "mismatch.iso", + URL: srv.URL + "/mismatch.iso", + SHA256: strings.Repeat("0", 64), + } + if _, err := Download(context.Background(), r, nil); !errors.Is(err, ErrChecksumMismatch) { + t.Errorf("Download() = %v, want ErrChecksumMismatch", err) + } +} diff --git a/internal/qemu/errors.go b/internal/qemu/errors.go new file mode 100644 index 00000000..fc85c07c --- /dev/null +++ b/internal/qemu/errors.go @@ -0,0 +1,29 @@ +package qemu + +import "errors" + +// Sentinels for the failures a caller branches on. Each is wrapped in front +// of the message the call site already printed, so the text is unchanged and +// errors.Is reaches the sentinel through core's own wrapping. +// +// ErrNoXattr lives in xattr.go, beside the probe that raises it. +// +// ErrAlreadyRunning duplicates core.ErrAlreadyRunning by necessity: core +// imports qemu, so qemu cannot name core's sentinel. wire maps both to +// already_running. +var ( + ErrBinaryMissing = errors.New("qemu binary not found") + ErrKVMUnusable = errors.New("/dev/kvm not usable") + ErrStartFailed = errors.New("qemu failed to start") + ErrMonitorUnreachable = errors.New("qemu monitor unreachable") + // ErrMonitorRejected is qemu answering a command with an error object. + // The command reached qemu; qemu refused it. + ErrMonitorRejected = errors.New("qemu monitor rejected the command") + ErrNoConsolePassword = errors.New("no console password set") + ErrShareInvalid = errors.New("share is not a directory") + ErrAlreadyRunning = errors.New("already running") + ErrScreenshotFailed = errors.New("screenshot failed") + // ErrNotRunning duplicates core.ErrNotRunning: core imports qemu, so + // qemu cannot name core's sentinel. wire maps both to not_running. + ErrNotRunning = errors.New("not running") +) diff --git a/internal/qemu/errors_test.go b/internal/qemu/errors_test.go new file mode 100644 index 00000000..679e92a5 --- /dev/null +++ b/internal/qemu/errors_test.go @@ -0,0 +1,52 @@ +package qemu + +import ( + "errors" + "os" + "path/filepath" + "testing" + + "github.com/novusedge/stoat/internal/config" +) + +// A qemu binary that is not on PATH is the first thing Preflight checks, so +// an empty PATH reaches ErrBinaryMissing before the /dev/kvm open. +func TestPreflightReportsAMissingBinary(t *testing.T) { + t.Setenv("PATH", t.TempDir()) + if err := Preflight(); !errors.Is(err, ErrBinaryMissing) { + t.Errorf("Preflight() = %v, want ErrBinaryMissing", err) + } +} + +// A share that names a file, not a directory, is a vm.toml mistake a caller +// corrects by editing the field, so it is told apart from a missing path. +func TestPrepareSharesReportsAShareThatIsAFile(t *testing.T) { + dir := t.TempDir() + t.Setenv("STOAT_HOME", dir) + file := filepath.Join(dir, "notadir") + if err := os.WriteFile(file, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + v := &config.VM{Name: "work", Dir: filepath.Join(dir, "work"), Share: file} + if err := prepareShares(v); !errors.Is(err, ErrShareInvalid) { + t.Errorf("prepareShares() = %v, want ErrShareInvalid", err) + } +} + +// A stopped VM has no QMP socket to dial, and a caller that gets +// ErrMonitorUnreachable knows to start the VM rather than to retry. +func TestDialQMPReportsAnUnreachableMonitor(t *testing.T) { + v := &config.VM{Name: "work", Dir: t.TempDir()} + if _, err := dialQMP(v); !errors.Is(err, ErrMonitorUnreachable) { + t.Errorf("dialQMP() = %v, want ErrMonitorUnreachable", err) + } +} + +// TypeConsolePassword refuses a VM with no password rather than typing +// nothing at the login prompt and reporting success. +func TestTypeConsolePasswordReportsNoPassword(t *testing.T) { + v := &config.VM{Name: "work", Dir: t.TempDir()} + if err := TypeConsolePassword(v); !errors.Is(err, ErrNoConsolePassword) { + t.Errorf("TypeConsolePassword() = %v, want ErrNoConsolePassword", err) + } +} diff --git a/internal/qemu/qmp.go b/internal/qemu/qmp.go index fff54f73..a6de11d6 100644 --- a/internal/qemu/qmp.go +++ b/internal/qemu/qmp.go @@ -35,7 +35,7 @@ type qmp struct { func dialQMP(v *config.VM) (*qmp, error) { c, err := net.Dial("unix", v.QMPPath()) if err != nil { - return nil, fmt.Errorf("qmp: %w", err) + return nil, fmt.Errorf("%w: qmp: %w", ErrMonitorUnreachable, err) } if err := c.SetDeadline(time.Now().Add(qmpTimeout)); err != nil { _ = c.Close() @@ -50,11 +50,11 @@ func dialQMP(v *config.VM) (*qmp, error) { } if err := q.dec.Decode(&greeting); err != nil { _ = c.Close() - return nil, fmt.Errorf("qmp greeting: %w", err) + return nil, fmt.Errorf("%w: qmp greeting: %w", ErrMonitorUnreachable, err) } if greeting.QMP == nil { _ = c.Close() - return nil, fmt.Errorf("qmp: not a QMP socket (no greeting)") + return nil, fmt.Errorf("%w: qmp: not a QMP socket (no greeting)", ErrMonitorUnreachable) } if _, err := q.command("qmp_capabilities", nil); err != nil { _ = c.Close() @@ -97,7 +97,7 @@ func (q *qmp) command(name string, args map[string]any) (json.RawMessage, error) continue // asynchronous event, not our reply } if resp.Error != nil { - return nil, fmt.Errorf("qmp %s: %s", name, resp.Error.Desc) + return nil, fmt.Errorf("%w: qmp %s: %s", ErrMonitorRejected, name, resp.Error.Desc) } return resp.Return, nil } diff --git a/internal/qemu/run.go b/internal/qemu/run.go index 7424d353..9802d095 100644 --- a/internal/qemu/run.go +++ b/internal/qemu/run.go @@ -18,11 +18,11 @@ import ( // Preflight reports why VMs cannot start, or nil if they can. func Preflight() error { if _, err := exec.LookPath(Binary); err != nil { - return fmt.Errorf("%s not found in PATH", Binary) + return fmt.Errorf("%w: %s not found in PATH", ErrBinaryMissing, Binary) } f, err := os.OpenFile("/dev/kvm", os.O_RDWR, 0) if err != nil { - return fmt.Errorf("/dev/kvm not usable: %w (are you in the kvm group?)", err) + return fmt.Errorf("%w: /dev/kvm not usable: %w (are you in the kvm group?)", ErrKVMUnusable, err) } _ = f.Close() return nil @@ -104,7 +104,7 @@ func diskWritten(v *config.VM) bool { // nothing and tracks the process by pidfile. func Start(v *config.VM) error { if Running(v) { - return fmt.Errorf("%s is already running", v.Name) + return fmt.Errorf("%w: %s is already running", ErrAlreadyRunning, v.Name) } _ = os.Remove(v.MonitorPath()) // The interactive install happens inside the guest, where stoat can't @@ -141,7 +141,7 @@ func Start(v *config.VM) error { msg = err.Error() } logx.L().Error("start failed", "vm", v.Name, "err", msg) - return fmt.Errorf("qemu failed to start: %s%s", msg, explainDisplayFailure(msg)) + return fmt.Errorf("%w: qemu failed to start: %s%s", ErrStartFailed, msg, explainDisplayFailure(msg)) } // Log how to get in, not just that it started. This is the line someone // greps for when a VM is up but unreachable. diff --git a/internal/qemu/screenshot.go b/internal/qemu/screenshot.go new file mode 100644 index 00000000..a902dec5 --- /dev/null +++ b/internal/qemu/screenshot.go @@ -0,0 +1,36 @@ +package qemu + +import ( + "fmt" + "path/filepath" + + "github.com/novusedge/stoat/internal/config" +) + +// Screenshot writes the guest's framebuffer to path as a PNG. +// +// QMP screendump takes format: "png" from QEMU 7.1 on, verified by hand +// against QEMU 11.1, so nothing here converts a PPM afterwards. The +// framebuffer exists whatever the display backend is, so this answers for a +// GTK window and for a VNC socket alike. +// +// qemu writes the file itself, as its own user, so path must be absolute: +// a relative one resolves against qemu's working directory, not the +// caller's. +func Screenshot(v *config.VM, path string) error { + if !Running(v) { + return fmt.Errorf("%w: %s is not running", ErrNotRunning, v.Name) + } + if !filepath.IsAbs(path) { + return fmt.Errorf("%w: screenshot path %s is not absolute", ErrScreenshotFailed, path) + } + q, err := dialQMP(v) + if err != nil { + return err + } + defer func() { _ = q.Close() }() + if _, err := q.command("screendump", map[string]any{"filename": path, "format": "png"}); err != nil { + return fmt.Errorf("%w: %v", ErrScreenshotFailed, err) + } + return nil +} diff --git a/internal/qemu/screenshot_test.go b/internal/qemu/screenshot_test.go new file mode 100644 index 00000000..2ab9e969 --- /dev/null +++ b/internal/qemu/screenshot_test.go @@ -0,0 +1,20 @@ +package qemu + +import ( + "errors" + "path/filepath" + "testing" + + "github.com/novusedge/stoat/internal/config" +) + +// A stopped VM has no framebuffer, and its dead monitor socket would +// otherwise report the transport rather than the reason. +func TestScreenshotRefusesAStoppedVM(t *testing.T) { + dir := t.TempDir() + v := &config.VM{Name: "work", Dir: dir} + err := Screenshot(v, filepath.Join(dir, "shot.png")) + if !errors.Is(err, ErrNotRunning) { + t.Errorf("Screenshot() = %v, want ErrNotRunning", err) + } +} diff --git a/internal/qemu/sendkey.go b/internal/qemu/sendkey.go index 876a9a2c..7548e3ae 100644 --- a/internal/qemu/sendkey.go +++ b/internal/qemu/sendkey.go @@ -54,7 +54,7 @@ func passwordKeys(password string) ([]string, error) { // password to send. func TypeConsolePassword(v *config.VM) error { if v.ConsolePassword == "" { - return fmt.Errorf("%s has no console password set", v.Name) + return fmt.Errorf("%w: %s has no console password set", ErrNoConsolePassword, v.Name) } keys, err := passwordKeys(v.ConsolePassword) if err != nil { @@ -62,12 +62,12 @@ func TypeConsolePassword(v *config.VM) error { } c, err := dialMonitor(v) if err != nil { - return fmt.Errorf("qemu monitor: %w", err) + return fmt.Errorf("%w: qemu monitor: %w", ErrMonitorUnreachable, err) } defer func() { _ = c.Close() }() for _, k := range keys { if _, err := fmt.Fprintln(c, "sendkey "+k); err != nil { - return fmt.Errorf("qemu monitor: %w", err) + return fmt.Errorf("%w: qemu monitor: %w", ErrMonitorUnreachable, err) } time.Sleep(consoleKeyDelay) } diff --git a/internal/qemu/share.go b/internal/qemu/share.go index 34ef1098..aaa81430 100644 --- a/internal/qemu/share.go +++ b/internal/qemu/share.go @@ -34,7 +34,7 @@ func prepareShares(v *config.VM) error { return fmt.Errorf("share %s for %s: %w", v.Share, v.Name, err) } if !st.IsDir() { - return fmt.Errorf("share %s for %s is not a directory", v.Share, v.Name) + return fmt.Errorf("%w: share %s for %s is not a directory", ErrShareInvalid, v.Share, v.Name) } return nil }