Refactor CLI: birdc-style help, collapsed nouns, ReloadConfig, bug fixes

maglevc - Rewrite '?' handler (birdc-style): show full command paths from current position to every leaf, right-aligned help column, dynamic slot values displayed as an indented block when cursor is at a slot position. - Collapse show frontends/frontend, backends/backend, healthchecks/healthcheck into single plural-noun nodes with an optional <name> slot. Allows 'sh ba' (list all) and 'sh ba nginx0' (show one) without ambiguity. - Add 'config reload' command. - Fix tabwriter ANSI alignment: continuation lines in transition output now carry the same label() byte overhead as the header line. - Fix broken Walk for 'set frontend' command: setFrontendPoolName and setWeightValue were fixed-word nodes that couldn't capture user input; mark them as slot nodes with dynNone. - Add tree_test.go covering expandPaths, cycle detection, prefix matching, and the full weight-command walk. gRPC / proto - Add ReloadConfig RPC: checks config then applies it to the running checker, returning ok/parse_error/semantic_error/reload_error. - Add logging to CheckConfig (config-check-start/config-check-done at INFO level). maglevd - SIGHUP handler now calls maglevServer.TriggerReload(), sharing the same code path as the gRPC ReloadConfig RPC. docs - Collapse show command documentation to use [<name>] optional syntax. - Remove developer-facing 'Command tree and parser' section. - Document 'config reload'.
2026-04-11 18:20:43 +02:00
parent 58391f5463
commit 3bd30b69f4
11 changed files with 657 additions and 222 deletions
--- a/cmd/maglevc/commands.go
+++ b/cmd/maglevc/commands.go
@@ -27,58 +27,55 @@ func buildTree() *Node {
 	exit := &Node{Word: "exit", Help: "exit the shell", Run: runQuit}

 	// show version
-	showVersion := &Node{Word: "version", Help: "show build version", Run: runShowVersion}
+	showVersion := &Node{Word: "version", Help: "Show build version", Run: runShowVersion}

-	// show frontends
-	showFrontends := &Node{Word: "frontends", Help: "list all frontends", Run: runShowFrontends}
-	// show frontend <name>
+	// show frontends [<name>]  — without name: list all, with name: show details
 	showFrontendName := &Node{
 		Word:    "<name>",
-		Help:    "frontend name",
+		Help:    "Show details for a single frontend",
 		Dynamic: dynFrontends,
 		Run:     runShowFrontend,
 	}
-	showFrontend := &Node{
-		Word:     "frontend",
-		Help:     "show a single frontend",
+	showFrontends := &Node{
+		Word:     "frontends",
+		Help:     "List all frontends",
+		Run:      runShowFrontends,
 		Children: []*Node{showFrontendName},
 	}

-	// show backends
-	showBackends := &Node{Word: "backends", Help: "list all backends", Run: runShowBackends}
-	// show backend <name>
+	// show backends [<name>]  — without name: list all, with name: show details
 	showBackendName := &Node{
 		Word:    "<name>",
-		Help:    "backend name",
+		Help:    "Show details for a single backend",
 		Dynamic: dynBackends,
 		Run:     runShowBackend,
 	}
-	showBackend := &Node{
-		Word:     "backend",
-		Help:     "show a single backend",
+	showBackends := &Node{
+		Word:     "backends",
+		Help:     "List all backends",
+		Run:      runShowBackends,
 		Children: []*Node{showBackendName},
 	}

-	// show healthchecks
-	showHealthChecks := &Node{Word: "healthchecks", Help: "list all health checks", Run: runShowHealthChecks}
-	// show healthcheck <name>
+	// show healthchecks [<name>]  — without name: list all, with name: show details
 	showHealthCheckName := &Node{
 		Word:    "<name>",
-		Help:    "health check name",
+		Help:    "Show details for a single health check",
 		Dynamic: dynHealthChecks,
 		Run:     runShowHealthCheck,
 	}
-	showHealthCheck := &Node{
-		Word:     "healthcheck",
-		Help:     "show a single health check",
+	showHealthChecks := &Node{
+		Word:     "healthchecks",
+		Help:     "List all health checks",
+		Run:      runShowHealthChecks,
 		Children: []*Node{showHealthCheckName},
 	}

 	show.Children = []*Node{
 		showVersion,
-		showFrontends, showFrontend,
-		showBackends, showBackend,
-		showHealthChecks, showHealthCheck,
+		showFrontends,
+		showBackends,
+		showHealthChecks,
 	}

 	// set backend <name> pause|resume|disabled|enabled
@@ -99,9 +96,10 @@ func buildTree() *Node {
 	}
 	// set frontend <name> pool <pool> backend <name> weight <0-100>
 	setWeightValue := &Node{
-		Word: "<weight>",
-		Help: "weight 0-100",
-		Run:  runSetFrontendPoolBackendWeight,
+		Word:    "<weight>",
+		Help:    "Set weight of a backend in a pool (0-100)",
+		Dynamic: dynNone, // accepts any integer; no tab-completion candidates
+		Run:     runSetFrontendPoolBackendWeight,
 	}
 	setFrontendPoolBackendWeight := &Node{Word: "weight", Help: "set backend weight in pool", Children: []*Node{setWeightValue}}
 	setFrontendPoolBackendName := &Node{
@@ -114,6 +112,7 @@ func buildTree() *Node {
 	setFrontendPoolName := &Node{
 		Word:     "<pool>",
 		Help:     "pool name",
+		Dynamic:  dynNone, // pool names aren't listed via gRPC; accepts any input
 		Children: []*Node{setFrontendPoolBackend},
 	}
 	setFrontendPool := &Node{Word: "pool", Help: "select a pool", Children: []*Node{setFrontendPoolName}}
@@ -139,7 +138,7 @@ func buildTree() *Node {
 	var watchEventsOptSlot *Node
 	watchEventsOptSlot = &Node{
 		Word:    "<opt>",
-		Help:    "watch option",
+		Help:    "Stream events with options",
 		Dynamic: dynWatchEventOpts,
 		Run:     runWatchEvents,
 	}
@@ -157,12 +156,13 @@ func buildTree() *Node {
 		Children: []*Node{watchEvents},
 	}

-	// config check
-	configCheck := &Node{Word: "check", Help: "check configuration file", Run: runConfigCheck}
+	// config check / reload
+	configCheck := &Node{Word: "check", Help: "Check configuration file", Run: runConfigCheck}
+	configReload := &Node{Word: "reload", Help: "Check and reload configuration", Run: runConfigReload}
 	configNode := &Node{
 		Word:     "config",
 		Help:     "configuration commands",
-		Children: []*Node{configCheck},
+		Children: []*Node{configCheck, configReload},
 	}

 	root.Children = []*Node{show, set, watch, configNode, quit, exit}
@@ -195,6 +195,10 @@ func dynHealthChecks(ctx context.Context, client grpcapi.MaglevClient) []string
 	return resp.Names
 }

+// dynNone marks a slot node that accepts any input but provides no
+// tab-completion candidates (e.g. a pool name or numeric weight value).
+func dynNone(_ context.Context, _ grpcapi.MaglevClient) []string { return nil }
+
 // ---- run functions ---------------------------------------------------------

 func runShowVersion(_ context.Context, _ grpcapi.MaglevClient, _ []string) error {
@@ -315,9 +319,14 @@ func runShowBackend(ctx context.Context, client grpcapi.MaglevClient, args []str
 	fmt.Fprintf(w, "%s\t%s\n", label("healthcheck"), info.Healthcheck)
 	for i, t := range info.Transitions {
 		ts := time.Unix(0, t.AtUnixNs)
-		lbl := ""
+		var lbl string
 		if i == 0 {
 			lbl = label("transitions")
+		} else {
+			// Pad to same visible width as "transitions" and wrap through
+			// label() so tabwriter sees the same byte count (ANSI overhead
+			// is identical on every row, keeping columns aligned).
+			lbl = label("           ")
 		}
 		fmt.Fprintf(w, "%s\t%s → %s\t%s\t%s\n",
 			lbl,
@@ -501,6 +510,26 @@ func runConfigCheck(ctx context.Context, client grpcapi.MaglevClient, _ []string
 	return fmt.Errorf("semantic error: %s", resp.SemanticError)
 }

+func runConfigReload(ctx context.Context, client grpcapi.MaglevClient, _ []string) error {
+	ctx, cancel := context.WithTimeout(ctx, callTimeout)
+	defer cancel()
+	resp, err := client.ReloadConfig(ctx, &grpcapi.ReloadConfigRequest{})
+	if err != nil {
+		return err
+	}
+	if resp.Ok {
+		fmt.Println("config reloaded")
+		return nil
+	}
+	if resp.ParseError != "" {
+		return fmt.Errorf("parse error: %s", resp.ParseError)
+	}
+	if resp.SemanticError != "" {
+		return fmt.Errorf("semantic error: %s", resp.SemanticError)
+	}
+	return fmt.Errorf("reload error: %s", resp.ReloadError)
+}
+
 // formatDuration formats a duration as Xd Xh Xm Xs without milliseconds.
 func formatDuration(d time.Duration) string {
 	if d < 0 {
--- a/cmd/maglevc/complete.go
+++ b/cmd/maglevc/complete.go
@@ -67,15 +67,16 @@ func (ql *questionListener) OnChange(line []rune, pos int, key rune) (newLine []
 		return line, pos, false
 	}

-	// line[:pos] includes the '?' just typed — strip it before tokenizing.
+	// Strip the '?' that was just appended to line[:pos].
 	before := string(line[:pos])
 	if len(before) > 0 && before[len(before)-1] == '?' {
 		before = before[:len(before)-1]
 	}
 	tokens := splitTokens(before)

-	var partial string
+	// Split into confirmed prefix tokens and the partial token being typed.
 	var prefix []string
+	var partial string
 	if len(before) == 0 || before[len(before)-1] == ' ' {
 		prefix = tokens
 		partial = ""
@@ -84,25 +85,65 @@ func (ql *questionListener) OnChange(line []rune, pos int, key rune) (newLine []
 		partial = tokens[len(tokens)-1]
 	}

+	// Walk the confirmed prefix to the current node, then try to advance one
+	// more step using the partial token (via prefix-match or slot fallback).
+	// This mirrors birdc: "sh?" expands "sh" to "show" and shows show's subtree.
+	node, _ := Walk(ql.root, prefix)
+	displayPrefix := strings.Join(prefix, " ")
+	if partial != "" {
+		if next := matchFixedChild(node.Children, partial); next != nil {
+			// Partial uniquely matched a fixed child — descend into it.
+			node = next
+			displayPrefix = strings.Join(tokens, " ")
+		} else if slot := findSlotChild(node.Children); slot != nil {
+			// Partial is filling a slot node.
+			node = slot
+			displayPrefix = strings.Join(tokens, " ")
+		}
+		// If partial matched nothing (ambiguous or dead end), stay at the
+		// current node and show its subcommands with the confirmed prefix.
+	}
+
+	// Expand all leaf paths reachable from the current node.
+	lines := expandPaths(node, displayPrefix, make(map[*Node]bool))
+
+	// If the cursor is at a position where the next input is a dynamic slot,
+	// fetch live values now and show them below the syntax lines.
 	ctx, cancel := context.WithTimeout(context.Background(), completeTimeout)
 	defer cancel()
+	var dynValues []string
+	var dynWord string
+	if slot := findSlotChild(node.Children); slot != nil && slot.Dynamic != nil {
+		dynValues = slot.Dynamic(ctx, ql.client)
+		dynWord = slot.Word
+	}

-	candidates := Candidates(ql.root, prefix, partial, ctx, ql.client)
-
-	fmt.Fprintf(ql.rl.Stderr(), "\r\n")
-	if len(candidates) == 0 {
-		fmt.Fprintf(ql.rl.Stderr(), "  <no completions>\r\n")
-	} else {
-		for _, c := range candidates {
-			if c.Help != "" {
-				fmt.Fprintf(ql.rl.Stderr(), "  %-20s  %s\r\n", c.Word, c.Help)
-			} else {
-				fmt.Fprintf(ql.rl.Stderr(), "  %s\r\n", c.Word)
-			}
+	// Right-align the help column at the width of the longest path + 2.
+	maxLen := 0
+	for _, l := range lines {
+		if len(l.path) > maxLen {
+			maxLen = len(l.path)
 		}
 	}

-	// Remove the '?' from the line and return with cursor one step back.
+	// Emit output. Raw terminal mode requires \r\n.
+	fmt.Fprintf(ql.rl.Stderr(), "\r\n")
+	if len(lines) == 0 {
+		fmt.Fprintf(ql.rl.Stderr(), "  <no completions>\r\n")
+	} else {
+		for _, l := range lines {
+			if l.help != "" {
+				fmt.Fprintf(ql.rl.Stderr(), "%-*s  %s\r\n", maxLen+2, l.path, l.help)
+			} else {
+				fmt.Fprintf(ql.rl.Stderr(), "%s\r\n", l.path)
+			}
+		}
+		if len(dynValues) > 0 {
+			fmt.Fprintf(ql.rl.Stderr(), "  %s:  %s\r\n", dynWord, strings.Join(dynValues, "  "))
+		}
+	}
+
+	// Remove the '?' from the line and step cursor back one position.
 	newLine = append(append([]rune{}, line[:pos-1]...), line[pos:]...)
 	return newLine, pos - 1, true
 }
--- a/cmd/maglevc/shell.go
+++ b/cmd/maglevc/shell.go
@@ -7,6 +7,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"strings"

 	"github.com/chzyer/readline"

@@ -75,20 +76,28 @@ func dispatch(ctx context.Context, root *Node, client grpcapi.MaglevClient, toke
 	return node.Run(ctx, client, args)
 }

-// showHelp prints available completions for the given token path.
+// showHelp prints all reachable commands from the given token path, birdc-style.
 func showHelp(root *Node, tokens []string) {
 	node, _ := Walk(root, tokens)
+	prefix := strings.Join(tokens, " ")
+	lines := expandPaths(node, prefix, make(map[*Node]bool))

-	candidates := filterFixedChildren(node.Children, "")
-	if len(candidates) == 0 {
+	maxLen := 0
+	for _, l := range lines {
+		if len(l.path) > maxLen {
+			maxLen = len(l.path)
+		}
+	}
+
+	if len(lines) == 0 {
 		fmt.Println("  <no completions>")
 		return
 	}
-	for _, c := range candidates {
-		if c.Help != "" {
-			fmt.Printf("  %-20s  %s\n", c.Word, c.Help)
+	for _, l := range lines {
+		if l.help != "" {
+			fmt.Printf("%-*s  %s\n", maxLen+2, l.path, l.help)
 		} else {
-			fmt.Printf("  %s\n", c.Word)
+			fmt.Printf("%s\n", l.path)
 		}
 	}
 }
--- a/cmd/maglevc/tree.go
+++ b/cmd/maglevc/tree.go
@@ -91,6 +91,36 @@ func findSlotChild(children []*Node) *Node {
 	return nil
 }

+// helpLine is a (path, help) pair used when displaying '?' output.
+type helpLine struct {
+	path string
+	help string
+}
+
+// expandPaths returns all (path, help) pairs for every node reachable from
+// node that has a Run function. prefix is the display string accumulated so
+// far (e.g. "show frontend"). visited prevents infinite loops through
+// self-referencing slot nodes like watchEventsOptSlot.
+func expandPaths(node *Node, prefix string, visited map[*Node]bool) []helpLine {
+	if visited[node] {
+		return nil
+	}
+	visited[node] = true
+
+	var lines []helpLine
+	if node.Run != nil {
+		lines = append(lines, helpLine{path: prefix, help: node.Help})
+	}
+	for _, child := range node.Children {
+		childPrefix := child.Word
+		if prefix != "" {
+			childPrefix = prefix + " " + child.Word
+		}
+		lines = append(lines, expandPaths(child, childPrefix, visited)...)
+	}
+	return lines
+}
+
 // Candidates returns the completable children at the current position given
 // the already-typed tokens and the partial token being completed.
 func Candidates(root *Node, tokens []string, partial string, ctx context.Context, client grpcapi.MaglevClient) []*Node {
--- a/cmd/maglevc/tree_test.go
+++ b/cmd/maglevc/tree_test.go
@@ -0,0 +1,153 @@
+// Copyright (c) 2026, Pim van Pelt <pim@ipng.ch>
+
+package main
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestExpandPathsRoot(t *testing.T) {
+	root := buildTree()
+	lines := expandPaths(root, "", make(map[*Node]bool))
+
+	// Should include well-known leaf paths.
+	want := []string{
+		"show version",
+		"show frontends",
+		"show frontends <name>",
+		"show backends",
+		"show backends <name>",
+		"show healthchecks",
+		"show healthchecks <name>",
+		"set backend <name> pause",
+		"set backend <name> resume",
+		"set backend <name> disable",
+		"set backend <name> enable",
+		"set frontend <name> pool <pool> backend <backend> weight <weight>",
+		"watch events",
+		"watch events <opt>",
+		"config check",
+		"config reload",
+		"quit",
+		"exit",
+	}
+
+	paths := make(map[string]bool, len(lines))
+	for _, l := range lines {
+		paths[l.path] = true
+	}
+
+	for _, w := range want {
+		if !paths[w] {
+			t.Errorf("expandPaths(root) missing %q", w)
+		}
+	}
+}
+
+func TestExpandPathsShow(t *testing.T) {
+	root := buildTree()
+	showNode, _ := Walk(root, []string{"show"})
+	lines := expandPaths(showNode, "show", make(map[*Node]bool))
+
+	for _, l := range lines {
+		if !strings.HasPrefix(l.path, "show ") {
+			t.Errorf("unexpected path %q: should start with 'show '", l.path)
+		}
+		if l.help == "" {
+			t.Errorf("path %q has empty help", l.path)
+		}
+	}
+	// version, frontends, frontends <name>, backends, backends <name>,
+	// healthchecks, healthchecks <name> = 7 lines
+	if len(lines) != 7 {
+		t.Errorf("expected exactly 7 show subcommands, got %d", len(lines))
+	}
+}
+
+func TestExpandPathsNoCycles(t *testing.T) {
+	root := buildTree()
+	// watch events has a self-referencing slot; expandPaths must terminate.
+	watchEvents, _ := Walk(root, []string{"watch", "events"})
+	lines := expandPaths(watchEvents, "watch events", make(map[*Node]bool))
+
+	// Should produce exactly 2 lines: "watch events" and "watch events <opt>".
+	if len(lines) != 2 {
+		t.Errorf("watch events: expected 2 lines, got %d: %v", len(lines), lines)
+	}
+}
+
+func TestExpandPathsSetBackendName(t *testing.T) {
+	root := buildTree()
+	// Walk to the name slot so displayPrefix carries the actual arg.
+	node, _ := Walk(root, []string{"set", "backend", "mybackend"})
+	lines := expandPaths(node, "set backend mybackend", make(map[*Node]bool))
+
+	want := []string{
+		"set backend mybackend pause",
+		"set backend mybackend resume",
+		"set backend mybackend disable",
+		"set backend mybackend enable",
+	}
+	if len(lines) != len(want) {
+		t.Fatalf("expected %d lines, got %d: %v", len(want), len(lines), lines)
+	}
+	for i, w := range want {
+		if lines[i].path != w {
+			t.Errorf("line %d: got %q, want %q", i, lines[i].path, w)
+		}
+	}
+}
+
+func TestPrefixMatchCollapsedNouns(t *testing.T) {
+	root := buildTree()
+
+	// "sh ba" → show backends (list all) via prefix matching.
+	node, args := Walk(root, []string{"sh", "ba"})
+	if node.Run == nil {
+		t.Fatal("'sh ba' did not reach a Run node")
+	}
+	if len(args) != 0 {
+		t.Errorf("'sh ba' should have 0 args, got %v", args)
+	}
+
+	// "sh ba nginx0" → show backends <name> (get specific) via slot.
+	node, args = Walk(root, []string{"sh", "ba", "nginx0"})
+	if node.Run == nil {
+		t.Fatal("'sh ba nginx0' did not reach a Run node")
+	}
+	if len(args) != 1 || args[0] != "nginx0" {
+		t.Errorf("'sh ba nginx0' args: got %v, want [nginx0]", args)
+	}
+
+	// "sh fr" → show frontends (list all).
+	node, _ = Walk(root, []string{"sh", "fr"})
+	if node.Run == nil {
+		t.Fatal("'sh fr' did not reach a Run node")
+	}
+
+	// "sh he icmp" → show healthchecks icmp (get specific).
+	node, args = Walk(root, []string{"sh", "he", "icmp"})
+	if node.Run == nil {
+		t.Fatal("'sh he icmp' did not reach a Run node")
+	}
+	if len(args) != 1 || args[0] != "icmp" {
+		t.Errorf("'sh he icmp' args: got %v, want [icmp]", args)
+	}
+}
+
+func TestExpandPathsWeightSlotWalk(t *testing.T) {
+	// Verify the weight command is fully walkable (fixes bug: setWeightValue
+	// and setFrontendPoolName were non-slot nodes that couldn't capture tokens).
+	root := buildTree()
+	node, args := Walk(root, []string{"set", "frontend", "web", "pool", "primary", "backend", "be0", "weight", "42"})
+	if node.Run == nil {
+		t.Fatal("Walk did not reach a Run node for full weight command")
+	}
+	if len(args) != 4 {
+		t.Errorf("expected 4 args (name, pool, backend, weight), got %d: %v", len(args), args)
+	}
+	if args[3] != "42" {
+		t.Errorf("args[3] (weight): got %q, want 42", args[3])
+	}
+}
--- a/cmd/maglevd/main.go
+++ b/cmd/maglevd/main.go
@@ -93,7 +93,8 @@ func run() error {
 		return fmt.Errorf("listen %s: %w", *grpcAddr, err)
 	}
 	srv := grpc.NewServer()
-	grpcapi.RegisterMaglevServer(srv, grpcapi.NewServer(ctx, chkr, logBroadcaster, *configPath))
+	maglevServer := grpcapi.NewServer(ctx, chkr, logBroadcaster, *configPath)
+	grpcapi.RegisterMaglevServer(srv, maglevServer)
 	if *enableReflection {
 		reflection.Register(srv)
 	}
@@ -112,21 +113,7 @@ func run() error {
 	for sig := range sigCh {
 		switch sig {
 		case syscall.SIGHUP:
-			slog.Info("config-reload-start")
-			newCfg, result := config.Check(*configPath)
-			if !result.OK() {
-				if result.ParseError != "" {
-					slog.Error("config-check-failed", "type", "parse", "err", result.ParseError)
-				} else {
-					slog.Error("config-check-failed", "type", "semantic", "err", result.SemanticError)
-				}
-				continue
-			}
-			if err := chkr.Reload(ctx, newCfg); err != nil {
-				slog.Error("checker-reload-error", "err", err)
-				continue
-			}
-			slog.Info("config-reload-done", "frontends", len(newCfg.Frontends))
+			maglevServer.TriggerReload()

 		case syscall.SIGTERM, syscall.SIGINT:
 			slog.Info("shutdown", "signal", sig)