Skip to content

Commit 8aadf42

Browse files
committed
feat(facts): add requires_command guard sentinel and preconditions() hook
Background ---------- Two related reliability gaps existed in the fact-collection layer: 1. The old shell guard for requires_command used: ! command -v <bin> >/dev/null || (<fact-command>) When the binary is absent this produces *no stdout*, making it impossible to distinguish "binary not installed" from "binary installed but returned no data". Both cases silently return default(), masking real errors. 2. There was no way for a fact to express runtime prerequisites beyond "this binary must exist" — e.g. ZFS datasets cannot be listed when the zfs kernel module is not loaded, even if the zfs binary is present. Changes ------- Replace the old `! … ||` guard with an explicit if/then/else that emits a unique marker when the binary is absent: if command -v <bin> >/dev/null 2>&1; then (<fact-command>); else echo '##PYINFRA_NOCMD##'; fi The marker `_MISSING_COMMAND_MARKER = "##PYINFRA_NOCMD##"` is detected in `_get_fact` and converted into a `MissingCommandError` exception so callers can tell the two cases apart. FactError └── FactNotCollected # base: fact skipped, not a data error ├── MissingCommandError # requires_command binary absent └── FactPreconditionError # preconditions() not satisfied All three are exported from `pyinfra.api`. Both exceptions obey the deploy phase in `get_fact()`: - **Prepare phase**: silently return `default()`. The binary / module may simply not be installed yet; a later operation will install it. - **Execute phase**: re-raise. The binary / module should already be present; its absence indicates incorrect ordering in the deploy. New optional override on `FactBase`: def preconditions(self, state, host) -> bool | tuple[Literal[False], str] | None: Return `True` / `None` to proceed, `False` to skip with a generic message, or `(False, "human reason")` to skip with context. The framework raises `FactPreconditionError` automatically — fact authors never need to import exception classes. * `ZfsPools` added with `requires_command = "zpool"`. * `ZfsDatasets.preconditions()` uses the existing `server.KernelModules` fact to verify the zfs kernel module is loaded before attempting `zfs get -H all` — a real-world scenario where `requires_command` alone is not sufficient. Tests ----- * tests/test_api/test_api_facts_missing_command.py — phase-aware behaviour for MissingCommandError and the ##PYINFRA_NOCMD## sentinel. * tests/test_api/test_api_facts_preconditions.py — phase-aware behaviour for FactPreconditionError via preconditions().
1 parent cedf47e commit 8aadf42

8 files changed

Lines changed: 364 additions & 22 deletions

File tree

CHANGELOG.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,16 @@
1+
# Unreleased
2+
3+
Core:
4+
5+
- api.facts: `requires_command` now uses an if/then/else shell guard with a sentinel marker
6+
(`##PYINFRA_NOCMD##`) so that "binary absent" can be distinguished from "binary returned no data"
7+
- api.facts: add `preconditions(state, host)` hook on `FactBase` for runtime prerequisite checks
8+
(e.g. kernel module loaded, service running) — return `False` or `(False, "reason")`
9+
- api.exceptions: add `FactNotCollected`, `MissingCommandError`, `FactPreconditionError` exception
10+
hierarchy; both exceptions are phase-aware (silent during prepare, raised during execute)
11+
- facts.zfs: fix `ZfsDatasets.requires_command` returning `"zpool"` instead of `"zfs"`; add
12+
`ZfsPools` fact; add `ZfsDatasets.preconditions()` checking kernel module via `server.KernelModules`
13+
114
# v3.7
215

316
Thank you to all contributors - particular shout out to @wowi42 for an incredible run of PRs!

docs/api/facts.md

Lines changed: 72 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,78 @@ and a ``process`` function. The command is executed on the target host and the o
55
passed (as a ``list`` of lines) to the ``process`` handler to generate fact data. Facts can output anything, normally a ``list`` or ``dict``.
66

