Follow-up to review feedback on #7003 and #7004: when removing the
hosts file in CoreDNS.reset() or cleaning up a backup tarfile in
BackupManager.import_backup(), pass any OSError to
sys_resolution.check_oserror() so known filesystem issues (e.g.
EBADMSG) mark the system unhealthy early. For the backup paths the
check only applies to local locations (default and cloud backup),
matching the existing handling in remove() and _copy_to_location().
This also covers the second unlink in import_backup() (cleanup after
a failed load) and stops OSError from bubbling out of both cleanup
paths unhandled to the job: the error is now logged, and the method
still returns None or raises the original BackupInvalidError.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Derive App state from container state
The App.state setter mixed two responsibilities: it both mutated a
private `_state` field and dispatched side effects (WebSocket events,
issue dismissal, startup_event signaling). On top of that, an installed
but never-started app stayed in AppState.UNKNOWN forever, because the
attach() image-only fallback never fires a container state-change event
and the AppState therefore kept its constructor default. Conceptually,
ContainerState.UNKNOWN ("container does not exist") and AppState.UNKNOWN
("nothing observed yet") happened to share a name but meant different
things, which made the distinction easy to lose.
Make App.state a pure derived property. The source of truth is the last
observed ContainerState (cached on the App), plus a sticky operation-
error flag for start/stop failures that the docker event stream cannot
reflect. When no container has been observed yet, the derivation falls
back to install signals: an attached instance (image present) is
STOPPED, otherwise UNKNOWN. As a side effect, an installed-but-never-
started app now correctly reports STOPPED instead of UNKNOWN.
container_state_changed updates the cached container state and routes
all side effects through a single _emit_state_change helper that diffs
old vs new derived state. The two start/stop failure paths route
through _set_operation_error. Uninstall resets the cached signals so
the derivation naturally returns UNKNOWN.
Tests use a new tests/common.force_app_state helper that pokes the
underlying signals directly; the production class no longer carries
test-only setters.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Fix App state drive to AppState.UNKNOWN
* Unify state mutation through _update_state
Previously, state-driving signal changes were spread across two helpers
(_set_operation_error, _emit_state_change(old_state)) and required each
caller to capture self.state before mutating a private field — leaking
implementation details to call sites and raising the "why am I emitting
the old state?" question pointed out in code review.
Replace both helpers with a single _update_state(*, container_state=,
operation_error=) entry point. Callers describe what changed via
keyword arguments (None leaves a signal untouched); the helper captures
the previous state, applies the updates, recomputes the derived state
and emits side effects if anything changed.
Diff against a tracked _last_state instead of a freshly derived
"current" state, so that an out-of-band mutation between updates does
not silently shift the comparison baseline. The concrete case is
App.uninstall: instance.remove() clears the docker meta mid-flow, which
would otherwise reshape the derivation (RUNNING with no healthcheck
becomes STARTED instead of STARTUP) and suppress the STARTUP transition
that resolves the start-wait task. As a side effect, the initial
UNKNOWN -> STOPPED transition on attach is also now reliably emitted.
Switch the uninstall path to ContainerState.UNKNOWN ("we know there is
no container") rather than the constructor sentinel None.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Cache app state instead of deriving on every read
Building on the previous commit, make App.state a plain read of a
cached _state field rather than re-deriving on every property access.
The derivation moves to _derive_state(), and _update_state() is the
sole place that recomputes and assigns _state, so the value consumers
read always matches what was last emitted to listeners.
This removes the _last_state bookkeeping introduced previously: with a
single cached value there is no longer a separate "derived now" vs
"last emitted" distinction to reconcile, and out-of-band mutations
(e.g. instance.remove() clearing _meta during uninstall) can no longer
silently shift what state returns between updates.
Call _update_state() at the end of load() so the cached state settles
once attach() has run. Image-only attaches do not fire a docker event,
so without this an installed app would stay in the constructor-default
UNKNOWN until first start; this also makes the initial transition on
attach observable to listeners.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Pass operation error to _derive_state instead of storing it
The two state-driving signals were not symmetric. _container_state is
genuinely persisted state ("the last thing docker told us") that
re-derivation legitimately reads across calls. _operation_error, on the
other hand, is a momentary "force ERROR for this transition" signal; the
persistence of an error condition already lives in the cached _state.
Storing it as an instance attribute implied a sticky cross-call behavior
that no call path actually exercised: every caller either set it
explicitly right before deriving (start/stop failures, container events)
or ran argless only at load time, where no failure has occurred.
Drop the _operation_error field and pass operation_error as a parameter
to _derive_state(), defaulting to False in _update_state(). A container
observation now supersedes a prior error implicitly via the default,
which lets the container-event and uninstall call sites drop their
explicit operation_error=False.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Settle load state synchronously from current_state
The argless _update_state() settle at the end of load() raced attach()'s
container-state event. attach() fires DOCKER_CONTAINER_STATE_CHANGE via
the bus, which schedules the container_state_changed listener as a task
rather than running it inline. In the deprecated-arch early-return path
there is no await between attach() and the settle, so the listener had
not run yet: _container_state was still None and the settle derived
STOPPED (instance attached) — emitting a transient UNKNOWN->STOPPED even
for a running container before the listener corrected it. The main path
only avoided this incidentally, by having awaits (check_image,
save_persist) in between for the listener to run.
Derive the load-time state synchronously from instance.current_state()
instead of relying on the asynchronously delivered event. current_state()
returns the real container state, or UNKNOWN when only an image is
present (which derives to STOPPED), so both paths settle correctly
without racing the event.
Add a regression test that loading a running container settles to
STARTED, and mock current_state() in the state-listener test which
relies on a clean UNKNOWN baseline.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* apps: avoid blocking is_file() check in restore
Restoring an app off a backup called `Path.is_file()` directly on the
event loop to check whether the bundle contained an `image.tar`, which
triggered a blockbuster BlockingError on `os.stat` in environments where
blocking-call detection is enabled. Wrap the check in
`self.sys_run_in_executor(...)` so the stat happens off the main thread,
matching the pattern already used in `api/backups.py`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* tests: cover restore path where addon image is missing
The blocking ``is_file()`` call that the previous commit replaced lived
in the ``not instance.exists()`` branch of ``App.restore()`` -- reached
only when the local Docker image for the addon is absent. Existing
app-restore tests start from ``install_app_ssh`` / ``install_app_example``
and rely on the mocked ``docker images.inspect`` which always reports
the image as present, so that branch never executed and blockbuster
never had a chance to flag the blocking stat.
Add a focused test that uninstalls the app after backup, patches
``DockerApp.exists`` to ``False``, and stubs ``install`` / ``import_image``
so the restore drives through the ``image.tar`` check. Verified that
this test raises ``BlockingError: Blocking call to os.stat`` when run
against the unpatched code.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* 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>
* Rename addon/addons to app/apps in filenames and imports
Continues the addon→app terminology migration (#6786).
Renames all source files, test files, fixture files, and
directories that contained 'addon'/'addons' in their names,
and updates all imports accordingly.
Resolution check files in supervisor/resolution/checks/ that were
renamed override the slug property to preserve the existing API
contract (slugs are exposed via the resolution info API and used
to run checks by name).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Rename add-on.json fixture
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Replace fixed-duration sleeps after bus events with gather
Several tests use ``await asyncio.sleep(...)`` to "wait for the
listener to run" after firing a bus event. The fixed duration is
real wall-clock time and the wait can be indeterministic — if the
handler chain happens to need slightly more time on a busy CI
runner, the assertion races the handler.
``Bus.fire_event`` returns the listener tasks since #6252; capture
and ``await asyncio.gather(*tasks)`` instead of sleeping. Touches
test_bus.py (the bus tests were poking scheduling instead of
verifying their assertions), test_home_assistant_watchdog.py,
test_plugin_base.py, addons/test_manager.py, docker/test_addon.py,
and test_store_execute_reload.py.
Other cleanups in the same spirit:
- ``_fire_test_event`` in addons/test_addon.py becomes ``async def``
and gathers the listener tasks itself, so its 17 call sites
collapse to a single ``await _fire_test_event(...)``.
- The two test_store_execute_reload.py sites that used the private
``_update_connectivity()`` helper are reworked to set the cached
connectivity flag directly and fire the event themselves so they
can gather the listener tasks the same way.
- The two ``sleep(1)`` post-pull drains in docker/test_interface.py
collapse to ``sleep(0)`` (handler tasks are already gathered
inside pull_image), saving ~2s.
- The ``sleep(0.01)`` waits inside ``container_events()`` task
bodies (api/test_addons.py, api/test_store.py,
backups/test_manager.py) are just one-yield-to-the-parent and
become ``sleep(0)``.
Switching to ``gather`` exposes a few latent test mocks that were
silently swallowing TypeErrors as background-task failures before:
- ``CGroup.add_devices_allowed`` is ``async def`` but was patched
as a plain MagicMock in docker/test_addon.py — now patched via
``new_callable=AsyncMock``.
- The watchdog does ``await (await self.start())`` /
``await (await self.restart())`` because ``App.start`` /
``App.restart`` return ``asyncio.Task``. The mocks in
addons/test_addon.py (test_app_watchdog, test_watchdog_on_stop,
test_watchdog_during_attach) needed
``AsyncMock(return_value=<settled future>)`` to mirror that
shape rather than a plain MagicMock.
* Factor bus.fire_event + gather pattern into a helper
Per review feedback, the ``await asyncio.gather(*coresys.bus.fire_event(...))``
incantation was scattered across many call sites. Add
``tests.common.fire_bus_event`` that takes the coresys, event and data,
fires the event and awaits the spawned listener tasks. Convert all
matching sites to use it, including the ``_fire_test_event`` wrapper
in addons/test_addon.py which now just builds the
``DockerContainerStateEvent`` and delegates.
* Rename addon→app in docstrings and comments
Updates all docstrings and inline comments across supervisor/ and
tests/ to use the new app/apps terminology. No runtime behaviour
is changed by this commit.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Rename addon→app in code (variables, args, class names, functions)
Renames all internal Python identifiers from addon/addons to app/apps:
- Variable and argument names
- Function and method names
- Class names (Addon→App, AddonManager→AppManager, DockerAddon→DockerApp,
all exception, check, and fixup classes, etc.)
- String literals used as Python identifiers (pytest fixtures,
parametrize param names, patch.object attribute strings,
URL route match_info keys)
External API contracts are preserved: JSON keys, error codes,
discovery protocol fields, TypedDict/attr.s field names.
Import module paths (supervisor/addons/) are also unchanged.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix partial backup/restore API to remap addons key to apps
The external API accepts `addons` as the request body key (since
ATTR_APPS = "addons"), but do_backup_partial and do_restore_partial
now take an `apps` parameter after the rename. The **body expansion
in both endpoints would pass `addons=...` causing a TypeError.
Remap the key before expansion in both backup_partial and
restore_partial:
if ATTR_APPS in body:
body["apps"] = body.pop(ATTR_APPS)
Also adds test_restore_partial_with_addons_key to verify the restore
path correctly receives apps= when addons is passed in the request
body. This path had no existing test coverage.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix merge error
* Adjust AppLoggerAdapter to use app_name
Co-authored-by: Stefan Agner <stefan@agner.ch>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Stefan Agner <stefan@agner.ch>
* Change addons to apps in all user-facing strings
* Fix grammar in errors
* Apply suggestions from code review
Co-authored-by: Jan Čermák <sairon@users.noreply.github.com>
* Apply suggestions from code review
Co-authored-by: Stefan Agner <stefan@agner.ch>
---------
Co-authored-by: Jan Čermák <sairon@users.noreply.github.com>
Co-authored-by: Stefan Agner <stefan@agner.ch>
* Include network storage mount configurations in backups
When creating backups, now stores mount configurations (CIFS/NFS shares)
including server, share, credentials, and other settings. When restoring
from a backup, mount configurations are automatically restored.
This fixes the issue where network storage definitions for backups
were lost after restoring from a backup, requiring users to manually
reconfigure their network storage mounts.
Changes:
- Add ATTR_MOUNTS schema to backup validation
- Add store_mounts() method to save mount configs during backup
- Add restore_mounts() method to restore mount configs during restore
- Add MOUNTS stage to backup/restore job stages
- Update BackupManager to call mount backup/restore methods
- Add tests for mount backup/restore functionality
Fixes home-assistant/core#148663
* Address reviewer feedback for mount backup/restore
Changes based on PR review:
- Store mount configs in encrypted mounts.tar instead of unencrypted
backup metadata (security fix for passwords)
- Separate mount restore into config save + async activation tasks
(mounts activate in background, failures don't block restore)
- Add replace_default_backup_mount parameter to control whether to
overwrite existing default mount setting
- Remove unnecessary broad exception handler for default mount setter
- Simplify schema: ATTR_MOUNTS is now just a boolean flag since
actual data is in the encrypted tar file
- Update tests to reflect new async API and return types
* Fix code review issues in mount backup/restore
- Add bind mount handling for MEDIA and SHARE usage types in
_activate_restored_mount() to mirror MountManager.create_mount()
- Fix double save_data() call by using needs_save flag
- Import MountUsage const for usage type checks
* Add pylint disable comments for protected member access
* Tighten broad exception handlers in mount backup restore
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Address second round of reviewer feedback
- Catch OSError separately and check errno.EBADMSG for drive health
- Validate mounts JSON against SCHEMA_MOUNTS_CONFIG before importing
- Use mount_data[ATTR_NAME] instead of .get("name", "unknown")
- Overwrite existing mounts on restore instead of skipping
- Move restore_mount/activate logic to MountManager (no more
protected-access in Backup)
- Drop unused replace_default_backup_mount parameter
- Fix test_backup_progress: add mounts stage to expected events
- Fix test_store_mounts: avoid create_mount which requires dbus
* Rename mounts.tar to supervisor.tar for generic supervisor config
Rename the inner tar from mounts.tar to supervisor.tar so it can hold
multiple config files (mounts.json now, docker credentials later).
Rename store_mounts/restore_mounts to store_supervisor_config/
restore_supervisor_config and update stage names accordingly.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Fix pylint protected-access and test timeouts in backup tests
- Add pylint disable comment for _mounts protected access in test_backup.py
- Mock restore_supervisor_config in test_full_backup_to_mount and
test_partial_backup_to_mount to avoid D-Bus mount activation during
restore that causes timeouts in the test environment
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Address agners review feedback
- Change create_inner_tar() to create_tar() per #6575
- Remove "r" argument from SecureTarFile (now read-only by default)
- Change warning to info for missing mounts tar (old backups won't have it)
- Narrow exception handler to (MountError, vol.Invalid, KeyError, OSError)
* Update supervisor/backups/backup.py
* Address agners feedback: remove metadata flag, add mount feature check
- Remove ATTR_SUPERVISOR boolean flag from backup metadata; instead
check for physical presence of supervisor.tar (like folder backups)
- Remove has_supervisor_config property
- Always attempt supervisor config restore (tar existence check handles it)
- Add HostFeature.MOUNT check in _activate_restored_mount before
attempting to activate mounts on systems without mount support
---------
Co-authored-by: Stefan Agner <stefan@agner.ch>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Mike Degatano <michael.degatano@gmail.com>
* Migrate info and events to aiodocker
* Migrate container logs to aiodocker
* Fix dns plugin loop test
* Fix mocking for docker info
* Fixes from feedback
* Harden monitor error handling
* Deleted failing tests because they were not useful
* Migrate all docker container interactions to aiodocker
* Remove containers_legacy since its no longer used
* Add back remove color logic
* Revert accidental invert of conditional in setup_network
* Fix typos found by copilot
* Apply suggestions from code review
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Revert "Apply suggestions from code review"
This reverts commit 0a475433ea.
---------
Co-authored-by: Stefan Agner <stefan@agner.ch>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Send progress updates during image pull for install/update
* Add extra to tests about job APIs
* Sent out of date progress to sentry and combine done event
* Pulling container image layer
* Harmonize folder and add-on backup error handling
Align add-on and folder backup error handling in that in both cases
errors are recorded on the respective backup Jobs, but not raised to
the caller. This allows the backup to complete successfully even if
some add-ons or folders fail to back up.
Along with this, also record errors in the per-add-on and per-folder
backup jobs, as well as the add-on and folder root job.
And finally, align the exception handling to only catch expected
exceptions for add-ons too.
* Fix pytest
* Improve backup upload location determination
For local backup upload locations, check if the location is on the same
file system an thuse allows to move the backup file after upload. This
allows custom backup mounts. Currently there is no documented,
persistent way to create such mounts in with Home Assistant OS
installations, but since we might add local mounts in the future this
seems a worthwhile addition.
Fixes: #5837
* Fix pytests
* Fix mypy issues in backups module
* Fix mypy issues in dbus module
* Fix mypy issues in api after rebase
* TypedDict to dataclass and other small fixes
* Finish fixing mypy errors in dbus
* local_where must exist
* Fix references to name in tests
* Report stage with error in jobs
* Copy doesn't lose track of the successful copies
* Add stage to errors in api output test
* revert unneessary change to import
* Add tests for a bit more coverage of copy_additional_locations
* Finish out effort of adding and enabling blockbuster
* Skip getting addon file size until securetar fixed
* Fix test for devcontainer and blocking I/O
* Fix docker fixture and load_config to post_init
* Avoid IO in event loop when removing backup
* Refactor backup size calculation
Currently size is lazy loaded when required via properties. This
however is blocking the async event loop.
Backup sizes don't change. Instead of lazy loading the size of a backup
simply determine it on loading/after creation.
* Fix tests for backup size change
* Avoid IO in event loop when loading backups
* Avoid IO in event loop when importing a backup
* Validate Backup always before restoring
Since #5519 we check the encryption password early in restore case.
This has the side effect that we check the file existance early too.
However, in the non-encryption case, the file is not checked early.
This PR changes the behavior to always validate the backup file before
restoring, ensuring both encryption and non-encryption cases are
handled consistently.
In particular, the last case of test_restore_immediate_errors actually
validates that behavior. That test should actually have failed so far.
But it seems that because we validate the backup shortly after freeze
anyways, the exception still got raised early enough.
A simply `await asyncio.sleep(10)` right after the freeze makes the
test case fail. With this change, the test works consistently.
* Address pylint
* Fix backup_manager tests
* Drop warning message
* Drop Docker config from Supervisor backup
The Docker config is part of the main backup metadata. Because we
consolidate encrypted and unencrypted backups today, this leads to
potential bugs when restoring a backup.
* Drop obsolete encrypt/decrypt functions
* Drop unused Backup Job stage
* Handle non-existing file in Backup password check too
Make sure we handle a non-existing backup file also when validating
the password.
* Update supervisor/backups/manager.py
Co-authored-by: Mike Degatano <michael.degatano@gmail.com>
* Add test case and fix password check when multiple locations
* Mock default backup unprotected by default
Instead of setting the protected property which we might not use
everywhere, simply mock the default backup to be unprotected.
* Fix mock of protected backup
* Introduce test for validate_password
Testing showed that validate_password doesn't return anything. Extend
tests to cover this case and fix the actual code.
---------
Co-authored-by: Mike Degatano <michael.degatano@gmail.com>
* Make the API return 404 for non-existing backup files
* Introduce BackupFileNotFoundError exception
* Return 404 on full restore as well
* Fix remaining API tests
* Improve error handling in delete
* Fix pytest
* Fix tests and change error handling to agreed logic
---------
Co-authored-by: Mike Degatano <michael.degatano@gmail.com>
* Backup protected status can vary per location
* Fix test_backup_remove_error test
* Update supervisor/backups/backup.py
* Add Docker registry configuration to backup metadata
* Make use of backup location fixture
* Address pylint
---------
Co-authored-by: Stefan Agner <stefan@agner.ch>
* Extend backup API with file name field
Allow to specify a backup file name when creating a backup. This allows
for user friendly backup file names. If none is specified, the current
behavior remains (backup file name is the backup slug).
* Check passed file name using regex
* Use custom filename on download only if backup file name is backup slug
* ruff format
* Remove path from location for download file name
Introduce a validate password method which only peaks into the archive
to validate the password before starting the actual restore process.
This makes sure that a wrong password returns an error even when
restoring the backup in background.
* Fix and extend cloud backup support
* Clean up task for cloud backup and remove by location
* Args to kwargs on backup methods
* Fix backup remove error test and typing clean up
* Stop backup if pre backup failed in Core
* Fix API tests
* Partial backup in ci since there's no Home assistant
* Add ssl folder to partial backup
* Allow backups when Home Assistant is not running
* Undo change to skip db test
* Migrate to Ruff for lint and format
* Fix pylint issues
* DBus property sets into normal awaitable methods
* Fix tests relying on separate tasks in connect
* Fixes from feedback
* Add background option to backup APIs
* Fix decorator tests
* Working error handling, initial test cases
* Change to schedule_job and always return job id
* Add tests
* Reorder call at/later args
* Validation errors return immediately in background
* None is invalid option for background
* Must pop the background option from body
* Bad message error marks system as unhealthy
* Finish adding test cases for changes
* Rename test file for uniqueness
* bad_message to oserror_bad_message
* Omit some checks and check for network mounts
François MartinandGitHub
