From 775525b59270a69c29f1c96f3e15e3600cd73cd3 Mon Sep 17 00:00:00 2001 From: Thales Maciel Date: Tue, 28 Apr 2026 17:53:32 -0300 Subject: [PATCH] cli,doctor: --version flag + CLI/install drift check MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two pre-release polish items on the version-display surface. * --version on both binaries: cobra's Version field on the banger and bangerd roots renders a one-line summary (banger v0.1.0 (commit abcd1234, built 2026-04-28T20:45:50Z)). The SetVersionTemplate override drops cobra's "{{.Name}} version" prefix — our string is already a complete sentence. The multi-line `banger version` subcommand is unchanged for callers that want the full SHA / built_at on separate lines. * Doctor "banger version" row: prints the running CLI's version + short commit + built-at, plus what /etc/banger/install.toml recorded at install time. Disagreement is the most common version-skew pitfall (stale CLI against fresh daemon, or vice versa) and a one-line warn is friendlier than tracking that down from a launch failure. Drift detection is suppressed when either side is dev/unknown (untagged build) — comparing a dev CLI against a tagged install is the developer-machine case, not a real problem. formatVersionLine is in internal/cli (banger.go) and reused by bangerd.go via a strings.Replace because bangerd's version line should say "bangerd" not "banger". Slightly tilt-feeling but cheaper than parameterising the helper for one caller. Tests: TestVersionsDriftToleratesDevAndUnknown pins the four branches (match, version diff, commit diff, dev-suppression). The existing version-format test already runs through formatVersionLine indirectly. Live exercise: $ banger --version banger dev (commit 1c1ca7d6, built 2026-04-28T20:52:33Z) $ bangerd --version bangerd dev (commit 1c1ca7d6, built 2026-04-28T20:52:33Z) $ banger doctor | head ... PASS banger version - CLI dev (commit 1c1ca7d6, built 2026-04-28T20:52:33Z) Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/cli/banger.go | 22 +++++++++++-- internal/cli/bangerd.go | 4 +++ internal/daemon/doctor.go | 58 ++++++++++++++++++++++++++++++++++ internal/daemon/doctor_test.go | 39 +++++++++++++++++++++++ 4 files changed, 121 insertions(+), 2 deletions(-) diff --git a/internal/cli/banger.go b/internal/cli/banger.go index ee0d716..b1f5a48 100644 --- a/internal/cli/banger.go +++ b/internal/cli/banger.go @@ -21,8 +21,9 @@ func NewBangerCommand() *cobra.Command { func (d *deps) newRootCommand() *cobra.Command { root := &cobra.Command{ - Use: "banger", - Short: "Run development sandboxes as Firecracker microVMs", + Use: "banger", + Version: formatVersionLine(buildinfo.Current()), + Short: "Run development sandboxes as Firecracker microVMs", Long: strings.TrimSpace(` banger runs disposable development sandboxes as Firecracker microVMs. Each sandbox boots in a few seconds, gets its own root filesystem and @@ -50,6 +51,9 @@ to diagnose host readiness problems. SilenceErrors: true, RunE: helpNoArgs, } + // Drop cobra's default "{{.Name}} version {{.Version}}" wrapper — + // our Version string is already a complete sentence. + root.SetVersionTemplate("{{.Version}}\n") root.AddCommand( d.newDaemonCommand(), d.newDoctorCommand(), @@ -186,3 +190,17 @@ func absolutizePaths(values ...*string) error { func formatBuildInfoBlock(info buildinfo.Info) string { return fmt.Sprintf("version: %s\ncommit: %s\nbuilt_at: %s\n", info.Version, info.Commit, info.BuiltAt) } + +// formatVersionLine renders a buildinfo.Info as a single line — +// "banger v0.1.0 (commit abcd1234, built 2026-04-28T20:45:50Z)" — for +// the `--version` flag. Long commit strings are truncated to the +// first 8 hex chars so the line stays scannable. The verbose +// multi-line form lives on `banger version` for callers that want +// the full SHA / built_at on separate lines. +func formatVersionLine(info buildinfo.Info) string { + commit := info.Commit + if len(commit) > 8 { + commit = commit[:8] + } + return fmt.Sprintf("banger %s (commit %s, built %s)", info.Version, commit, info.BuiltAt) +} diff --git a/internal/cli/bangerd.go b/internal/cli/bangerd.go index 6911ce0..23c98d4 100644 --- a/internal/cli/bangerd.go +++ b/internal/cli/bangerd.go @@ -2,7 +2,9 @@ package cli import ( "errors" + "strings" + "banger/internal/buildinfo" "banger/internal/daemon" "banger/internal/roothelper" @@ -14,6 +16,7 @@ func NewBangerdCommand() *cobra.Command { var rootHelperMode bool cmd := &cobra.Command{ Use: "bangerd", + Version: strings.Replace(formatVersionLine(buildinfo.Current()), "banger ", "bangerd ", 1), Short: "Run the banger daemon", SilenceUsage: true, SilenceErrors: true, @@ -44,6 +47,7 @@ func NewBangerdCommand() *cobra.Command { } cmd.Flags().BoolVar(&systemMode, "system", false, "run as the owner-user system service") cmd.Flags().BoolVar(&rootHelperMode, "root-helper", false, "run as the privileged root helper service") + cmd.SetVersionTemplate("{{.Version}}\n") cmd.CompletionOptions.DisableDefaultCmd = true return cmd } diff --git a/internal/daemon/doctor.go b/internal/daemon/doctor.go index bb6dfcf..1f563d6 100644 --- a/internal/daemon/doctor.go +++ b/internal/daemon/doctor.go @@ -9,6 +9,9 @@ import ( "strings" "syscall" + "time" + + "banger/internal/buildinfo" "banger/internal/config" "banger/internal/firecracker" "banger/internal/imagecat" @@ -72,6 +75,7 @@ func (d *Daemon) doctorReport(ctx context.Context, storeErr error, storeMissing report := system.Report{} addArchitectureCheck(&report) + addBangerVersionCheck(&report, installmeta.DefaultPath) switch { case storeMissing: @@ -439,6 +443,60 @@ func (d *Daemon) addSSHShortcutCheck(report *system.Report) { ) } +// addBangerVersionCheck reports the running CLI's version + commit +// alongside whatever's recorded in /etc/banger/install.toml. When +// the installed copy and the running binary disagree on version or +// commit, doctor warns: a stale `banger` running against a freshly- +// installed daemon (or vice versa) is the most common version-skew +// pitfall, and a one-line warning is friendlier than tracking down +// which side is wrong from a launch failure. +// +// Drift detection is suppressed when EITHER side is "dev"/"unknown" +// (untagged build) — those don't have a real version to compare. +func addBangerVersionCheck(report *system.Report, installPath string) { + cli := buildinfo.Current() + cliLine := fmt.Sprintf("CLI %s (commit %s, built %s)", cli.Version, shortCommit(cli.Commit), cli.BuiltAt) + + meta, err := installmeta.Load(installPath) + if err != nil { + // Non-system mode (no install.toml). Just report what we have. + report.AddPass("banger version", cliLine) + return + } + installLine := fmt.Sprintf("install %s (commit %s, installed %s)", meta.Version, shortCommit(meta.Commit), meta.InstalledAt.Format(time.RFC3339)) + if versionsDrift(cli, meta) { + report.AddWarn("banger version", + cliLine, + installLine, + "CLI and installed banger disagree; run `sudo banger system install` to refresh, or run the matching CLI binary") + return + } + report.AddPass("banger version", cliLine, installLine+" (matches CLI)") +} + +func versionsDrift(cli buildinfo.Info, meta installmeta.Metadata) bool { + // Treat dev/unknown as "no real version on this side" — comparing + // a dev build against a tagged install is the local-development + // case, not a drift problem worth surfacing. + if cli.Version == "dev" || strings.TrimSpace(meta.Version) == "" { + return false + } + if cli.Version != meta.Version { + return true + } + if cli.Commit != "unknown" && strings.TrimSpace(meta.Commit) != "" && cli.Commit != meta.Commit { + return true + } + return false +} + +func shortCommit(c string) string { + if len(c) > 8 { + return c[:8] + } + return c +} + // addArchitectureCheck surfaces a hard-fail when banger is running on // a non-amd64 host. Companion binaries are pinned to amd64 in the // Makefile, the published kernel catalog ships only x86_64 images, and diff --git a/internal/daemon/doctor_test.go b/internal/daemon/doctor_test.go index 58b379e..fdc9c6d 100644 --- a/internal/daemon/doctor_test.go +++ b/internal/daemon/doctor_test.go @@ -8,7 +8,9 @@ import ( "strings" "testing" + "banger/internal/buildinfo" "banger/internal/firecracker" + "banger/internal/installmeta" "banger/internal/model" "banger/internal/paths" "banger/internal/system" @@ -371,6 +373,43 @@ func TestDoctorReport_MissingFirecrackerFailsFirecrackerBinaryCheck(t *testing.T } } +// TestVersionsDriftToleratesDevAndUnknown pins the suppression +// branches: a "dev"/"unknown" build on either side is the local- +// development case, not a drift problem; we don't want every +// developer-machine doctor run to emit a noisy warn. +func TestVersionsDriftToleratesDevAndUnknown(t *testing.T) { + t.Parallel() + cliReleased := buildinfo.Info{Version: "0.1.0", Commit: "abcd1234efgh", BuiltAt: "2026-04-28"} + metaReleased := installmeta.Metadata{Version: "0.1.0", Commit: "abcd1234efgh"} + + // Match → no drift. + if versionsDrift(cliReleased, metaReleased) { + t.Fatal("identical CLI and install metadata reported as drifted") + } + // Real version mismatch → drift. + bumped := metaReleased + bumped.Version = "0.2.0" + if !versionsDrift(cliReleased, bumped) { + t.Fatal("differing version not flagged as drift") + } + // Same version, different commit → drift (rebuilt without retag). + differCommit := metaReleased + differCommit.Commit = "deadbeefdead" + if !versionsDrift(cliReleased, differCommit) { + t.Fatal("differing commit at same version not flagged as drift") + } + // "dev" CLI vs released install → suppressed. + devCLI := buildinfo.Info{Version: "dev", Commit: "f00fb00b", BuiltAt: "now"} + if versionsDrift(devCLI, metaReleased) { + t.Fatal("dev CLI vs released install incorrectly flagged as drift") + } + // Empty install version → suppressed (predates the field). + emptyMeta := installmeta.Metadata{} + if versionsDrift(cliReleased, emptyMeta) { + t.Fatal("empty install metadata incorrectly flagged as drift") + } +} + // 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