fix(cli): stop --print-effective-config from starting a container
argparse requires exactly one action, so the documented diagnostic form `--print-effective-config --config … --start` never took the early-return branch in main.run(): it printed the resolved config and then executed a real do_start(), silently replacing a running container with the [default] model. README, installation guide and manual all recommended that form. Make it an action in the mutually exclusive group. It can no longer be combined with --start/--check/--stop/--change/--chat (argparse error, exit 2), and it skips the docker_available() check, so it now really is the offline config check the docs promise. --dry-run remains a modifier and still requires a reachable daemon. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
218c3fc791
commit
3e3dd1c150
8 changed files with 104 additions and 30 deletions
11
README.md
11
README.md
|
|
@ -44,9 +44,10 @@ Voraussetzungen auf dem Zielsystem:
|
|||
- Docker (CLI + laufender Daemon), für `--start`/--check`/--stop`/--change`
|
||||
- Netzwerkzugriff auf den Container-Host-Port für `--chat`/--check`
|
||||
|
||||
`--print-effective-config` und `--dry-run` benötigen keinen laufenden Docker-Daemon
|
||||
für die reine Konfigurationsprüfung; `--start`/--check`/--stop`/--change` erfordern
|
||||
einen erreichbaren Docker-Daemon.
|
||||
`--print-effective-config` ist eine eigenständige, nebenwirkungsfreie Aktion und
|
||||
benötigt keinen laufenden Docker-Daemon. `--start`/--check`/--stop`/--change`
|
||||
erfordern einen erreichbaren Docker-Daemon — auch zusammen mit `--dry-run`, weil
|
||||
die Verfügbarkeit geprüft wird, bevor der Trockenlauf greift.
|
||||
|
||||
## Konfiguration
|
||||
|
||||
|
|
@ -93,8 +94,8 @@ llamacppctl --start --config llama.cpp.config
|
|||
# Nur den geplanten docker-run-Befehl anzeigen, nichts ausführen
|
||||
llamacppctl --start --config llama.cpp.config --dry-run
|
||||
|
||||
# Effektive Konfiguration als JSON ausgeben
|
||||
llamacppctl --print-effective-config --config llama.cpp.config --start
|
||||
# Effektive Konfiguration als JSON ausgeben (startet nichts, braucht kein Docker)
|
||||
llamacppctl --print-effective-config --config llama.cpp.config
|
||||
|
||||
# Status prüfen
|
||||
llamacppctl --check --config llama.cpp.config
|
||||
|
|
|
|||
|
|
@ -49,7 +49,7 @@ Siehe [`INSTALL_FROM_ARCHIVE.md`](INSTALL_FROM_ARCHIVE.md).
|
|||
|
||||
```bash
|
||||
llamacppctl --help
|
||||
llamacppctl --print-effective-config --config llama.cpp.config --start --dry-run
|
||||
llamacppctl --print-effective-config --config llama.cpp.config
|
||||
```
|
||||
|
||||
---
|
||||
|
|
|
|||
|
|
@ -64,7 +64,7 @@ cp llama.cpp.config.example llama.cpp.config
|
|||
Auflösung prüfen, ohne etwas zu starten:
|
||||
|
||||
```bash
|
||||
llamacppctl --print-effective-config --config llama.cpp.config --start --dry-run
|
||||
llamacppctl --print-effective-config --config llama.cpp.config
|
||||
```
|
||||
|
||||
Das zeigt die vollständig aufgelöste Konfiguration als JSON **und** das
|
||||
|
|
|
|||
|
|
@ -258,22 +258,29 @@ keinen vermeidbaren Ausfall verursacht.
|
|||
|
||||
## 6. `--dry-run` und `--print-effective-config` als Sicherheitswerkzeuge
|
||||
|
||||
- `--print-effective-config` gibt die vollständig aufgelöste Konfiguration
|
||||
(Server- und Prompt-Konfiguration) als JSON aus, **bevor** irgendeine
|
||||
Docker- oder HTTP-Aktion ausgeführt wird. Damit lässt sich prüfen, welche
|
||||
Werte aus welcher Quelle (Defaults/[default]/[model.*]/CLI) tatsächlich
|
||||
gewonnen haben, ohne einen Container anzufassen.
|
||||
- `--print-effective-config` ist eine **eigene Aktion** in der
|
||||
Mutually-Exclusive-Gruppe: sie gibt die vollständig aufgelöste Konfiguration
|
||||
(Server- und Prompt-Konfiguration) als JSON aus und beendet sich. Damit lässt
|
||||
sich prüfen, welche Werte aus welcher Quelle (Defaults/[default]/[model.*]/CLI)
|
||||
tatsächlich gewonnen haben, ohne einen Container anzufassen.
|
||||
|
||||
Bis einschließlich 0.1.0 war dies ein *Flag*. Da argparse immer eine Aktion
|
||||
verlangt, führte `--print-effective-config --start` die Konfigurationsausgabe
|
||||
**und anschließend einen echten `do_start()`** aus — der einen laufenden
|
||||
Container stillschweigend ersetzte. Die Kombination ist jetzt ein
|
||||
argparse-Fehler (Exit-Code 2).
|
||||
- `--dry-run` (in Kombination mit `--start`/--change`) zeigt den vollständig
|
||||
zusammengesetzten `docker run`-Befehl (Shell-quotiert zur Anzeige) an,
|
||||
**ohne** ihn auszuführen. Empfohlen vor jeder Änderung an einer
|
||||
Produktionskonfiguration, insbesondere nach Anpassungen an
|
||||
`llama.cpp.config`.
|
||||
|
||||
Beide Flags erfordern **keinen** erreichbaren Docker-Daemon für ihre reine
|
||||
Ausgabe — `main.run()` prüft `docker_available()` derzeit vor der
|
||||
Konfigurationsauflösung; auf einem Host ganz ohne Docker (z. B. zur reinen
|
||||
Konfigurationsvalidierung) schlägt der Aufruf entsprechend mit einer
|
||||
expliziten Fehlermeldung fehl statt still falsche Annahmen zu treffen.
|
||||
`--print-effective-config` erfordert **keinen** erreichbaren Docker-Daemon;
|
||||
`main.run()` überspringt die `docker_available()`-Prüfung für diese Aktion. Für
|
||||
`--dry-run` gilt das **nicht**: es ist ein Modifikator von `--start`/`--change`,
|
||||
und die Docker-Prüfung läuft vor der Konfigurationsauflösung. Auf einem Host ganz
|
||||
ohne Docker schlägt `--start --dry-run` daher mit einer expliziten Fehlermeldung
|
||||
fehl, statt still falsche Annahmen zu treffen.
|
||||
|
||||
## 7. Betriebsmodell auf einem dedizierten GPU-Host
|
||||
|
||||
|
|
|
|||
|
|
@ -20,6 +20,14 @@ def build_parser() -> argparse.ArgumentParser:
|
|||
action.add_argument("--stop", action="store_true", help="Stop/remove the container")
|
||||
action.add_argument("--change", action="store_true", help="Stop, reconfigure, and restart")
|
||||
action.add_argument("--chat", action="store_true", help="Send a prompt to a running server")
|
||||
# A diagnostic action, not a modifier: combining it with --start used to print
|
||||
# the config and then really start the container. It is mutually exclusive with
|
||||
# the other actions so it can never have a side effect.
|
||||
action.add_argument(
|
||||
"--print-effective-config",
|
||||
action="store_true",
|
||||
help="Print the fully merged configuration as JSON and exit. Touches nothing",
|
||||
)
|
||||
|
||||
p.add_argument("--config", default="llama.cpp.config", help="Path to llama.cpp.config")
|
||||
p.add_argument("--profile", help="Model profile name: [model.<profile>] in config")
|
||||
|
|
@ -146,11 +154,6 @@ def build_parser() -> argparse.ArgumentParser:
|
|||
p.add_argument("--log-lines", type=int, default=100)
|
||||
p.add_argument("--dry-run", action="store_true", help="Show the docker run command, do not execute it")
|
||||
p.add_argument("--force", action="store_true", help="Force stop/change despite inconsistencies")
|
||||
p.add_argument(
|
||||
"--print-effective-config",
|
||||
action="store_true",
|
||||
help="Print the fully merged configuration as JSON and exit (unless combined with an action)",
|
||||
)
|
||||
|
||||
return p
|
||||
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@
|
|||
Orchestration order:
|
||||
1. Parse CLI arguments (cli.build_parser)
|
||||
2. Run semantic validation (cli.validate_args)
|
||||
3. Check that docker is available
|
||||
3. Check that docker is available (skipped for --print-effective-config)
|
||||
4. Resolve prompt sources (prompt_io) under an InputPolicy built from CLI flags
|
||||
5. Resolve effective server/prompt config (config.resolve_effective_config)
|
||||
6. Dispatch to the requested action (actions.do_*)
|
||||
|
|
@ -55,7 +55,9 @@ def run(argv: Optional[Sequence[str]] = None) -> int:
|
|||
args = parser.parse_args(argv)
|
||||
validate_args(args, parser)
|
||||
|
||||
if not docker_available():
|
||||
# --print-effective-config is purely diagnostic: it must work on a host
|
||||
# without a Docker daemon, and it must never touch a container.
|
||||
if not args.print_effective_config and not docker_available():
|
||||
print("docker is not available on PATH (or the daemon is not reachable)", file=sys.stderr)
|
||||
return 1
|
||||
|
||||
|
|
@ -74,9 +76,6 @@ def run(argv: Optional[Sequence[str]] = None) -> int:
|
|||
|
||||
if args.print_effective_config:
|
||||
actions.print_effective_config(server_cfg, prompt_cfg)
|
||||
# An action is always required by argparse; only exit early if the run
|
||||
# is purely diagnostic (no action would actually do anything).
|
||||
if not (args.start or args.check or args.stop or args.change or args.chat):
|
||||
return 0
|
||||
|
||||
if args.start:
|
||||
|
|
|
|||
|
|
@ -147,9 +147,16 @@ def test_dry_run_flag():
|
|||
assert args.dry_run is True
|
||||
|
||||
|
||||
def test_print_effective_config_flag():
|
||||
args = parse(["--start", "--print-effective-config"])
|
||||
def test_print_effective_config_is_a_standalone_action():
|
||||
args = parse(["--print-effective-config"])
|
||||
assert args.print_effective_config is True
|
||||
assert args.start is False
|
||||
|
||||
|
||||
def test_print_effective_config_cannot_be_combined_with_start():
|
||||
# Regression: as a plain flag this printed the config and then really started
|
||||
# the container, silently replacing a running one.
|
||||
parse_error(["--print-effective-config", "--start"])
|
||||
|
||||
|
||||
def test_reasoning_choice_validated():
|
||||
|
|
|
|||
57
tests/test_main.py
Normal file
57
tests/test_main.py
Normal file
|
|
@ -0,0 +1,57 @@
|
|||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "src"))
|
||||
|
||||
from llamacppctl import actions, main as main_mod # noqa: E402
|
||||
|
||||
CONFIG = """
|
||||
[default]
|
||||
hf_home = /srv/models
|
||||
model_path = qwen3/default.gguf
|
||||
container_name = test_main
|
||||
"""
|
||||
|
||||
|
||||
def _config(tmp_path: Path) -> Path:
|
||||
path = tmp_path / "llama.cpp.config"
|
||||
path.write_text(CONFIG, encoding="utf-8")
|
||||
return path
|
||||
|
||||
|
||||
def test_print_effective_config_has_no_side_effects(tmp_path, monkeypatch, capsys):
|
||||
"""Regression: --print-effective-config used to fall through into do_start()
|
||||
because argparse always required an action, silently replacing a running
|
||||
container."""
|
||||
started = []
|
||||
monkeypatch.setattr(actions, "do_start", lambda *a, **k: started.append(1))
|
||||
monkeypatch.setattr(actions, "do_change", lambda *a, **k: started.append(1))
|
||||
monkeypatch.setattr(actions, "do_stop", lambda *a, **k: started.append(1))
|
||||
|
||||
rc = main_mod.run(["--print-effective-config", "--config", str(_config(tmp_path))])
|
||||
|
||||
assert rc == 0
|
||||
assert started == []
|
||||
assert '"container_name": "test_main"' in capsys.readouterr().out
|
||||
|
||||
|
||||
def test_print_effective_config_works_without_docker(tmp_path, monkeypatch, capsys):
|
||||
"""It is a pure config check, so it must not require a reachable daemon."""
|
||||
def boom() -> bool:
|
||||
raise AssertionError("docker_available() must not be consulted")
|
||||
|
||||
monkeypatch.setattr(main_mod, "docker_available", boom)
|
||||
|
||||
rc = main_mod.run(["--print-effective-config", "--config", str(_config(tmp_path))])
|
||||
|
||||
assert rc == 0
|
||||
assert '"hf_home": "/srv/models"' in capsys.readouterr().out
|
||||
|
||||
|
||||
def test_start_still_requires_docker(tmp_path, monkeypatch, capsys):
|
||||
monkeypatch.setattr(main_mod, "docker_available", lambda: False)
|
||||
|
||||
rc = main_mod.run(["--start", "--config", str(_config(tmp_path))])
|
||||
|
||||
assert rc == 1
|
||||
assert "docker is not available" in capsys.readouterr().err
|
||||
Loading…
Add table
Add a link
Reference in a new issue