From cc097bddc2534e938f62d60b545df2d5bc5d6a74 Mon Sep 17 00:00:00 2001 From: Biowilko Date: Wed, 19 Aug 2026 12:35:21 +0100 Subject: [PATCH 1/2] Add run-id tag to CLI messages for clarity --- pyproject.toml | 2 +- squarepeg/cli.py | 24 +++++++++++-- squarepeg/ui.py | 30 ++++++++++++++++- tests/conftest.py | 14 ++++++++ tests/test_cli.py | 86 +++++++++++++++++++++++++++++++++++++++++++++++ tests/test_ui.py | 55 +++++++++++++++++++++++++++++- 6 files changed, 206 insertions(+), 5 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index d9b2495..35b4823 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "squarepeg" -version = "0.2.1" +version = "0.3.0" description = "Run a docker-run-style command as a Kubernetes Pod or Job, streaming its output like a local process" readme = "README.md" requires-python = ">=3.11" diff --git a/squarepeg/cli.py b/squarepeg/cli.py index 8614696..5425dea 100644 --- a/squarepeg/cli.py +++ b/squarepeg/cli.py @@ -1,3 +1,5 @@ +import secrets + import click import yaml @@ -17,6 +19,7 @@ from squarepeg.k8s.dryrun import server_dry_run from squarepeg.k8s.runner import run_manifest from squarepeg.k8s.session import Session +from squarepeg.labels import RUN_ID_LABEL from squarepeg.log import chatter from squarepeg.manifest import build_job, build_pod from squarepeg.naming import generate_name, validate_rfc1123 @@ -217,6 +220,7 @@ def run( ): """Run IMAGE [COMMAND...] as a Kubernetes Pod or Job.""" ui.set_color_override(False if no_color else None) + ui.set_run_tag(None) check_supported_image(image) if rm_flag and keep: raise UsageError("--rm and --keep are mutually exclusive") @@ -302,6 +306,9 @@ def run( else: name = generate_name(image) + run_id = secrets.token_hex(4) + ui.set_run_tag(run_id) + if namespace is not None: claims.add("/metadata/namespace") else: @@ -393,9 +400,21 @@ def run( kubernetes_passthrough = resolved_config.get("kubernetes") job_passthrough = resolved_config.get("job") if spec.mode == "job": - manifest = build_job(spec, kubernetes_passthrough, job_passthrough) + manifest = build_job(spec, kubernetes_passthrough, job_passthrough, run_id=run_id) else: - manifest = build_pod(spec, kubernetes_passthrough) + manifest = build_pod(spec, kubernetes_passthrough, run_id=run_id) + + # kubernetes.metadata passthrough can legally overwrite the run-id label (it's not a + # claimed field) -- if it did, the tag on screen must match what's actually on the + # cluster, not the value we generated, so the printed tag is always kubectl -l ready. + effective_run_id = manifest.get("metadata", {}).get("labels", {}).get(RUN_ID_LABEL) + if effective_run_id and effective_run_id != run_id: + chatter( + f"config passthrough overrode the {RUN_ID_LABEL!r} label " + f"(was {run_id!r}, now {effective_run_id!r})", + level="warn", + ) + ui.set_run_tag(effective_run_id) if dry_run_server: session = Session(namespace=spec.namespace, context=context, quiet=spec.quiet) @@ -414,6 +433,7 @@ def run( def _print_config(config_paths, no_default_config, profile): + ui.set_run_tag(None) # config has no run context, even if invoked right after a 'run' in-process resolved_config, sources = _resolve_config(config_paths, no_default_config, profile) for path, origin in sources: chatter(f"{path} ({origin})", level="info") diff --git a/squarepeg/ui.py b/squarepeg/ui.py index 74189e8..7e993a7 100644 --- a/squarepeg/ui.py +++ b/squarepeg/ui.py @@ -10,6 +10,12 @@ Colour/animation precedence (highest first): --quiet (nothing at all) > --no-color (forces colour off) > $NO_COLOR (forces colour off) > $FORCE_COLOR (forces colour on) > autodetect from sys.stderr.isatty() (click's own default behaviour). + +When a run is in progress, the prefix also carries that run's id -- `[squarepeg:1a2b3c4] +...` -- so that several `squarepeg run` invocations whose stderr gets interleaved (parallel +shell jobs, a CI matrix, an aggregated log) stay attributable line by line. The tag lives +strictly in the prefix span, never inside the message body, so it can never break a test +asserting a message body is a contiguous substring of the output. """ import itertools @@ -42,6 +48,8 @@ _color_override: bool | None = None # None = no override, resolved from env/TTY instead +_run_tag: str | None = None # None = no run associated with current output (config, pre-run errors) + def set_color_override(value: bool | None) -> None: """Called once from cli.run() for --no-color. None restores auto-detection.""" @@ -49,6 +57,24 @@ def set_color_override(value: bool | None) -> None: _color_override = value +def set_run_tag(tag: str | None) -> None: + """Set the run id shown in every subsequent status line's prefix. None (the default, + and what cli.py's `config` command resets it to) means no run is associated with the + current output, producing the legacy plain `[squarepeg] ` prefix. + + Written exactly once per real run, on the main thread, before any thread that reads it + (the log-streaming thread, a Status spinner thread) is created -- threading.Thread.start() + establishes a happens-before edge, so no lock is needed for readers. The signal handler in + _InterruptHandler also only reads this value, which is what makes it signal-handler-safe + to read (acquiring a lock there would not be).""" + global _run_tag + _run_tag = tag + + +def current_run_tag() -> str | None: + return _run_tag + + def color_enabled() -> bool | None: """Tri-state, passed straight through to click.echo(color=...): True/False force colour on/off, None lets click autodetect from sys.stderr.isatty().""" @@ -79,7 +105,9 @@ def _spinner_frames() -> list[str]: def _styled_line(message: str, level: Level) -> str: - prefix = click.style("[squarepeg] ", dim=True) + tag = current_run_tag() + label = f"[squarepeg:{tag}] " if tag else "[squarepeg] " + prefix = click.style(label, dim=True) body = click.style(message, **_STYLES[level]) return prefix + body diff --git a/tests/conftest.py b/tests/conftest.py index 4498ead..3b04422 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,9 +5,23 @@ import pytest +from squarepeg import ui + INTEGRATION_ENV_VAR = "SQUAREPEG_INTEGRATION" +@pytest.fixture(autouse=True) +def _reset_ui_globals(): + """squarepeg.ui's colour override and run-tag are module-level globals, set once per + real invocation -- reset them around every test regardless of module/order, so a tag + set (or a colour override changed) in one test can never leak into another.""" + ui.set_color_override(None) + ui.set_run_tag(None) + yield + ui.set_color_override(None) + ui.set_run_tag(None) + + @pytest.fixture(scope="session") def k8s_cluster(): """Guard integration tests behind an explicit opt-in and a reachable cluster. diff --git a/tests/test_cli.py b/tests/test_cli.py index 16bfed1..480a1dd 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -367,3 +367,89 @@ def test_config_unknown_subcommand_errors(): result = CliRunner().invoke(cli, ["config", "bogus"]) assert result.exit_code != 0 assert "No such command 'bogus'" in result.output + + +# --- run-id tag --- + + +def _mock_run(monkeypatch): + """Patches Session/run_manifest so 'run' never touches a cluster, and returns a dict + that gets filled in with the manifest run_manifest was actually handed.""" + captured = {} + monkeypatch.setattr(cli_module, "Session", FakeSession) + + def fake_run_manifest(session, spec, manifest): + captured["manifest"] = manifest + return 0 + + monkeypatch.setattr(cli_module, "run_manifest", fake_run_manifest) + return captured + + +def test_run_stderr_tag_matches_manifest_run_id_label(monkeypatch): + """-t (without -i) triggers a real chatter() warning inside cli.py itself, before Session + is even constructed -- a guaranteed, already-existing status line to check the tag on.""" + import re + + captured = _mock_run(monkeypatch) + result = CliRunner(mix_stderr=False).invoke(cli, ["run", "--no-default-config", "-t", "alpine"]) + assert result.exit_code == 0, result.output + + label = captured["manifest"]["metadata"]["labels"]["squarepeg.io/run-id"] + assert re.fullmatch(r"[0-9a-f]{8}", label) + assert f"[squarepeg:{label}]" in result.stderr + + +def test_run_job_mode_stderr_tag_matches_manifest_run_id_label(monkeypatch): + captured = _mock_run(monkeypatch) + result = CliRunner(mix_stderr=False).invoke( + cli, ["run", "--no-default-config", "--mode", "job", "-t", "alpine"] + ) + assert result.exit_code == 0, result.output + + label = captured["manifest"]["metadata"]["labels"]["squarepeg.io/run-id"] + assert f"[squarepeg:{label}]" in result.stderr + + +def test_config_output_has_no_run_tag(): + result = CliRunner(mix_stderr=False).invoke(cli, ["config", "--no-default-config"]) + assert result.exit_code == 0, result.stderr + assert "[squarepeg:" not in result.stderr + assert "[squarepeg] no config files loaded" in result.stderr + + +def test_config_after_run_has_no_run_tag(monkeypatch): + """A 'run' followed by 'config' in the same process (as CliRunner does) must not leak + the run's tag into config's output.""" + _mock_run(monkeypatch) + CliRunner(mix_stderr=False).invoke(cli, ["run", "--no-default-config", "alpine"]) + result = CliRunner(mix_stderr=False).invoke(cli, ["config", "--no-default-config"]) + assert result.exit_code == 0, result.stderr + assert "[squarepeg:" not in result.stderr + + +def test_dry_run_stdout_has_no_tag(): + result = CliRunner(mix_stderr=False).invoke(cli, ["run", "--no-default-config", "--dry-run", "alpine"]) + assert result.exit_code == 0, result.stderr + assert "[squarepeg" not in result.stdout + + +def test_dry_run_server_stderr_is_tagged(monkeypatch): + monkeypatch.setattr(cli_module, "Session", FakeSession) + monkeypatch.setattr(cli_module, "server_dry_run", lambda session, spec, manifest: manifest) + result = CliRunner(mix_stderr=False).invoke( + cli, ["run", "--no-default-config", "-t", "--dry-run-server", "alpine"] + ) + assert result.exit_code == 0, result.stderr + assert "[squarepeg:" in result.stderr + + +def test_two_run_invocations_get_different_tags(monkeypatch): + captured1 = _mock_run(monkeypatch) + CliRunner().invoke(cli, ["run", "--no-default-config", "alpine"]) + captured2 = _mock_run(monkeypatch) + CliRunner().invoke(cli, ["run", "--no-default-config", "alpine"]) + + label1 = captured1["manifest"]["metadata"]["labels"]["squarepeg.io/run-id"] + label2 = captured2["manifest"]["metadata"]["labels"]["squarepeg.io/run-id"] + assert label1 != label2 diff --git a/tests/test_ui.py b/tests/test_ui.py index f2360bf..e6a84ff 100644 --- a/tests/test_ui.py +++ b/tests/test_ui.py @@ -8,10 +8,12 @@ @pytest.fixture(autouse=True) -def _reset_color_override(): +def _reset_ui_state(): ui.set_color_override(None) + ui.set_run_tag(None) yield ui.set_color_override(None) + ui.set_run_tag(None) # --- emit: colour / env var precedence --- @@ -108,3 +110,54 @@ def test_status_propagates_exceptions_and_leaves_no_live_thread(): with ui.Status("doing a thing"): raise RunnerError("boom") assert threading.active_count() == before + + +# --- run tag --- + + +def test_set_run_tag_appears_in_prefix(capsys): + ui.set_run_tag("abc12345") + ui.emit("hello") + assert "[squarepeg:abc12345] hello" in capsys.readouterr().err + + +def test_no_run_tag_produces_legacy_prefix(capsys): + ui.emit("hello") + err = capsys.readouterr().err + assert err.strip() == "[squarepeg] hello" + assert "[squarepeg:" not in err + + +@pytest.mark.parametrize("level", LEVELS) +def test_run_tag_does_not_corrupt_body_substring(monkeypatch, capsys, level): + monkeypatch.setenv("FORCE_COLOR", "1") + ui.set_run_tag("abc12345") + ui.emit("swept 3 orphaned resource(s) from previous runs", level=level) + assert "swept 3 orphaned resource(s) from previous runs" in capsys.readouterr().err + + +def test_status_line_carries_run_tag(capsys): + ui.set_run_tag("abc12345") + with ui.Status("waiting for pod to start", slow_hint="still pulling", slow_after=999): + pass + err = capsys.readouterr().err + assert "[squarepeg:abc12345]" in err + assert "waiting for pod to start" in err + assert "still pulling" in err + + +def test_status_success_line_carries_run_tag(capsys): + ui.set_run_tag("abc12345") + with ui.Status("doing a thing", success="all done"): + pass + err = capsys.readouterr().err + assert "[squarepeg:abc12345] all done" in err + + +def test_set_run_tag_none_clears_a_previously_set_tag(capsys): + ui.set_run_tag("abc12345") + ui.set_run_tag(None) + ui.emit("hello") + err = capsys.readouterr().err + assert "[squarepeg:" not in err + assert "[squarepeg] hello" in err From 8cb6d5c017578c588652c618fd3d138cf7438ef0 Mon Sep 17 00:00:00 2001 From: Biowilko Date: Wed, 19 Aug 2026 12:39:06 +0100 Subject: [PATCH 2/2] fix CI --- tests/test_cli.py | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 480a1dd..b45e31e 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -7,6 +7,16 @@ from squarepeg.errors import RunnerError +def _separated_runner(): + """A CliRunner with stdout/stderr captured separately, across the click 8.1/8.2 API + split: 8.1.x needs mix_stderr=False to get a usable .stderr; 8.2+ removed that kwarg + entirely and always separates the two streams by default.""" + try: + return CliRunner(mix_stderr=False) + except TypeError: + return CliRunner() + + class FakeSession: """Stands in for squarepeg.k8s.session.Session so CLI-level dry-run-server tests never touch a real cluster; records the args it was constructed with for assertions.""" @@ -392,7 +402,7 @@ def test_run_stderr_tag_matches_manifest_run_id_label(monkeypatch): import re captured = _mock_run(monkeypatch) - result = CliRunner(mix_stderr=False).invoke(cli, ["run", "--no-default-config", "-t", "alpine"]) + result = _separated_runner().invoke(cli, ["run", "--no-default-config", "-t", "alpine"]) assert result.exit_code == 0, result.output label = captured["manifest"]["metadata"]["labels"]["squarepeg.io/run-id"] @@ -402,7 +412,7 @@ def test_run_stderr_tag_matches_manifest_run_id_label(monkeypatch): def test_run_job_mode_stderr_tag_matches_manifest_run_id_label(monkeypatch): captured = _mock_run(monkeypatch) - result = CliRunner(mix_stderr=False).invoke( + result = _separated_runner().invoke( cli, ["run", "--no-default-config", "--mode", "job", "-t", "alpine"] ) assert result.exit_code == 0, result.output @@ -412,7 +422,7 @@ def test_run_job_mode_stderr_tag_matches_manifest_run_id_label(monkeypatch): def test_config_output_has_no_run_tag(): - result = CliRunner(mix_stderr=False).invoke(cli, ["config", "--no-default-config"]) + result = _separated_runner().invoke(cli, ["config", "--no-default-config"]) assert result.exit_code == 0, result.stderr assert "[squarepeg:" not in result.stderr assert "[squarepeg] no config files loaded" in result.stderr @@ -422,14 +432,14 @@ def test_config_after_run_has_no_run_tag(monkeypatch): """A 'run' followed by 'config' in the same process (as CliRunner does) must not leak the run's tag into config's output.""" _mock_run(monkeypatch) - CliRunner(mix_stderr=False).invoke(cli, ["run", "--no-default-config", "alpine"]) - result = CliRunner(mix_stderr=False).invoke(cli, ["config", "--no-default-config"]) + _separated_runner().invoke(cli, ["run", "--no-default-config", "alpine"]) + result = _separated_runner().invoke(cli, ["config", "--no-default-config"]) assert result.exit_code == 0, result.stderr assert "[squarepeg:" not in result.stderr def test_dry_run_stdout_has_no_tag(): - result = CliRunner(mix_stderr=False).invoke(cli, ["run", "--no-default-config", "--dry-run", "alpine"]) + result = _separated_runner().invoke(cli, ["run", "--no-default-config", "--dry-run", "alpine"]) assert result.exit_code == 0, result.stderr assert "[squarepeg" not in result.stdout @@ -437,7 +447,7 @@ def test_dry_run_stdout_has_no_tag(): def test_dry_run_server_stderr_is_tagged(monkeypatch): monkeypatch.setattr(cli_module, "Session", FakeSession) monkeypatch.setattr(cli_module, "server_dry_run", lambda session, spec, manifest: manifest) - result = CliRunner(mix_stderr=False).invoke( + result = _separated_runner().invoke( cli, ["run", "--no-default-config", "-t", "--dry-run-server", "alpine"] ) assert result.exit_code == 0, result.stderr