Mount falls back on restart if reload fails (#6197)

Author: mdegat01

supervisor/mounts/mount.py

@@ -343,25 +343,51 @@ async def reload(self) -> None:
             )
             await self.mount()
         except DBusError as err:
-            raise MountError(
-                f"Could not reload mount {self.name} due to: {err!s}", _LOGGER.error
-            ) from err
+            _LOGGER.error(
+                "Could not reload mount %s due to: %s. Trying a restart", self.name, err
+            )
+            await self._restart()
         else:
             if unit := await self._update_unit():
                 await self._update_state_await(
                     unit, not_state=UnitActiveState.ACTIVATING
                 )
 
             if not await self.is_mounted():
-                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,
+                _LOGGER.info(
+                    "Mount %s not correctly mounted after a reload. Trying a restart",
+                    self.name,
                 )
+                await self._restart()
 
         # If it is mounted now, dismiss corresponding issue if present
         if self.failed_issue in self.sys_resolution.issues:
             self.sys_resolution.dismiss_issue(self.failed_issue)
 
+    async def _restart(self) -> None:
+        """Restart mount unit to re-mount."""
+        try:
+            await self.sys_dbus.systemd.restart_unit(self.unit_name, StartUnitMode.FAIL)
+        except DBusSystemdNoSuchUnit:
+            _LOGGER.info(
+                "Mount %s is not mounted, mounting instead of restarting", self.name
+            )
+            await self.mount()
+            return
+        except DBusError as err:
+            raise MountError(
+                f"Could not restart mount {self.name} due to: {err!s}", _LOGGER.error
+            ) from err
+
+        if unit := await self._update_unit():
+            await self._update_state_await(unit, not_state=UnitActiveState.ACTIVATING)
+
+        if not await self.is_mounted():
+            raise MountActivationError(
+                f"Restarting {self.name} did not succeed. Check host logs for errors from mount or systemd unit {self.unit_name} for details.",
+                _LOGGER.error,
+            )
+
 
 class NetworkMount(Mount, ABC):
     """A network mount."""

tests/dbus_service_mocks/systemd.py

@@ -36,6 +36,7 @@ class Systemd(DBusServiceMock):
     response_reload_or_restart_unit: str | DBusError = (
         "/org/freedesktop/systemd1/job/7623"
     )
+    response_restart_unit: str | DBusError = "/org/freedesktop/systemd1/job/7623"
     response_start_transient_unit: str | DBusError = (
         "/org/freedesktop/systemd1/job/7623"
     )
@@ -683,9 +684,11 @@ def ReloadOrRestartUnit(self, name: "s", mode: "s") -> "o":
     @dbus_method()
     def RestartUnit(self, name: "s", mode: "s") -> "o":
         """Restart a service unit."""
+        if isinstance(self.response_restart_unit, DBusError):
+            raise self.response_restart_unit  # pylint: disable=raising-bad-type
         if self.mock_systemd_unit:
             self.mock_systemd_unit.active_state = "active"
-        return "/org/freedesktop/systemd1/job/7623"
+        return self.response_restart_unit
 
     @dbus_method()
     def StartTransientUnit(

tests/mounts/test_mount.py

@@ -7,7 +7,7 @@
 from pathlib import Path
 import stat
 from typing import Any
-from unittest.mock import MagicMock
+from unittest.mock import MagicMock, patch
 
 from dbus_fast import DBusError, ErrorType, Variant
 import pytest
@@ -514,18 +514,16 @@ async def test_unmount_failure(
     assert systemd_service.StopUnit.calls == []
 
 
+@pytest.mark.usefixtures("tmp_supervisor_data", "path_extern", "mock_is_mount")
 async def test_reload_failure(
-    coresys: CoreSys,
-    all_dbus_services: dict[str, DBusServiceMock],
-    tmp_supervisor_data,
-    path_extern,
-    mock_is_mount,
+    coresys: CoreSys, all_dbus_services: dict[str, DBusServiceMock]
 ):
     """Test failure to reload."""
     systemd_service: SystemdService = all_dbus_services["systemd"]
     systemd_unit_service: SystemdUnitService = all_dbus_services["systemd_unit"]
     systemd_service.StartTransientUnit.calls.clear()
     systemd_service.ReloadOrRestartUnit.calls.clear()
+    systemd_service.RestartUnit.calls.clear()
     systemd_service.GetUnit.calls.clear()
 
     mount = Mount.from_dict(
@@ -539,28 +537,46 @@ async def test_reload_failure(
         },
     )
 
-    # Raise error on ReloadOrRestartUnit error
+    # Raise error on ReloadOrRestartUnit and RestartUnit error
     systemd_service.response_reload_or_restart_unit = ERROR_FAILURE
+    systemd_service.response_restart_unit = ERROR_FAILURE
     with pytest.raises(MountError):
         await mount.reload()
 
     assert mount.state is None
     assert len(systemd_service.ReloadOrRestartUnit.calls) == 1
+    assert len(systemd_service.RestartUnit.calls) == 1
     assert systemd_service.GetUnit.calls == []
     assert systemd_service.StartTransientUnit.calls == []
 
-    # Raise error if state is not "active" after reload
+    # RestartUnit if ReloadOrRestartUnit does not get it mounted
     systemd_service.ReloadOrRestartUnit.calls.clear()
+    systemd_service.RestartUnit.calls.clear()
     systemd_service.response_reload_or_restart_unit = (
         "/org/freedesktop/systemd1/job/7623"
     )
+    systemd_service.response_restart_unit = "/org/freedesktop/systemd1/job/7623"
+    with patch("supervisor.mounts.mount.Path.is_mount", side_effect=[False, True]):
+        await mount.reload()
+
+    assert mount.state == UnitActiveState.ACTIVE
+    assert len(systemd_service.ReloadOrRestartUnit.calls) == 1
+    assert len(systemd_service.RestartUnit.calls) == 1
+    assert len(systemd_service.GetUnit.calls) == 2
+    assert systemd_service.StartTransientUnit.calls == []
+
+    # Raise error if state is not "active" after reload
+    systemd_service.ReloadOrRestartUnit.calls.clear()
+    systemd_service.RestartUnit.calls.clear()
+    systemd_service.GetUnit.calls.clear()
     systemd_unit_service.active_state = "failed"
     with pytest.raises(MountError):
         await mount.reload()
 
     assert mount.state == UnitActiveState.FAILED
     assert len(systemd_service.ReloadOrRestartUnit.calls) == 1
-    assert len(systemd_service.GetUnit.calls) == 1
+    assert len(systemd_service.RestartUnit.calls) == 1
+    assert len(systemd_service.GetUnit.calls) == 2
     assert systemd_service.StartTransientUnit.calls == []
 
     # If error is NoSuchUnit then don't raise just mount instead as its not mounted