From 221fb03d68f1773524c04aeab49614bee8dfa368 Mon Sep 17 00:00:00 2001 From: Thales Maciel Date: Sun, 19 Apr 2026 12:17:46 -0300 Subject: [PATCH] =?UTF-8?q?cli=20QoL:=20vm=20prune,=20list=E2=86=92ls=20al?= =?UTF-8?q?iases,=20delete=E2=86=92rm=20aliases?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `banger vm prune` sweeps every non-running VM (stopped, created, error) with an interactive confirmation; -f/--force skips the prompt. Partial failures report which VM failed and exit non-zero. - list commands gain `ls` alias: vm list already had it; added to image list, kernel list, and vm session list. - delete commands gain `rm` alias: vm delete and image delete. kernel rm already aliased delete/remove. Uses new test seams (vmListFunc) plus the existing vmDeleteFunc so prune unit-tests without touching the daemon socket. --- docs/advanced.md | 7 ++ internal/cli/aliases_test.go | 103 ++++++++++++++++++ internal/cli/banger.go | 124 +++++++++++++++++++-- internal/cli/prune_test.go | 204 +++++++++++++++++++++++++++++++++++ 4 files changed, 430 insertions(+), 8 deletions(-) create mode 100644 internal/cli/aliases_test.go create mode 100644 internal/cli/prune_test.go diff --git a/docs/advanced.md b/docs/advanced.md index d416b77..8863739 100644 --- a/docs/advanced.md +++ b/docs/advanced.md @@ -17,6 +17,13 @@ banger vm stop testbox banger vm delete testbox ``` +Sweep every non-running VM (stopped, created, error) with: + +```bash +banger vm prune # interactive confirmation +banger vm prune -f # skip the prompt +``` + `vm create` is synchronous by default, but on a TTY it shows live progress until the VM is fully ready. diff --git a/internal/cli/aliases_test.go b/internal/cli/aliases_test.go new file mode 100644 index 0000000..12853e4 --- /dev/null +++ b/internal/cli/aliases_test.go @@ -0,0 +1,103 @@ +package cli + +import ( + "testing" + + "github.com/spf13/cobra" +) + +// findSubcommand walks cmd's subtree along path and returns the +// matching command, or nil. +func findSubcommand(root *cobra.Command, path ...string) *cobra.Command { + cur := root + for _, name := range path { + var next *cobra.Command + for _, sub := range cur.Commands() { + if sub.Name() == name { + next = sub + break + } + } + if next == nil { + return nil + } + cur = next + } + return cur +} + +func assertHasAlias(t *testing.T, cmd *cobra.Command, alias string) { + t.Helper() + if cmd == nil { + t.Fatal("command is nil") + } + for _, a := range cmd.Aliases { + if a == alias { + return + } + } + t.Errorf("%q missing alias %q; have %v", cmd.Name(), alias, cmd.Aliases) +} + +func TestListCommandsHaveLsAlias(t *testing.T) { + root := NewBangerCommand() + + cases := [][]string{ + {"vm", "list"}, + {"image", "list"}, + {"kernel", "list"}, + {"vm", "session", "list"}, + } + for _, path := range cases { + t.Run(path[len(path)-1], func(t *testing.T) { + cmd := findSubcommand(root, path...) + if cmd == nil { + t.Fatalf("missing command: %v", path) + } + assertHasAlias(t, cmd, "ls") + }) + } +} + +func TestDeleteCommandsHaveRmAlias(t *testing.T) { + root := NewBangerCommand() + + cases := [][]string{ + {"vm", "delete"}, + {"image", "delete"}, + } + for _, path := range cases { + t.Run(path[len(path)-1], func(t *testing.T) { + cmd := findSubcommand(root, path...) + if cmd == nil { + t.Fatalf("missing command: %v", path) + } + assertHasAlias(t, cmd, "rm") + }) + } +} + +func TestVMCommandRegistersPrune(t *testing.T) { + root := NewBangerCommand() + cmd := findSubcommand(root, "vm", "prune") + if cmd == nil { + t.Fatal("vm prune not registered") + } + if flag := cmd.Flags().Lookup("force"); flag == nil { + t.Error("vm prune missing --force flag") + } + if flag := cmd.Flags().ShorthandLookup("f"); flag == nil { + t.Error("vm prune missing -f shorthand") + } +} + +func TestKernelRmHasDeleteAlias(t *testing.T) { + // This already existed prior to this feature — guard against regressions. + root := NewBangerCommand() + cmd := findSubcommand(root, "kernel", "rm") + if cmd == nil { + t.Fatal("kernel rm missing") + } + assertHasAlias(t, cmd, "delete") + assertHasAlias(t, cmd, "remove") +} diff --git a/internal/cli/banger.go b/internal/cli/banger.go index 02de1f9..9b794db 100644 --- a/internal/cli/banger.go +++ b/internal/cli/banger.go @@ -2,6 +2,7 @@ package cli import ( "archive/tar" + "bufio" "bytes" "context" "crypto/sha256" @@ -80,6 +81,9 @@ var ( _, err := rpc.Call[api.VMShowResult](ctx, socketPath, "vm.delete", api.VMRefParams{IDOrName: idOrName}) return err } + vmListFunc = func(ctx context.Context, socketPath string) (api.VMListResult, error) { + return rpc.Call[api.VMListResult](ctx, socketPath, "vm.list", api.Empty{}) + } daemonPingFunc = func(ctx context.Context, socketPath string) (api.PingResult, error) { return rpc.Call[api.PingResult](ctx, socketPath, "ping", api.Empty{}) } @@ -732,7 +736,8 @@ func newVMCommand() *cobra.Command { newVMActionCommand("stop", "Stop a VM", "vm.stop"), newVMKillCommand(), newVMActionCommand("restart", "Restart a VM", "vm.restart"), - newVMActionCommand("delete", "Delete a VM", "vm.delete"), + newVMActionCommand("delete", "Delete a VM", "vm.delete", "rm"), + newVMPruneCommand(), newVMSetCommand(), newVMSSHCommand(), newVMWorkspaceCommand(), @@ -894,6 +899,104 @@ func newVMKillCommand() *cobra.Command { return cmd } +func newVMPruneCommand() *cobra.Command { + var force bool + cmd := &cobra.Command{ + Use: "prune", + Short: "Delete every VM that isn't running", + Long: "Scan for VMs in state other than 'running' (stopped, created, error) and delete them after confirmation. Use -f to skip the prompt.", + Args: noArgsUsage("usage: banger vm prune [-f|--force]"), + RunE: func(cmd *cobra.Command, args []string) error { + if err := system.EnsureSudo(cmd.Context()); err != nil { + return err + } + layout, _, err := ensureDaemon(cmd.Context()) + if err != nil { + return err + } + return runVMPrune(cmd, layout.SocketPath, force) + }, + } + cmd.Flags().BoolVarP(&force, "force", "f", false, "skip the confirmation prompt") + return cmd +} + +func runVMPrune(cmd *cobra.Command, socketPath string, force bool) error { + ctx := cmd.Context() + stdout := cmd.OutOrStdout() + stderr := cmd.ErrOrStderr() + + list, err := vmListFunc(ctx, socketPath) + if err != nil { + return err + } + var victims []model.VMRecord + for _, vm := range list.VMs { + if vm.State != model.VMStateRunning { + victims = append(victims, vm) + } + } + if len(victims) == 0 { + _, err := fmt.Fprintln(stdout, "no non-running VMs to prune") + return err + } + + fmt.Fprintf(stdout, "The following %d VM(s) will be deleted:\n", len(victims)) + w := tabwriter.NewWriter(stdout, 0, 0, 2, ' ', 0) + fmt.Fprintln(w, " ID\tNAME\tSTATE") + for _, vm := range victims { + fmt.Fprintf(w, " %s\t%s\t%s\n", shortID(vm.ID), vm.Name, vm.State) + } + if err := w.Flush(); err != nil { + return err + } + + if !force { + ok, err := promptYesNo(cmd.InOrStdin(), stdout, "Delete these VMs? [y/N] ") + if err != nil { + return err + } + if !ok { + _, err := fmt.Fprintln(stdout, "aborted") + return err + } + } + + var failed int + for _, vm := range victims { + ref := vm.Name + if ref == "" { + ref = shortID(vm.ID) + } + if err := vmDeleteFunc(ctx, socketPath, vm.ID); err != nil { + fmt.Fprintf(stderr, "delete %s: %v\n", ref, err) + failed++ + continue + } + fmt.Fprintln(stdout, "deleted", ref) + } + if failed > 0 { + return fmt.Errorf("%d VM(s) failed to delete", failed) + } + return nil +} + +// promptYesNo reads a line from in and returns true iff the trimmed +// lowercase answer is "y" or "yes". EOF is treated as "no". Any other +// read error is surfaced to the caller. +func promptYesNo(in io.Reader, out io.Writer, prompt string) (bool, error) { + if _, err := fmt.Fprint(out, prompt); err != nil { + return false, err + } + reader := bufio.NewReader(in) + line, err := reader.ReadString('\n') + if err != nil && err != io.EOF { + return false, err + } + answer := strings.ToLower(strings.TrimSpace(line)) + return answer == "y" || answer == "yes", nil +} + func newVMCreateCommand() *cobra.Command { var ( name string @@ -1035,9 +1138,10 @@ func newVMShowCommand() *cobra.Command { } } -func newVMActionCommand(use, short, method string) *cobra.Command { +func newVMActionCommand(use, short, method string, aliases ...string) *cobra.Command { return &cobra.Command{ Use: use + " ...", + Aliases: aliases, Short: short, Args: minArgsUsage(1, fmt.Sprintf("usage: banger vm %s ...", use)), ValidArgsFunction: completeVMNames, @@ -1351,6 +1455,7 @@ func newVMSessionStartCommand() *cobra.Command { func newVMSessionListCommand() *cobra.Command { return &cobra.Command{ Use: "list ", + Aliases: []string{"ls"}, Short: "List managed guest commands for a VM", Args: exactArgsUsage(1, "usage: banger vm session list "), ValidArgsFunction: completeVMNameOnlyAtPos0, @@ -1862,9 +1967,10 @@ func newImagePromoteCommand() *cobra.Command { func newImageListCommand() *cobra.Command { return &cobra.Command{ - Use: "list", - Short: "List images", - Args: noArgsUsage("usage: banger image list"), + Use: "list", + Aliases: []string{"ls"}, + Short: "List images", + Args: noArgsUsage("usage: banger image list"), RunE: func(cmd *cobra.Command, args []string) error { layout, _, err := ensureDaemon(cmd.Context()) if err != nil { @@ -1902,6 +2008,7 @@ func newImageShowCommand() *cobra.Command { func newImageDeleteCommand() *cobra.Command { return &cobra.Command{ Use: "delete ", + Aliases: []string{"rm"}, Short: "Delete an image", Args: exactArgsUsage(1, "usage: banger image delete "), ValidArgsFunction: completeImageNameOnlyAtPos0, @@ -2002,9 +2109,10 @@ func newKernelImportCommand() *cobra.Command { func newKernelListCommand() *cobra.Command { var available bool cmd := &cobra.Command{ - Use: "list", - Short: "List kernels (local by default, or --available for the catalog)", - Args: noArgsUsage("usage: banger kernel list [--available]"), + Use: "list", + Aliases: []string{"ls"}, + Short: "List kernels (local by default, or --available for the catalog)", + Args: noArgsUsage("usage: banger kernel list [--available]"), RunE: func(cmd *cobra.Command, args []string) error { layout, _, err := ensureDaemon(cmd.Context()) if err != nil { diff --git a/internal/cli/prune_test.go b/internal/cli/prune_test.go new file mode 100644 index 0000000..32372cb --- /dev/null +++ b/internal/cli/prune_test.go @@ -0,0 +1,204 @@ +package cli + +import ( + "bytes" + "context" + "errors" + "fmt" + "strings" + "testing" + + "banger/internal/api" + "banger/internal/model" + + "github.com/spf13/cobra" +) + +// stubPruneSeams installs fakes for vmListFunc and vmDeleteFunc, and +// restores originals on cleanup. +func stubPruneSeams(t *testing.T, vms []model.VMRecord, listErr error, deleteErr map[string]error) *[]string { + t.Helper() + origList := vmListFunc + origDelete := vmDeleteFunc + t.Cleanup(func() { + vmListFunc = origList + vmDeleteFunc = origDelete + }) + + var deleted []string + vmListFunc = func(ctx context.Context, socketPath string) (api.VMListResult, error) { + return api.VMListResult{VMs: vms}, listErr + } + vmDeleteFunc = func(ctx context.Context, socketPath, idOrName string) error { + if err, ok := deleteErr[idOrName]; ok { + return err + } + deleted = append(deleted, idOrName) + return nil + } + return &deleted +} + +func newPruneTestCmd(stdin string) (*cobra.Command, *bytes.Buffer, *bytes.Buffer) { + cmd := &cobra.Command{Use: "prune"} + cmd.SetContext(context.Background()) + stdout := &bytes.Buffer{} + stderr := &bytes.Buffer{} + cmd.SetIn(strings.NewReader(stdin)) + cmd.SetOut(stdout) + cmd.SetErr(stderr) + return cmd, stdout, stderr +} + +func TestPromptYesNo(t *testing.T) { + cases := map[string]bool{ + "y\n": true, + "Y\n": true, + "yes\n": true, + "YES\n": true, + " y \n": true, + "n\n": false, + "no\n": false, + "\n": false, + "anything\n": false, + } + for input, want := range cases { + out := &bytes.Buffer{} + got, err := promptYesNo(strings.NewReader(input), out, "go? ") + if err != nil { + t.Errorf("input %q: error %v", input, err) + continue + } + if got != want { + t.Errorf("input %q: got %v, want %v", input, got, want) + } + if !strings.Contains(out.String(), "go?") { + t.Errorf("input %q: prompt not written; got %q", input, out.String()) + } + } +} + +func TestPromptYesNoEOF(t *testing.T) { + got, err := promptYesNo(strings.NewReader(""), &bytes.Buffer{}, "? ") + if err != nil { + t.Fatalf("EOF should not error: %v", err) + } + if got { + t.Fatal("EOF should be treated as no") + } +} + +func TestRunVMPruneNoVictims(t *testing.T) { + stubPruneSeams(t, []model.VMRecord{ + {ID: "id-1", Name: "running-vm", State: model.VMStateRunning}, + }, nil, nil) + + cmd, stdout, _ := newPruneTestCmd("") + if err := runVMPrune(cmd, "sock", false); err != nil { + t.Fatalf("runVMPrune: %v", err) + } + if !strings.Contains(stdout.String(), "no non-running VMs") { + t.Errorf("expected no-op message, got %q", stdout.String()) + } +} + +func TestRunVMPruneAbortedByUser(t *testing.T) { + deleted := stubPruneSeams(t, []model.VMRecord{ + {ID: "id-1", Name: "stale", State: model.VMStateStopped}, + }, nil, nil) + + cmd, stdout, _ := newPruneTestCmd("n\n") + if err := runVMPrune(cmd, "sock", false); err != nil { + t.Fatalf("runVMPrune: %v", err) + } + if !strings.Contains(stdout.String(), "aborted") { + t.Errorf("expected 'aborted' output, got %q", stdout.String()) + } + if len(*deleted) != 0 { + t.Errorf("should not have deleted anything, got %v", *deleted) + } +} + +func TestRunVMPruneConfirmedDeletesNonRunning(t *testing.T) { + deleted := stubPruneSeams(t, []model.VMRecord{ + {ID: "id-run", Name: "keeper", State: model.VMStateRunning}, + {ID: "id-stop", Name: "stale", State: model.VMStateStopped}, + {ID: "id-err", Name: "broken", State: model.VMStateError}, + {ID: "id-created", Name: "fresh", State: model.VMStateCreated}, + }, nil, nil) + + cmd, stdout, _ := newPruneTestCmd("y\n") + if err := runVMPrune(cmd, "sock", false); err != nil { + t.Fatalf("runVMPrune: %v", err) + } + // Deleted must be exactly the three non-running IDs, in list order. + want := []string{"id-stop", "id-err", "id-created"} + if len(*deleted) != len(want) { + t.Fatalf("deleted = %v, want %v", *deleted, want) + } + for i, id := range want { + if (*deleted)[i] != id { + t.Errorf("deleted[%d] = %q, want %q", i, (*deleted)[i], id) + } + } + for _, want := range []string{"stale", "broken", "fresh"} { + if !strings.Contains(stdout.String(), "deleted "+want) { + t.Errorf("output missing 'deleted %s':\n%s", want, stdout.String()) + } + } + if strings.Contains(stdout.String(), "deleted keeper") { + t.Errorf("running VM should not be deleted:\n%s", stdout.String()) + } +} + +func TestRunVMPruneForceSkipsPrompt(t *testing.T) { + deleted := stubPruneSeams(t, []model.VMRecord{ + {ID: "id-1", Name: "stale", State: model.VMStateStopped}, + }, nil, nil) + + // Empty stdin + force=true: must not block on prompt. + cmd, stdout, _ := newPruneTestCmd("") + if err := runVMPrune(cmd, "sock", true); err != nil { + t.Fatalf("runVMPrune: %v", err) + } + if len(*deleted) != 1 || (*deleted)[0] != "id-1" { + t.Errorf("deleted = %v, want [id-1]", *deleted) + } + // Prompt should not appear in output. + if strings.Contains(stdout.String(), "Delete these VMs?") { + t.Errorf("force=true should skip prompt:\n%s", stdout.String()) + } +} + +func TestRunVMPruneReportsPartialFailure(t *testing.T) { + stubPruneSeams(t, + []model.VMRecord{ + {ID: "id-a", Name: "a", State: model.VMStateStopped}, + {ID: "id-b", Name: "b", State: model.VMStateStopped}, + }, + nil, + map[string]error{"id-a": errors.New("simulated")}, + ) + + cmd, _, stderr := newPruneTestCmd("") + err := runVMPrune(cmd, "sock", true) + if err == nil { + t.Fatal("expected non-zero exit when any delete fails") + } + if !strings.Contains(err.Error(), "1 VM(s) failed") { + t.Errorf("unexpected error: %v", err) + } + if !strings.Contains(stderr.String(), "delete a:") { + t.Errorf("stderr missing failure log: %q", stderr.String()) + } +} + +func TestRunVMPruneListErrorPropagates(t *testing.T) { + stubPruneSeams(t, nil, fmt.Errorf("rpc failed"), nil) + + cmd, _, _ := newPruneTestCmd("") + err := runVMPrune(cmd, "sock", true) + if err == nil || !strings.Contains(err.Error(), "rpc failed") { + t.Fatalf("expected rpc error to propagate, got %v", err) + } +}