feat(facts): add requires_command guard sentinel and check_preconditions() hook

Author: maisim

CHANGELOG.md

@@ -1,3 +1,16 @@
+# Unreleased
+
+Core:
+
+- api.facts: `requires_command` now uses an if/then/else shell guard with a sentinel marker
+  (`##PYINFRA_NOCMD##`) so that "binary absent" can be distinguished from "binary returned no data"
+- api.facts: add `check_preconditions(state, host)` hook on `FactBase` for runtime prerequisite checks
+  (e.g. kernel module loaded, service running) — return a reason string or `None`
+- api.exceptions: add `FactNotCollected`, `MissingCommandError`, `FactPreconditionError` exception
+  hierarchy; both exceptions are phase-aware (silent during prepare, raised during execute)
+- facts.zfs: fix `ZfsDatasets.requires_command` returning `"zpool"` instead of `"zfs"`; add
+  `ZfsPools` fact; add `ZfsDatasets.check_preconditions()` checking kernel module via `server.KernelModules`
+
 # v3.7
 
 Thank you to all contributors - particular shout out to @wowi42 for an incredible run of PRs!

docs/api/facts.md

@@ -5,9 +5,80 @@ and a ``process`` function. The command is executed on the target host and the o
 passed (as a ``list`` of lines) to the ``process`` handler to generate fact data. Facts can output anything, normally a ``list`` or ``dict``.
 
 Fact classes may provide a ``default`` function that takes no arguments (except ``self``). The return value of this function is used if an error
-occurs during fact collection. Additionally, a ``requires_command`` variable can be set on the fact that specifies a command that must be available
-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
-is available.
+occurs during fact collection.
+
+## Guarding against missing binaries: `requires_command`
+
+Override `requires_command` to declare a binary that must be present on the remote host before
+the fact command is run. When the binary is absent pyinfra emits a unique sentinel instead of
+executing the command, then raises `MissingCommandError` internally:
+
+```py
+from pyinfra.api import FactBase
+
+class ZfsPools(FactBase):
+    def requires_command(self) -> str:
+        return "zpool"
+
+    def command(self) -> str:
+        return "zpool get -H all"
+```
+
+**Phase-aware behaviour** — The exception is handled differently depending on the deploy phase:
+
+- **Prepare phase**: the fact returns `default()` silently. The binary may simply not be
+  installed yet; a later operation will install it.
+- **Execute phase** (v3): a warning is logged and `default()` is returned. This preserves
+  backwards compatibility for deploys that rely on `default()` being returned when a binary
+  is absent.
+- **Execute phase** (v4): `MissingCommandError` will be raised so the developer knows the
+  deploy is incorrectly ordered (the install step must come first).
+
+## Checking runtime prerequisites: `check_preconditions()`
+
+Some facts require more than just a binary — they need a specific runtime state (e.g. a kernel
+module loaded, a service running). Override `check_preconditions` to express these checks:
+
+```py
+from pyinfra.api import FactBase
+
+class ZfsDatasets(FactBase):
+    def requires_command(self) -> str:
+        return "zfs"
+
+    def check_preconditions(self, state, host):
+        from pyinfra.facts.server import KernelModules
+        modules = host.get_fact(KernelModules) or {}
+        if "zfs" not in modules:
+            return "kernel module 'zfs' is not loaded"
+
+    def command(self) -> str:
+        return "zfs get -H all"
+```
+
+Return values:
+
+| Return value | Meaning |
+|---|---|
+| `None` (or no return) | Prerequisites satisfied — proceed normally |
+| `"reason"` | Prerequisite not satisfied with a human-readable explanation |
+
+The framework raises `FactPreconditionError` automatically and applies the same
+phase-aware behaviour as `requires_command`: silent during prepare, raised during execute.
+Fact authors never need to import any exception class.
+
+## Exception hierarchy
+
+All "fact skipped" situations use a common base class so callers can catch at any level:
+
+```
+FactError
+└── FactNotCollected          # base: fact could not be collected
+    ├── MissingCommandError   # requires_command binary absent
+    └── FactPreconditionError # check_preconditions() not satisfied
+```
+
+All three are exported from `pyinfra.api`.
 
 ## Importing & Using Facts
 

src/pyinfra/api/__init__.py

