diff --git a/supervisor/exceptions.py b/supervisor/exceptions.py index 5a45a0901d3..c73b658f4aa 100644 --- a/supervisor/exceptions.py +++ b/supervisor/exceptions.py @@ -579,6 +579,10 @@ class MountError(HassioError): """Raise on an error related to mounting/unmounting.""" +class MountActivationError(MountError): + """Raise on mount not reaching active state after mount/reload.""" + + class MountInvalidError(MountError): """Raise on invalid mount attempt.""" diff --git a/supervisor/mounts/manager.py b/supervisor/mounts/manager.py index 1384facf197..d26de27cf58 100644 --- a/supervisor/mounts/manager.py +++ b/supervisor/mounts/manager.py @@ -8,7 +8,7 @@ from ..const import ATTR_NAME from ..coresys import CoreSys, CoreSysAttributes from ..dbus.const import UnitActiveState -from ..exceptions import MountError, MountNotFound +from ..exceptions import MountActivationError, MountError, MountNotFound from ..resolution.const import ContextType, IssueType, SuggestionType from ..utils.common import FileConfiguration from ..utils.sentry import capture_exception @@ -111,16 +111,20 @@ async def create_mount(self, mount: Mount) -> None: """Add/update a mount.""" if mount.name in self._mounts: _LOGGER.debug("Mount '%s' exists, unmounting then mounting from new config") - await self.remove_mount(mount.name) + await self.remove_mount(mount.name, retain_entry=True) _LOGGER.info("Creating or updating mount: %s", mount.name) - self._mounts[mount.name] = mount - await mount.load() + try: + await mount.load() + except MountActivationError as err: + await mount.unmount() + raise err + self._mounts[mount.name] = mount if mount.usage == MountUsage.MEDIA: await self._bind_media(mount) - async def remove_mount(self, name: str) -> None: + async def remove_mount(self, name: str, *, retain_entry: bool = False) -> None: """Remove a mount.""" if name not in self._mounts: raise MountNotFound( @@ -133,7 +137,8 @@ async def remove_mount(self, name: str) -> None: del self._bound_mounts[name] await self._mounts[name].unmount() - del self._mounts[name] + if not retain_entry: + del self._mounts[name] async def reload_mount(self, name: str) -> None: """Reload a mount to retry mounting with same config.""" diff --git a/supervisor/mounts/mount.py b/supervisor/mounts/mount.py index 27539caabe2..97b86f4ab31 100644 --- a/supervisor/mounts/mount.py +++ b/supervisor/mounts/mount.py @@ -20,7 +20,13 @@ UnitActiveState, ) from ..dbus.systemd import SystemdUnit -from ..exceptions import DBusError, DBusSystemdNoSuchUnit, MountError, MountInvalidError +from ..exceptions import ( + DBusError, + DBusSystemdNoSuchUnit, + MountActivationError, + MountError, + MountInvalidError, +) from ..utils.sentry import capture_exception from .const import ATTR_PATH, ATTR_SERVER, ATTR_SHARE, ATTR_USAGE, MountType, MountUsage from .validate import MountData @@ -200,6 +206,9 @@ async def mount(self) -> None: if self.options else [] ) + if self.type != MountType.BIND: + options += [(DBUS_ATTR_TYPE, Variant("s", self.type.value))] + await self.sys_dbus.systemd.start_transient_unit( self.unit_name, StartUnitMode.FAIL, @@ -207,7 +216,6 @@ async def mount(self) -> None: + [ (DBUS_ATTR_DESCRIPTION, Variant("s", self.description)), (DBUS_ATTR_WHAT, Variant("s", self.what)), - (DBUS_ATTR_TYPE, Variant("s", self.type.value)), ], ) except DBusError as err: @@ -218,15 +226,20 @@ async def mount(self) -> None: await self._update_await_activating() if self.state != UnitActiveState.ACTIVE: - raise MountError( + raise MountActivationError( f"Mounting {self.name} did not succeed. Check host logs for errors from mount or systemd unit {self.unit_name} for details.", _LOGGER.error, ) async def unmount(self) -> None: """Unmount using systemd.""" + await self.update() + try: - await self.sys_dbus.systemd.stop_unit(self.unit_name, StopUnitMode.FAIL) + if self.state == UnitActiveState.FAILED: + await self.sys_dbus.systemd.reset_failed_unit(self.unit_name) + else: + await self.sys_dbus.systemd.stop_unit(self.unit_name, StopUnitMode.FAIL) except DBusSystemdNoSuchUnit: _LOGGER.info("Mount %s is not mounted, skipping unmount", self.name) except DBusError as err: @@ -255,7 +268,7 @@ async def reload(self) -> None: await self._update_await_activating() if self.state != UnitActiveState.ACTIVE: - raise MountError( + raise MountActivationError( f"Reloading {self.name} did not succeed. Check host logs for errors from mount or systemd unit {self.unit_name} for details.", _LOGGER.error, ) @@ -407,4 +420,4 @@ def where(self) -> PurePath: @property def options(self) -> list[str]: """List of options to use to mount.""" - return [] + return ["bind"] diff --git a/tests/api/test_mounts.py b/tests/api/test_mounts.py index 41cc587b439..ecb73c10cf2 100644 --- a/tests/api/test_mounts.py +++ b/tests/api/test_mounts.py @@ -1,6 +1,7 @@ """Test mounts API.""" from aiohttp.test_utils import TestClient +from dbus_fast import DBusError, ErrorType import pytest from supervisor.coresys import CoreSys @@ -8,6 +9,7 @@ from tests.dbus_service_mocks.base import DBusServiceMock from tests.dbus_service_mocks.systemd import Systemd as SystemdService +from tests.dbus_service_mocks.systemd_unit import SystemdUnit as SystemdUnitService @pytest.fixture(name="mount") @@ -87,6 +89,70 @@ async def test_api_create_error_mount_exists(api_client: TestClient, mount): assert result["message"] == "A mount already exists with name backup_test" +async def test_api_create_dbus_error_mount_not_added( + api_client: TestClient, + all_dbus_services: dict[str, DBusServiceMock], + tmp_supervisor_data, + path_extern, +): + """Test mount not added to list of mounts if a dbus error occurs.""" + systemd_service: SystemdService = all_dbus_services["systemd"] + systemd_unit_service: SystemdUnitService = all_dbus_services["systemd_unit"] + systemd_service.response_get_unit = DBusError( + "org.freedesktop.systemd1.NoSuchUnit", "error" + ) + systemd_service.response_start_transient_unit = DBusError(ErrorType.FAILED, "fail") + + resp = await api_client.post( + "/mounts", + json={ + "name": "backup_test", + "type": "cifs", + "usage": "backup", + "server": "backup.local", + "share": "backups", + }, + ) + assert resp.status == 400 + result = await resp.json() + assert result["result"] == "error" + assert result["message"] == "Could not mount backup_test due to: fail" + + resp = await api_client.get("/mounts") + result = await resp.json() + assert result["data"]["mounts"] == [] + + systemd_service.response_get_unit = [ + DBusError("org.freedesktop.systemd1.NoSuchUnit", "error"), + "/org/freedesktop/systemd1/unit/tmp_2dyellow_2emount", + "/org/freedesktop/systemd1/unit/tmp_2dyellow_2emount", + ] + systemd_service.response_start_transient_unit = "/org/freedesktop/systemd1/job/7623" + systemd_unit_service.active_state = "failed" + + resp = await api_client.post( + "/mounts", + json={ + "name": "backup_test", + "type": "cifs", + "usage": "backup", + "server": "backup.local", + "share": "backups", + }, + ) + assert resp.status == 400 + result = await resp.json() + assert result["result"] == "error" + assert ( + result["message"] + == "Mounting backup_test did not succeed. Check host logs for errors from mount or systemd unit mnt-data-supervisor-mounts-backup_test.mount for details." + ) + + resp = await api_client.get("/mounts") + result = await resp.json() + assert result["data"]["mounts"] == [] + + async def test_api_update_mount(api_client: TestClient, coresys: CoreSys, mount): """Test updating a mount via API.""" resp = await api_client.put( @@ -134,6 +200,89 @@ async def test_api_update_error_mount_missing(api_client: TestClient): assert result["message"] == "No mount exists with name backup_test" +async def test_api_update_dbus_error_mount_remains( + api_client: TestClient, + all_dbus_services: dict[str, DBusServiceMock], + mount, + tmp_supervisor_data, + path_extern, +): + """Test mount remains in list with unsuccessful state if dbus error occurs during update.""" + systemd_service: SystemdService = all_dbus_services["systemd"] + systemd_unit_service: SystemdUnitService = all_dbus_services["systemd_unit"] + systemd_service.response_get_unit = [ + "/org/freedesktop/systemd1/unit/tmp_2dyellow_2emount", + DBusError("org.freedesktop.systemd1.NoSuchUnit", "error"), + ] + systemd_service.response_start_transient_unit = DBusError(ErrorType.FAILED, "fail") + + resp = await api_client.put( + "/mounts/backup_test", + json={ + "type": "cifs", + "usage": "backup", + "server": "backup.local", + "share": "backups1", + }, + ) + assert resp.status == 400 + result = await resp.json() + assert result["result"] == "error" + assert result["message"] == "Could not mount backup_test due to: fail" + + resp = await api_client.get("/mounts") + result = await resp.json() + assert result["data"]["mounts"] == [ + { + "name": "backup_test", + "type": "cifs", + "usage": "backup", + "server": "backup.local", + "share": "backups", + "state": None, + } + ] + + systemd_service.response_get_unit = [ + "/org/freedesktop/systemd1/unit/tmp_2dyellow_2emount", + DBusError("org.freedesktop.systemd1.NoSuchUnit", "error"), + "/org/freedesktop/systemd1/unit/tmp_2dyellow_2emount", + "/org/freedesktop/systemd1/unit/tmp_2dyellow_2emount", + ] + systemd_service.response_start_transient_unit = "/org/freedesktop/systemd1/job/7623" + systemd_unit_service.active_state = "failed" + + resp = await api_client.put( + "/mounts/backup_test", + json={ + "type": "cifs", + "usage": "backup", + "server": "backup.local", + "share": "backups2", + }, + ) + assert resp.status == 400 + result = await resp.json() + assert result["result"] == "error" + assert ( + result["message"] + == "Mounting backup_test did not succeed. Check host logs for errors from mount or systemd unit mnt-data-supervisor-mounts-backup_test.mount for details." + ) + + resp = await api_client.get("/mounts") + result = await resp.json() + assert result["data"]["mounts"] == [ + { + "name": "backup_test", + "type": "cifs", + "usage": "backup", + "server": "backup.local", + "share": "backups", + "state": None, + } + ] + + async def test_api_reload_mount( api_client: TestClient, all_dbus_services: dict[str, DBusServiceMock], mount ): diff --git a/tests/backups/test_manager.py b/tests/backups/test_manager.py index bdf74bd1a6c..7903081391a 100644 --- a/tests/backups/test_manager.py +++ b/tests/backups/test_manager.py @@ -377,6 +377,8 @@ async def test_backup_media_with_mounts( "/org/freedesktop/systemd1/unit/tmp_2dyellow_2emount", DBusError("org.freedesktop.systemd1.NoSuchUnit", "error"), "/org/freedesktop/systemd1/unit/tmp_2dyellow_2emount", + "/org/freedesktop/systemd1/unit/tmp_2dyellow_2emount", + "/org/freedesktop/systemd1/unit/tmp_2dyellow_2emount", ] # Make some normal test files diff --git a/tests/mounts/test_manager.py b/tests/mounts/test_manager.py index 46b448811c6..802281203f0 100644 --- a/tests/mounts/test_manager.py +++ b/tests/mounts/test_manager.py @@ -4,13 +4,13 @@ import os from pathlib import Path -from dbus_fast import DBusError, Variant +from dbus_fast import DBusError, ErrorType, Variant from dbus_fast.aio.message_bus import MessageBus import pytest from supervisor.coresys import CoreSys from supervisor.dbus.const import UnitActiveState -from supervisor.exceptions import MountNotFound +from supervisor.exceptions import MountActivationError, MountError, MountNotFound from supervisor.mounts.manager import MountManager from supervisor.mounts.mount import Mount from supervisor.resolution.const import ContextType, IssueType, SuggestionType @@ -101,9 +101,9 @@ async def test_load( "mnt-data-supervisor-mounts-backup_test.mount", "fail", [ + ["Type", Variant("s", "cifs")], ["Description", Variant("s", "Supervisor cifs mount: backup_test")], ["What", Variant("s", "//backup.local/backups")], - ["Type", Variant("s", "cifs")], ], [], ), @@ -111,9 +111,9 @@ async def test_load( "mnt-data-supervisor-mounts-media_test.mount", "fail", [ + ["Type", Variant("s", "nfs")], ["Description", Variant("s", "Supervisor nfs mount: media_test")], ["What", Variant("s", "media.local:/media")], - ["Type", Variant("s", "nfs")], ], [], ), @@ -121,9 +121,9 @@ async def test_load( "mnt-data-supervisor-media-media_test.mount", "fail", [ + ["Options", Variant("s", "bind")], ["Description", Variant("s", "Supervisor bind mount: bind_media_test")], ["What", Variant("s", "/mnt/data/supervisor/mounts/media_test")], - ["Type", Variant("s", "bind")], ], [], ), @@ -229,12 +229,12 @@ async def test_mount_failed_during_load( "mnt-data-supervisor-media-media_test.mount", "fail", [ + ["Options", Variant("s", "bind")], [ "Description", Variant("s", "Supervisor bind mount: emergency_media_test"), ], ["What", Variant("s", "/mnt/data/supervisor/emergency/media_test")], - ["Type", Variant("s", "bind")], ], [], ) @@ -295,6 +295,8 @@ async def test_update_mount( assert mount_new.state is None systemd_service.response_get_unit = [ + "/org/freedesktop/systemd1/unit/tmp_2dyellow_2emount", + "/org/freedesktop/systemd1/unit/tmp_2dyellow_2emount", ERROR_NO_UNIT, "/org/freedesktop/systemd1/unit/tmp_2dyellow_2emount", ERROR_NO_UNIT, @@ -404,3 +406,69 @@ async def test_save_data(coresys: CoreSys, tmp_supervisor_data: Path, path_exter "password": "password", } ] + + +async def test_create_mount_start_unit_failure( + coresys: CoreSys, + all_dbus_services: dict[str, DBusServiceMock], + tmp_supervisor_data, + path_extern, +): + """Test failure to start mount unit does not add mount to the list.""" + systemd_service: SystemdService = all_dbus_services["systemd"] + systemd_service.StartTransientUnit.calls.clear() + systemd_service.ResetFailedUnit.calls.clear() + systemd_service.StopUnit.calls.clear() + + systemd_service.response_get_unit = ERROR_NO_UNIT + systemd_service.response_start_transient_unit = DBusError(ErrorType.FAILED, "fail") + + await coresys.mounts.load() + + mount = Mount.from_dict(coresys, BACKUP_TEST_DATA) + + with pytest.raises(MountError): + await coresys.mounts.create_mount(mount) + + assert mount.state is None + assert mount not in coresys.mounts + + assert len(systemd_service.StartTransientUnit.calls) == 1 + assert not systemd_service.ResetFailedUnit.calls + assert not systemd_service.StopUnit.calls + + +async def test_create_mount_activation_failure( + coresys: CoreSys, + all_dbus_services: dict[str, DBusServiceMock], + tmp_supervisor_data, + path_extern, +): + """Test activation failure during create mount does not add mount to the list and unmounts new mount.""" + systemd_service: SystemdService = all_dbus_services["systemd"] + systemd_unit_service: SystemdUnitService = all_dbus_services["systemd_unit"] + + systemd_service.StartTransientUnit.calls.clear() + systemd_service.ResetFailedUnit.calls.clear() + systemd_service.StopUnit.calls.clear() + + systemd_service.response_get_unit = [ + ERROR_NO_UNIT, + "/org/freedesktop/systemd1/unit/tmp_2dyellow_2emount", + "/org/freedesktop/systemd1/unit/tmp_2dyellow_2emount", + ] + systemd_unit_service.active_state = "failed" + + await coresys.mounts.load() + + mount = Mount.from_dict(coresys, BACKUP_TEST_DATA) + + with pytest.raises(MountActivationError): + await coresys.mounts.create_mount(mount) + + assert mount.state is None + assert mount not in coresys.mounts + + assert len(systemd_service.StartTransientUnit.calls) == 1 + assert len(systemd_service.ResetFailedUnit.calls) == 1 + assert not systemd_service.StopUnit.calls diff --git a/tests/mounts/test_mount.py b/tests/mounts/test_mount.py index 8ce070e5cad..aaff6418344 100644 --- a/tests/mounts/test_mount.py +++ b/tests/mounts/test_mount.py @@ -73,9 +73,9 @@ async def test_cifs_mount( "fail", [ ["Options", Variant("s", "username=admin,password=password")], + ["Type", Variant("s", "cifs")], ["Description", Variant("s", "Supervisor cifs mount: test")], ["What", Variant("s", "//test.local/camera")], - ["Type", Variant("s", "cifs")], ], [], ) @@ -130,9 +130,9 @@ async def test_nfs_mount( "fail", [ ["Options", Variant("s", "port=1234")], + ["Type", Variant("s", "nfs")], ["Description", Variant("s", "Supervisor nfs mount: test")], ["What", Variant("s", "test.local:/media/camera")], - ["Type", Variant("s", "nfs")], ], [], ) @@ -176,9 +176,9 @@ async def test_load( "mnt-data-supervisor-mounts-test.mount", "fail", [ + ["Type", Variant("s", "cifs")], ["Description", Variant("s", "Supervisor cifs mount: test")], ["What", Variant("s", "//test.local/share")], - ["Type", Variant("s", "cifs")], ], [], )