77
Fact classes may provide a ``default`` function that takes no arguments (except ``self``). The return value of this function is used if an error
8-
occurs during fact collection. Additionally, a ``requires_command`` variable can be set on the fact that specifies a command that must be available
9-
on the host to collect the fact. If this command is not present on the host, the fact will be set to the default, or empty if no ``default`` function
10-
is available.
8+
occurs during fact collection.
9+
10+
## Guarding against missing binaries: `requires_command`
11+
12+
Override `requires_command` to declare a binary that must be present on the remote host before
13+
the fact command is run. When the binary is absent pyinfra emits a unique sentinel instead of
14+
executing the command, then raises `MissingCommandError` internally:
15+
16+
```py
17+
from pyinfra.api import FactBase
18+
19+
class ZfsPools(FactBase):
20+
def requires_command(self) -> str:
21+
return "zpool"
22+
23+
def command(self) -> str:
24+
return "zpool get -H all"
25+
```
26+
27+
**Phase-aware behaviour** — The exception is handled differently depending on the deploy phase:
28+
29+
- **Prepare phase**: the fact returns `default()` silently. The binary may simply not be
30+
installed yet; a later operation will install it.
31+
- **Execute phase**: `MissingCommandError` is re-raised so the developer knows the deploy
32+
is incorrectly ordered (the install step must come first).
33+
34+
## Checking runtime prerequisites: `preconditions()`
35+
36+
Some facts require more than just a binary — they need a specific runtime state (e.g. a kernel
37+
module loaded, a service running). Override `preconditions` to express these checks:
38+
39+
```py
40+
from pyinfra.api import FactBase
41+
42+
class ZfsDatasets(FactBase):
43+
def requires_command(self) -> str:
44+
return "zfs"
45+
46+
def preconditions(self, state, host):
47+
from pyinfra.facts.server import KernelModules
48+
modules = host.get_fact(KernelModules) or {}
49+
if "zfs" not in modules:
50+
return False, "kernel module 'zfs' is not loaded"
51+
52+
def command(self) -> str:
53+
return "zfs get -H all"
54+
```
55+
56+
Return values:
57+
58+
| Return value | Meaning |
59+
|---|---|
60+
| `True` or `None` | Prerequisites satisfied — proceed normally |
61+
| `False` | Prerequisite not satisfied, no reason given |
62+
| `(False, "reason")` | Prerequisite not satisfied with a human-readable explanation |
63+
64+
The framework raises `FactPreconditionError` automatically and applies the same
65+
phase-aware behaviour as `requires_command`: silent during prepare, raised during execute.
66+
Fact authors never need to import any exception class.
67+
68+
## Exception hierarchy
69+
70+
All "fact skipped" situations use a common base class so callers can catch at any level:
71+
72+
```
73+
FactError
74+
└── FactNotCollected # base: fact could not be collected
75+
├── MissingCommandError # requires_command binary absent
76+
└── FactPreconditionError # preconditions() not satisfied
77+
```
78+
79+
All three are exported from `pyinfra.api`.
1180

1281
## Importing & Using Facts
1382

