From dc0ca8295d5753657f7a3c2478bc1ab0b92c08d0 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Fri, 24 Jul 2026 16:06:22 +0200 Subject: [PATCH] Add repair to move local data blocking a mount target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When an add-on writes into a media/share directory while its network mount is not in place (#7037), the local data blocks re-creating the mount: mounting over a non-empty directory is refused. Until now this failed silently at Supervisor startup — the bind mounts were created as fire-and-forget tasks — and the only way out was to remove the data manually over SSH/Samba and re-create the mount via the API. Surface the condition as a new mount_target_not_empty issue and offer a move_local_data suggestion. The fixup moves the blocking data to a _local_recovery folder in a user-accessible location — media or share for bind mount targets, local backup storage for backup mounts (their data mount directory is not reachable for users) — then reloads the mount. Nothing is deleted; users can inspect and clean up the recovered data via the media browser or the share and backup folders. Bind mount failures during load are now awaited and routed into resolution issues instead of being swallowed as fire-and-forget tasks; bind failures other than blocking local data create the existing mount_failed issue. A successful mount reload dismisses a stale local data issue. Co-Authored-By: Claude Fable 5 --- supervisor/mounts/manager.py | 136 ++++++++++++++-- supervisor/resolution/const.py | 2 + .../resolution/fixups/mount_execute_remove.py | 2 +- .../fixups/mount_move_local_data.py | 53 ++++++ tests/mounts/test_manager.py | 84 ++++++++++ .../fixup/test_mount_move_local_data.py | 151 ++++++++++++++++++ 6 files changed, 412 insertions(+), 16 deletions(-) create mode 100644 supervisor/resolution/fixups/mount_move_local_data.py create mode 100644 tests/resolution/fixup/test_mount_move_local_data.py diff --git a/supervisor/mounts/manager.py b/supervisor/mounts/manager.py index 5339ab16625..378d48687eb 100644 --- a/supervisor/mounts/manager.py +++ b/supervisor/mounts/manager.py @@ -5,7 +5,7 @@ from contextlib import suppress from dataclasses import dataclass, replace import logging -from pathlib import PurePath +from pathlib import Path, PurePath from typing import Self from ..const import ATTR_NAME @@ -21,7 +21,8 @@ from ..host.const import HostFeature from ..jobs.const import JobCondition from ..jobs.decorator import Job -from ..resolution.const import SuggestionType +from ..resolution.const import ContextType, IssueType, SuggestionType +from ..resolution.data import Issue from ..utils.common import FileConfiguration from ..utils.sentry import async_capture_exception from .const import ( @@ -135,22 +136,21 @@ async def load(self) -> None: self.mounts.copy(), [mount.load() for mount in self.mounts] ) - # Bind all media mounts to directories in media + # Bind all media mounts to directories in media. Bind failures used + # to be silently swallowed as fire-and-forget tasks — route them into + # resolution issues so the user learns about e.g. local data blocking + # the bind mount target. if self.media_mounts: - await asyncio.wait( - [ - self.sys_create_task(self._bind_media(mount)) - for mount in self.media_mounts - ] + await self._mount_errors_to_issues( + self.media_mounts, + [self._bind_media(mount) for mount in self.media_mounts], ) # Bind all share mounts to directories in share if self.share_mounts: - await asyncio.wait( - [ - self.sys_create_task(self._bind_share(mount)) - for mount in self.share_mounts - ] + await self._mount_errors_to_issues( + self.share_mounts, + [self._bind_share(mount) for mount in self.share_mounts], ) @Job(name="mount_manager_reload", conditions=[JobCondition.MOUNT_AVAILABLE]) @@ -175,12 +175,15 @@ async def reload(self) -> None: async def _mount_errors_to_issues( self, mounts: list[Mount], mount_tasks: list[Awaitable[None]] ) -> None: - """Await a list of tasks on mounts and turn each error into a failed mount issue.""" + """Await a list of tasks on mounts and turn each error into a resolution issue.""" errors = await asyncio.gather(*mount_tasks, return_exceptions=True) for i in range(len(errors)): # pylint: disable=consider-using-enumerate if not (err := errors[i]): continue + if isinstance(err, MountTargetNotEmptyError | MountTargetNotDirectoryError): + self._add_local_data_issue(mounts[i].name) + continue if mounts[i].failed_issue in self.sys_resolution.issues: continue if not isinstance(err, MountError): @@ -194,6 +197,25 @@ async def _mount_errors_to_issues( ], ) + def _local_data_issue(self, name: str) -> Issue: + """Return the issue used when local data blocks a mount target.""" + return Issue( + IssueType.MOUNT_TARGET_NOT_EMPTY, ContextType.MOUNT, reference=name + ) + + def _add_local_data_issue(self, name: str) -> None: + """Add an issue for local data blocking a mount target.""" + if not self.sys_resolution.get_issue_if_present( + issue := self._local_data_issue(name) + ): + self.sys_resolution.add_issue( + issue, + suggestions=[ + SuggestionType.MOVE_LOCAL_DATA, + SuggestionType.EXECUTE_REMOVE, + ], + ) + @Job( name="mount_manager_create_mount", conditions=[JobCondition.MOUNT_AVAILABLE], @@ -323,7 +345,91 @@ async def reload_mount(self, name: str) -> None: # restarting a failed data mount tears down the bind mount as well — # our BoundMount bookkeeping cannot know whether that happened. if bound_mount := self._bound_mounts.get(name): - await self._bind_mount(bound_mount.mount, bound_mount.bind_mount.where) + try: + await self._bind_mount(bound_mount.mount, bound_mount.bind_mount.where) + except MountTargetNotEmptyError, MountTargetNotDirectoryError: + self._add_local_data_issue(name) + raise + + # Everything is mounted again, local data can no longer be in the way + if issue := self.sys_resolution.get_issue_if_present( + self._local_data_issue(name) + ): + self.sys_resolution.dismiss_issue(issue) + + @Job( + name="mount_manager_relocate_local_data", + conditions=[JobCondition.MOUNT_AVAILABLE], + on_condition=MountJobError, + ) + async def relocate_local_data(self, name: str) -> None: + """Move local data out of a mount's target directories, then remount. + + Local data ends up in a mount's target directory when something + wrote into it while the mount was not in place (e.g. an add-on + recording to its media directory before network storage was set up + or after the bind mount was torn down). The data is moved to a + `_local_recovery` folder in a user-accessible location + (media, share or local backup storage) instead of being deleted. + """ + # Add mount name to job + self.sys_jobs.current.reference = name + + if name not in self._mounts: + raise MountNotFound( + f"Cannot relocate local data for '{name}', no mount exists with that name" + ) + mount = self._mounts[name] + + paths = [mount.local_where] + if mount.usage == MountUsage.MEDIA: + recovery_base = self.sys_config.path_media + paths.append(self.sys_config.path_media / name) + elif mount.usage == MountUsage.SHARE: + recovery_base = self.sys_config.path_share + paths.append(self.sys_config.path_share / name) + else: + # Backup mounts have no bind mount and their data mount directory + # is not user-accessible — move the data to local backup storage, + # which is reachable via the backup share and add-ons. + recovery_base = self.sys_config.path_backup + + def move_aside() -> list[tuple[Path, Path]]: + moved: list[tuple[Path, Path]] = [] + for path in paths: + try: + if path.is_mount() or not path.exists(): + continue + if path.is_dir() and not any(path.iterdir()): + continue + except OSError: + continue + + target = recovery_base / f"{name}_local_recovery" + counter = 1 + while target.exists(): + counter += 1 + target = recovery_base / f"{name}_local_recovery_{counter}" + path.rename(target) + moved.append((path, target)) + return moved + + try: + moved = await self.sys_run_in_executor(move_aside) + except OSError as err: + raise MountError( + f"Could not move local data for mount {name}: {err!s}", _LOGGER.error + ) from err + + for path, target in moved: + _LOGGER.info( + "Moved local data blocking mount %s from %s to %s", + name, + path.as_posix(), + target.as_posix(), + ) + + await self.reload_mount(name) async def _bind_media(self, mount: Mount) -> None: """Bind a media mount to media directory.""" diff --git a/supervisor/resolution/const.py b/supervisor/resolution/const.py index 1545704f50d..2e8c4ba0fa6 100644 --- a/supervisor/resolution/const.py +++ b/supervisor/resolution/const.py @@ -102,6 +102,7 @@ class IssueType(StrEnum): IPV4_CONNECTION_PROBLEM = "ipv4_connection_problem" MISSING_IMAGE = "missing_image" MOUNT_FAILED = "mount_failed" + MOUNT_TARGET_NOT_EMPTY = "mount_target_not_empty" MULTIPLE_DATA_DISKS = "multiple_data_disks" NO_CURRENT_BACKUP = "no_current_backup" NTP_SYNC_FAILED = "ntp_sync_failed" @@ -133,5 +134,6 @@ class SuggestionType(StrEnum): EXECUTE_START = "execute_start" EXECUTE_STOP = "execute_stop" EXECUTE_UPDATE = "execute_update" + MOVE_LOCAL_DATA = "move_local_data" REGISTRY_LOGIN = "registry_login" RENAME_DATA_DISK = "rename_data_disk" diff --git a/supervisor/resolution/fixups/mount_execute_remove.py b/supervisor/resolution/fixups/mount_execute_remove.py index eaebf042a67..2cd2eaa6c90 100644 --- a/supervisor/resolution/fixups/mount_execute_remove.py +++ b/supervisor/resolution/fixups/mount_execute_remove.py @@ -41,7 +41,7 @@ def context(self) -> ContextType: @property def issues(self) -> list[IssueType]: """Return a IssueType enum list.""" - return [IssueType.MOUNT_FAILED] + return [IssueType.MOUNT_FAILED, IssueType.MOUNT_TARGET_NOT_EMPTY] @property def auto(self) -> bool: diff --git a/supervisor/resolution/fixups/mount_move_local_data.py b/supervisor/resolution/fixups/mount_move_local_data.py new file mode 100644 index 00000000000..8d8fd479483 --- /dev/null +++ b/supervisor/resolution/fixups/mount_move_local_data.py @@ -0,0 +1,53 @@ +"""Helper to fix an issue with a mount by moving local data out of its target.""" + +import logging + +from ...coresys import CoreSys +from ...exceptions import MountError, MountNotFound, ResolutionFixupError +from ..const import ContextType, IssueType, SuggestionType +from ..data import Suggestion +from .base import FixupBase + +_LOGGER: logging.Logger = logging.getLogger(__name__) + + +def setup(coresys: CoreSys) -> FixupBase: + """Check setup function.""" + return FixupMountMoveLocalData(coresys) + + +class FixupMountMoveLocalData(FixupBase): + """Storage class for fixup.""" + + async def process_fixup(self, suggestion: Suggestion) -> None: + """Move local data out of the mount target directories and remount.""" + try: + await self.sys_mounts.relocate_local_data(suggestion.reference) + except MountNotFound: + _LOGGER.warning("Can't find mount %s for fixup", suggestion.reference) + except MountError as err: + # Leave the issue/suggestion in place so the user can try again + _LOGGER.warning( + "Could not move local data for mount %s: %s", suggestion.reference, err + ) + raise ResolutionFixupError from err + + @property + def suggestion(self) -> SuggestionType: + """Return a SuggestionType enum.""" + return SuggestionType.MOVE_LOCAL_DATA + + @property + def context(self) -> ContextType: + """Return a ContextType enum.""" + return ContextType.MOUNT + + @property + def issues(self) -> list[IssueType]: + """Return a IssueType enum list.""" + return [IssueType.MOUNT_TARGET_NOT_EMPTY] + + @property + def auto(self) -> bool: + """Return if a fixup can be apply as auto fix.""" + return False diff --git a/tests/mounts/test_manager.py b/tests/mounts/test_manager.py index fcb8e7efc70..3db2c21a76c 100644 --- a/tests/mounts/test_manager.py +++ b/tests/mounts/test_manager.py @@ -596,6 +596,90 @@ async def test_save_data( ] +async def test_load_bind_failure_creates_local_data_issue( + coresys: CoreSys, + all_dbus_services: dict[str, DBusServiceMock], + tmp_supervisor_data, + path_extern, + mount_propagation, + mock_is_mount, +): + """Test local data blocking the bind mount at load creates a repair issue.""" + systemd_service: SystemdService = all_dbus_services["systemd"] + + mount = Mount.from_dict(coresys, MEDIA_TEST_DATA) + coresys.mounts._mounts = {"media_test": mount} # pylint: disable=protected-access + + media_dir = coresys.config.path_media / "media_test" + media_dir.mkdir() + (media_dir / "recording.mp4").touch() + + systemd_service.response_get_unit = { + "mnt-data-supervisor-mounts-media_test.mount": [ + "/org/freedesktop/systemd1/unit/tmp_2dyellow_2emount" + ], + "mnt-data-supervisor-media-media_test.mount": [ERROR_NO_UNIT], + } + await coresys.mounts.load() + + issue = Issue( + IssueType.MOUNT_TARGET_NOT_EMPTY, ContextType.MOUNT, reference="media_test" + ) + assert issue in coresys.resolution.issues + assert coresys.resolution.suggestions_for_issue(issue) == { + Suggestion( + SuggestionType.MOVE_LOCAL_DATA, ContextType.MOUNT, reference="media_test" + ), + Suggestion( + SuggestionType.EXECUTE_REMOVE, ContextType.MOUNT, reference="media_test" + ), + } + + +async def test_reload_mount_dismisses_local_data_issue( + coresys: CoreSys, + all_dbus_services: dict[str, DBusServiceMock], + mount: Mount, +): + """Test a successful reload dismisses a stale local data issue.""" + systemd_service: SystemdService = all_dbus_services["systemd"] + + coresys.resolution.create_issue( + IssueType.MOUNT_TARGET_NOT_EMPTY, + ContextType.MOUNT, + reference="media_test", + suggestions=[SuggestionType.MOVE_LOCAL_DATA, SuggestionType.EXECUTE_REMOVE], + ) + + systemd_service.response_get_unit = [ + "/org/freedesktop/systemd1/unit/tmp_2dyellow_2emount", + ERROR_NO_UNIT, + "/org/freedesktop/systemd1/unit/tmp_2dyellow_2emount", + ] + await coresys.mounts.reload_mount(mount.name) + + assert coresys.resolution.issues == [] + assert coresys.resolution.suggestions == [] + + +async def test_relocate_local_data_recovery_name_collision( + coresys: CoreSys, + all_dbus_services: dict[str, DBusServiceMock], + mount: Mount, +): + """Test relocating local data picks a free recovery folder name.""" + media_dir = coresys.config.path_media / "media_test" + media_dir.mkdir(exist_ok=True) + (media_dir / "recording.mp4").touch() + (coresys.config.path_media / "media_test_local_recovery").mkdir() + + await coresys.mounts.relocate_local_data(mount.name) + + recovery_dir = coresys.config.path_media / "media_test_local_recovery_2" + assert (recovery_dir / "recording.mp4").exists() + assert not media_dir.exists() + + async def test_create_mount_blocked_by_existing_local_data( coresys: CoreSys, all_dbus_services: dict[str, DBusServiceMock], diff --git a/tests/resolution/fixup/test_mount_move_local_data.py b/tests/resolution/fixup/test_mount_move_local_data.py new file mode 100644 index 00000000000..15887942645 --- /dev/null +++ b/tests/resolution/fixup/test_mount_move_local_data.py @@ -0,0 +1,151 @@ +"""Test fixup mount move local data.""" + +from unittest.mock import patch + +from supervisor.coresys import CoreSys +from supervisor.exceptions import MountError +from supervisor.mounts.manager import MountManager +from supervisor.mounts.mount import Mount +from supervisor.resolution.const import ContextType, IssueType, SuggestionType +from supervisor.resolution.fixups.mount_move_local_data import FixupMountMoveLocalData + +from tests.dbus_service_mocks.base import DBusServiceMock + +MEDIA_TEST_DATA = { + "name": "media_test", + "type": "nfs", + "usage": "media", + "server": "media.local", + "path": "/media", +} +BACKUP_TEST_DATA = { + "name": "backup_test", + "type": "cifs", + "usage": "backup", + "server": "backup.local", + "share": "backups", +} + + +async def test_fixup_media_mount( + coresys: CoreSys, + all_dbus_services: dict[str, DBusServiceMock], + tmp_supervisor_data, + path_extern, + mount_propagation, + mock_is_mount, +): + """Test fixup moves local data out of the media directory and remounts.""" + mount_move_local_data = FixupMountMoveLocalData(coresys) + + assert mount_move_local_data.auto is False + + await coresys.mounts.create_mount(Mount.from_dict(coresys, MEDIA_TEST_DATA)) + + media_dir = coresys.config.path_media / "media_test" + media_dir.mkdir(exist_ok=True) + (media_dir / "recording.mp4").touch() + + coresys.resolution.create_issue( + IssueType.MOUNT_TARGET_NOT_EMPTY, + ContextType.MOUNT, + reference="media_test", + suggestions=[SuggestionType.MOVE_LOCAL_DATA, SuggestionType.EXECUTE_REMOVE], + ) + + await mount_move_local_data() + + recovery_dir = coresys.config.path_media / "media_test_local_recovery" + assert (recovery_dir / "recording.mp4").exists() + assert not media_dir.exists() + assert coresys.resolution.issues == [] + assert coresys.resolution.suggestions == [] + assert "media_test" in coresys.mounts + + +async def test_fixup_backup_mount( + coresys: CoreSys, + all_dbus_services: dict[str, DBusServiceMock], + tmp_supervisor_data, + path_extern, + mount_propagation, + mock_is_mount, +): + """Test fixup moves local data of a backup mount to local backup storage.""" + mount_move_local_data = FixupMountMoveLocalData(coresys) + + await coresys.mounts.create_mount(Mount.from_dict(coresys, BACKUP_TEST_DATA)) + + mount_dir = coresys.mounts.get("backup_test").local_where + mount_dir.mkdir(parents=True, exist_ok=True) + (mount_dir / "stranded_backup.tar").touch() + + coresys.resolution.create_issue( + IssueType.MOUNT_TARGET_NOT_EMPTY, + ContextType.MOUNT, + reference="backup_test", + suggestions=[SuggestionType.MOVE_LOCAL_DATA, SuggestionType.EXECUTE_REMOVE], + ) + + await mount_move_local_data() + + recovery_dir = coresys.config.path_backup / "backup_test_local_recovery" + assert (recovery_dir / "stranded_backup.tar").exists() + assert not mount_dir.exists() + assert coresys.resolution.issues == [] + assert coresys.resolution.suggestions == [] + + +async def test_fixup_failure_keeps_suggestion( + coresys: CoreSys, + all_dbus_services: dict[str, DBusServiceMock], + tmp_supervisor_data, + path_extern, + mount_propagation, + mock_is_mount, +): + """Test failing to relocate keeps the issue and suggestion for retry.""" + mount_move_local_data = FixupMountMoveLocalData(coresys) + + await coresys.mounts.create_mount(Mount.from_dict(coresys, MEDIA_TEST_DATA)) + + coresys.resolution.create_issue( + IssueType.MOUNT_TARGET_NOT_EMPTY, + ContextType.MOUNT, + reference="media_test", + suggestions=[SuggestionType.MOVE_LOCAL_DATA, SuggestionType.EXECUTE_REMOVE], + ) + + with patch.object( + MountManager, "relocate_local_data", side_effect=MountError("fail") + ): + await mount_move_local_data() + + assert len(coresys.resolution.issues) == 1 + assert len(coresys.resolution.suggestions) == 2 + + +async def test_fixup_missing_mount( + coresys: CoreSys, + all_dbus_services: dict[str, DBusServiceMock], + tmp_supervisor_data, + path_extern, + mount_propagation, + mock_is_mount, +): + """Test fixup dismisses the issue if the mount no longer exists.""" + mount_move_local_data = FixupMountMoveLocalData(coresys) + + await coresys.mounts.load() + + coresys.resolution.create_issue( + IssueType.MOUNT_TARGET_NOT_EMPTY, + ContextType.MOUNT, + reference="does_not_exist", + suggestions=[SuggestionType.MOVE_LOCAL_DATA, SuggestionType.EXECUTE_REMOVE], + ) + + await mount_move_local_data() + + assert coresys.resolution.issues == [] + assert coresys.resolution.suggestions == []