From ca850208fe082e37512134c18062c0cddd9340fe Mon Sep 17 00:00:00 2001 From: Mike Langmayr <1809691+mikelangmayr@users.noreply.github.com> Date: Thu, 8 Oct 2026 18:03:45 -0700 Subject: [PATCH 1/3] Move the instrument's identity into a config the CLI reads --- pyproject.toml | 3 + src/hispec/cli/__init__.py | 18 +++-- src/hispec/cli/commands.py | 114 +++++++++++++++++------------ src/hispec/cli/doctor.py | 130 ++++++++++++++++++--------------- src/hispec/cli/instances.py | 88 +++++++++++----------- src/hispec/cli/instrument.toml | 30 ++++++++ src/hispec/cli/product.py | 110 ++++++++++++++++++++++++++++ src/hispec/cli/units.py | 34 ++++----- 8 files changed, 353 insertions(+), 174 deletions(-) create mode 100644 src/hispec/cli/instrument.toml create mode 100644 src/hispec/cli/product.py diff --git a/pyproject.toml b/pyproject.toml index 0ac3802..ba19909 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -2,6 +2,9 @@ where = ["src"] include = ["hispec", "hispec.*"] +[tool.setuptools.package-data] +"hispec.cli" = ["instrument.toml"] + [build-system] requires = ["setuptools>=61.0"] build-backend = "setuptools.build_meta" diff --git a/src/hispec/cli/__init__.py b/src/hispec/cli/__init__.py index 2c2d452..d33d834 100644 --- a/src/hispec/cli/__init__.py +++ b/src/hispec/cli/__init__.py @@ -13,10 +13,14 @@ from . import commands from .doctor import doctor from .instances import Paths +from .product import Product TARGETS_HELP = ("instance names (hsfei_adc), subsystems (hsfei, or just fei), " "or 'all'") +# Examples read better with real instance names than with placeholders, so +# they are written for this product rather than generated. + EPILOG = """\ examples: hispec status what is deployed here, and is it running? @@ -34,10 +38,11 @@ """ -def _parser() -> argparse.ArgumentParser: +def _parser(product: Product) -> argparse.ArgumentParser: parser = argparse.ArgumentParser( - prog="hispec", - description="Deploy, run and inspect the HISPEC daemons on this host.", + prog=product.name, + description=f"Deploy, run and inspect the {product.name} daemons " + "on this host.", epilog=EPILOG, formatter_class=argparse.RawDescriptionHelpFormatter, ) @@ -106,13 +111,14 @@ def add(name, func, help_text, description=None): def main(argv: Optional[List[str]] = None) -> int: - """Entry point for the ``hispec`` console script.""" - parser = _parser() + """Entry point for the console script.""" + product = Product.load() + parser = _parser(product) args = parser.parse_args(argv) if not getattr(args, "func", None): parser.print_help() return 2 try: - return args.func(Paths.from_env(), args) + return args.func(Paths.from_env(product), args) except KeyboardInterrupt: return 130 diff --git a/src/hispec/cli/commands.py b/src/hispec/cli/commands.py index 21dfe92..8b6970d 100644 --- a/src/hispec/cli/commands.py +++ b/src/hispec/cli/commands.py @@ -12,23 +12,28 @@ from . import instances as inst from . import units from .instances import Paths, TargetError +from .product import Product from .output import paint -def _err(message: str) -> None: - print(f"hispec: {message}", file=sys.stderr) +def _err(product: Product, message: str) -> None: + print(f"{product.name}: {message}", file=sys.stderr) def _resolve(paths: Paths, targets: List[str], names: List[str]) -> Optional[List[str]]: """Expand targets, explaining a miss. None means an error was printed.""" try: - return inst.resolve(targets, names) + return inst.resolve(paths.product, targets, names) except TargetError as exc: - target = str(exc) + target, prog = str(exc), paths.product.name if target in inst.in_repo(paths): - _err(f"{target} is not deployed on this host; deploy it with: hispec deploy {target}") + _err(paths.product, + f"{target} is not deployed on this host; " + f"deploy it with: {prog} deploy {target}") else: - _err(f"no instance or subsystem matches '{target}' (see: hispec status --all)") + _err(paths.product, + f"no instance or subsystem matches '{target}' " + f"(see: {prog} status --all)") return None @@ -48,8 +53,8 @@ def _install_file(src: Path, dst: Path, overwrite: bool, dry_run: bool) -> str: if dry_run: return {"copied": "would copy", "updated": "would update"}[outcome] # Write beside the target and rename, so a daemon starting meanwhile never - # reads half a file. Group-writable so the rest of hispec-ops can edit it; - # the directory is setgid hispec-ops, which takes care of the group. + # reads half a file. Group-writable so the rest of the ops group can edit + # it; the directory is setgid, which takes care of the group. tmp = dst.with_name(f".{dst.name}.{os.getpid()}.tmp") try: shutil.copyfile(src, tmp) @@ -78,24 +83,27 @@ def _deploy_plan(paths: Paths, names: List[str], for name in names: env_src = paths.repo_instances / f"{name}.env" if not inst.NAME_RE.match(name) or not env_src.is_file(): - _err(f"no {env_src}; is '{name}' a hispec instance? (see: hispec status --all)") + _err(paths.product, f"no {env_src}; is '{name}' an instance? " + f"(see: {paths.product.name} status --all)") problems += 1 continue - claimed = inst.assigned_host(env_src) + claimed = inst.assigned_host(paths.product, env_src) if claimed and claimed != role and not any_host: - _err(f"{name} belongs on '{claimed}', not '{role}'; " + _err(paths.product, f"{name} belongs on '{claimed}', not '{role}'; " "deploy it there, or pass --any-host") problems += 1 continue - config = inst.read_env(env_src).get("HISPEC_CONFIG", "") + config = inst.read_env(env_src).get(paths.product.env_key("CONFIG"), "") if not config: - _err(f"{env_src} sets no HISPEC_CONFIG") + _err(paths.product, + f"{env_src} sets no {paths.product.env_key('CONFIG')}") problems += 1 continue found = inst.repo_configs(paths, name) if len(found) != 1: where = ", ".join(str(p) for p in found) or "none" - _err(f"{name}: expected one {paths.repo}/config/*/{name}.yaml, found {where}") + _err(paths.product, + f"{name}: expected one {paths.repo}/config/*/{name}.yaml, found {where}") problems += 1 continue plan.append(_Deployment(name, env_src, paths.instances / f"{name}.env", @@ -119,9 +127,11 @@ def _copy(paths: Paths, plan: List[_Deployment], force: bool, dry_run: bool) -> outcome = _install_file(src, dst, overwrite, dry_run) except OSError as exc: print() - _err(f"cannot write {dst}: {exc.strerror}") + _err(paths.product, f"cannot write {dst}: {exc.strerror}") if isinstance(exc, PermissionError): - _err("deploying needs hispec-ops membership; run: hispec doctor") + _err(paths.product, + f"deploying needs {paths.product.ops_group} membership; " + f"run: {paths.product.name} doctor") return None note = "" if outcome == "kept": @@ -148,7 +158,7 @@ def deploy(paths: Paths, args: argparse.Namespace) -> int: "is already deployed.") return 0 elif not names: - _err("name the instances to deploy, or pass --new") + _err(paths.product, "name the instances to deploy, or pass --new") return 2 plan = _deploy_plan(paths, list(dict.fromkeys(names)), args.any_host) @@ -161,36 +171,37 @@ def deploy(paths: Paths, args: argparse.Namespace) -> int: names = [item.name for item in plan] rc = 0 if not args.no_enable: - rc |= _enable(names, [], args.dry_run) + rc |= _enable(paths.product, names, [], args.dry_run) if not args.no_start: print() - rc |= _act(names, "start", args.dry_run) + rc |= _act(paths.product, names, "start", args.dry_run) - running = units.states(updated) if updated and not args.dry_run else {} + running = units.states(paths.product, updated) if updated and not args.dry_run else {} restart = [n for n, s in running.items() if s.running] if restart: print() print("Already running with the old files; to pick up the new ones:") - print(f" hispec restart {' '.join(restart)}") + print(f" {paths.product.name} restart {' '.join(restart)}") return rc -def _enable(names: List[str], options: List[str], dry_run: bool) -> int: +def _enable(product: Product, names: List[str], options: List[str], + dry_run: bool) -> int: verb = "disable" if "--disable" in options else "enable" if dry_run: print(f"would {verb} at boot: {' '.join(names)}") return 0 - proc = units.enable_helper([*options, *names]) + proc = units.enable_helper(product, [*options, *names]) if proc.returncode == 0: print(f"{verb}d at boot: {' '.join(names)}") return 0 detail = (proc.stderr or proc.stdout).strip() if "password is required" in detail or proc.returncode == 127: - _err(f"cannot run {units.ENABLE_HELPER} without a password: {detail}") - _err("an admin needs to add you to hispec-ops, or re-run install.sh; " - "run: hispec doctor") + _err(product, f"cannot run {product.enable_helper} without a password: {detail}") + _err(product, f"an admin needs to add you to {product.ops_group}, or re-run\n" + f"install.sh; run: {product.name} doctor") else: - _err(detail or f"{units.ENABLE_HELPER} failed") + _err(product, detail or f"{product.enable_helper} failed") return 1 @@ -200,15 +211,15 @@ def enable(paths: Paths, args: argparse.Namespace) -> int: if names is None: return 1 options = (["--disable"] if args.verb == "disable" else []) + (["--now"] if args.now else []) - return _enable(names, options, args.dry_run) + return _enable(paths.product, names, options, args.dry_run) _DONE = {"start": "started", "stop": "stopped", "restart": "restarted"} -def _act(names: List[str], verb: str, dry_run: bool) -> int: +def _act(product: Product, names: List[str], verb: str, dry_run: bool) -> int: """Apply a verb to every instance, carrying on past failures.""" - state = units.states(names) + state = units.states(product, names) failed = [] for name in names: if verb == "start" and state[name].active == "active": @@ -220,14 +231,16 @@ def _act(names: List[str], verb: str, dry_run: bool) -> int: if dry_run: print(f"{name:<24} would {verb}") continue - if units.systemctl(verb, name): + if units.systemctl(product, verb, name): print(f"{name:<24} {_DONE[verb]}") else: - print(f"{name:<24} " + paint("FAILED", "31") + f" (hispec logs {name})") + print(f"{name:<24} " + paint("FAILED", "31") + + f" ({product.name} logs {name})") failed.append(name) if failed: print() - _err(f"{len(failed)} of {len(names)} failed to {verb}: {' '.join(failed)}") + _err(product, f"{len(failed)} of {len(names)} failed to {verb}: " + f"{' '.join(failed)}") return 1 return 0 @@ -238,10 +251,10 @@ def act(paths: Paths, args: argparse.Namespace) -> int: if names is None: return 1 if args.verb == "stop": - # Undo a start in the opposite order: `hispec stop power fei` would + # Undo a start in the opposite order: `stop power fei` would # otherwise cut power before the mechanisms had parked. names.reverse() - return _act(names, args.verb, args.dry_run) + return _act(paths.product, names, args.verb, args.dry_run) _STATE_COLOR = {"active": "32", "failed": "31", "activating": "33", "deactivating": "33"} @@ -261,7 +274,7 @@ def status(paths: Paths, args: argparse.Namespace) -> int: names = universe if not names: print(f"No instances deployed in {paths.instances}.") - print("See what the repo defines with: hispec status --all") + print(f"See what the repo defines with: {paths.product.name} status --all") return 0 if args.verbose: return _status_verbose(paths, names, args.lines) @@ -278,21 +291,23 @@ def status(paths: Paths, args: argparse.Namespace) -> int: if missing: print() print(f"{len(missing)} more in the repo, not deployed here " - "(hispec status --all; hispec deploy --new)") + f"({paths.product.name} status --all; " + f"{paths.product.name} deploy --new)") return 0 def _status_rows(paths: Paths, names: List[str], here: List[str]) -> List[Tuple[str, ...]]: - state = units.states([n for n in names if n in here]) + state = units.states(paths.product, [n for n in names if n in here]) rows = [] for name in names: if name not in here: - daemon = _daemon(paths.repo_instances / f"{name}.env") + daemon = _daemon(paths.product, paths.repo_instances / f"{name}.env") rows.append((name, "not deployed", "", "", daemon)) continue s = state[name] since = s.since if s.active != "inactive" else "" - rows.append((name, s.active, s.enabled, since, _daemon(paths.instances / f"{name}.env"))) + rows.append((name, s.active, s.enabled, since, + _daemon(paths.product, paths.instances / f"{name}.env"))) return rows @@ -307,11 +322,12 @@ def _status_verbose(paths: Paths, names: List[str], lines: int) -> int: print(f" {label:<9} {value}") print() sys.stdout.flush() # systemctl writes to the same terminal - if not units.show_status(name, lines): + if not units.show_status(paths.product, name, lines): failed.append(name) if failed: print() - _err(f"systemctl could not report on: {' '.join(failed)} (run: hispec doctor)") + _err(paths.product, f"systemctl could not report on: {' '.join(failed)} " + f"(run: {paths.product.name} doctor)") return 1 return 0 @@ -323,8 +339,9 @@ def _files(paths: Paths, name: str) -> List[Tuple[str, str]]: env = inst.read_env(env_file) except OSError as exc: return [("instance", f"{env_file} " + paint(f"(unreadable: {exc.strerror})", "31"))] - rows = [("daemon", env.get("HISPEC_DAEMON", "?")), ("instance", str(env_file))] - config = env.get("HISPEC_CONFIG", "") + rows = [("daemon", env.get(paths.product.env_key("DAEMON"), "?")), + ("instance", str(env_file))] + config = env.get(paths.product.env_key("CONFIG"), "") if not config: rows.append(("config", paint("not set in the instance file", "31"))) return rows @@ -338,9 +355,9 @@ def _files(paths: Paths, name: str) -> List[Tuple[str, str]]: return rows -def _daemon(env: Path) -> str: +def _daemon(product: Product, env: Path) -> str: try: - return inst.read_env(env).get("HISPEC_DAEMON", "?") + return inst.read_env(env).get(product.env_key("DAEMON"), "?") except OSError: return "?" @@ -353,7 +370,7 @@ def logs(paths: Paths, args: argparse.Namespace) -> int: return 1 cmd = ["journalctl"] for name in names: - cmd += ["-u", inst.unit(name)] + cmd += ["-u", paths.product.unit(name)] if args.follow: cmd.append("-f") if args.lines is not None: @@ -367,5 +384,6 @@ def logs(paths: Paths, args: argparse.Namespace) -> int: try: os.execvp(cmd[0], cmd) except FileNotFoundError: - _err("journalctl not found; this host does not look like it runs systemd") + _err(paths.product, + "journalctl not found; this host does not look like it runs systemd") return 1 diff --git a/src/hispec/cli/doctor.py b/src/hispec/cli/doctor.py index d21cbaa..1a9ceef 100644 --- a/src/hispec/cli/doctor.py +++ b/src/hispec/cli/doctor.py @@ -1,4 +1,4 @@ -"""``hispec doctor``: check this host's setup and say what is wrong. +"""``doctor``: check this host's setup and say what is wrong. """ from __future__ import annotations @@ -10,17 +10,14 @@ import socket import stat from pathlib import Path +from typing import Optional from . import instances as inst from . import units from .instances import Paths from .output import paint -UNIT_FILE = Path("/etc/systemd/system/hispec@.service") -OLD_UNIT_FILE = Path("/etc/systemd/system/hispec-daemon@.service") -POLKIT_RULE = Path("/etc/polkit-1/rules.d/49-hispec.rules") -OLD_POLKIT_RULE = Path("/etc/polkit-1/rules.d/49-hispec-daemons.rules") -SUDOERS = Path("/etc/sudoers.d/hispec-ops") +LOG_DIR_TEMPLATE = "/var/log/{name}" class Report: @@ -66,8 +63,10 @@ def _my_groups() -> set: return names -def _is_file(path: Path) -> bool: +def _is_file(path: Optional[Path]) -> bool: """Whether path is a file, treating one we may not stat as absent.""" + if path is None: + return False try: return path.is_file() except OSError: @@ -102,26 +101,28 @@ def _version_tuple(text: str) -> tuple: return () -def _account(r: Report) -> None: +def _account(r: Report, paths: Paths) -> None: + product = paths.product + group = product.ops_group r.section("Your account") me = pwd.getpwuid(os.geteuid()).pw_name groups = _my_groups() if os.geteuid() == 0: r.warn("running as root, so the permission checks below say nothing about operators") - r.hint("re-run as your own user: hispec doctor") - if not _group_exists("hispec-ops"): - r.bad("group hispec-ops does not exist") + r.hint(f"re-run as your own user: {product.name} doctor") + if not _group_exists(group): + r.bad(f"group {group} does not exist") r.hint("an admin needs to run systemd/install.sh on this host") - elif "hispec-ops" in groups: - r.ok("you are in hispec-ops") + elif group in groups: + r.ok(f"you are in {group}") else: - r.bad("you are NOT in hispec-ops — this is why systemctl asks for a password") - r.hint(f"admin: sudo /opt/hispec/app/systemd/install.sh {me}") - r.hint("then log out and back in (or run: newgrp hispec-ops)") + r.bad(f"you are NOT in {group}, which is why systemctl asks for a password") + r.hint(f"admin: sudo {paths.repo}/systemd/install.sh {me}") + r.hint(f"then log out and back in (or run: newgrp {group})") if "systemd-journal" in groups: r.ok("you are in systemd-journal (can read other units' logs)") else: - r.warn("you are not in systemd-journal; hispec logs will be empty") + r.warn(f"you are not in systemd-journal; {product.name} logs will be empty") r.hint(f"admin: sudo usermod -aG systemd-journal {me}") @@ -130,38 +131,43 @@ def _install_hint(paths: Paths) -> str: def _unit_and_polkit(r: Report, paths: Paths) -> None: + product = paths.product install = _install_hint(paths) + unit_file = product.unit_template + polkit_rule = product.polkit_rule r.section("Unit and polkit") - repo_unit = paths.repo / "systemd" / "hispec@.service" - if UNIT_FILE.is_file(): - r.ok(f"{UNIT_FILE} installed") - if repo_unit.is_file() and not filecmp.cmp(UNIT_FILE, repo_unit, shallow=False): + repo_unit = paths.repo / "systemd" / unit_file.name + if unit_file.is_file(): + r.ok(f"{unit_file} installed") + if repo_unit.is_file() and not filecmp.cmp(unit_file, repo_unit, shallow=False): r.warn(f"installed unit differs from {repo_unit}") r.hint(install) else: - r.bad(f"{UNIT_FILE} is missing") + r.bad(f"{unit_file} is missing") r.hint(install) - if OLD_UNIT_FILE.is_file(): - r.warn("the old hispec-daemon@.service is still installed") - r.hint(f"{install} # migrates instances to hispec@") + if _is_file(product.legacy_unit_path): + r.warn(f"the old {product.legacy_unit} is still installed") + r.hint(f"{install} # migrates instances to {product.name}@") try: - rule = POLKIT_RULE.read_text(encoding="utf-8") + rule = polkit_rule.read_text(encoding="utf-8") except FileNotFoundError: - r.bad(f"{POLKIT_RULE} is missing, so every start/stop will prompt") + r.bad(f"{polkit_rule} is missing, so every start/stop will prompt") r.hint(install) except OSError: # rules.d is 0750 root:polkitd on some hosts, so an operator cannot # even stat inside it; unreadable means unknown, not missing - r.warn(f"cannot read {POLKIT_RULE}, so the polkit rule is unchecked") - r.hint("sudo hispec doctor") + r.warn(f"cannot read {polkit_rule}, so the polkit rule is unchecked") + r.hint(f"sudo {product.name} doctor") else: - r.ok(f"{POLKIT_RULE} installed") - if 'indexOf("hispec@")' not in rule: - r.bad("the installed polkit rule does not match hispec@ units") + r.ok(f"{polkit_rule} installed") + if f'indexOf("{product.name}@")' not in rule: + r.bad(f"the installed polkit rule does not match {product.name}@ units") r.hint(f"it is probably the pre-rename copy; {install}") - if _is_file(OLD_POLKIT_RULE): - r.warn(f"the old {OLD_POLKIT_RULE.name} is still installed (harmless, matches nothing now)") - r.hint(f"admin: sudo rm {OLD_POLKIT_RULE}") + legacy_rule = product.legacy_polkit_path + if _is_file(legacy_rule): + r.warn(f"the old {legacy_rule.name} is still installed " + "(harmless, matches nothing now)") + r.hint(f"admin: sudo rm {legacy_rule}") _polkit_daemon(r) @@ -187,24 +193,27 @@ def _polkit_daemon(r: Report) -> None: def _sudo(r: Report, paths: Paths) -> None: + product = paths.product install = _install_hint(paths) - if not SUDOERS.is_file(): - r.warn(f"no {SUDOERS}; enabling an instance at boot needs an admin") + sudoers, helper = product.sudoers, str(product.enable_helper) + if not sudoers.is_file(): + r.warn(f"no {sudoers}; enabling an instance at boot needs an admin") r.hint(install) - elif os.geteuid() != 0 and "hispec-ops" in _my_groups(): - if units.run(["sudo", "-n", "-l", units.ENABLE_HELPER], capture=True).returncode == 0: - r.ok("you can run hispec enable without a password") + elif os.geteuid() != 0 and product.ops_group in _my_groups(): + if units.run(["sudo", "-n", "-l", helper], capture=True).returncode == 0: + r.ok(f"you can run {product.name} enable without a password") else: - r.bad("sudo will not run hispec-enable for you without a password") - r.hint(f"{SUDOERS} or {units.ENABLE_HELPER} is out of date; {install}") + r.bad(f"sudo will not run {helper} for you without a password") + r.hint(f"{sudoers} or {helper} is out of date; {install}") else: - r.ok(f"{SUDOERS} installed (passwordless hispec enable)") + r.ok(f"{sudoers} installed (passwordless {product.name} enable)") def _paths(r: Report, paths: Paths) -> None: install = _install_hint(paths) r.section("Paths") - for directory in (paths.etc, paths.instances, Path("/var/log/hispec")): + log_dir = Path(LOG_DIR_TEMPLATE.format(name=paths.product.name)) + for directory in (paths.etc, paths.instances, log_dir): if directory.is_dir(): r.ok(f"{directory} exists ({_owner(directory)})") else: @@ -226,31 +235,34 @@ def _deployed(r: Report, paths: Paths) -> None: names = inst.deployed(paths) if not names: r.warn(f"no instance files in {paths.instances} — nothing is deployed on this host") - r.hint("hispec deploy # or: hispec deploy --new") + r.hint(f"{paths.product.name} deploy " + f"# or: {paths.product.name} deploy --new") return - state = units.states(names) + product = paths.product + state = units.states(product, names) for name in names: env_file = paths.instances / f"{name}.env" env = inst.read_env(env_file) - script = env.get("HISPEC_DAEMON", "") - config = env.get("HISPEC_CONFIG", "") + script = env.get(product.env_key("DAEMON"), "") + config = env.get(product.env_key("CONFIG"), "") repo_env = paths.repo_instances / f"{name}.env" if not script: - r.bad(f"{name}: {env_file} sets no HISPEC_DAEMON") + r.bad(f"{name}: {env_file} sets no {product.env_key('DAEMON')}") elif not (paths.repo / "daemons" / script).is_file(): r.bad(f"{name}: daemon script {paths.repo}/daemons/{script} does not exist") elif not config or not Path(config).is_file(): r.bad(f"{name}: config {config or ''} does not exist") - r.hint(f"hispec deploy {name}") + r.hint(f"{product.name} deploy {name}") elif state[name].active == "active": r.ok(f"{name}: running") else: r.warn(f"{name}: {state[name].active}") - r.hint(f"hispec start {name}") - r.hint(f"hispec logs {name}") + r.hint(f"{product.name} start {name}") + r.hint(f"{product.name} logs {name}") if repo_env.is_file() and not filecmp.cmp(env_file, repo_env, shallow=False): r.warn(f"{name}: deployed instance file differs from {repo_env}") - r.hint(f"hispec deploy {name} # updates the instance file, keeps the config") + r.hint(f"{product.name} deploy {name} " + "# updates the instance file, keeps the config") elif not repo_env.is_file(): r.warn(f"{name}: deployed here but not defined in {paths.repo_instances}") @@ -261,7 +273,7 @@ def _secrets(r: Report, paths: Paths) -> None: present = inst.secrets_set(paths) if present is None: r.warn(f"cannot read {paths.secrets}, so the credentials below are unchecked") - r.hint("sudo hispec doctor") + r.hint(f"sudo {paths.product.name} doctor") return for variable in sorted(needed): if present.get(variable): @@ -275,29 +287,29 @@ def _placement(r: Report, paths: Paths) -> None: """Flag instances running on the wrong host, or claiming no host at all. Only looks at what is deployed: which of the repo's instances a host runs - is an operational decision, and ``hispec status`` already lists the rest. + is an operational decision, and ``status`` already lists the rest. """ r.section("Host assignment") role = inst.host_role(paths) r.ok(f"this host is '{role}'") unassigned = [] for name in inst.deployed(paths): - claimed = inst.assigned_host(paths.instances / f"{name}.env") + claimed = inst.assigned_host(paths.product, paths.instances / f"{name}.env") if claimed is None: unassigned.append(name) elif claimed != role: r.bad(f"{name}: deployed here but assigned to '{claimed}'") if unassigned: - r.warn(f"no {inst.HOST_KEY}, so nothing says where they belong: " + r.warn(f"no {paths.product.env_key('HOST')}, so nothing says where they belong: " f"{' '.join(unassigned)}") def doctor(paths: Paths, _args: argparse.Namespace) -> int: """Run every check and exit 1 if any FAILed.""" me = pwd.getpwuid(os.geteuid()).pw_name - print(f"hispec doctor: {socket.gethostname()}, running as {me}") + print(f"{paths.product.name} doctor: {socket.gethostname()}, running as {me}") r = Report() - _account(r) + _account(r, paths) _unit_and_polkit(r, paths) _sudo(r, paths) _paths(r, paths) diff --git a/src/hispec/cli/instances.py b/src/hispec/cli/instances.py index 00cde9f..c6efb39 100644 --- a/src/hispec/cli/instances.py +++ b/src/hispec/cli/instances.py @@ -4,11 +4,11 @@ - ``/systemd/instances/.env``, which says which daemon script to run and where its config is deployed; - ``/config//.yaml``, the config as committed; -- their deployed copies, ``/etc/hispec/instances/.env`` and whatever - path the ``.env`` gives as ``HISPEC_CONFIG``. +- their deployed copies, ``/instances/.env`` and whatever path the + ``.env`` gives as the config. -Deployed means the ``.env`` is in ``/etc/hispec/instances/``, since that is -what ``hispec@.service`` reads. +Deployed means the ``.env`` is in ``/instances/``, since that is what the +template unit reads. """ from __future__ import annotations @@ -21,46 +21,52 @@ import yaml -NAME_RE = re.compile(r"^[a-z][a-z0-9_]*$") - -# Set in an instance file, so deploying it on the wrong host is caught before -# two daemons reach for one device -HOST_KEY = "HISPEC_HOST" +from .product import Product -# A config names the variable holding a credential, never the value itself -SECRET_KEY_SUFFIX = "_env" +NAME_RE = re.compile(r"^[a-z][a-z0-9_]*$") -def _default_repo_dir() -> Path: +def _default_repo_dir(product: Product) -> Path: # install.sh does an editable install, so this file is inside the checkout # the daemons run from. Anything else (a wheel) gets the standard path. here = Path(__file__).resolve().parents[3] if (here / "systemd" / "instances").is_dir(): return here - return Path("/opt/hispec/app") + return Path(f"/opt/{product.name}/app") @dataclass(frozen=True) class Paths: - """The filesystem layout, overridable for testing and odd hosts.""" + """The filesystem layout, overridable for testing and odd hosts. + Carries the product so every command reaches it through the one argument + they all already take. + """ + + product: Product repo: Path venv: Path etc: Path @classmethod - def from_env(cls, environ: Mapping[str, str] = os.environ) -> "Paths": - """Build from HISPEC_REPO_DIR / HISPEC_VENV_DIR / HISPEC_ETC_DIR.""" - repo = environ.get("HISPEC_REPO_DIR") + def from_env(cls, product: Optional[Product] = None, + environ: Optional[Mapping[str, str]] = None) -> "Paths": + """Build from _REPO_DIR / _VENV_DIR / _ETC_DIR.""" + product = product or Product.load() + environ = os.environ if environ is None else environ + repo = environ.get(product.env_key("REPO_DIR")) return cls( - repo=Path(repo) if repo else _default_repo_dir(), - venv=Path(environ.get("HISPEC_VENV_DIR", "/opt/hispec/venv")), - etc=Path(environ.get("HISPEC_ETC_DIR", "/etc/hispec")), + product=product, + repo=Path(repo) if repo else _default_repo_dir(product), + venv=Path(environ.get(product.env_key("VENV_DIR"), + f"/opt/{product.name}/venv")), + etc=Path(environ.get(product.env_key("ETC_DIR"), + f"/etc/{product.name}")), ) @property def instances(self) -> Path: - """Deployed instance files, read by hispec@.service.""" + """Deployed instance files, read by the template unit.""" return self.etc / "instances" @property @@ -75,7 +81,7 @@ def secrets(self) -> Path: def host_role(paths: Paths) -> str: - """Return this host's role, from /etc/hispec/host or its short hostname.""" + """Return this host's role, from /host or its short hostname.""" path = paths.etc / "host" if path.is_file(): role = path.read_text(encoding="utf-8").strip() @@ -84,22 +90,17 @@ def host_role(paths: Paths) -> str: return socket.gethostname().split(".")[0] -def assigned_host(env_file: Path) -> Optional[str]: +def assigned_host(product: Product, env_file: Path) -> Optional[str]: """Return the host an instance file claims, or None when it claims none.""" if not env_file.is_file(): return None - return read_env(env_file).get(HOST_KEY) or None + return read_env(env_file).get(product.env_key("HOST")) or None class TargetError(Exception): """A name on the command line matched nothing.""" -def unit(name: str) -> str: - """The systemd unit for an instance.""" - return f"hispec@{name}.service" - - def _names_in(directory: Path) -> List[str]: if not directory.is_dir(): return [] @@ -136,16 +137,17 @@ def read_env(path: Path) -> Dict[str, str]: return values -def _secret_keys(node: Any, prefix: str = "") -> Iterator[Tuple[str, str]]: - """Yield (config key, variable name) for every ``*_env`` key in a config.""" +def _secret_keys(node: Any, suffix: str, + prefix: str = "") -> Iterator[Tuple[str, str]]: + """Yield (config key, variable name) for every credential key in a config.""" if not isinstance(node, Mapping): return for key, value in node.items(): path = f"{prefix}.{key}" if prefix else str(key) - if str(key).endswith(SECRET_KEY_SUFFIX) and isinstance(value, str) and value: + if str(key).endswith(suffix) and isinstance(value, str) and value: yield path, value else: - yield from _secret_keys(value, path) + yield from _secret_keys(value, suffix, path) def _broker_secrets() -> List[str]: @@ -171,11 +173,12 @@ def required_secrets(paths: Paths) -> Dict[str, List[str]]: name: ["every daemon (broker login)"] for name in _broker_secrets() } for instance in deployed(paths): - config = read_env(paths.instances / f"{instance}.env").get("HISPEC_CONFIG", "") + env = read_env(paths.instances / f"{instance}.env") + config = env.get(paths.product.env_key("CONFIG"), "") if not config or not Path(config).is_file(): continue parsed = yaml.safe_load(Path(config).read_text(encoding="utf-8")) or {} - for key, variable in _secret_keys(parsed): + for key, variable in _secret_keys(parsed, paths.product.secret_key_suffix): needed.setdefault(variable, []).append(f"{instance} ({key})") return needed @@ -188,15 +191,16 @@ def secrets_set(paths: Paths) -> Optional[Dict[str, str]]: return None -def resolve(targets: Iterable[str], names: List[str]) -> List[str]: +def resolve(product: Product, targets: Iterable[str], names: List[str]) -> List[str]: """Expand command-line targets against the instances in ``names``. A target is an instance name (``hsfei_adc``), a subsystem prefix - (``hsfei``, or just ``fei``), or ``all``. Targets keep their command-line - order, so ``hispec start power fei`` powers up before it starts the - mechanisms. Duplicates are dropped, and a target matching nothing is an - error. + (``hsfei``, or just ``fei`` where the product sets one), or ``all``. + Targets keep their command-line order, so ``start power fei`` powers up + before it starts the mechanisms. Duplicates are dropped, and a target + matching nothing is an error. """ + shorthand = product.subsystem_prefix selected: List[str] = [] for target in targets: if target == "all": @@ -206,8 +210,8 @@ def resolve(targets: Iterable[str], names: List[str]) -> List[str]: else: prefix = target.rstrip("_") + "_" hits = [n for n in names if n.startswith(prefix)] - if not hits and not target.startswith("hs"): - hits = [n for n in names if n.startswith("hs" + prefix)] + if not hits and shorthand and not target.startswith(shorthand): + hits = [n for n in names if n.startswith(shorthand + prefix)] if not hits: raise TargetError(target) selected.extend(n for n in hits if n not in selected) diff --git a/src/hispec/cli/instrument.toml b/src/hispec/cli/instrument.toml new file mode 100644 index 0000000..2b9830f --- /dev/null +++ b/src/hispec/cli/instrument.toml @@ -0,0 +1,30 @@ +# Identity of the instrument this CLI drives. Read by hispec.cli.product. +# +# Paths follow from `name`: /opt//app, /opt//venv, /etc/, +# @.service, /usr/local/sbin/-enable. Operators override the +# directories with _REPO_DIR, _VENV_DIR and _ETC_DIR. +# +# Which credentials a host needs is deliberately not here. It is derived from +# the deployed configs and from the daemon code that reads them, so it cannot +# go stale the way a hand-written list does. + +[instrument] +name = "hispec" +user = "hispec" +ops_group = "hispec-ops" +# Instance files set HISPEC_DAEMON, HISPEC_CONFIG and HISPEC_HOST +env_prefix = "HISPEC" +# So `fei` finds hsfei_* as well as fei_* +subsystem_prefix = "hs" +docs_url = "https://caltechopticalobservatories.github.io/hispec/operations/systemd.html" + +[secrets] +# A config key ending in this names the variable holding a credential, never +# the credential itself +key_suffix = "_env" + +# Installed by older versions. doctor reports them so a host that predates the +# rename is recognised rather than looking merely misconfigured. +[legacy] +unit = "hispec-daemon@.service" +polkit_rule = "49-hispec-daemons.rules" diff --git a/src/hispec/cli/product.py b/src/hispec/cli/product.py new file mode 100644 index 0000000..e9e2eec --- /dev/null +++ b/src/hispec/cli/product.py @@ -0,0 +1,110 @@ +"""Which instrument this CLI drives. + +Everything that differs between instruments is here, so the rest of the package +is about deploying daemons rather than about HISPEC specifically. The values +come from ``instrument.toml`` beside this module; a second instrument ships its +own and reuses the code unchanged. + +These are identity, not operator settings. Where an operator needs to override +something, such as a non-standard checkout path, that stays an environment +variable on ``Paths``. +""" +from __future__ import annotations + +import tomllib +from dataclasses import dataclass, field +from importlib import resources +from pathlib import Path +from typing import Optional + +CONFIG_NAME = "instrument.toml" + +SYSTEMD_DIR = Path("/etc/systemd/system") +POLKIT_RULES_DIR = Path("/etc/polkit-1/rules.d") +SUDOERS_DIR = Path("/etc/sudoers.d") +SBIN_DIR = Path("/usr/local/sbin") + + +# Each attribute is one fact about the instrument rather than hidden state +@dataclass(frozen=True) +class Product: # pylint: disable=too-many-instance-attributes + """One instrument's names, and the paths and units derived from them.""" + + name: str + user: str + ops_group: str + env_prefix: str + subsystem_prefix: str + secret_key_suffix: str = "_env" + docs_url: str = "" + # Units and polkit rules this product used to install, reported by doctor + # so a host that predates a rename can be recognised + legacy_unit: Optional[str] = field(default=None) + legacy_polkit_rule: Optional[str] = field(default=None) + + @classmethod + def load(cls) -> "Product": + """Read ``instrument.toml`` from beside this module.""" + with resources.files(__package__).joinpath(CONFIG_NAME).open("rb") as handle: + data = tomllib.load(handle) + instrument = data["instrument"] + secrets = data.get("secrets", {}) + legacy = data.get("legacy", {}) + return cls( + name=instrument["name"], + user=instrument.get("user", instrument["name"]), + ops_group=instrument.get("ops_group", f"{instrument['name']}-ops"), + env_prefix=instrument.get("env_prefix", instrument["name"].upper()), + subsystem_prefix=instrument.get("subsystem_prefix", ""), + secret_key_suffix=secrets.get("key_suffix", "_env"), + docs_url=instrument.get("docs_url", ""), + legacy_unit=legacy.get("unit"), + legacy_polkit_rule=legacy.get("polkit_rule"), + ) + + def unit(self, instance: str) -> str: + """The systemd unit for an instance.""" + return f"{self.name}@{instance}.service" + + def instance_of(self, unit_id: str) -> Optional[str]: + """The instance a unit id names, or None if it is not one of ours.""" + prefix, suffix = f"{self.name}@", ".service" + if not unit_id.startswith(prefix) or not unit_id.endswith(suffix): + return None + return unit_id[len(prefix):-len(suffix)] + + def env_key(self, name: str) -> str: + """An instance file's variable, e.g. ``HISPEC_DAEMON`` for ``DAEMON``.""" + return f"{self.env_prefix}_{name}" + + @property + def unit_template(self) -> Path: + """The installed template unit every instance runs from.""" + return SYSTEMD_DIR / f"{self.name}@.service" + + @property + def polkit_rule(self) -> Path: + """The rule granting the ops group start/stop without a password.""" + return POLKIT_RULES_DIR / f"49-{self.name}.rules" + + @property + def sudoers(self) -> Path: + """The drop-in granting passwordless use of the enable helper.""" + return SUDOERS_DIR / f"{self.name}-ops" + + @property + def enable_helper(self) -> Path: + """Root helper behind ``enable``, which polkit cannot scope per unit.""" + return SBIN_DIR / f"{self.name}-enable" + + @property + def legacy_unit_path(self) -> Optional[Path]: + """Where a pre-rename template unit would still be installed.""" + return SYSTEMD_DIR / self.legacy_unit if self.legacy_unit else None + + @property + def legacy_polkit_path(self) -> Optional[Path]: + """Where a pre-rename polkit rule would still be installed.""" + if not self.legacy_polkit_rule: + return None + return POLKIT_RULES_DIR / self.legacy_polkit_rule diff --git a/src/hispec/cli/units.py b/src/hispec/cli/units.py index 00d6041..e9b26f6 100644 --- a/src/hispec/cli/units.py +++ b/src/hispec/cli/units.py @@ -9,22 +9,18 @@ from dataclasses import dataclass from typing import Dict, List, Sequence -from .instances import unit +from .product import Product -# Root-owned and installed by install.sh, with a NOPASSWD sudoers entry for -# hispec-ops. Deliberately not part of this package: the venv is writable by -# the hispec user, so root must never run code from it. -ENABLE_HELPER = "/usr/local/sbin/hispec-enable" - -def run(cmd: Sequence[str], capture: bool = False) -> subprocess.CompletedProcess: +def run(cmd: Sequence[str], capture: bool = False, + prog: str = "") -> subprocess.CompletedProcess: """Run a command, inheriting the terminal unless ``capture``.""" try: return subprocess.run(list(cmd), check=False, text=True, capture_output=capture) except FileNotFoundError: message = f"{cmd[0]}: not found" if not capture: - print(f"hispec: {message}", file=sys.stderr) + print(f"{prog or cmd[0]}: {message}", file=sys.stderr) return subprocess.CompletedProcess(list(cmd), 127, "", message + "\n") @@ -45,23 +41,22 @@ def running(self) -> bool: UNKNOWN = UnitState(active="unknown", enabled="", since="") -def states(names: List[str]) -> Dict[str, UnitState]: +def states(product: Product, names: List[str]) -> Dict[str, UnitState]: """Query every instance in one systemctl call.""" if not names: return {} proc = run( ["systemctl", "show", "--no-pager", "--property=Id,ActiveState,UnitFileState,StateChangeTimestamp", - *[unit(n) for n in names]], + *[product.unit(n) for n in names]], capture=True, ) found = {} for block in proc.stdout.split("\n\n"): props = dict(line.split("=", 1) for line in block.splitlines() if "=" in line) - unit_id = props.get("Id", "") - if not unit_id.startswith("hispec@"): + name = product.instance_of(props.get("Id", "")) + if name is None: continue - name = unit_id[len("hispec@"):-len(".service")] found[name] = UnitState( active=props.get("ActiveState", "unknown"), enabled=props.get("UnitFileState", ""), @@ -76,26 +71,27 @@ def _short_time(stamp: str) -> str: return " ".join(parts[1:3]) if len(parts) >= 3 else stamp -def systemctl(verb: str, name: str) -> bool: +def systemctl(product: Product, verb: str, name: str) -> bool: """start/stop/restart one instance. Output goes to the terminal.""" - return run(["systemctl", verb, unit(name)]).returncode == 0 + return run(["systemctl", verb, product.unit(name)], + prog=product.name).returncode == 0 -def show_status(name: str, lines: int) -> bool: +def show_status(product: Product, name: str, lines: int) -> bool: """Print ``systemctl status`` for one instance, straight to the terminal. False only if systemctl could not report on it at all: an exit code of 3 just means the daemon is not running, which is an answer, not a failure. """ rc = run(["systemctl", "status", "--no-pager", "--full", f"--lines={lines}", - unit(name)]).returncode + product.unit(name)], prog=product.name).returncode return rc in (0, 3) -def enable_helper(args: List[str]) -> subprocess.CompletedProcess: +def enable_helper(product: Product, args: List[str]) -> subprocess.CompletedProcess: """Run the root enable/disable helper without a password prompt. ``-n`` makes sudo fail instead of prompting, so a missing sudoers entry is reported as such rather than as a mystery password request. """ - return run(["sudo", "-n", ENABLE_HELPER, *args], capture=True) + return run(["sudo", "-n", str(product.enable_helper), *args], capture=True) From abd3a73c7441515c4f6c0bc1c9a0df39b8b2cfad Mon Sep 17 00:00:00 2001 From: Mike Langmayr <1809691+mikelangmayr@users.noreply.github.com> Date: Fri, 9 Oct 2026 14:40:40 -0700 Subject: [PATCH 2/3] Take the operator CLI from instrumentctl instead of carrying it --- docs/operations/systemd.md | 15 +- pyproject.toml | 4 +- src/hispec/cli/__init__.py | 129 ++--------- src/hispec/cli/commands.py | 389 --------------------------------- src/hispec/cli/doctor.py | 324 --------------------------- src/hispec/cli/instances.py | 218 ------------------ src/hispec/cli/instrument.toml | 7 +- src/hispec/cli/output.py | 10 - src/hispec/cli/product.py | 110 ---------- src/hispec/cli/units.py | 97 -------- systemd/README.md | 2 +- systemd/bin/hispec | 2 +- 12 files changed, 36 insertions(+), 1271 deletions(-) delete mode 100644 src/hispec/cli/commands.py delete mode 100644 src/hispec/cli/doctor.py delete mode 100644 src/hispec/cli/instances.py delete mode 100644 src/hispec/cli/output.py delete mode 100644 src/hispec/cli/product.py delete mode 100644 src/hispec/cli/units.py diff --git a/docs/operations/systemd.md b/docs/operations/systemd.md index e1682bc..dafa463 100644 --- a/docs/operations/systemd.md +++ b/docs/operations/systemd.md @@ -62,6 +62,13 @@ None of these need `sudo`, as long as you are in the `hispec-ops` group. If you are being asked for a password, see [systemctl keeps asking for a password](#systemctl-keeps-asking-for-a-password). +The `hispec` command itself comes from +[instrumentctl](https://github.com/CaltechOpticalObservatories/instrumentctl), +a dependency shared with other instruments. What makes it speak HISPEC is +`src/hispec/cli/instrument.toml`, which names the unit, the ops group and the +`HISPEC_` variable prefix. So a bug in `deploy` or `doctor` is fixed there, +while anything naming HISPEC is fixed here. + ```bash hispec status # every deployed daemon: running? at boot? since when? hispec status -v hsfei_adc # in detail: its files, PID, exit code, last log lines @@ -525,9 +532,11 @@ You do **not** need it to add a daemon or to pick up code changes; `HISPEC_REPO_DIR` and `HISPEC_VENV_DIR` override the paths. `/usr/local/bin/hispec` is a tiny wrapper that runs the CLI from the venv. The -CLI lives in `src/hispec/cli/`, uses only the standard library so that -`hispec doctor` works on a half-broken venv, and, being an editable install, -follows `git pull`. +commands come from `instrumentctl`; what stays here is +`src/hispec/cli/instrument.toml` and a few lines that pass it. Both are an +editable install, so `git pull` changes HISPEC's own configuration without a +reinstall, while updating the commands themselves means reinstalling the +dependency. ### What the unit does diff --git a/pyproject.toml b/pyproject.toml index ba19909..025e753 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -19,7 +19,7 @@ authors = [ { name="Prakriti Gupta", email="pgupta@astro.caltech.edu" } ] readme = "README.md" -requires-python = ">=3.10" +requires-python = ">=3.11" dependencies = [ "keyring", "pipython", @@ -33,6 +33,7 @@ dependencies = [ # influxdb extra: needed by daemons/generic/keygrabber "libby[influxdb]@git+https://github.com/CaltechOpticalObservatories/libby.git", "hardware_device_base@git+https://github.com/COO-Utilities/hardware_device_base", + "instrumentctl@git+https://github.com/CaltechOpticalObservatories/instrumentctl.git", "camera_daemon@git+https://github.com/COO-Utilities/camera-daemon.git" ] @@ -48,6 +49,7 @@ ignored-modules = [ # Third-party dependencies, not installed in the lint environment "astropy", "camera_daemon", + "instrumentctl", "libby", "matplotlib", "numpy", diff --git a/src/hispec/cli/__init__.py b/src/hispec/cli/__init__.py index d33d834..73a681e 100644 --- a/src/hispec/cli/__init__.py +++ b/src/hispec/cli/__init__.py @@ -1,124 +1,21 @@ -"""``hispec``: deploy, run and inspect the HISPEC daemons on this host. +"""The ``hispec`` command: instrumentctl, driven by this instrument's config. -One command for operators and developers, replacing the separate -hispec-fei-start / hispec-fei-stop / hispec-doctor scripts. Host setup -(users, groups, unit file, polkit, venv) is still systemd/install.sh, run by -an admin; nothing here needs root. +Everything the command does lives in instrumentctl, which knows nothing about +HISPEC. What stays here is the configuration describing this instrument, and +where to find the checkout. """ -from __future__ import annotations - -import argparse +from importlib import resources +from pathlib import Path from typing import List, Optional -from . import commands -from .doctor import doctor -from .instances import Paths -from .product import Product - -TARGETS_HELP = ("instance names (hsfei_adc), subsystems (hsfei, or just fei), " - "or 'all'") - -# Examples read better with real instance names than with placeholders, so -# they are written for this product rather than generated. - -EPILOG = """\ -examples: - hispec status what is deployed here, and is it running? - hispec status -v hsfei_adc why did it stop? PID, exit code, last log lines - hispec deploy hsfei_newthing copy its files from the repo, enable, start - hispec deploy --new the same for every instance not deployed yet - hispec start fei start every deployed hsfei_* daemon - hispec stop fei stop them, in reverse order - hispec restart hsfei_adc e.g. after editing /etc/hispec/hsfei_adc.yaml - hispec logs hsfei_adc -f follow its log - hispec doctor why isn't it working? - -Every subcommand takes --help. Full guide: - https://caltechopticalobservatories.github.io/hispec/operations/systemd.html -""" - - -def _parser(product: Product) -> argparse.ArgumentParser: - parser = argparse.ArgumentParser( - prog=product.name, - description=f"Deploy, run and inspect the {product.name} daemons " - "on this host.", - epilog=EPILOG, - formatter_class=argparse.RawDescriptionHelpFormatter, - ) - sub = parser.add_subparsers(dest="command", metavar="") - - def add(name, func, help_text, description=None): - p = sub.add_parser(name, help=help_text, description=description or help_text, - formatter_class=argparse.RawDescriptionHelpFormatter) - p.set_defaults(func=func) - return p - - p = add("deploy", commands.deploy, - "copy instances from the repo to this host, enable and start them", - "Copy each instance's .env and config from the repo into /etc/hispec, add\n" - "it to the boot set, and start it. No daemon-reload or install.sh needed.\n\n" - "The instance file always follows the repo. A deployed config that differs\n" - "from the repo is kept, since it holds this host's real hardware values;\n" - "pass --force to replace it.") - p.add_argument("names", nargs="*", metavar="name", help="instances to deploy") - p.add_argument("--new", action="store_true", - help="deploy every repo instance not yet deployed here") - p.add_argument("--force", action="store_true", - help="replace deployed configs that differ from the repo") - p.add_argument("--any-host", action="store_true", - help="deploy even if the instance names a different host") - p.add_argument("--no-start", action="store_true", help="do not start them now") - p.add_argument("--no-enable", action="store_true", help="do not start them at boot") - p.add_argument("-n", "--dry-run", action="store_true", help="print what would happen") - - for verb, text in (("start", "start daemons (already-running ones are left alone)"), - ("stop", "stop daemons, in reverse order (they still start at boot)"), - ("restart", "restart daemons, e.g. after a config edit or git pull")): - p = add(verb, commands.act, text) - p.set_defaults(verb=verb) - p.add_argument("targets", nargs="+", metavar="target", help=TARGETS_HELP) - p.add_argument("-n", "--dry-run", action="store_true", help="print what would happen") - - for verb, text in (("enable", "start daemons at every boot (--now: and start now)"), - ("disable", "stop starting daemons at boot (--now: and stop now)")): - p = add(verb, commands.enable, text) - p.set_defaults(verb=verb) - p.add_argument("targets", nargs="+", metavar="target", help=TARGETS_HELP) - now = "start" if verb == "enable" else "stop" - p.add_argument("--now", action="store_true", help=f"also {now} them now") - p.add_argument("-n", "--dry-run", action="store_true", help="print what would happen") - - p = add("status", commands.status, "show each daemon: running? enabled at boot? since when?") - p.add_argument("targets", nargs="*", metavar="target", help=TARGETS_HELP) - p.add_argument("-a", "--all", action="store_true", - help="include instances defined in the repo but not deployed here") - p.add_argument("-v", "--verbose", action="store_true", - help="full detail per daemon: its files, then systemctl status " - "(PID, memory, exit code, recent log lines)") - p.add_argument("-n", "--lines", type=int, default=10, - help="with -v, how many log lines to show (default 10)") - - p = add("logs", commands.logs, "show daemon logs (journalctl)") - p.add_argument("targets", nargs="+", metavar="target", help=TARGETS_HELP) - p.add_argument("-f", "--follow", action="store_true", help="keep printing new lines") - p.add_argument("-n", "--lines", type=int, help="how many lines (default 100)") - p.add_argument("--since", help='e.g. "1 hour ago", today, "2026-09-21 09:00"') - p.add_argument("-p", "--priority", help="e.g. err, warning") +from instrumentctl import run - add("doctor", doctor, "check this host and your account, and say what to fix") - return parser +CONFIG = "instrument.toml" def main(argv: Optional[List[str]] = None) -> int: - """Entry point for the console script.""" - product = Product.load() - parser = _parser(product) - args = parser.parse_args(argv) - if not getattr(args, "func", None): - parser.print_help() - return 2 - try: - return args.func(Paths.from_env(product), args) - except KeyboardInterrupt: - return 130 + """Entry point for the ``hispec`` console script.""" + # parents[3] is the checkout under an editable install, which is what lets + # a `git pull` change the CLI without reinstalling + return run(resources.files(__package__).joinpath(CONFIG), + repo_hint=Path(__file__).resolve().parents[3], argv=argv) diff --git a/src/hispec/cli/commands.py b/src/hispec/cli/commands.py deleted file mode 100644 index 8b6970d..0000000 --- a/src/hispec/cli/commands.py +++ /dev/null @@ -1,389 +0,0 @@ -"""The operator subcommands: deploy, start/stop/restart, enable/disable, -status and logs. ``doctor`` is in its own module.""" -from __future__ import annotations - -import argparse -import os -import shutil -import sys -from pathlib import Path -from typing import List, NamedTuple, Optional, Tuple - -from . import instances as inst -from . import units -from .instances import Paths, TargetError -from .product import Product -from .output import paint - - -def _err(product: Product, message: str) -> None: - print(f"{product.name}: {message}", file=sys.stderr) - - -def _resolve(paths: Paths, targets: List[str], names: List[str]) -> Optional[List[str]]: - """Expand targets, explaining a miss. None means an error was printed.""" - try: - return inst.resolve(paths.product, targets, names) - except TargetError as exc: - target, prog = str(exc), paths.product.name - if target in inst.in_repo(paths): - _err(paths.product, - f"{target} is not deployed on this host; " - f"deploy it with: {prog} deploy {target}") - else: - _err(paths.product, - f"no instance or subsystem matches '{target}' " - f"(see: {prog} status --all)") - return None - - -def _install_file(src: Path, dst: Path, overwrite: bool, dry_run: bool) -> str: - """Copy src to dst, replacing a different dst only if ``overwrite``. - - Returns what happened, for printing. - """ - if dst.exists(): - if dst.read_bytes() == src.read_bytes(): - return "unchanged" - if not overwrite: - return "kept" - outcome = "updated" - else: - outcome = "copied" - if dry_run: - return {"copied": "would copy", "updated": "would update"}[outcome] - # Write beside the target and rename, so a daemon starting meanwhile never - # reads half a file. Group-writable so the rest of the ops group can edit - # it; the directory is setgid, which takes care of the group. - tmp = dst.with_name(f".{dst.name}.{os.getpid()}.tmp") - try: - shutil.copyfile(src, tmp) - os.chmod(tmp, 0o664) - os.replace(tmp, dst) - finally: - if tmp.exists(): - tmp.unlink() - return outcome - - -class _Deployment(NamedTuple): - name: str - env_src: Path - env_dst: Path - config_src: Path - config_dst: Path - - -def _deploy_plan(paths: Paths, names: List[str], - any_host: bool = False) -> Optional[List[_Deployment]]: - """Work out every copy up front, so a bad name copies nothing at all.""" - plan = [] - problems = 0 - role = inst.host_role(paths) - for name in names: - env_src = paths.repo_instances / f"{name}.env" - if not inst.NAME_RE.match(name) or not env_src.is_file(): - _err(paths.product, f"no {env_src}; is '{name}' an instance? " - f"(see: {paths.product.name} status --all)") - problems += 1 - continue - claimed = inst.assigned_host(paths.product, env_src) - if claimed and claimed != role and not any_host: - _err(paths.product, f"{name} belongs on '{claimed}', not '{role}'; " - "deploy it there, or pass --any-host") - problems += 1 - continue - config = inst.read_env(env_src).get(paths.product.env_key("CONFIG"), "") - if not config: - _err(paths.product, - f"{env_src} sets no {paths.product.env_key('CONFIG')}") - problems += 1 - continue - found = inst.repo_configs(paths, name) - if len(found) != 1: - where = ", ".join(str(p) for p in found) or "none" - _err(paths.product, - f"{name}: expected one {paths.repo}/config/*/{name}.yaml, found {where}") - problems += 1 - continue - plan.append(_Deployment(name, env_src, paths.instances / f"{name}.env", - found[0], Path(config))) - return None if problems else plan - - -def _copy(paths: Paths, plan: List[_Deployment], force: bool, dry_run: bool) -> Optional[List[str]]: - """Copy every file in the plan. Returns the instances whose files changed. - - The instance file only points at things, so it always follows the repo. - The config is another matter: once deployed it holds this host's real - ports and addresses, and a silent overwrite would lose them. - """ - updated = [] - for item in plan: - print(item.name) - for src, dst, overwrite in ((item.env_src, item.env_dst, True), - (item.config_src, item.config_dst, force)): - try: - outcome = _install_file(src, dst, overwrite, dry_run) - except OSError as exc: - print() - _err(paths.product, f"cannot write {dst}: {exc.strerror}") - if isinstance(exc, PermissionError): - _err(paths.product, - f"deploying needs {paths.product.ops_group} membership; " - f"run: {paths.product.name} doctor") - return None - note = "" - if outcome == "kept": - where = src.relative_to(paths.repo) - note = paint(f" (differs from {where}; --force replaces it)", "33") - elif outcome == "updated" and item.name not in updated: - updated.append(item.name) - print(f" {outcome:<12} {dst}{note}") - return updated - - -def deploy(paths: Paths, args: argparse.Namespace) -> int: - """Copy instance files and configs from the repo, then enable and start.""" - # Against the repo rather than what is deployed: putting something new on - # this host is the point - names = _resolve(paths, args.names, inst.in_repo(paths)) if args.names else [] - if names is None: - return 1 - if args.new: - have = set(inst.deployed(paths)) - names = [n for n in inst.in_repo(paths) if n not in have] + names - if not names: - print(f"Nothing to deploy: every instance in {paths.repo_instances} " - "is already deployed.") - return 0 - elif not names: - _err(paths.product, "name the instances to deploy, or pass --new") - return 2 - - plan = _deploy_plan(paths, list(dict.fromkeys(names)), args.any_host) - if plan is None: - return 1 - updated = _copy(paths, plan, args.force, args.dry_run) - if updated is None: - return 1 - - names = [item.name for item in plan] - rc = 0 - if not args.no_enable: - rc |= _enable(paths.product, names, [], args.dry_run) - if not args.no_start: - print() - rc |= _act(paths.product, names, "start", args.dry_run) - - running = units.states(paths.product, updated) if updated and not args.dry_run else {} - restart = [n for n, s in running.items() if s.running] - if restart: - print() - print("Already running with the old files; to pick up the new ones:") - print(f" {paths.product.name} restart {' '.join(restart)}") - return rc - - -def _enable(product: Product, names: List[str], options: List[str], - dry_run: bool) -> int: - verb = "disable" if "--disable" in options else "enable" - if dry_run: - print(f"would {verb} at boot: {' '.join(names)}") - return 0 - proc = units.enable_helper(product, [*options, *names]) - if proc.returncode == 0: - print(f"{verb}d at boot: {' '.join(names)}") - return 0 - detail = (proc.stderr or proc.stdout).strip() - if "password is required" in detail or proc.returncode == 127: - _err(product, f"cannot run {product.enable_helper} without a password: {detail}") - _err(product, f"an admin needs to add you to {product.ops_group}, or re-run\n" - f"install.sh; run: {product.name} doctor") - else: - _err(product, detail or f"{product.enable_helper} failed") - return 1 - - -def enable(paths: Paths, args: argparse.Namespace) -> int: - """Add instances to (or remove them from) the set started at boot.""" - names = _resolve(paths, args.targets, inst.deployed(paths)) - if names is None: - return 1 - options = (["--disable"] if args.verb == "disable" else []) + (["--now"] if args.now else []) - return _enable(paths.product, names, options, args.dry_run) - - -_DONE = {"start": "started", "stop": "stopped", "restart": "restarted"} - - -def _act(product: Product, names: List[str], verb: str, dry_run: bool) -> int: - """Apply a verb to every instance, carrying on past failures.""" - state = units.states(product, names) - failed = [] - for name in names: - if verb == "start" and state[name].active == "active": - print(f"{name:<24} already running") - continue - if verb == "stop" and not state[name].running: - print(f"{name:<24} not running") - continue - if dry_run: - print(f"{name:<24} would {verb}") - continue - if units.systemctl(product, verb, name): - print(f"{name:<24} {_DONE[verb]}") - else: - print(f"{name:<24} " + paint("FAILED", "31") - + f" ({product.name} logs {name})") - failed.append(name) - if failed: - print() - _err(product, f"{len(failed)} of {len(names)} failed to {verb}: " - f"{' '.join(failed)}") - return 1 - return 0 - - -def act(paths: Paths, args: argparse.Namespace) -> int: - """start/stop/restart instances or whole subsystems.""" - names = _resolve(paths, args.targets, inst.deployed(paths)) - if names is None: - return 1 - if args.verb == "stop": - # Undo a start in the opposite order: `stop power fei` would - # otherwise cut power before the mechanisms had parked. - names.reverse() - return _act(paths.product, names, args.verb, args.dry_run) - - -_STATE_COLOR = {"active": "32", "failed": "31", "activating": "33", "deactivating": "33"} - - -def status(paths: Paths, args: argparse.Namespace) -> int: - """One line per instance: running? enabled at boot? since when?""" - here = inst.deployed(paths) - repo = inst.in_repo(paths) - # systemd knows nothing about an undeployed instance, so -v ignores --all - universe = sorted(set(here) | set(repo)) if args.all and not args.verbose else here - if args.targets: - names = _resolve(paths, args.targets, universe) - if names is None: - return 1 - else: - names = universe - if not names: - print(f"No instances deployed in {paths.instances}.") - print(f"See what the repo defines with: {paths.product.name} status --all") - return 0 - if args.verbose: - return _status_verbose(paths, names, args.lines) - - rows = _status_rows(paths, names, here) - width = max(len("INSTANCE"), *(len(r[0]) for r in rows)) - print(f"{'INSTANCE':<{width}} {'STATE':<12} {'BOOT':<8} {'SINCE':<19} DAEMON") - for name, active, enabled, since, daemon in rows: - colored = paint(f"{active:<12}", _STATE_COLOR.get(active, "")) - print(f"{name:<{width}} {colored} {enabled:<8} {since:<19} {daemon}") - - if not args.all and not args.targets: - missing = [n for n in repo if n not in here] - if missing: - print() - print(f"{len(missing)} more in the repo, not deployed here " - f"({paths.product.name} status --all; " - f"{paths.product.name} deploy --new)") - return 0 - - -def _status_rows(paths: Paths, names: List[str], here: List[str]) -> List[Tuple[str, ...]]: - state = units.states(paths.product, [n for n in names if n in here]) - rows = [] - for name in names: - if name not in here: - daemon = _daemon(paths.product, paths.repo_instances / f"{name}.env") - rows.append((name, "not deployed", "", "", daemon)) - continue - s = state[name] - since = s.since if s.active != "inactive" else "" - rows.append((name, s.active, s.enabled, since, - _daemon(paths.product, paths.instances / f"{name}.env"))) - return rows - - -def _status_verbose(paths: Paths, names: List[str], lines: int) -> int: - """For each instance, the files behind it and then systemctl status.""" - failed = [] - for i, name in enumerate(names): - if i: - print() - print(paint(name, "1")) - for label, value in _files(paths, name): - print(f" {label:<9} {value}") - print() - sys.stdout.flush() # systemctl writes to the same terminal - if not units.show_status(paths.product, name, lines): - failed.append(name) - if failed: - print() - _err(paths.product, f"systemctl could not report on: {' '.join(failed)} " - f"(run: {paths.product.name} doctor)") - return 1 - return 0 - - -def _files(paths: Paths, name: str) -> List[Tuple[str, str]]: - """What an operator checks first: which daemon, which config, is it the repo's?""" - env_file = paths.instances / f"{name}.env" - try: - env = inst.read_env(env_file) - except OSError as exc: - return [("instance", f"{env_file} " + paint(f"(unreadable: {exc.strerror})", "31"))] - rows = [("daemon", env.get(paths.product.env_key("DAEMON"), "?")), - ("instance", str(env_file))] - config = env.get(paths.product.env_key("CONFIG"), "") - if not config: - rows.append(("config", paint("not set in the instance file", "31"))) - return rows - note = "" - found = inst.repo_configs(paths, name) - if not Path(config).is_file(): - note = paint(" (missing)", "31") - elif len(found) == 1 and Path(config).read_bytes() != found[0].read_bytes(): - note = paint(f" (differs from {found[0].relative_to(paths.repo)})", "33") - rows.append(("config", config + note)) - return rows - - -def _daemon(product: Product, env: Path) -> str: - try: - return inst.read_env(env).get(product.env_key("DAEMON"), "?") - except OSError: - return "?" - - -def logs(paths: Paths, args: argparse.Namespace) -> int: - """Hand over to journalctl for the selected instances.""" - known = sorted(set(inst.deployed(paths)) | set(inst.in_repo(paths))) - names = _resolve(paths, args.targets, known) - if names is None: - return 1 - cmd = ["journalctl"] - for name in names: - cmd += ["-u", paths.product.unit(name)] - if args.follow: - cmd.append("-f") - if args.lines is not None: - cmd += ["-n", str(args.lines)] - elif not args.since and not args.follow: - cmd += ["-n", "100"] - if args.since: - cmd += ["--since", args.since] - if args.priority: - cmd += ["-p", args.priority] - try: - os.execvp(cmd[0], cmd) - except FileNotFoundError: - _err(paths.product, - "journalctl not found; this host does not look like it runs systemd") - return 1 diff --git a/src/hispec/cli/doctor.py b/src/hispec/cli/doctor.py deleted file mode 100644 index 1a9ceef..0000000 --- a/src/hispec/cli/doctor.py +++ /dev/null @@ -1,324 +0,0 @@ -"""``doctor``: check this host's setup and say what is wrong. -""" -from __future__ import annotations - -import argparse -import filecmp -import grp -import os -import pwd -import socket -import stat -from pathlib import Path -from typing import Optional - -from . import instances as inst -from . import units -from .instances import Paths -from .output import paint - -LOG_DIR_TEMPLATE = "/var/log/{name}" - - -class Report: - """Collects ok/FAIL/warn lines; FAILs decide the exit status.""" - - def __init__(self) -> None: - self.problems = 0 - - @staticmethod - def section(title: str) -> None: - """Start a group of checks.""" - print() - print(title) - - @staticmethod - def ok(text: str) -> None: - """A check that passed.""" - print(f" {paint('ok', '32')} {text}") - - def bad(self, text: str) -> None: - """A check that failed: something an operator needs is missing.""" - print(f" {paint('FAIL', '31')} {text}") - self.problems += 1 - - @staticmethod - def warn(text: str) -> None: - """Worth knowing, but not broken.""" - print(f" {paint('warn', '33')} {text}") - - @staticmethod - def hint(text: str) -> None: - """The command that fixes the line above.""" - print(f" → {text}") - - -def _my_groups() -> set: - names = set() - for gid in os.getgroups(): - try: - names.add(grp.getgrgid(gid).gr_name) - except KeyError: - pass - return names - - -def _is_file(path: Optional[Path]) -> bool: - """Whether path is a file, treating one we may not stat as absent.""" - if path is None: - return False - try: - return path.is_file() - except OSError: - return False - - -def _group_exists(name: str) -> bool: - try: - grp.getgrnam(name) - return True - except KeyError: - return False - - -def _owner(path: Path) -> str: - st = path.stat() - try: - user = pwd.getpwuid(st.st_uid).pw_name - except KeyError: - user = str(st.st_uid) - try: - group = grp.getgrgid(st.st_gid).gr_name - except KeyError: - group = str(st.st_gid) - return f"{user}:{group} {stat.S_IMODE(st.st_mode):o}" - - -def _version_tuple(text: str) -> tuple: - try: - return tuple(int(p) for p in text.split(".")) - except ValueError: - return () - - -def _account(r: Report, paths: Paths) -> None: - product = paths.product - group = product.ops_group - r.section("Your account") - me = pwd.getpwuid(os.geteuid()).pw_name - groups = _my_groups() - if os.geteuid() == 0: - r.warn("running as root, so the permission checks below say nothing about operators") - r.hint(f"re-run as your own user: {product.name} doctor") - if not _group_exists(group): - r.bad(f"group {group} does not exist") - r.hint("an admin needs to run systemd/install.sh on this host") - elif group in groups: - r.ok(f"you are in {group}") - else: - r.bad(f"you are NOT in {group}, which is why systemctl asks for a password") - r.hint(f"admin: sudo {paths.repo}/systemd/install.sh {me}") - r.hint(f"then log out and back in (or run: newgrp {group})") - if "systemd-journal" in groups: - r.ok("you are in systemd-journal (can read other units' logs)") - else: - r.warn(f"you are not in systemd-journal; {product.name} logs will be empty") - r.hint(f"admin: sudo usermod -aG systemd-journal {me}") - - -def _install_hint(paths: Paths) -> str: - return f"admin: sudo {paths.repo}/systemd/install.sh" - - -def _unit_and_polkit(r: Report, paths: Paths) -> None: - product = paths.product - install = _install_hint(paths) - unit_file = product.unit_template - polkit_rule = product.polkit_rule - r.section("Unit and polkit") - repo_unit = paths.repo / "systemd" / unit_file.name - if unit_file.is_file(): - r.ok(f"{unit_file} installed") - if repo_unit.is_file() and not filecmp.cmp(unit_file, repo_unit, shallow=False): - r.warn(f"installed unit differs from {repo_unit}") - r.hint(install) - else: - r.bad(f"{unit_file} is missing") - r.hint(install) - if _is_file(product.legacy_unit_path): - r.warn(f"the old {product.legacy_unit} is still installed") - r.hint(f"{install} # migrates instances to {product.name}@") - try: - rule = polkit_rule.read_text(encoding="utf-8") - except FileNotFoundError: - r.bad(f"{polkit_rule} is missing, so every start/stop will prompt") - r.hint(install) - except OSError: - # rules.d is 0750 root:polkitd on some hosts, so an operator cannot - # even stat inside it; unreadable means unknown, not missing - r.warn(f"cannot read {polkit_rule}, so the polkit rule is unchecked") - r.hint(f"sudo {product.name} doctor") - else: - r.ok(f"{polkit_rule} installed") - if f'indexOf("{product.name}@")' not in rule: - r.bad(f"the installed polkit rule does not match {product.name}@ units") - r.hint(f"it is probably the pre-rename copy; {install}") - legacy_rule = product.legacy_polkit_path - if _is_file(legacy_rule): - r.warn(f"the old {legacy_rule.name} is still installed " - "(harmless, matches nothing now)") - r.hint(f"admin: sudo rm {legacy_rule}") - _polkit_daemon(r) - - -def _polkit_daemon(r: Report) -> None: - proc = units.run(["pkaction", "--version"], capture=True) - if proc.returncode == 127: - r.warn("pkaction not found; cannot check the polkit version") - else: - version = (proc.stdout.split() or [""])[-1] - # JavaScript .rules files need polkit 0.106+; older polkit ignores - # them silently and every operator gets an auth prompt instead. - if _version_tuple(version) >= (0, 106): - r.ok(f"polkit {version} supports JavaScript .rules") - else: - r.bad(f"polkit {version or 'unknown'} is too old for .rules files (need 0.106+)") - r.hint("the rule is ignored silently; operators will keep getting auth prompts") - if any(units.run(["systemctl", "is-active", "--quiet", svc]).returncode == 0 - for svc in ("polkit.service", "polkitd.service")): - r.ok("polkit is running") - else: - r.warn("polkit does not look like it is running") - r.hint("admin: sudo systemctl start polkit") - - -def _sudo(r: Report, paths: Paths) -> None: - product = paths.product - install = _install_hint(paths) - sudoers, helper = product.sudoers, str(product.enable_helper) - if not sudoers.is_file(): - r.warn(f"no {sudoers}; enabling an instance at boot needs an admin") - r.hint(install) - elif os.geteuid() != 0 and product.ops_group in _my_groups(): - if units.run(["sudo", "-n", "-l", helper], capture=True).returncode == 0: - r.ok(f"you can run {product.name} enable without a password") - else: - r.bad(f"sudo will not run {helper} for you without a password") - r.hint(f"{sudoers} or {helper} is out of date; {install}") - else: - r.ok(f"{sudoers} installed (passwordless {product.name} enable)") - - -def _paths(r: Report, paths: Paths) -> None: - install = _install_hint(paths) - r.section("Paths") - log_dir = Path(LOG_DIR_TEMPLATE.format(name=paths.product.name)) - for directory in (paths.etc, paths.instances, log_dir): - if directory.is_dir(): - r.ok(f"{directory} exists ({_owner(directory)})") - else: - r.bad(f"{directory} is missing") - r.hint(install) - if paths.repo.is_dir(): - r.ok(f"{paths.repo} checked out") - else: - r.bad(f"{paths.repo} is missing") - if (paths.venv / "bin" / "python3").exists(): - r.ok(f"{paths.venv} venv present") - else: - r.bad(f"{paths.venv}/bin/python3 is missing") - r.hint(install) - - -def _deployed(r: Report, paths: Paths) -> None: - r.section("Deployed instances") - names = inst.deployed(paths) - if not names: - r.warn(f"no instance files in {paths.instances} — nothing is deployed on this host") - r.hint(f"{paths.product.name} deploy " - f"# or: {paths.product.name} deploy --new") - return - product = paths.product - state = units.states(product, names) - for name in names: - env_file = paths.instances / f"{name}.env" - env = inst.read_env(env_file) - script = env.get(product.env_key("DAEMON"), "") - config = env.get(product.env_key("CONFIG"), "") - repo_env = paths.repo_instances / f"{name}.env" - if not script: - r.bad(f"{name}: {env_file} sets no {product.env_key('DAEMON')}") - elif not (paths.repo / "daemons" / script).is_file(): - r.bad(f"{name}: daemon script {paths.repo}/daemons/{script} does not exist") - elif not config or not Path(config).is_file(): - r.bad(f"{name}: config {config or ''} does not exist") - r.hint(f"{product.name} deploy {name}") - elif state[name].active == "active": - r.ok(f"{name}: running") - else: - r.warn(f"{name}: {state[name].active}") - r.hint(f"{product.name} start {name}") - r.hint(f"{product.name} logs {name}") - if repo_env.is_file() and not filecmp.cmp(env_file, repo_env, shallow=False): - r.warn(f"{name}: deployed instance file differs from {repo_env}") - r.hint(f"{product.name} deploy {name} " - "# updates the instance file, keeps the config") - elif not repo_env.is_file(): - r.warn(f"{name}: deployed here but not defined in {paths.repo_instances}") - - -def _secrets(r: Report, paths: Paths) -> None: - r.section("Credentials") - needed = inst.required_secrets(paths) - present = inst.secrets_set(paths) - if present is None: - r.warn(f"cannot read {paths.secrets}, so the credentials below are unchecked") - r.hint(f"sudo {paths.product.name} doctor") - return - for variable in sorted(needed): - if present.get(variable): - r.ok(f"{variable} is set") - else: - r.bad(f"{variable} is unset, needed by {'; '.join(needed[variable])}") - r.hint(f"add {variable}= to {paths.secrets}, then restart that daemon") - - -def _placement(r: Report, paths: Paths) -> None: - """Flag instances running on the wrong host, or claiming no host at all. - - Only looks at what is deployed: which of the repo's instances a host runs - is an operational decision, and ``status`` already lists the rest. - """ - r.section("Host assignment") - role = inst.host_role(paths) - r.ok(f"this host is '{role}'") - unassigned = [] - for name in inst.deployed(paths): - claimed = inst.assigned_host(paths.product, paths.instances / f"{name}.env") - if claimed is None: - unassigned.append(name) - elif claimed != role: - r.bad(f"{name}: deployed here but assigned to '{claimed}'") - if unassigned: - r.warn(f"no {paths.product.env_key('HOST')}, so nothing says where they belong: " - f"{' '.join(unassigned)}") - - -def doctor(paths: Paths, _args: argparse.Namespace) -> int: - """Run every check and exit 1 if any FAILed.""" - me = pwd.getpwuid(os.geteuid()).pw_name - print(f"{paths.product.name} doctor: {socket.gethostname()}, running as {me}") - r = Report() - _account(r, paths) - _unit_and_polkit(r, paths) - _sudo(r, paths) - _paths(r, paths) - _deployed(r, paths) - _placement(r, paths) - _secrets(r, paths) - print() - if r.problems == 0: - print("No problems found.") - return 0 - print(f"{r.problems} problem(s) found — see the FAIL lines above.") - return 1 diff --git a/src/hispec/cli/instances.py b/src/hispec/cli/instances.py deleted file mode 100644 index c6efb39..0000000 --- a/src/hispec/cli/instances.py +++ /dev/null @@ -1,218 +0,0 @@ -""" -An instance is a name, e.g. ``hsfei_adc``, with three files behind it: - -- ``/systemd/instances/.env``, which says which daemon script to - run and where its config is deployed; -- ``/config//.yaml``, the config as committed; -- their deployed copies, ``/instances/.env`` and whatever path the - ``.env`` gives as the config. - -Deployed means the ``.env`` is in ``/instances/``, since that is what the -template unit reads. -""" -from __future__ import annotations - -import os -import re -import socket -from dataclasses import dataclass -from pathlib import Path -from typing import Any, Dict, Iterable, Iterator, List, Mapping, Optional, Tuple - -import yaml - -from .product import Product - -NAME_RE = re.compile(r"^[a-z][a-z0-9_]*$") - - -def _default_repo_dir(product: Product) -> Path: - # install.sh does an editable install, so this file is inside the checkout - # the daemons run from. Anything else (a wheel) gets the standard path. - here = Path(__file__).resolve().parents[3] - if (here / "systemd" / "instances").is_dir(): - return here - return Path(f"/opt/{product.name}/app") - - -@dataclass(frozen=True) -class Paths: - """The filesystem layout, overridable for testing and odd hosts. - - Carries the product so every command reaches it through the one argument - they all already take. - """ - - product: Product - repo: Path - venv: Path - etc: Path - - @classmethod - def from_env(cls, product: Optional[Product] = None, - environ: Optional[Mapping[str, str]] = None) -> "Paths": - """Build from _REPO_DIR / _VENV_DIR / _ETC_DIR.""" - product = product or Product.load() - environ = os.environ if environ is None else environ - repo = environ.get(product.env_key("REPO_DIR")) - return cls( - product=product, - repo=Path(repo) if repo else _default_repo_dir(product), - venv=Path(environ.get(product.env_key("VENV_DIR"), - f"/opt/{product.name}/venv")), - etc=Path(environ.get(product.env_key("ETC_DIR"), - f"/etc/{product.name}")), - ) - - @property - def instances(self) -> Path: - """Deployed instance files, read by the template unit.""" - return self.etc / "instances" - - @property - def repo_instances(self) -> Path: - """Instance files as committed.""" - return self.repo / "systemd" / "instances" - - @property - def secrets(self) -> Path: - """Shared secrets, read by every unit.""" - return self.etc / "secrets.env" - - -def host_role(paths: Paths) -> str: - """Return this host's role, from /host or its short hostname.""" - path = paths.etc / "host" - if path.is_file(): - role = path.read_text(encoding="utf-8").strip() - if role: - return role - return socket.gethostname().split(".")[0] - - -def assigned_host(product: Product, env_file: Path) -> Optional[str]: - """Return the host an instance file claims, or None when it claims none.""" - if not env_file.is_file(): - return None - return read_env(env_file).get(product.env_key("HOST")) or None - - -class TargetError(Exception): - """A name on the command line matched nothing.""" - - -def _names_in(directory: Path) -> List[str]: - if not directory.is_dir(): - return [] - return sorted(p.stem for p in directory.glob("*.env") if p.is_file()) - - -def deployed(paths: Paths) -> List[str]: - """Instances deployed on this host, sorted.""" - return _names_in(paths.instances) - - -def in_repo(paths: Paths) -> List[str]: - """Instances defined in the repo, sorted.""" - return _names_in(paths.repo_instances) - - -def repo_configs(paths: Paths, name: str) -> List[Path]: - """Committed configs for an instance. Exactly one is the healthy case.""" - return sorted((paths.repo / "config").glob(f"*/{name}.yaml")) - - -def read_env(path: Path) -> Dict[str, str]: - """Parse a systemd EnvironmentFile: KEY=VALUE lines, # comments.""" - values = {} - for line in path.read_text(encoding="utf-8").splitlines(): - line = line.strip() - if not line or line.startswith("#") or "=" not in line: - continue - key, value = line.split("=", 1) - value = value.strip() - if len(value) >= 2 and value[0] == value[-1] and value[0] in "'\"": - value = value[1:-1] - values[key.strip()] = value - return values - - -def _secret_keys(node: Any, suffix: str, - prefix: str = "") -> Iterator[Tuple[str, str]]: - """Yield (config key, variable name) for every credential key in a config.""" - if not isinstance(node, Mapping): - return - for key, value in node.items(): - path = f"{prefix}.{key}" if prefix else str(key) - if str(key).endswith(suffix) and isinstance(value, str) and value: - yield path, value - else: - yield from _secret_keys(value, suffix, path) - - -def _broker_secrets() -> List[str]: - """Return the broker credential variables every daemon needs.""" - # Deferred: importing hispec.daemon pulls in libby, and the CLI has to keep - # working on a host whose venv is broken, which is when doctor is run - try: - from ..daemon import ( # pylint: disable=import-outside-toplevel - BROKER_PASSWORD_ENV, BROKER_USER_ENV, - ) - except ImportError: - return [] - return [BROKER_USER_ENV, BROKER_PASSWORD_ENV] - - -def required_secrets(paths: Paths) -> Dict[str, List[str]]: - """Map each variable this host needs to the reasons it is needed. - - Derived from the deployed configs rather than a list someone maintains, - which is how the installer's own list went stale. - """ - needed: Dict[str, List[str]] = { - name: ["every daemon (broker login)"] for name in _broker_secrets() - } - for instance in deployed(paths): - env = read_env(paths.instances / f"{instance}.env") - config = env.get(paths.product.env_key("CONFIG"), "") - if not config or not Path(config).is_file(): - continue - parsed = yaml.safe_load(Path(config).read_text(encoding="utf-8")) or {} - for key, variable in _secret_keys(parsed, paths.product.secret_key_suffix): - needed.setdefault(variable, []).append(f"{instance} ({key})") - return needed - - -def secrets_set(paths: Paths) -> Optional[Dict[str, str]]: - """Return the variables secrets.env defines, or None if it cannot be read.""" - try: - return read_env(paths.secrets) - except OSError: - return None - - -def resolve(product: Product, targets: Iterable[str], names: List[str]) -> List[str]: - """Expand command-line targets against the instances in ``names``. - - A target is an instance name (``hsfei_adc``), a subsystem prefix - (``hsfei``, or just ``fei`` where the product sets one), or ``all``. - Targets keep their command-line order, so ``start power fei`` powers up - before it starts the mechanisms. Duplicates are dropped, and a target - matching nothing is an error. - """ - shorthand = product.subsystem_prefix - selected: List[str] = [] - for target in targets: - if target == "all": - hits = list(names) - elif target in names: - hits = [target] - else: - prefix = target.rstrip("_") + "_" - hits = [n for n in names if n.startswith(prefix)] - if not hits and shorthand and not target.startswith(shorthand): - hits = [n for n in names if n.startswith(shorthand + prefix)] - if not hits: - raise TargetError(target) - selected.extend(n for n in hits if n not in selected) - return selected diff --git a/src/hispec/cli/instrument.toml b/src/hispec/cli/instrument.toml index 2b9830f..fe46743 100644 --- a/src/hispec/cli/instrument.toml +++ b/src/hispec/cli/instrument.toml @@ -1,4 +1,4 @@ -# Identity of the instrument this CLI drives. Read by hispec.cli.product. +# Identity of this instrument, read by instrumentctl via hispec.cli.main. # # Paths follow from `name`: /opt//app, /opt//venv, /etc/, # @.service, /usr/local/sbin/-enable. Operators override the @@ -18,6 +18,11 @@ env_prefix = "HISPEC" subsystem_prefix = "hs" docs_url = "https://caltechopticalobservatories.github.io/hispec/operations/systemd.html" +[help] +# --help reads better with names an operator can actually type +example_instance = "hsfei_adc" +example_subsystem = "fei" + [secrets] # A config key ending in this names the variable holding a credential, never # the credential itself diff --git a/src/hispec/cli/output.py b/src/hispec/cli/output.py deleted file mode 100644 index f1b0f16..0000000 --- a/src/hispec/cli/output.py +++ /dev/null @@ -1,10 +0,0 @@ -"""Terminal colour, only when stdout is a terminal.""" -import os -import sys - - -def paint(text: str, code: str) -> str: - """Wrap text in an ANSI colour code, or return it as is when piped.""" - if not code or not sys.stdout.isatty() or os.environ.get("NO_COLOR"): - return text - return f"\033[{code}m{text}\033[0m" diff --git a/src/hispec/cli/product.py b/src/hispec/cli/product.py deleted file mode 100644 index e9e2eec..0000000 --- a/src/hispec/cli/product.py +++ /dev/null @@ -1,110 +0,0 @@ -"""Which instrument this CLI drives. - -Everything that differs between instruments is here, so the rest of the package -is about deploying daemons rather than about HISPEC specifically. The values -come from ``instrument.toml`` beside this module; a second instrument ships its -own and reuses the code unchanged. - -These are identity, not operator settings. Where an operator needs to override -something, such as a non-standard checkout path, that stays an environment -variable on ``Paths``. -""" -from __future__ import annotations - -import tomllib -from dataclasses import dataclass, field -from importlib import resources -from pathlib import Path -from typing import Optional - -CONFIG_NAME = "instrument.toml" - -SYSTEMD_DIR = Path("/etc/systemd/system") -POLKIT_RULES_DIR = Path("/etc/polkit-1/rules.d") -SUDOERS_DIR = Path("/etc/sudoers.d") -SBIN_DIR = Path("/usr/local/sbin") - - -# Each attribute is one fact about the instrument rather than hidden state -@dataclass(frozen=True) -class Product: # pylint: disable=too-many-instance-attributes - """One instrument's names, and the paths and units derived from them.""" - - name: str - user: str - ops_group: str - env_prefix: str - subsystem_prefix: str - secret_key_suffix: str = "_env" - docs_url: str = "" - # Units and polkit rules this product used to install, reported by doctor - # so a host that predates a rename can be recognised - legacy_unit: Optional[str] = field(default=None) - legacy_polkit_rule: Optional[str] = field(default=None) - - @classmethod - def load(cls) -> "Product": - """Read ``instrument.toml`` from beside this module.""" - with resources.files(__package__).joinpath(CONFIG_NAME).open("rb") as handle: - data = tomllib.load(handle) - instrument = data["instrument"] - secrets = data.get("secrets", {}) - legacy = data.get("legacy", {}) - return cls( - name=instrument["name"], - user=instrument.get("user", instrument["name"]), - ops_group=instrument.get("ops_group", f"{instrument['name']}-ops"), - env_prefix=instrument.get("env_prefix", instrument["name"].upper()), - subsystem_prefix=instrument.get("subsystem_prefix", ""), - secret_key_suffix=secrets.get("key_suffix", "_env"), - docs_url=instrument.get("docs_url", ""), - legacy_unit=legacy.get("unit"), - legacy_polkit_rule=legacy.get("polkit_rule"), - ) - - def unit(self, instance: str) -> str: - """The systemd unit for an instance.""" - return f"{self.name}@{instance}.service" - - def instance_of(self, unit_id: str) -> Optional[str]: - """The instance a unit id names, or None if it is not one of ours.""" - prefix, suffix = f"{self.name}@", ".service" - if not unit_id.startswith(prefix) or not unit_id.endswith(suffix): - return None - return unit_id[len(prefix):-len(suffix)] - - def env_key(self, name: str) -> str: - """An instance file's variable, e.g. ``HISPEC_DAEMON`` for ``DAEMON``.""" - return f"{self.env_prefix}_{name}" - - @property - def unit_template(self) -> Path: - """The installed template unit every instance runs from.""" - return SYSTEMD_DIR / f"{self.name}@.service" - - @property - def polkit_rule(self) -> Path: - """The rule granting the ops group start/stop without a password.""" - return POLKIT_RULES_DIR / f"49-{self.name}.rules" - - @property - def sudoers(self) -> Path: - """The drop-in granting passwordless use of the enable helper.""" - return SUDOERS_DIR / f"{self.name}-ops" - - @property - def enable_helper(self) -> Path: - """Root helper behind ``enable``, which polkit cannot scope per unit.""" - return SBIN_DIR / f"{self.name}-enable" - - @property - def legacy_unit_path(self) -> Optional[Path]: - """Where a pre-rename template unit would still be installed.""" - return SYSTEMD_DIR / self.legacy_unit if self.legacy_unit else None - - @property - def legacy_polkit_path(self) -> Optional[Path]: - """Where a pre-rename polkit rule would still be installed.""" - if not self.legacy_polkit_rule: - return None - return POLKIT_RULES_DIR / self.legacy_polkit_rule diff --git a/src/hispec/cli/units.py b/src/hispec/cli/units.py deleted file mode 100644 index e9b26f6..0000000 --- a/src/hispec/cli/units.py +++ /dev/null @@ -1,97 +0,0 @@ -"""Thin wrappers around systemctl and the root enable helper. - -Everything that touches systemd goes through ``run``, so tests can replace it. -""" -from __future__ import annotations - -import subprocess -import sys -from dataclasses import dataclass -from typing import Dict, List, Sequence - -from .product import Product - - -def run(cmd: Sequence[str], capture: bool = False, - prog: str = "") -> subprocess.CompletedProcess: - """Run a command, inheriting the terminal unless ``capture``.""" - try: - return subprocess.run(list(cmd), check=False, text=True, capture_output=capture) - except FileNotFoundError: - message = f"{cmd[0]}: not found" - if not capture: - print(f"{prog or cmd[0]}: {message}", file=sys.stderr) - return subprocess.CompletedProcess(list(cmd), 127, "", message + "\n") - - -@dataclass(frozen=True) -class UnitState: - """What systemd says about one instance.""" - - active: str # active, inactive, failed, activating, deactivating - enabled: str # enabled, disabled, or empty if systemd does not know it - since: str # when ``active`` last changed, as systemd prints it - - @property - def running(self) -> bool: - """Up, or on its way up.""" - return self.active in ("active", "activating", "reloading") - - -UNKNOWN = UnitState(active="unknown", enabled="", since="") - - -def states(product: Product, names: List[str]) -> Dict[str, UnitState]: - """Query every instance in one systemctl call.""" - if not names: - return {} - proc = run( - ["systemctl", "show", "--no-pager", - "--property=Id,ActiveState,UnitFileState,StateChangeTimestamp", - *[product.unit(n) for n in names]], - capture=True, - ) - found = {} - for block in proc.stdout.split("\n\n"): - props = dict(line.split("=", 1) for line in block.splitlines() if "=" in line) - name = product.instance_of(props.get("Id", "")) - if name is None: - continue - found[name] = UnitState( - active=props.get("ActiveState", "unknown"), - enabled=props.get("UnitFileState", ""), - since=_short_time(props.get("StateChangeTimestamp", "")), - ) - return {n: found.get(n, UNKNOWN) for n in names} - - -def _short_time(stamp: str) -> str: - # "Mon 2026-09-21 09:14:02 HST" -> "2026-09-21 09:14:02" - parts = stamp.split() - return " ".join(parts[1:3]) if len(parts) >= 3 else stamp - - -def systemctl(product: Product, verb: str, name: str) -> bool: - """start/stop/restart one instance. Output goes to the terminal.""" - return run(["systemctl", verb, product.unit(name)], - prog=product.name).returncode == 0 - - -def show_status(product: Product, name: str, lines: int) -> bool: - """Print ``systemctl status`` for one instance, straight to the terminal. - - False only if systemctl could not report on it at all: an exit code of 3 - just means the daemon is not running, which is an answer, not a failure. - """ - rc = run(["systemctl", "status", "--no-pager", "--full", f"--lines={lines}", - product.unit(name)], prog=product.name).returncode - return rc in (0, 3) - - -def enable_helper(product: Product, args: List[str]) -> subprocess.CompletedProcess: - """Run the root enable/disable helper without a password prompt. - - ``-n`` makes sudo fail instead of prompting, so a missing sudoers entry - is reported as such rather than as a mystery password request. - """ - return run(["sudo", "-n", str(product.enable_helper), *args], capture=True) diff --git a/systemd/README.md b/systemd/README.md index 58ea573..21de388 100644 --- a/systemd/README.md +++ b/systemd/README.md @@ -14,7 +14,7 @@ directory. | `hispec@.service` | The template unit. One file runs every daemon; `hispec@` is an instance of it. Installed to `/etc/systemd/system/`. | | `instances/.env` | Per-instance settings: `HISPEC_DAEMON` (script, relative to `daemons/`) and `HISPEC_CONFIG` (deployed config path). Deployed to `/etc/hispec/instances/`. | | `polkit/49-hispec.rules` | Lets `hispec-ops` members start/stop/restart `hispec@*` without sudo. Installed to `/etc/polkit-1/rules.d/`. | -| `bin/hispec` | Wrapper that runs the `hispec` CLI (`src/hispec/cli/`) from the venv. Installed to `/usr/local/bin/`. | +| `bin/hispec` | Wrapper that runs the `hispec` CLI from the venv. Installed to `/usr/local/bin/`. | | `bin/hispec-enable` | Root helper behind `hispec enable` / `hispec deploy`: enables/disables instances at boot, which `systemctl enable` cannot be granted per-unit. Installed to `/usr/local/sbin/`, invoked via `sudo -n` by a NOPASSWD drop-in. | | `install.sh` | Host setup for admins / IT: users, groups, directories, venv, and all of the above. Idempotent, run as root. Not needed to add a daemon. | diff --git a/systemd/bin/hispec b/systemd/bin/hispec index 924c053..60e8383 100755 --- a/systemd/bin/hispec +++ b/systemd/bin/hispec @@ -1,5 +1,5 @@ #!/usr/bin/env bash -# Front door for the hispec CLI (src/hispec/cli), installed to /usr/local/bin. +# Front door for the hispec CLI, installed to /usr/local/bin. # # The CLI itself lives in the venv, installed editable from the repo, so a # `git pull` updates it with no reinstall. This wrapper only exists so that a From 417579fa8007a893a77ada55905dbbb42dda5eac Mon Sep 17 00:00:00 2001 From: Mike Langmayr <1809691+mikelangmayr@users.noreply.github.com> Date: Fri, 9 Oct 2026 15:30:42 -0700 Subject: [PATCH 3/3] Name subsystems in full now the shorthand is gone --- docs/operations/systemd.md | 22 +++++++++++----------- src/hispec/cli/instrument.toml | 4 +--- systemd/README.md | 2 +- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/docs/operations/systemd.md b/docs/operations/systemd.md index dafa463..293cc3e 100644 --- a/docs/operations/systemd.md +++ b/docs/operations/systemd.md @@ -105,18 +105,18 @@ still work if you prefer them: `systemctl start hispec@hsfei_adc`, ### Starting and stopping a whole subsystem -Wherever `hispec` takes an instance name it also takes a subsystem, written -either in full or without the `hs`, or `all`: +Wherever `hispec` takes an instance name it also takes the prefix a subsystem's +instances share, or `all`: ```bash -hispec start fei # every deployed hsfei_* daemon -hispec stop fei # all of them, in reverse order -hispec restart cal # every hscal_* daemon -hispec start power fei # PDUs first, then the FEI -hispec stop power fei # FEI first, then the PDUs +hispec start hsfei # every deployed hsfei_* daemon +hispec stop hsfei # all of them, in reverse order +hispec restart hscal # every hscal_* daemon +hispec start hspower hsfei # PDUs first, then the FEI +hispec stop hspower hsfei # FEI first, then the PDUs hispec start hsfei_adc hsfei_ms # or name the ones you want -hispec start --dry-run fei # print what it would do, do nothing -hispec status fei # just the FEI +hispec start --dry-run hsfei # print what it would do, do nothing +hispec status hsfei # just the FEI ``` Names are matched against what is deployed in `/etc/hispec/instances/`, so @@ -190,7 +190,7 @@ subsystem, or everything the repo defines that this host does not have yet: ```bash hispec deploy hsfei_adc hsfei_ms -hispec deploy fei # every hsfei_* the repo defines +hispec deploy hsfei # every hsfei_* the repo defines hispec deploy all --no-start --no-enable # refresh the files, change nothing else hispec deploy --new # e.g. after a git pull added instances hispec deploy --new --dry-run # see what that would do first @@ -524,7 +524,7 @@ You do **not** need it to add a daemon or to pick up code changes; - installs `hispec@.service` (then runs `daemon-reload`), the polkit rule, the sudoers drop-in, `/usr/local/bin/hispec` and `/usr/local/sbin/hispec-enable`; - removes the retired `hispec-fei-start`, `hispec-fei-stop` and `hispec-doctor` - scripts (now `hispec start fei`, `hispec stop fei` and `hispec doctor`); + scripts (now `hispec start hsfei`, `hispec stop hsfei` and `hispec doctor`); - migrates any instance still running under the old `hispec-daemon@` name, preserving whether it was enabled and whether it was up; - lists instances that are deployed but not set to start at boot. diff --git a/src/hispec/cli/instrument.toml b/src/hispec/cli/instrument.toml index fe46743..d656d31 100644 --- a/src/hispec/cli/instrument.toml +++ b/src/hispec/cli/instrument.toml @@ -14,14 +14,12 @@ user = "hispec" ops_group = "hispec-ops" # Instance files set HISPEC_DAEMON, HISPEC_CONFIG and HISPEC_HOST env_prefix = "HISPEC" -# So `fei` finds hsfei_* as well as fei_* -subsystem_prefix = "hs" docs_url = "https://caltechopticalobservatories.github.io/hispec/operations/systemd.html" [help] # --help reads better with names an operator can actually type example_instance = "hsfei_adc" -example_subsystem = "fei" +example_subsystem = "hsfei" [secrets] # A config key ending in this names the variable holding a credential, never diff --git a/systemd/README.md b/systemd/README.md index 21de388..73d1e3d 100644 --- a/systemd/README.md +++ b/systemd/README.md @@ -27,7 +27,7 @@ sudo ./systemd/install.sh alice bob # admin, once per host: setup + two operat hispec deploy hsfei_adc # copy files from the repo, enable, start hispec deploy --new # everything the repo has that this host doesn't hispec status -hispec start fei # the whole FEI subsystem +hispec start hsfei # the whole FEI subsystem hispec logs hsfei_adc -f hispec doctor # why isn't it working? ```