@@ -11,11 +11,14 @@
 from .deploy import deploy  # noqa: F401 # pragma: no cover
 from .exceptions import (  # noqa: F401
     DeployError,  # noqa: F401 # pragma: no cover
+    FactPreconditionError,
     FactError,
+    FactNotCollected,
     FactProcessError,
     FactTypeError,
     FactValueError,
     InventoryError,
+    MissingCommandError,
     OperationError,
     OperationTypeError,
     OperationValueError,

src/pyinfra/api/exceptions.py

@@ -60,6 +60,38 @@ class NestedOperationError(OperationError):
     """
 
 
+class FactNotCollected(FactError):
+    """
+    Base exception raised when a fact could not be collected (e.g. binary absent
+    on the remote host, or the fact was skipped by a condition).
+    """
+
+
+class MissingCommandError(FactNotCollected):
+    """
+    Exception raised when ``requires_command`` specifies a binary that is
+    not present on the remote host.  The fact returns its ``default()`` value
+    instead of raising, unless explicitly configured otherwise.
+    """
+
+    def __init__(self, command: str) -> None:
+        self.command = command
+        super().__init__(f"Command not found on remote host: {command}")
+
+
+class FactPreconditionError(FactNotCollected):
+    """
+    Exception raised when a fact's ``check_preconditions()`` returns a reason string
+    (e.g. a kernel module is not loaded).  Like ``MissingCommandError``, this is
+    silenced during the prepare phase and re-raised during execute.
+    """
+
+    def __init__(self, fact_cls: type, reason: str) -> None:
+        self.fact_cls = fact_cls
+        self.reason = reason
+        super().__init__(f"Fact precondition not satisfied ({fact_cls.__name__}): {reason}")
+
+
 class DeployError(PyinfraError):
     """
     User exception for raising in deploys or sub deploys.

src/pyinfra/api/facts.py

@@ -24,7 +24,7 @@
 from pyinfra.api.output import format_text
 from pyinfra.api import StringCommand
 from pyinfra.api.arguments import all_global_arguments, pop_global_arguments
-from pyinfra.api.exceptions import FactProcessError
+from pyinfra.api.exceptions import FactPreconditionError, FactProcessError, MissingCommandError
 from pyinfra.api.util import (
     get_kwargs_str,
     log_error_or_warning,
@@ -36,10 +36,14 @@
 from pyinfra.progress import progress_spinner
 
 from .arguments import CONNECTOR_ARGUMENT_KEYS
+from .state import StateStage
 
 if TYPE_CHECKING:
     from pyinfra.api import Host, State
 
+# Sentinel output line emitted when skip_unless_command binary is absent on the remote host.
+_MISSING_COMMAND_MARKER = "##PYINFRA_NOCMD##"
+
 SUDO_REGEX = r"^sudo: unknown user"
 SU_REGEXES = (
     r"^su: user .+ does not exist",
@@ -60,6 +64,22 @@ class FactBase(Generic[T]):
     command: Callable[..., str | StringCommand]
 
     def requires_command(self, *args, **kwargs) -> str | None:
+        """Return the binary name that must exist on the remote host for this fact to run.
+        If the binary is absent the fact returns its ``default()`` value silently.
+        """
+        return None
+
+    def check_preconditions(self, state: "State", host: "Host") -> str | None:
+        """Check that this fact's prerequisites are satisfied before running.
+
+        Override this method to call ``host.get_fact(...)`` and return:
+
+        - ``None`` (or no return) — all prerequisites satisfied, proceed normally
+        - ``"reason message"`` — prerequisite not satisfied with explanation
+
+        The framework handles raising ``FactPreconditionError`` and phase-awareness
+        automatically; fact authors never need to import exception classes.
+        """
         return None
 
     @override
@@ -186,15 +206,51 @@ def get_fact(
             apply_failed_hosts=apply_failed_hosts,
         )
 
-    return _get_fact(
-        state,
-        host,
-        cls,
-        args,
-        kwargs,
-        ensure_hosts,
-        apply_failed_hosts,
-    )
+    try:
+        return _get_fact(
+            state,
+            host,
+            cls,
+            args,
+            kwargs,
+            ensure_hosts,
+            apply_failed_hosts,
+        )
+    except MissingCommandError as e:
+        # During the prepare phase the binary might not yet be installed (a prior
+        # operation will install it).  Silently return the default so change
+        # detection can proceed normally.
+        if state.current_stage != StateStage.Execute:
+            logger.debug(
+                "Fact %s skipped on %s during prepare: %s",
+                cls.__name__,
+                host.print_prefix,
+                e,
+            )
+            return cls().default()
+        # During the execute phase the binary should already be present. If it
+        # isn't, the deploy is incorrectly ordered (missing an install step?).
+        # TODO(v4): remove this compat shim and let the exception propagate.
+        logger.warning(
+            "Fact %s skipped on %s: command not found: %s (this will raise an exception in v4)",
+            cls.__name__,
+            host.print_prefix,
+            e,
+        )
+        return cls().default()
+    except FactPreconditionError as e:
+        # Same phase-aware logic: a precondition not satisfied during prepare
+        # is normal (e.g. kernel module not yet loaded); during execute it is an
+        # ordering error in the deploy.
+        if state.current_stage != StateStage.Execute:
+            logger.debug(
+                "Fact %s skipped on %s during prepare: %s",
+                cls.__name__,
+                host.print_prefix,
+                e,
+            )
+            return cls().default()
+        raise
 
 
 def _get_fact(
@@ -229,20 +285,30 @@ def _get_fact(
     if fact.shell_executable:
         global_kwargs["_shell_executable"] = fact.shell_executable
 
+    # Check preconditions before running this fact's command.
+    if reason := fact.check_preconditions(state, host):
+        raise FactPreconditionError(cls, reason)
+
     command = _make_command(fact.command, fact_kwargs)
     requires_command = _make_command(fact.requires_command, fact_kwargs)
     if requires_command:
         command = StringCommand(
-            # Command doesn't exist, return 0 *or* run & return fact command
-            "!",
+            # If binary exists → run the fact command; otherwise emit the sentinel so
+            # pyinfra can distinguish "binary absent" from "no output".
+            "if",
             "command",
             "-v",
             requires_command,
             ">/dev/null",
-            "||",
+            "2>&1;",
+            "then",
             "(",
             command,
-            ")",
+            ");",
+            "else",
+            "echo",
+            f"'{_MISSING_COMMAND_MARKER}';",
+            "fi",
         )
 
     status = False
@@ -268,6 +334,17 @@ def _get_fact(
 
     stdout_lines, stderr_lines = output.stdout_lines, output.stderr_lines
 
+    # Detect the "binary absent" sentinel from the if/then/else shell guard.
+    if status and stdout_lines == [_MISSING_COMMAND_MARKER]:
+        cmd_str = str(requires_command) if requires_command else ""
+        logger.debug(
+            "Skipping fact %s on %s: command not found: %s",
+            name,
+            host.print_prefix,
+            cmd_str,
+        )
+        raise MissingCommandError(cmd_str)
+
     data = fact.default()
 
     if status:

tests/test_api/test_api_facts_missing_command.py

@@ -0,0 +1,71 @@
+"""
+Tests for MissingCommandError / requires_command phase-aware behaviour.
+
+When a fact declares ``requires_command`` and the requested binary is absent:
+
+- **Prepare phase** – silent: the binary might not yet be installed.
+  The fact must return ``default()`` so change-detection can proceed.
+- **Execute phase** – compat shim (v3): warning logged, ``default()`` returned.
+  Will raise ``MissingCommandError`` in v4.
+"""
+
+from unittest import TestCase
+from unittest.mock import MagicMock, patch
+
+from pyinfra.api.exceptions import FactError, FactNotCollected, MissingCommandError
+from pyinfra.api.facts import _MISSING_COMMAND_MARKER, get_fact
+from pyinfra.api.state import StateStage
+from pyinfra.facts.zfs import ZfsPools
+
+
+def _make_state(stage: StateStage) -> MagicMock:
+    state = MagicMock()
+    state.current_stage = stage
+    return state
+
+
+class TestMissingCommandPhaseAwareness(TestCase):
+    """Phase-aware handling of MissingCommandError in the public get_fact() wrapper."""
+
+    def test_prepare_phase_returns_default(self):
+        """Binary absent during prepare → fact returns default() silently."""
+        state = _make_state(StateStage.Prepare)
+        host = MagicMock()
+
+        with patch("pyinfra.api.facts._get_fact", side_effect=MissingCommandError("zpool")):
+            result = get_fact(state, host, ZfsPools)
+
+        assert result == ZfsPools().default()
+
+    def test_execute_phase_returns_default_with_warning(self):
+        """Binary absent during execute → compat shim: warning logged, default() returned (v3).
+        TODO(v4): this should raise MissingCommandError instead."""
+        state = _make_state(StateStage.Execute)
+        host = MagicMock()
+
+        with patch("pyinfra.api.facts._get_fact", side_effect=MissingCommandError("zpool")):
+            result = get_fact(state, host, ZfsPools)
+
+        assert result == ZfsPools().default()
+
+    def test_missing_command_error_message(self):
+        """MissingCommandError carries a human-readable message."""
+        exc = MissingCommandError("zpool")
+        assert "zpool" in str(exc)
+
+    def test_missing_command_inherits_fact_not_collected(self):
+        """MissingCommandError is a FactNotCollected (and a FactError)."""
+        exc = MissingCommandError("zpool")
+        assert isinstance(exc, FactNotCollected)
+        assert isinstance(exc, FactError)
+
+
+class TestMissingCommandMarker(TestCase):
+    """The sentinel constant is stable – callers may depend on it."""
+
+    def test_marker_is_unique_string(self):
+        assert isinstance(_MISSING_COMMAND_MARKER, str)
+        assert len(_MISSING_COMMAND_MARKER) > 0
+        # Must not look like valid shell output from any standard command
+        assert _MISSING_COMMAND_MARKER.startswith("##")
+        assert _MISSING_COMMAND_MARKER.endswith("##")