Turn on the PTH rule set in ruff and convert the remaining call sites
that triggered violations:
- `supervisor/utils/yaml.py` and `tests/homeassistant/test_module.py`
use `Path.open()` instead of the builtin `open()`.
- `tests/backups/test_backup.py` and
`tests/resolution/fixup/test_store_execute_reset.py` use
`Path.iterdir()` instead of `os.listdir()`.
- `tests/mounts/test_mount.py` uses `Path.unlink()` instead of
`os.remove()`.
- `tests/store/test_translation_load.py` uses `Path.mkdir(parents=True)`
instead of `os.makedirs()`.
- `tests/hardware/test_disk.py` captures the unpatched `Path.is_dir`,
`Path.is_symlink` and `Path.stat` before the `patch.object(Path, ...)`
block so the mocks can delegate to the real implementations without
reaching for `os.path.isdir`/`os.path.islink`/`os.stat`.
* tests: enable flake8-pytest-style (PT) ruff rules
Enable the `PT` ruff rule set and fix the resulting violations across the
test suite:
- PT006: pass parametrize argument names as tuples instead of a single
comma-separated string.
- PT022: switch fixtures that have no teardown from `yield` to `return`
so the lack of cleanup is obvious at a glance.
- PT011: add `match=` to broad `pytest.raises(ValueError)` blocks so the
expected error is anchored to a specific message.
- PT012: hoist setup (patches, branching) out of `pytest.raises()`
blocks so only the call that is expected to raise remains inside.
- PT013: replace `from pytest import X` with `import pytest` and access
attributes via the module.
- PT015: replace `try/except` + `assert False` patterns with
`pytest.raises(...)`.
- PT017: replace `assert` on exceptions inside `except` blocks with
`pytest.raises(...) as exc_info` and assert on `exc_info.value`.
No behavioral changes to the tests; the full suite still passes.
* tests: address review feedback on PT ruff rule enablement
- Fix fixture return-type annotations after switching `yield` to `return`
in tests/conftest.py: drop the `Generator[...]`/`AsyncGenerator[...]`
wrapper for `dns_manager_service`, `supervisor_internet`, `websession`,
and `mock_update_data` so the annotation matches what the fixture
actually returns.
- Correct the return-type annotation of `fixture_ip6config_service` from
`IP4ConfigService` to `IP6ConfigService`.
- Fix recurring "excepiton" typo in tests/utils/test_exception_helper.py.
* tests: verify backup cleanup on permission error
After `test_new_backup_permission_error` raises `BackupPermissionError`,
assert that no tarfile was left behind and `tmp_path` is empty. The
previous version only checked that the exception was raised, which
missed any regression where a partial tarfile would survive the failed
create.
* tests: rename DNS_GOOD_V6 to DNS_V6_UNSUPPORTED
The constant was named "good" but its tests assert that the URLs are
rejected by the DNS validator. The IPv6 URLs are well-formed but
currently rejected because IPv6 doesn't work with the Docker network
(see `dns_url` in supervisor/validate.py). Rename the constant and the
related test to make the intent obvious.
* mounts: use softerr for NFS instead of soft
Switch the NFS mount option from `soft` to `softerr`. For HAOS-style
supervisor mounts (media, share, backup — not the root filesystem) the
error semantics matter:
* `softerr` returns `ETIMEDOUT` on timeout instead of `EIO` (`soft`).
`EIO` is indistinguishable from "the disk is dying"; tools like
SQLite, restic, rsync, ffmpeg tend to treat it as a hard storage
failure (mark database corrupt, abort backup with a hard error,
etc.). `ETIMEDOUT` is unambiguously "the network/server is gone,
transient" and is more commonly handled as retry-later. Supervisor
can also surface a clear "server unreachable" notification rather
than a generic I/O error.
* `softerr` was added in kernel 5.10 precisely to give the fail-fast
behavior of `soft` with a distinct errno so well-behaved apps can
do the right thing.
* For writes-must-not-be-lost use cases (databases, paid storage,
evidence-grade logging) one would want `hard,intr` and a different
recovery story. HAOS NFS mounts are not that — they're add-on
storage where "the share went offline, try again later" is the
correct user-visible behavior.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* mounts: make is_mounted honest about server reachability
systemd's "active/mounted" state is derived from /proc/self/mountinfo
and doesn't reflect whether the backing server actually answers. For
CIFS in particular, smb3_reconfigure never contacts the server, so a
reload of a dead share returns active/mounted with no recovery
attempted. PR #4882 added a Path.is_mount() cross-check to catch this,
but Path.is_mount() relies on os.stat() of the mountpoint root — and
both NFS (softreval) and CIFS (cached root inode attrs) can serve that
stat from local state without going to the wire, so it lies in exactly
the dead-share scenario it was meant to detect.
The visible failure: an API reload of a backup mount whose server had
gone away "succeeded", supervisor then kicked off sys_backups.reload()
against the dead share, and the executor fanned out hundreds of
"Task exception was never retrieved" OSError(112) tracebacks as each
backup tarball's stat() parked in the kernel and failed.
Replace the local-state checks with a statvfs() probe in
NetworkMount.is_mounted(). os.statvfs() returns per-filesystem data
(free blocks, total blocks) that has no client-side cache in either
kernel — neither cifs_statfs() nor nfs_statfs() has an early-return on
cache freshness; both build and send a real FSSTAT / QUERY_FS_INFO
request. So the kernel either reaches the server or gives up with
ETIMEDOUT / EHOSTDOWN / ECONNABORTED. is_mounted() now reflects actual
reachability, finally fulfilling PR #4882's stated intent.
With is_mounted() honest, the reload/restart machinery falls out
naturally: Mount.reload()'s fast path is just `if await self.is_mounted()`,
the post-reload check is the same call, and the "reload succeeded
systemd-wise but probe failed" branch collapses into the existing
"not mounted after reload, try restart" branch. mount(), _restart()
and update() already call is_mounted(); they now get the probe for
free.
To make the probe meaningful, the network mount option strings get
explicit kernel-side timeouts:
* NFS switches from `soft` to `softerr` so timeouts surface as
ETIMEDOUT rather than EIO. EIO is indistinguishable from
"disk dying" and gets misinterpreted by SQLite/restic/rsync as a
hard storage failure; ETIMEDOUT is unambiguously transient.
* CIFS gains `soft,echo_interval=10,retrans=0`, giving a ~30s
per-operation detection budget (3 x echo_interval since last server
response) that matches the NFS budget from `timeo=100,retrans=2`.
Both protocols now fail bounded operations in roughly the same time.
The probe is intentionally not wrapped in an asyncio timeout: the
kernel-side bound is authoritative, and an asyncio timeout would only
orphan the executor thread without unblocking the syscall. The probe
emits debug logs with timing so the ~30s syscall wait on a dead share
is visible in LOGLEVEL=debug traces instead of appearing as a hang.
Tests:
* mock_is_mount fixture extended to also patch os.statvfs so existing
tests that rely on a healthy mount don't need to know about the
probe.
* New manager test split into _healthy_skips_systemd (probe succeeds,
fast path, no systemd call) and _probe_failure_triggers_systemd_reload
(probe fails, escalation runs). API reload test covers both paths.
* Existing tests that simulated "mount is down" via
mock_is_mount.return_value=False updated to simulate probe failure
via OSError(EHOSTDOWN), since is_mount is no longer the signal.
* mounts: drive systemd job waits via JobRemoved instead of state polling
`_update_state_await` had a race that has been present since network
mounts were introduced (#4269) and survived every subsequent rewrite
(#4733, c75f36305): the wait function reads the unit's ActiveState
*after* the systemd dispatch returns, then matches against a target
set that almost always includes the pre-call state (ACTIVE). If
systemd has not yet started dispatching the queued job — and for
fast operations like CIFS reload (smb3_reconfigure completes in
milliseconds with no server contact) it routinely has not — the wait
matches immediately on the pre-call state and returns "done" before
the operation has actually begun. The race was invisible until the
probe commit started doing honest network work after the wait
returned, at which point we spent a full ~30s NFS probe wait on a
mount that systemd was still in the middle of restarting.
Switch the job-dispatching paths (mount/unmount/reload/restart) to
systemd's `JobRemoved` Manager signal. The pattern:
async with sys_dbus.systemd.job_removed() as jobs:
job_path = await sys_dbus.systemd.restart_unit(...)
await jobs.wait_for_job(job_path)
is structurally race-free: subscription is set up before dispatch,
the job_path is allocated synchronously inside the dispatch call,
and any JobRemoved for that path after subscription is queued. Even
for operations that complete faster than we can process the dispatch
return value (the CIFS reload case), the signal cannot be missed.
`_update_state_await` is kept for `load()` — that path observes
existing state without having dispatched a job, so the
PropertiesChanged-driven wait is the right primitive there.
* mounts: verify the path is a mount point before trusting statvfs
The userspace probe trusted statvfs() alone to declare a network mount
healthy. statvfs is uncacheable client-side for both NFS and CIFS, so
on a real mount it forces a wire RPC — but if the mount is gone from
the kernel mount table (e.g., after a restart cycle whose umount
succeeded but whose mount step failed, leaving the unit ACTIVE in
systemd's view), statvfs() operates on a plain directory on the
underlying root filesystem and happily returns the root fs's stats.
The mount is reported healthy when in fact it doesn't exist.
Add a pre-check using `Path.is_mount` — a parent-vs-path `st_dev`
comparison via stat() — to detect "this path is not actually a mount
point" before issuing statvfs. For the ghost-mount case (path on the
root fs) both stats are local and return immediately. For a real
mount the path-stat may cross into the filesystem driver, but the
result is correct in either case and the statvfs that follows
catches server-unreachable mounts that is_mount can't.
The full is_mounted contract is now: ACTIVE per systemd, present as
a mount point in our namespace, and server-reachable per statvfs.
* mounts: skip wasted probes when systemd already reported job failed
JobRemoved tells us whether the systemd job completed successfully —
we just weren't using it. The previous reload/restart paths ran their
post-job probe unconditionally, including the case where systemd had
already told us the operation failed. On a dead NFS share the kernel
is in transport-reconnect churn for tens of seconds after a killed
mount helper exits, so that probe takes 90+ seconds — and confirms
exactly what systemd already said. Plain wasted time.
* `Mount.reload()` now uses the systemd job result. On "done" we
still probe (CIFS reload is local-only — smb3_reconfigure returns
"done" even against a dead server, so the probe is the only check
that catches the lying-CIFS case). On anything else (failed,
timeout, canceled, dependency, skipped, or our own timeout
returning None) we escalate directly to `_restart()` without
probing.
* `Mount._restart()` skips the probe entirely on non-"done" results.
The mount is definitively not active in that case and the probe
would just spend another 30-90s on a dead share.
Also bump the JobRemoved wait timeout from UPDATE_STATE_TIMEOUT (40s,
sized for a single-helper invocation) to a new SYSTEMD_JOB_TIMEOUT
(90s). RestartUnit runs as stop + start, each bounded by the unit's
TimeoutSec (35s from #6834), so the worst case is ~70s plus systemd
queue dispatch. The previous 40s budget caused supervisor to time
out exactly one second before JobRemoved fired in the observed
dead-NFS restart cycle. UPDATE_STATE_TIMEOUT is kept at 40s for the
PropertiesChanged-driven wait in `load()`, where the layered timeout
invariant from #6827 still applies.
* dbus: filter signals at the wrapper level, drop JobRemovedSignal class
Mike's review on #6838: instead of carrying a bespoke JobRemovedSignal
context-manager class, push the filtering concept into the generic
DBusSignalWrapper. Any caller that subscribes to a broadcast signal
but only cares about specific payloads can pass a predicate.
* DBus.signal() gains an optional `message_filter: Callable[..., bool]`.
* DBusSignalWrapper.wait_for_signal() loops past messages where the
filter returns False, returning the next match.
* JobRemovedSignal goes away. supervisor/dbus/systemd.py exposes a
small factory `job_removed_filter(get_job_path)` that returns the
matching predicate; the getter indirection lets callers subscribe
before the dispatch returns the job path (which is required to be
race-free — see the previous commit).
Mount._run_systemd_job() switches to:
job_path: str | None = None
async with systemd.connected_dbus.signal(
DBUS_SIGNAL_SYSTEMD_JOB_REMOVED,
job_removed_filter(lambda: job_path),
) as signal:
job_path = await dispatch
_id, _path, _unit, result = await signal.wait_for_signal()
The behavior is identical to the previous JobRemovedSignal-based
implementation. The signal queue still buffers messages received
after AddMatch is installed, so subscribing before dispatch keeps
the race-free guarantee.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When a network mount's backing server became unreachable while the
supervisor's periodic mount reload ran, the host could be force-reset by
the hardware watchdog within ~60 seconds. Fixes#6827.
Mechanism observed in a reproducer on HAOS x86 (iTCO_wdt, 30 s
heartbeat):
1. `Mount.reload()` issues `ReloadUnit` over D-Bus. systemd spawns
`mount -o remount` against the dead NFS server. The remount helper
parks in `TASK_UNINTERRUPTIBLE` inside the kernel NFS client, and
any unrelated userspace operation that touches the same mount
path (stat, getdents, readdir) joins it there.
2. After 30 s `_update_state_await()` gives up waiting for the unit
to leave `reloading`. The unit is still in `reloading` from
systemd's point of view; the helper is still pinned in the kernel.
3. Because the reload did not converge, `Mount.reload()` escalates by
calling `_restart()`, which issues `RestartUnit`. To restart,
systemd has to stop the active mount; the umount runs into the
same in-kernel stall as the remount helper. The job queue stops
making progress and PID 1 stops being able to pet
/dev/watchdog0. ~30 s later the iTCO chip resets the box.
The underlying PID 1 livelock is reported upstream as
systemd/systemd#42050. The bug is reproducible with NFS; we have not
been able to reproduce it with CIFS. HAOS also bumped the systemd
runtime watchdog timeout to give more headroom for slow workers
(home-assistant/operating-system#4705), but that does not on its own
prevent the wedge. The fix here removes the trigger from the supervisor
side.
Changes:
* Switch the NFS option string from `timeo=200` to
`timeo=100,retrans=2`, and pass `TimeoutUSec=35s` as a
unit-level property on each transient mount unit. The supervisor's
existing state-await is bumped from 30 s to 40 s. Three layered
timeouts now cooperate to avoid the reload → restart escalation that
triggers the bug:
NFS RPC timeout (timeo=100,retrans=2) ~30 s
< systemd unit TimeoutSec 35 s
< supervisor state-await 40 s
Ordering is what matters. The kernel RPC layer fails first so the
mount helper can exit; systemd then SIGTERMs anything that did not
and moves the unit to `failed` before supervisor abandons its
observation. Supervisor sees `failed`, never enters the
reload → restart escalation, and any subsequent fixup-triggered
restart runs from `failed` (no in-flight helper, no pinned kernel
task) — the safe path. systemd's `TimeoutSec` covers all
mount/umount/remount helpers for a `.mount` unit; no separate reload
timeout exists.
* Safety net for the reload → restart escalation. In `Mount.reload()`,
if the unit is still in `reloading` after the state-await returns,
raise `MountActivationError` directly instead of calling
`_restart()`. With the layered timeouts above this should not be
reachable; the guard exists so that if it ever does fire (e.g. a
future kernel/option regression), we surface a clear log at
CRITICAL level and refuse to issue the destructive `RestartUnit`
call rather than wedging PID 1 again. Normal reload → restart for
the "unit left reloading but is not active" case is preserved.
* `FixupMountExecuteReload` now catches `MountError`, logs a warning,
and re-raises `ResolutionFixupError` so `FixupBase` skips
issue/suggestion cleanup. The user retains the "execute reload"
suggestion to retry once the underlying problem is fixed, instead of
the fixup returning success when nothing was actually fixed (which
would dismiss the issue and let the next reload cycle hit the same
bug again).
Tests:
* New `test_reload_does_not_escalate_when_still_reloading` asserts that
`RestartUnit` is not called when the unit stays in `reloading` after
the state-await — the regression guard for the watchdog reset.
* `test_fixup_error_after_reload` updated for the new fixup behavior
(no longer propagates `MountActivationError`; issue/suggestion
remain in place).
* All existing mount/manager tests updated to include the new
`TimeoutUSec` property in expected `StartTransientUnit` option
lists, and NFS option strings updated to the new defaults.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Wait until mount unit is deactivated on unmount
The current code does not wait until the (bind) mount unit has been
actually deactivated (state "inactive"). This is especially problematic
when restoring a backup, where we deactivate all bind mounts before
restoring the target folder. Before the tarball is actually restored,
we delete all contents of the target folder. This lead to the situation
where the "rm -rf" command got executed before the bind mount actually
got unmounted.
The current code polls the state using an exponentially increasing
delay. Wait up to 30s for the bind mount to actually deactivate.
* Fix function name
* Fix missing await
* Address pytest errors
Change state of systemd unit according to use cases. Note that this
is currently rather fragile, and ideally we should have a smarter
mock service instead.
* Fix pylint
* Fix remaining
* Check transition fo failed as well
* Used alternative mocking mechanism
* Remove state lists in test_manager
---------
Co-authored-by: Mike Degatano <michael.degatano@gmail.com>
* Add mount to supported features
* Typo in enable
* Fix places mocking os available without version
* Increase resilence of problematic repeat task test
Stefan AgnerandGitHub
Mike DegatanoandGitHub
Joakim SørensenandGitHub