src/pyinfra/api/__init__.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,14 @@
1111
from .deploy import deploy # noqa: F401 # pragma: no cover
1212
from .exceptions import ( # noqa: F401
1313
DeployError, # noqa: F401 # pragma: no cover
14+
FactPreconditionError,
1415
FactError,
16+
FactNotCollected,
1517
FactProcessError,
1618
FactTypeError,
1719
FactValueError,
1820
InventoryError,
21+
MissingCommandError,
1922
OperationError,
2023
OperationTypeError,
2124
OperationValueError,

src/pyinfra/api/exceptions.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,39 @@ class NestedOperationError(OperationError):
6060
"""
6161

6262

63+
class FactNotCollected(FactError):
64+
"""
65+
Base exception raised when a fact could not be collected (e.g. binary absent
66+
on the remote host, or the fact was skipped by a condition).
67+
"""
68+
69+
70+
class MissingCommandError(FactNotCollected):
71+
"""
72+
Exception raised when ``requires_command`` specifies a binary that is
73+
not present on the remote host. The fact returns its ``default()`` value
74+
instead of raising, unless explicitly configured otherwise.
75+
"""
76+
77+
def __init__(self, command: str) -> None:
78+
self.command = command
79+
super().__init__(f"Command not found on remote host: {command}")
80+
81+
82+
class FactPreconditionError(FactNotCollected):
83+
"""
84+
Exception raised when a fact's ``preconditions()`` check is not satisfied
85+
on the remote host (e.g. a kernel module is not loaded). Like
86+
``MissingCommandError``, this is silenced during the prepare phase and
87+
re-raised during execute.
88+
"""
89+
90+
def __init__(self, fact_cls: type, reason: str) -> None:
91+
self.fact_cls = fact_cls
92+
self.reason = reason
93+
super().__init__(f"Fact precondition not satisfied ({fact_cls.__name__}): {reason}")
94+
95+
6396
class DeployError(PyinfraError):
6497
"""
6598
User exception for raising in deploys or sub deploys.

src/pyinfra/api/facts.py

Lines changed: 92 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
import re
1515
from inspect import getcallargs
1616
from socket import error as socket_error, timeout as timeout_error
17-
from typing import TYPE_CHECKING, Any, Callable, Generic, Optional, Type, TypeVar, cast
17+
from typing import TYPE_CHECKING, Any, Callable, Generic, Literal, Optional, Type, TypeVar, cast
1818

1919
import click
2020
import gevent
@@ -24,7 +24,7 @@
2424
from pyinfra import logger
2525
from pyinfra.api import StringCommand
2626
from pyinfra.api.arguments import all_global_arguments, pop_global_arguments
27-
from pyinfra.api.exceptions import FactProcessError
27+
from pyinfra.api.exceptions import FactPreconditionError, FactProcessError, MissingCommandError
2828
from pyinfra.api.util import (
2929
get_kwargs_str,
3030
log_error_or_warning,
@@ -36,10 +36,14 @@
3636
from pyinfra.progress import progress_spinner
3737

3838
from .arguments import CONNECTOR_ARGUMENT_KEYS
39+
from .state import StateStage
3940

4041
if TYPE_CHECKING:
4142
from pyinfra.api import Host, State
4243

44+
# Sentinel output line emitted when skip_unless_command binary is absent on the remote host.
45+
_MISSING_COMMAND_MARKER = "##PYINFRA_NOCMD##"
46+
4347
SUDO_REGEX = r"^sudo: unknown user"
4448
SU_REGEXES = (
4549
r"^su: user .+ does not exist",
@@ -60,8 +64,27 @@ class FactBase(Generic[T]):
6064
command: Callable[..., str | StringCommand]
6165

6266
def requires_command(self, *args, **kwargs) -> str | None:
67+
"""Return the binary name that must exist on the remote host for this fact to run.
68+
If the binary is absent the fact returns its ``default()`` value silently.
69+
"""
6370
return None
6471

72+
def preconditions(
73+
self, state: "State", host: "Host"
74+
) -> bool | tuple[Literal[False], str] | None:
75+
"""Check that this fact's prerequisites are satisfied before running.
76+
77+
Override this method to call ``host.get_fact(...)`` and return:
78+
79+
- ``True`` (or ``None``) — all prerequisites satisfied, proceed normally
80+
- ``False`` — prerequisite not satisfied, no reason given
81+
- ``(False, "reason message")`` — prerequisite not satisfied with explanation
82+
83+
The framework handles raising ``FactPreconditionError`` and phase-awareness
84+
automatically; fact authors never need to import exception classes.
85+
"""
86+
return True
87+
6588
@override
6689
def __init_subclass__(cls) -> None:
6790
super().__init_subclass__()
@@ -186,15 +209,45 @@ def get_fact(
186209
apply_failed_hosts=apply_failed_hosts,
187210
)
188211

189-
return _get_fact(
190-
state,
191-
host,
192-
cls,
193-
args,
194-
kwargs,
195-
ensure_hosts,
196-
apply_failed_hosts,
197-
)
212+
try:
213+
return _get_fact(
214+
state,
215+
host,
216+
cls,
217+
args,
218+
kwargs,
219+
ensure_hosts,
220+
apply_failed_hosts,
221+
)
222+
except MissingCommandError as e:
223+
# During the prepare phase the binary might not yet be installed (a prior
224+
# operation will install it). Silently return the default so change
225+
# detection can proceed normally.
226+
if state.current_stage != StateStage.Execute:
227+
logger.debug(
228+
"Fact %s skipped on %s during prepare: %s",
229+
cls.__name__,
230+
host.print_prefix,
231+
e,
232+
)
233+
return cls().default()
234+
# During the execute phase the binary should already be present. If it
235+
# isn't, the deploy is incorrectly ordered (missing an install step).
236+
# Re-raise so the error surface is clear and actionable.
237+
raise
238+
except FactPreconditionError as e:
239+
# Same phase-aware logic: a precondition not satisfied during prepare
240+
# is normal (e.g. kernel module not yet loaded); during execute it is an
241+
# ordering error in the deploy.
242+
if state.current_stage != StateStage.Execute:
243+
logger.debug(
244+
"Fact %s skipped on %s during prepare: %s",
245+
cls.__name__,
246+
host.print_prefix,
247+
e,
248+
)
249+
return cls().default()
250+
raise
198251

199252

200253
def _get_fact(
@@ -229,20 +282,33 @@ def _get_fact(
229282
if fact.shell_executable:
230283
global_kwargs["_shell_executable"] = fact.shell_executable
231284

285+
# Check preconditions before running this fact's command.
286+
# preconditions() returns True/None (ok), False (fail), or (False, reason).
287+
pre = fact.preconditions(state, host)
288+
if pre is not True and pre is not None:
289+
reason = pre[1] if isinstance(pre, tuple) else f"{cls.__name__} preconditions not met"
290+
raise FactPreconditionError(cls, reason)
291+
232292
command = _make_command(fact.command, fact_kwargs)
233293
requires_command = _make_command(fact.requires_command, fact_kwargs)
234294
if requires_command:
235295
command = StringCommand(
236-
# Command doesn't exist, return 0 *or* run & return fact command
237-
"!",
296+
# If binary exists → run the fact command; otherwise emit the sentinel so
297+
# pyinfra can distinguish "binary absent" from "no output".
298+
"if",
238299
"command",
239300
"-v",
240301
requires_command,
241302
">/dev/null",
242-
"||",
303+
"2>&1;",
304+
"then",
243305
"(",
244306
command,
245-
")",
307+
");",
308+
"else",
309+
"echo",
310+
f"'{_MISSING_COMMAND_MARKER}';",
311+
"fi",
246312
)
247313

248314
status = False
@@ -268,6 +334,17 @@ def _get_fact(
268334

269335
stdout_lines, stderr_lines = output.stdout_lines, output.stderr_lines
270336

337+
# Detect the "binary absent" sentinel from the if/then/else shell guard.
338+
if status and stdout_lines == [_MISSING_COMMAND_MARKER]:
339+
cmd_str = str(requires_command) if requires_command else ""
340+
logger.debug(
341+
"Skipping fact %s on %s: command not found: %s",
342+
name,
343+
host.print_prefix,
344+
cmd_str,
345+
)
346+
raise MissingCommandError(cmd_str)
347+
271348
data = fact.default()
272349

273350
if status:

src/pyinfra/facts/zfs.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,22 @@ def process(self, output):
3232

3333

3434
class ZfsDatasets(FactBase):
35-
@override
36-
def command(self) -> str:
37-
return "zfs get -H all"
38-
3935
@override
4036
def requires_command(self) -> str:
4137
return "zfs"
4238

39+
@override
40+
def preconditions(self, state, host):
41+
from pyinfra.facts.server import KernelModules
42+
43+
modules = host.get_fact(KernelModules) or {}
44+
if "zfs" not in modules:
45+
return False, "kernel module 'zfs' is not loaded"
46+
47+
@override
48+
def command(self) -> str:
49+
return "zfs get -H all"
50+
4351
@override
4452
def process(self, output):
4553
return _process_zfs_props_table(output)

0 commit comments

Comments
 (0)