diff --git a/DESIGN.md b/DESIGN.md index 28f0974..4154ad0 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -67,7 +67,7 @@ Supported: - multiple models (e.g. a charm model and a COS model) - cross-model relations via `juju offer` and `juju integrate .` - virtual bundles (e.g. `juju deploy cos-lite`) -- `juju run` (actions) on virtual charms +- `juju run` (actions) on real and virtual charms - app-managed and user secrets, including `secret-changed` event dispatch Not supported: @@ -116,6 +116,7 @@ The `.charm` file passed to deploy is a trigger only. `jjx` does not inspect or - `./.jjx/charm/` (staged runtime charm directory with `src/`, `lib/`, `metadata.yaml`, `config.yaml`, and `.unit-state.db`) - `./.jjx/socket` (Pebble API Unix socket, bind-mounted into both the workload and charm runner containers) - `./.jjx/..deploy` (marker files for in-flight background pebble-ready processes; created by deploy, deleted by the process on completion or by teardown) +- `./.jjx/action-.json` (transient per-action results file; created by `juju run`, read/written by the action hook tools, deleted after the hook exits) - `./.jjx/prom-config-/` (Prometheus config directory, bind-mounted into the Prometheus container) - `./.jjx/grafana-config-/` (Grafana provisioning directory, bind-mounted into the Grafana container) @@ -253,7 +254,7 @@ When `jjx down` tears down all models, models are destroyed in reverse creation jjx implements several juju commands that jubilant/pytest-jubilant may call during setup, teardown, or status checks. These are minimal stubs that return just enough data for jubilant to function: - `juju offer` — records a cross-model offer in model state -- `juju run` — executes actions on virtual charms (e.g. traefik's `show-proxied-endpoints`) +- `juju run` — executes actions on real charms (dispatching the `actions/` hook via `docker exec`) and on virtual charms (returning dynamically-computed results) - `juju switch` — no-op (jjx always uses `--model`) - `juju version` — returns a minimal version response - `juju show-model` — returns model metadata @@ -278,6 +279,13 @@ Implemented: - `secret-add`, `secret-get`, `secret-grant`, `secret-info-get`, `secret-ids`, `secret-remove`, `secret-revoke`, `secret-set` — secret management (see "secrets" below) - `network-get` — returns the workload container's IP address (from `state.json`, not Docker, since Docker isn't available inside the charm runner). All bindings resolve to the workload's IP. - `application-version-set` — sets the workload version in state +- `action-get`, `action-set`, `action-fail`, `action-log` — action parameter access and result collection (see "actions" below) + +## actions + +`juju run ` dispatches the charm's `actions/` hook via `docker exec` into the charm runner, with `JUJU_ACTION_NAME` and `JUJU_ACTION_UUID` set and `JUJU_HOOK_NAME` empty (per `ops`). The action hook tools communicate with `juju run` through a transient per-action results file (`./.jjx/action-.json`): `action-get` reads the params, `action-set` accumulates results, `action-fail` sets the failure message, and `action-log` appends progress messages. After the hook exits, `juju run` reads the file and emits the task JSON jubilant expects. + +A hook that exits non-zero (uncaught exception) marks the task `failed` and sets unit/app status to `error`, matching real Juju. A hook that calls `action-fail` marks the task `failed` without changing unit status. In both cases `juju run` exits non-zero with `task failed` on stderr (and the task JSON on stdout) so jubilant raises `TaskError`. ## secrets diff --git a/src/jjx/_cmd_hook_tool.py b/src/jjx/_cmd_hook_tool.py index 382e901..cf9364c 100644 --- a/src/jjx/_cmd_hook_tool.py +++ b/src/jjx/_cmd_hook_tool.py @@ -4,6 +4,7 @@ import json import os +import pathlib import sys from typing import Any @@ -238,6 +239,9 @@ def hook_tool(args: list[str]) -> int: if tool == "network-get": return _network_get(tool_args, app_state) + if tool in ("action-get", "action-set", "action-fail", "action-log"): + return _action_tool(tool, tool_args) + raise _engine.CliError(f"unsupported hook tool: {tool}") @@ -1084,3 +1088,177 @@ def _network_get(tool_args: list[str], app_state: dict[str, Any]) -> int: else: sys.stdout.write(yaml.safe_dump(network_info, default_flow_style=False)) return 0 + + +# --------------------------------------------------------------------------- +# Action hook tools +# --------------------------------------------------------------------------- + + +def _action_results_path() -> pathlib.Path: + """Return the path to the current action's results file. + + The file is keyed by ``JUJU_ACTION_UUID`` (set by the action dispatch in + ``_engine._run_action_event``). It holds the params (input) and the + accumulating results, log messages, failure message, and failed flag that + the action hook tools write during the hook. ``_run_action_event`` reads + it after the hook exits to construct the task result jubilant expects. + """ + action_uuid = os.environ.get("JUJU_ACTION_UUID") + if not action_uuid: + raise _engine.CliError("action hook tool called outside an action hook") + return _engine._jjx_dir() / f"action-{action_uuid}.json" + + +def _load_action_results() -> dict[str, Any]: + path = _action_results_path() + if not path.exists(): + raise _engine.CliError(f"action results file not found: {path.name}") + return json.loads(path.read_text(encoding="utf-8")) + + +def _save_action_results(data: dict[str, Any]) -> None: + path = _action_results_path() + tmp = path.with_suffix(".tmp") + tmp.write_text(json.dumps(data), encoding="utf-8") + os.replace(tmp, path) + + +def _action_get(tool_args: list[str]) -> int: + """action-get [--format=json] [key] + + Returns the action parameters. With a dotted key, recurses into the + params map (matching real Juju's ``action-get``). + """ + output_format = "json" + key: str | None = None + i = 0 + while i < len(tool_args): + token = tool_args[i] + if token == "--format" and i + 1 < len(tool_args): + output_format = tool_args[i + 1] + i += 2 + continue + if token.startswith("--format="): + output_format = token.split("=", 1)[1] + i += 1 + continue + if token.startswith("-"): + i += 1 + continue + key = token + i += 1 + + data = _load_action_results() + params = data.get("params", {}) + + if key is None: + answer: Any = params if params else {} + else: + answer = params + for part in key.split("."): + if isinstance(answer, dict) and part in answer: + answer = answer[part] + else: + answer = None + break + + if output_format == "yaml": + sys.stdout.write(yaml.safe_dump(answer, sort_keys=False)) + else: + sys.stdout.write(json.dumps(answer)) + return 0 + + +def _action_set(tool_args: list[str]) -> int: + """action-set = [= ...] + + Adds the given dotted key=value pairs to the action results map. Nested + keys (e.g. ``foo.bar=baz``) build a nested dict, matching real Juju. + """ + data = _load_action_results() + results = data.setdefault("results", {}) + for arg in tool_args: + if arg.startswith("-"): + continue + key, sep, value = arg.partition("=") + if not sep: + raise _engine.CliError(f"action-set argument must be key=value: {arg}") + _set_nested(results, key.split("."), value) + _save_action_results(data) + return 0 + + +def _set_nested(target: dict[str, Any], keys: list[str], value: Any) -> None: + for key in keys[:-1]: + target = target.setdefault(key, {}) + if not isinstance(target, dict): + raise _engine.CliError(f"cannot set nested key under non-dict value: {key}") + target[keys[-1]] = value + + +def _action_fail(tool_args: list[str]) -> int: + """action-fail [message] + + Marks the action as failed with the given message. + """ + message = "" + i = 0 + while i < len(tool_args): + token = tool_args[i] + if token == "--": + message = " ".join(tool_args[i + 1 :]) + break + if token.startswith("-"): + i += 1 + continue + if message: + message += " " + token + else: + message = token + i += 1 + + data = _load_action_results() + data["failed"] = True + data["message"] = message + _save_action_results(data) + return 0 + + +def _action_log(tool_args: list[str]) -> int: + """action-log + + Appends a progress message to the action's log. + """ + message = "" + i = 0 + while i < len(tool_args): + token = tool_args[i] + if token == "--": + message = " ".join(tool_args[i + 1 :]) + break + if token.startswith("-"): + i += 1 + continue + if message: + message += " " + token + else: + message = token + i += 1 + + data = _load_action_results() + data.setdefault("log", []).append(message) + _save_action_results(data) + return 0 + + +def _action_tool(tool: str, tool_args: list[str]) -> int: + if tool == "action-get": + return _action_get(tool_args) + if tool == "action-set": + return _action_set(tool_args) + if tool == "action-fail": + return _action_fail(tool_args) + if tool == "action-log": + return _action_log(tool_args) + raise _engine.CliError(f"unsupported action hook tool: {tool}") diff --git a/src/jjx/_cmd_run.py b/src/jjx/_cmd_run.py index 0704fe7..411f5b4 100644 --- a/src/jjx/_cmd_run.py +++ b/src/jjx/_cmd_run.py @@ -1,8 +1,8 @@ """Run command wrapper (actions). -Executes an action on a unit. For real charms, this would run the action -hook via ``docker exec`` into the charm runner (not yet implemented). For -virtual charms, it returns dynamically-computed results (e.g. traefik's +Executes an action on a unit. For real charms, this runs the action hook via +``docker exec`` into the charm runner (see :func:`jjx._engine._run_action_event`). +For virtual charms, it returns dynamically-computed results (e.g. traefik's ``show-proxied-endpoints`` discovers the URLs of other COS charms in the model). """ @@ -12,6 +12,8 @@ import sys from typing import Any +import yaml + from . import _engine, _virtual_traefik @@ -24,6 +26,7 @@ def run(args: list[str], model: str | None) -> int: output_format = "json" unit: str | None = None action: str | None = None + params_file: str | None = None i = 0 while i < len(args): @@ -43,8 +46,13 @@ def run(args: list[str], model: str | None) -> int: i += 1 continue if token == "--params" and i + 1 < len(args): + params_file = args[i + 1] i += 2 continue + if token.startswith("--params="): + params_file = token.split("=", 1)[1] + i += 1 + continue if token.startswith("--"): i += 1 continue @@ -76,11 +84,31 @@ def run(args: list[str], model: str | None) -> int: if app_state.get("virtual"): return _run_virtual_action(model_state, app_name, app_state, action, output_format) - # For real charms, we would need to run the action hook. This is not - # implemented yet — the k8s-5-observe tests only use actions on traefik - # (virtual) and the charm's own get-db-info action (which the tests - # don't call in the COS Lite test flow). - raise _engine.CliError(f"actions on real charms not yet supported: {action}") + # Real charm: dispatch the action hook and return the task result. + params: dict[str, Any] = {} + if params_file: + try: + loaded = yaml.safe_load(open(params_file).read()) # noqa: SIM115 + except OSError as exc: + raise _engine.CliError(f"failed to read params file: {exc}") from None + if loaded is not None: + if not isinstance(loaded, dict): + raise _engine.CliError( + f"action params must be a mapping, got {type(loaded).__name__}" + ) + params = loaded + + task = _engine._run_action_event(model_name, app_name, action, params=params) + unit_name = app_state.get("unit", f"{app_name}/0") + result = {unit_name: task} + sys.stdout.write(json.dumps(result)) + # jubilant expects the CLI to exit non-zero with "task failed" in stderr + # when the action fails, so it can distinguish a failed action from a + # command error. The task JSON is still on stdout either way. + if task["status"] != "completed": + sys.stderr.write(f"task failed: action {action!r} on {unit_name}\n") + return 1 + return 0 def _run_virtual_action( diff --git a/src/jjx/_engine.py b/src/jjx/_engine.py index 3eb0ca3..c7d43e9 100644 --- a/src/jjx/_engine.py +++ b/src/jjx/_engine.py @@ -1092,6 +1092,10 @@ def _ensure_hook_tools(python_exe: str) -> None: "secret-revoke", "secret-set", "network-get", + "action-get", + "action-set", + "action-fail", + "action-log", ] root = _hook_tools_dir() root.mkdir(parents=True, exist_ok=True) @@ -1186,6 +1190,8 @@ def _build_charm_env( relation: dict[str, Any] | None = None, secret: dict[str, Any] | None = None, secret_revision: int | None = None, + action_name: str | None = None, + action_uuid: str | None = None, ) -> dict[str, str]: unit_name = app_state.get("unit", f"{app_name}/0") @@ -1242,6 +1248,13 @@ def _build_charm_env( for var in ("JUJU_SECRET_ID", "JUJU_SECRET_LABEL", "JUJU_SECRET_REVISION"): env.pop(var, None) + if action_name is not None and action_uuid is not None: + env["JUJU_ACTION_NAME"] = action_name + env["JUJU_ACTION_UUID"] = action_uuid + else: + for var in ("JUJU_ACTION_NAME", "JUJU_ACTION_UUID"): + env.pop(var, None) + # Hook tools are at /jjx/hook-tools inside the charm runner container. env["PATH"] = "/jjx/hook-tools:/usr/bin:/bin" # Hook tools do `import jjx`, so they need the same PYTHONPATH as the @@ -1262,6 +1275,8 @@ def _run_charm_event( relation: dict[str, Any] | None = None, secret: dict[str, Any] | None = None, secret_revision: int | None = None, + action_name: str | None = None, + action_uuid: str | None = None, ) -> None: state = _load_state() model_state = state["models"][model_name] @@ -1295,6 +1310,8 @@ def _run_charm_event( relation=relation, secret=secret, secret_revision=secret_revision, + action_name=action_name, + action_uuid=action_uuid, ) container_name = app_state.get("container_name", "") @@ -1407,6 +1424,82 @@ def _run_deploy_event_flow(model_name: str, app_name: str, workload_name: str) - _run_pebble_ready_event(model_name, app_name, workload_name) +def _run_action_event( + model_name: str, + app_name: str, + action_name: str, + params: dict[str, Any] | None = None, +) -> dict[str, Any]: + """Run an action hook on the real charm and return the task result dict. + + This mirrors real Juju's action dispatch: the charm's ``actions/`` + hook is executed via ``docker exec`` into the charm runner, with + ``JUJU_ACTION_NAME`` and ``JUJU_ACTION_UUID`` set (and ``JUJU_HOOK_NAME`` + empty, per ops). The action hook tools (``action-get``, ``action-set``, + ``action-fail``, ``action-log``) write to a per-action results file in + ``.jjx/``; this function reads that file after the hook exits to build + the result jubilant expects. + + Returns a dict with keys: ``id``, ``status``, ``results``, ``return-code``, + ``stderr``, ``message``, ``log``. + """ + action_uuid = uuid.uuid4().hex[:8] + results_path = _jjx_dir() / f"action-{action_uuid}.json" + results_path.write_text( + json.dumps( + {"params": params or {}, "results": {}, "log": [], "message": "", "failed": False} + ), + encoding="utf-8", + ) + + try: + _run_charm_event( + model_name, + app_name, + hook_name="", + dispatch_path=f"actions/{action_name}", + action_name=action_name, + action_uuid=action_uuid, + ) + # _run_charm_event raises CliError on non-zero exit, so reaching here + # means the hook succeeded. + return_code = 0 + stderr = "" + except CliError as exc: + # The hook exited non-zero. Real Juju marks the task as "failed". + # The error status has already been written to state by + # _run_charm_event; the exception message goes to the task's stderr. + return_code = 1 + stderr = exc.message + finally: + if results_path.exists(): + data = json.loads(results_path.read_text(encoding="utf-8")) + results_path.unlink(missing_ok=True) + else: + data = {} + + results = data.get("results", {}) + failed = data.get("failed", False) + message = data.get("message", "") + log = data.get("log", []) + + status = "completed" if return_code == 0 and not failed else "failed" + + # jubilant's Task._from_dict pops these special keys out of results. + task_results = dict(results) + task_results["return-code"] = return_code + task_results["stdout"] = "" + task_results["stderr"] = stderr + + return { + "id": action_uuid, + "status": status, + "results": task_results, + "message": message, + "log": log, + } + + # Minimum delay (seconds) between deploy() returning and pebble-ready firing. # Must exceed jubilant's wait() minimum return time (~2.0s with delay=1.0, # successes=3) so that tests using wait() cannot race ahead of container