doctor: pin firecracker version range, distro-aware install hint
Pre-release polish: be explicit about which firecracker versions
banger has been validated against, and give users a one-line install
suggestion when the binary is missing rather than the previous
generic "install firecracker or set firecracker_bin".
internal/firecracker/version.go (new):
* MinSupportedVersion = "1.5.0" — the floor banger refuses to
launch below. Bumping this is a deliberate decision, paired
with whatever helper feature started requiring the newer
firecracker.
* KnownTestedVersion = "1.14.1" — what banger's smoke suite
actually runs against today.
* SemVer + Compare + ParseVersionOutput, table-tested. The parser
tolerates the trailing "exiting successfully" log line that
firecracker tacks onto --version; only the canonical
"Firecracker vX.Y.Z" line matters.
* QueryVersion shells `<bin> --version` through a CommandRunner-
shaped interface; doesn't import internal/system to keep the
firecracker package leaf-clean.
internal/daemon/doctor.go:
* New addFirecrackerVersionCheck replaces the previous bare
RequireExecutable preflight for firecracker. Three outcomes:
PASS within [Min, Tested], WARN above Tested (newer firecracker
usually works but is outside the tested window), FAIL below Min
or when the binary is missing.
* On missing binary, surfaces a distro-aware install command via
parseOSReleaseIDs(/etc/os-release) → guessFirecrackerInstall
Command. Pinned suggestions for debian (apt), arch/manjaro
(paru), and nixos (nix-env). Other distros get only the upstream
Releases URL — guessing wrong sends users on a wild goose chase.
* runtimeChecks no longer includes the firecracker preflight; the
new check subsumes it.
README.md:
* Requirements line now spells out the tested-against version
(v1.14.1) and the supported floor (≥ v1.5.0), and points at
`banger doctor` for the version check + install hint.
Tests: ParseVersionOutput across canonical/prerelease/garbage inputs,
SemVer.Compare across major/minor/patch boundaries, MustParseSemVer
panics on malformed inputs. Doctor-side: PASS on tested version,
FAIL below Min, WARN above Tested, FAIL with upstream URL when
missing, install-hint dispatch table covering debian/ubuntu (via
ID_LIKE)/arch/manjaro/nixos/fedora-fallback/missing-os-release.
The renamed TestDoctorReport_MissingFirecrackerFails... now asserts
against the new check name. Live `banger doctor` reports
"v1.14.1 at /usr/bin/firecracker (within tested range; min v1.5.0,
tested v1.14.1)" against the smoke host.
Smoke bare_run still green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
f7a6832ebf
commit
1c1ca7d6a4
5 changed files with 510 additions and 6 deletions
|
|
@ -10,6 +10,7 @@ import (
|
|||
"syscall"
|
||||
|
||||
"banger/internal/config"
|
||||
"banger/internal/firecracker"
|
||||
"banger/internal/imagecat"
|
||||
"banger/internal/installmeta"
|
||||
"banger/internal/model"
|
||||
|
|
@ -91,11 +92,132 @@ func (d *Daemon) doctorReport(ctx context.Context, storeErr error, storeMissing
|
|||
d.addVMDefaultsCheck(&report)
|
||||
d.addSSHShortcutCheck(&report)
|
||||
d.addCapabilityDoctorChecks(ctx, &report)
|
||||
d.addFirecrackerVersionCheck(ctx, &report)
|
||||
d.addSecurityPostureChecks(ctx, &report)
|
||||
|
||||
return report
|
||||
}
|
||||
|
||||
// addFirecrackerVersionCheck verifies the configured firecracker
|
||||
// binary exists, is recent enough for banger's expectations
|
||||
// (firecracker.MinSupportedVersion), and surfaces a distro-aware
|
||||
// install hint if it's missing. Three outcomes:
|
||||
//
|
||||
// - present + version in [Min, Tested]: PASS.
|
||||
// - present + version above Tested: WARN. Newer firecracker
|
||||
// usually works (the API is stable within a major), but it's
|
||||
// outside banger's tested window.
|
||||
// - present + version below Min: FAIL with the upgrade hint.
|
||||
// - missing entirely: FAIL with a guess at the user's package
|
||||
// manager plus the upstream Releases URL.
|
||||
//
|
||||
// We intentionally don't use the generic RequireExecutable preflight
|
||||
// for this check — its static hint string can't carry the distro
|
||||
// dispatch.
|
||||
func (d *Daemon) addFirecrackerVersionCheck(ctx context.Context, report *system.Report) {
|
||||
binPath := strings.TrimSpace(d.config.FirecrackerBin)
|
||||
if binPath == "" {
|
||||
binPath = "firecracker"
|
||||
}
|
||||
resolved, err := system.LookupExecutable(binPath)
|
||||
if err != nil {
|
||||
details := []string{fmt.Sprintf("not found: %s", binPath)}
|
||||
details = append(details, firecrackerInstallHint(osReleaseSource)...)
|
||||
report.AddFail("firecracker binary", details...)
|
||||
return
|
||||
}
|
||||
parsed, err := firecracker.QueryVersion(ctx, d.runner, resolved)
|
||||
if err != nil {
|
||||
report.AddFail("firecracker binary",
|
||||
fmt.Sprintf("`%s --version` failed: %v", resolved, err),
|
||||
"reinstall firecracker; see https://github.com/firecracker-microvm/firecracker/releases")
|
||||
return
|
||||
}
|
||||
reported := parsed.String()
|
||||
min := firecracker.MustParseSemVer(firecracker.MinSupportedVersion)
|
||||
tested := firecracker.MustParseSemVer(firecracker.KnownTestedVersion)
|
||||
switch {
|
||||
case parsed.Compare(min) < 0:
|
||||
report.AddFail("firecracker binary",
|
||||
fmt.Sprintf("%s at %s; banger requires ≥ v%s", reported, resolved, firecracker.MinSupportedVersion),
|
||||
"upgrade firecracker — see https://github.com/firecracker-microvm/firecracker/releases")
|
||||
case parsed.Compare(tested) > 0:
|
||||
report.AddWarn("firecracker binary",
|
||||
fmt.Sprintf("%s at %s (newer than banger's tested v%s; usually works)", reported, resolved, firecracker.KnownTestedVersion))
|
||||
default:
|
||||
report.AddPass("firecracker binary",
|
||||
fmt.Sprintf("%s at %s (within tested range; min v%s, tested v%s)",
|
||||
reported, resolved, firecracker.MinSupportedVersion, firecracker.KnownTestedVersion))
|
||||
}
|
||||
}
|
||||
|
||||
// osReleaseSource is the file the install-hint reads to detect the
|
||||
// host distro. Var rather than const so doctor tests can swap in a
|
||||
// fixture.
|
||||
var osReleaseSource = "/etc/os-release"
|
||||
|
||||
// firecrackerInstallHint returns 1-2 detail lines describing how to
|
||||
// install firecracker on the current host: a one-line guess based on
|
||||
// /etc/os-release when the distro is recognised, plus the upstream
|
||||
// Releases URL as a universal fallback. Anything we can't recognise
|
||||
// gets only the URL — better silence than wrong instructions.
|
||||
func firecrackerInstallHint(osReleasePath string) []string {
|
||||
hints := []string{}
|
||||
if cmd := guessFirecrackerInstallCommand(osReleasePath); cmd != "" {
|
||||
hints = append(hints, "install: "+cmd)
|
||||
}
|
||||
hints = append(hints, "or download a static binary from https://github.com/firecracker-microvm/firecracker/releases")
|
||||
return hints
|
||||
}
|
||||
|
||||
// guessFirecrackerInstallCommand reads osReleasePath and returns a
|
||||
// short, copy-pasteable install command for the detected distro, or
|
||||
// "" when no reliable mapping applies. We only suggest commands for
|
||||
// distros where firecracker is actually packaged — guessing wrong
|
||||
// here would send users on a wild goose chase.
|
||||
func guessFirecrackerInstallCommand(osReleasePath string) string {
|
||||
data, err := os.ReadFile(osReleasePath)
|
||||
if err != nil {
|
||||
return ""
|
||||
}
|
||||
id, idLike := parseOSReleaseIDs(string(data))
|
||||
candidates := append([]string{id}, strings.Fields(idLike)...)
|
||||
for _, c := range candidates {
|
||||
switch c {
|
||||
case "debian":
|
||||
// Packaged in Debian since trixie / bookworm-backports.
|
||||
return "sudo apt install firecracker"
|
||||
case "arch", "manjaro", "endeavouros":
|
||||
// AUR; we don't assume a specific helper, but `paru` is the
|
||||
// common one. Users who prefer yay/makepkg/etc. will
|
||||
// substitute mentally.
|
||||
return "paru -S firecracker # or your preferred AUR helper"
|
||||
case "nixos":
|
||||
return "nix-env -iA nixos.firecracker # or add to your configuration.nix"
|
||||
}
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
// parseOSReleaseIDs extracts the ID and ID_LIKE values from an
|
||||
// /etc/os-release blob. Both are returned with surrounding quotes
|
||||
// stripped; missing keys return empty strings. We don't validate
|
||||
// the format beyond `KEY=value` — os-release is a simple format and
|
||||
// any drift would manifest as a quiet "no distro hint" rather than
|
||||
// a false positive.
|
||||
func parseOSReleaseIDs(content string) (id, idLike string) {
|
||||
for _, line := range strings.Split(content, "\n") {
|
||||
line = strings.TrimSpace(line)
|
||||
if rest, ok := strings.CutPrefix(line, "ID="); ok {
|
||||
id = strings.Trim(rest, `"`)
|
||||
}
|
||||
if rest, ok := strings.CutPrefix(line, "ID_LIKE="); ok {
|
||||
idLike = strings.Trim(rest, `"`)
|
||||
}
|
||||
}
|
||||
return id, idLike
|
||||
}
|
||||
|
||||
// addSecurityPostureChecks verifies the install matches what
|
||||
// docs/privileges.md describes: helper + owner-daemon units active,
|
||||
// sockets at the expected mode/owner, unit files carrying the
|
||||
|
|
@ -358,7 +480,10 @@ func (d *Daemon) addVMDefaultsCheck(report *system.Report) {
|
|||
|
||||
func (d *Daemon) runtimeChecks() *system.Preflight {
|
||||
checks := system.NewPreflight()
|
||||
checks.RequireExecutable(d.config.FirecrackerBin, "firecracker binary", `install firecracker or set "firecracker_bin"`)
|
||||
// Firecracker presence + version is a separate top-level check (see
|
||||
// addFirecrackerVersionCheck) so the report can carry a distro-aware
|
||||
// install hint when the binary is missing — RequireExecutable's
|
||||
// static `hint` string can't do that.
|
||||
checks.RequireFile(d.config.SSHKeyPath, "ssh private key", `set "ssh_key_path" or let banger create its default key`)
|
||||
if helper, err := vsockAgentBinary(d.layout); err == nil {
|
||||
checks.RequireExecutable(helper, "vsock agent helper", `run 'make build' or reinstall banger`)
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ import (
|
|||
"strings"
|
||||
"testing"
|
||||
|
||||
"banger/internal/firecracker"
|
||||
"banger/internal/model"
|
||||
"banger/internal/paths"
|
||||
"banger/internal/system"
|
||||
|
|
@ -347,17 +348,166 @@ func TestDoctorReport_StoreSuccessSurfacesAsPass(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestDoctorReport_MissingFirecrackerFailsHostRuntime(t *testing.T) {
|
||||
func TestDoctorReport_MissingFirecrackerFailsFirecrackerBinaryCheck(t *testing.T) {
|
||||
d := buildDoctorDaemon(t)
|
||||
// Point at a nonexistent path. Note: the doctor's PATH lookup
|
||||
// looks for the basename, so use an absolute non-existent path
|
||||
// (that's the configured-path branch — bare-name lookups would
|
||||
// fall through to the test-fixture binDir which DOES contain a
|
||||
// fake `firecracker`).
|
||||
d.config.FirecrackerBin = filepath.Join(t.TempDir(), "does-not-exist")
|
||||
|
||||
report := d.doctorReport(context.Background(), nil, false)
|
||||
check := findCheck(report, "host runtime")
|
||||
check := findCheck(report, "firecracker binary")
|
||||
if check == nil {
|
||||
t.Fatal("host runtime check missing from report")
|
||||
t.Fatal("firecracker binary check missing from report")
|
||||
}
|
||||
if check.Status != system.CheckStatusFail {
|
||||
t.Fatalf("host runtime status = %q, want fail when firecracker binary missing", check.Status)
|
||||
t.Fatalf("firecracker binary status = %q, want fail when binary missing", check.Status)
|
||||
}
|
||||
joined := strings.Join(check.Details, " ")
|
||||
if !strings.Contains(joined, "firecracker-microvm/firecracker/releases") {
|
||||
t.Fatalf("missing-binary report should include the upstream URL; got %q", joined)
|
||||
}
|
||||
}
|
||||
|
||||
// TestFirecrackerInstallHintDispatchesByDistro pins the per-distro
|
||||
// install command guess. Pinned IDs are the ones banger is willing to
|
||||
// suggest a concrete command for; everything else gets only the
|
||||
// upstream URL.
|
||||
func TestFirecrackerInstallHintDispatchesByDistro(t *testing.T) {
|
||||
t.Parallel()
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
release string
|
||||
wantSub string
|
||||
wantNone bool
|
||||
}{
|
||||
{name: "debian", release: "ID=debian\nVERSION_CODENAME=bookworm\n", wantSub: "apt install firecracker"},
|
||||
{name: "ubuntu_id_like_debian", release: "ID=ubuntu\nID_LIKE=debian\n", wantSub: "apt install firecracker"},
|
||||
{name: "arch", release: "ID=arch\n", wantSub: "paru -S firecracker"},
|
||||
{name: "manjaro_via_id_like", release: "ID=manjaro\nID_LIKE=arch\n", wantSub: "paru -S firecracker"},
|
||||
{name: "nixos", release: "ID=nixos\n", wantSub: "nixos.firecracker"},
|
||||
{name: "fedora_falls_back_to_url", release: "ID=fedora\n", wantNone: true},
|
||||
{name: "missing_file", release: "", wantNone: true},
|
||||
} {
|
||||
tc := tc
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
osPath := filepath.Join(t.TempDir(), "os-release")
|
||||
if tc.release != "" {
|
||||
if err := os.WriteFile(osPath, []byte(tc.release), 0o644); err != nil {
|
||||
t.Fatalf("write os-release: %v", err)
|
||||
}
|
||||
}
|
||||
hints := firecrackerInstallHint(osPath)
|
||||
joined := strings.Join(hints, " ")
|
||||
if !strings.Contains(joined, "firecracker-microvm/firecracker/releases") {
|
||||
t.Fatalf("hints missing upstream URL; got %q", joined)
|
||||
}
|
||||
if tc.wantNone {
|
||||
// Distro-specific hint must NOT be present — only the URL.
|
||||
if len(hints) != 1 {
|
||||
t.Fatalf("unrecognised distro got distro-specific hint(s); want only the URL line, got %v", hints)
|
||||
}
|
||||
return
|
||||
}
|
||||
if !strings.Contains(joined, tc.wantSub) {
|
||||
t.Fatalf("hints %q do not contain expected substring %q", joined, tc.wantSub)
|
||||
}
|
||||
if len(hints) < 2 {
|
||||
t.Fatalf("expected distro hint + URL; got only %v", hints)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// firecrackerVersionRunner is a CommandRunner that actually executes
|
||||
// firecracker --version (via system.Runner) but stubs everything else
|
||||
// with the permissive default. The doctor uses d.runner for the
|
||||
// firecracker version query AND for several other checks; this tiny
|
||||
// dispatcher lets us run a real script for one command without
|
||||
// rewiring the rest.
|
||||
type firecrackerVersionRunner struct {
|
||||
real system.Runner
|
||||
canned []byte
|
||||
bin string
|
||||
}
|
||||
|
||||
func (r *firecrackerVersionRunner) Run(ctx context.Context, name string, args ...string) ([]byte, error) {
|
||||
if name == r.bin {
|
||||
return r.real.Run(ctx, name, args...)
|
||||
}
|
||||
return r.canned, nil
|
||||
}
|
||||
|
||||
func (r *firecrackerVersionRunner) RunSudo(_ context.Context, _ ...string) ([]byte, error) {
|
||||
return r.canned, nil
|
||||
}
|
||||
|
||||
// stubFirecrackerVersion replaces the test daemon's firecracker
|
||||
// stub with a script that prints the requested version line, then
|
||||
// swaps d.runner for one that actually executes the script when the
|
||||
// firecracker path is queried. Returns the resulting daemon ready
|
||||
// for doctorReport.
|
||||
func stubFirecrackerVersion(t *testing.T, d *Daemon, version string) {
|
||||
t.Helper()
|
||||
if err := os.WriteFile(d.config.FirecrackerBin, []byte("#!/bin/sh\necho 'Firecracker v"+version+"'\n"), 0o755); err != nil {
|
||||
t.Fatalf("write firecracker stub: %v", err)
|
||||
}
|
||||
d.runner = &firecrackerVersionRunner{
|
||||
real: system.NewRunner(),
|
||||
canned: []byte("default via 10.0.0.1 dev eth0 proto static\n"),
|
||||
bin: d.config.FirecrackerBin,
|
||||
}
|
||||
}
|
||||
|
||||
// TestFirecrackerVersionCheckPasses pins the happy path: when the
|
||||
// configured firecracker reports a tested-range version, doctor
|
||||
// emits a PASS row.
|
||||
func TestFirecrackerVersionCheckPasses(t *testing.T) {
|
||||
d := buildDoctorDaemon(t)
|
||||
stubFirecrackerVersion(t, d, firecracker.KnownTestedVersion)
|
||||
report := d.doctorReport(context.Background(), nil, false)
|
||||
check := findCheck(report, "firecracker binary")
|
||||
if check == nil {
|
||||
t.Fatal("firecracker binary check missing from report")
|
||||
}
|
||||
if check.Status != system.CheckStatusPass {
|
||||
t.Fatalf("status = %q, want pass; details=%v", check.Status, check.Details)
|
||||
}
|
||||
}
|
||||
|
||||
// TestFirecrackerVersionCheckFailsBelowMin pins the too-old path:
|
||||
// a binary reporting a version below MinSupportedVersion must FAIL
|
||||
// with the upgrade hint.
|
||||
func TestFirecrackerVersionCheckFailsBelowMin(t *testing.T) {
|
||||
d := buildDoctorDaemon(t)
|
||||
stubFirecrackerVersion(t, d, "0.25.0")
|
||||
report := d.doctorReport(context.Background(), nil, false)
|
||||
check := findCheck(report, "firecracker binary")
|
||||
if check == nil {
|
||||
t.Fatal("firecracker binary check missing from report")
|
||||
}
|
||||
if check.Status != system.CheckStatusFail {
|
||||
t.Fatalf("status = %q, want fail for below-min version", check.Status)
|
||||
}
|
||||
}
|
||||
|
||||
// TestFirecrackerVersionCheckWarnsAboveTested pins the over-tested
|
||||
// path: a binary reporting a version newer than KnownTestedVersion
|
||||
// must WARN — newer firecracker usually works, but it's outside the
|
||||
// tested window.
|
||||
func TestFirecrackerVersionCheckWarnsAboveTested(t *testing.T) {
|
||||
d := buildDoctorDaemon(t)
|
||||
stubFirecrackerVersion(t, d, "99.0.0")
|
||||
report := d.doctorReport(context.Background(), nil, false)
|
||||
check := findCheck(report, "firecracker binary")
|
||||
if check == nil {
|
||||
t.Fatal("firecracker binary check missing from report")
|
||||
}
|
||||
if check.Status != system.CheckStatusWarn {
|
||||
t.Fatalf("status = %q, want warn for above-tested version", check.Status)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue