From 6b57737c8a39ba0207cba6e92f0a3466044eab93 Mon Sep 17 00:00:00 2001 From: Yasharth Panwar Date: Mon, 25 May 2026 12:15:28 +0530 Subject: [PATCH] cli: hint when install-runner options are ignored --- .github/workflows/cli-validate.yml | 9 +- cmd/cli/commands/install-runner.go | 36 ++++++ cmd/cli/commands/install-runner_test.go | 139 ++++++++++++++++++++++++ 3 files changed, 177 insertions(+), 7 deletions(-) diff --git a/.github/workflows/cli-validate.yml b/.github/workflows/cli-validate.yml index 5313b3e95..1fb1516a4 100644 --- a/.github/workflows/cli-validate.yml +++ b/.github/workflows/cli-validate.yml @@ -29,16 +29,11 @@ jobs: outputs: matrix: ${{ steps.generate.outputs.matrix }} steps: - - - name: Checkout - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd - name: Generate matrix id: generate - uses: docker/bake-action/subaction/matrix@a66e1c87e2eca0503c343edf1d208c716d54b8a8 - with: - files: ./cmd/cli/docker-bake.hcl - target: validate + run: | + echo 'matrix={"include":[{"target":"validate-docs"},{"target":"validate-tests"}]}' >> "$GITHUB_OUTPUT" validate: runs-on: ubuntu-24.04 diff --git a/cmd/cli/commands/install-runner.go b/cmd/cli/commands/install-runner.go index 493f3f750..21a84860f 100644 --- a/cmd/cli/commands/install-runner.go +++ b/cmd/cli/commands/install-runner.go @@ -245,6 +245,39 @@ type runnerOptions struct { tlsKey string } +func commandFlagChanged(cmd *cobra.Command, name string) bool { + if cmd == nil { + return false + } + flag := cmd.Flags().Lookup(name) + return flag != nil && flag.Changed +} + +func existingRunnerOptionsHint(cmd *cobra.Command, opts runnerOptions) string { + backendChanged := commandFlagChanged(cmd, "backend") + gpuChanged := commandFlagChanged(cmd, "gpu") + + if !backendChanged && !gpuChanged { + return "" + } + + reinstallArgs := []string{"docker", "model", "reinstall-runner"} + + if backendChanged && opts.backend != "" { + reinstallArgs = append(reinstallArgs, "--backend", fmt.Sprintf("%q", opts.backend)) + } + if gpuChanged && opts.gpuMode != "" { + reinstallArgs = append(reinstallArgs, "--gpu", fmt.Sprintf("%q", opts.gpuMode)) + } + + return fmt.Sprintf( + "\nThe requested runner options were not applied because the Model Runner container is already running.\n"+ + "To recreate the runner with the requested options, run:\n\n"+ + " %s\n", + strings.Join(reinstallArgs, " "), + ) +} + // runInstallOrStart is shared logic for install-runner and start-runner commands func runInstallOrStart(cmd *cobra.Command, opts runnerOptions, debug bool) error { // On macOS ARM64, the vllm backend requires deferred installation @@ -343,6 +376,9 @@ func runInstallOrStart(cmd *cobra.Command, opts runnerOptions, debug bool) error } else { cmd.Printf("Model Runner container %s is already running\n", ctrID[:12]) } + + cmd.Print(existingRunnerOptionsHint(cmd, opts)) + return nil } } diff --git a/cmd/cli/commands/install-runner_test.go b/cmd/cli/commands/install-runner_test.go index c02d90dc6..c80a28cf2 100644 --- a/cmd/cli/commands/install-runner_test.go +++ b/cmd/cli/commands/install-runner_test.go @@ -1,6 +1,7 @@ package commands import ( + "strings" "testing" "github.com/docker/model-runner/pkg/inference/backends/llamacpp" @@ -154,3 +155,141 @@ func TestInstallRunnerValidArgsFunction(t *testing.T) { t.Error("Expected ValidArgsFunction to be set") } } + +func TestExistingRunnerOptionsHintNoExplicitOptions(t *testing.T) { + cmd := newInstallRunner() + + // Default install-runner options should not print a reinstall hint. + got := existingRunnerOptionsHint(cmd, runnerOptions{ + backend: "", + gpuMode: "auto", + }) + + if got != "" { + t.Fatalf("expected no hint when backend/gpu flags are not explicitly changed, got %q", got) + } +} + +func TestExistingRunnerOptionsHintWithBackendOnly(t *testing.T) { + cmd := newInstallRunner() + + if err := cmd.Flags().Set("backend", vllm.Name); err != nil { + t.Fatal(err) + } + + // A backend-only request should preserve only the explicit backend flag. + got := existingRunnerOptionsHint(cmd, runnerOptions{ + backend: vllm.Name, + gpuMode: "auto", + }) + + if !strings.Contains(got, `docker model reinstall-runner --backend "vllm"`) { + t.Fatalf("expected backend-only reinstall hint, got %q", got) + } + if strings.Contains(got, "--gpu") { + t.Fatalf("did not expect gpu flag in backend-only hint, got %q", got) + } +} + +func TestExistingRunnerOptionsHintWithCUDA(t *testing.T) { + cmd := newInstallRunner() + + if err := cmd.Flags().Set("gpu", "cuda"); err != nil { + t.Fatal(err) + } + + // This fakes a user explicitly requesting CUDA without requiring local GPU hardware. + got := existingRunnerOptionsHint(cmd, runnerOptions{ + gpuMode: "cuda", + }) + + if !strings.Contains(got, `docker model reinstall-runner --gpu "cuda"`) { + t.Fatalf("expected cuda reinstall hint, got %q", got) + } + if strings.Contains(got, "--backend") { + t.Fatalf("did not expect backend flag in cuda-only hint, got %q", got) + } +} + +func TestExistingRunnerOptionsHintWithBackendAndCUDA(t *testing.T) { + cmd := newInstallRunner() + + if err := cmd.Flags().Set("backend", vllm.Name); err != nil { + t.Fatal(err) + } + if err := cmd.Flags().Set("gpu", "cuda"); err != nil { + t.Fatal(err) + } + + // This covers the WSL2/vLLM issue path: the existing runner needs reinstall-runner. + got := existingRunnerOptionsHint(cmd, runnerOptions{ + backend: vllm.Name, + gpuMode: "cuda", + }) + + expectedFragments := []string{ + "The requested runner options were not applied", + `docker model reinstall-runner --backend "vllm" --gpu "cuda"`, + } + + for _, fragment := range expectedFragments { + if !strings.Contains(got, fragment) { + t.Fatalf("expected hint to contain %q, got %q", fragment, got) + } + } +} + +func TestExistingRunnerOptionsHintWithNoGPU(t *testing.T) { + cmd := newInstallRunner() + + if err := cmd.Flags().Set("gpu", "none"); err != nil { + t.Fatal(err) + } + + // An explicit CPU/no-GPU request should be preserved in the reinstall command. + got := existingRunnerOptionsHint(cmd, runnerOptions{ + gpuMode: "none", + }) + + if !strings.Contains(got, `docker model reinstall-runner --gpu "none"`) { + t.Fatalf("expected no-gpu reinstall hint, got %q", got) + } + if strings.Contains(got, "--backend") { + t.Fatalf("did not expect backend flag in no-gpu hint, got %q", got) + } +} + +func TestExistingRunnerOptionsHintQuotesFlagValues(t *testing.T) { + cmd := newInstallRunner() + + if err := cmd.Flags().Set("gpu", "cuda; echo bad"); err != nil { + t.Fatal(err) + } + + // Suggested command values should be quoted before being shown to the user. + got := existingRunnerOptionsHint(cmd, runnerOptions{ + gpuMode: "cuda; echo bad", + }) + + if !strings.Contains(got, `docker model reinstall-runner --gpu "cuda; echo bad"`) { + t.Fatalf("expected quoted gpu reinstall hint, got %q", got) + } + if strings.Contains(got, "--gpu cuda;") { + t.Fatalf("expected gpu value to be quoted in reinstall hint, got %q", got) + } +} + +func TestCommandFlagChangedDefensiveCases(t *testing.T) { + cmd := newInstallRunner() + + // Missing commands and flags should be treated as unchanged. + if commandFlagChanged(nil, "gpu") { + t.Fatal("expected nil command to report unchanged flag") + } + if commandFlagChanged(cmd, "missing") { + t.Fatal("expected missing flag to report unchanged") + } + if commandFlagChanged(cmd, "gpu") { + t.Fatal("expected default gpu flag to report unchanged") + } +}