Adopt a disabled data disk (#5010)

supervisor/os/const.py

@@ -1,6 +1,7 @@
 """Constants for OS."""
 
 FILESYSTEM_LABEL_DATA_DISK = "hassos-data"
+FILESYSTEM_LABEL_DISABLED_DATA_DISK = "hassos-data-dis"
 FILESYSTEM_LABEL_OLD_DATA_DISK = "hassos-data-old"
 PARTITION_NAME_EXTERNAL_DATA_DISK = "hassos-data-external"
 PARTITION_NAME_OLD_EXTERNAL_DATA_DISK = "hassos-data-external-old"

supervisor/resolution/checks/disabled_data_disk.py

@@ -0,0 +1,63 @@
+"""Helpers to check for a disabled data disk."""
+
+from pathlib import Path
+
+from ...const import CoreState
+from ...coresys import CoreSys
+from ...dbus.udisks2.block import UDisks2Block
+from ...dbus.udisks2.data import DeviceSpecification
+from ...os.const import FILESYSTEM_LABEL_DISABLED_DATA_DISK
+from ..const import ContextType, IssueType, SuggestionType
+from .base import CheckBase
+
+
+def setup(coresys: CoreSys) -> CheckBase:
+    """Check setup function."""
+    return CheckDisabledDataDisk(coresys)
+
+
+class CheckDisabledDataDisk(CheckBase):
+    """CheckDisabledDataDisk class for check."""
+
+    async def run_check(self) -> None:
+        """Run check if not affected by issue."""
+        for block_device in self.sys_dbus.udisks2.block_devices:
+            if self._is_disabled_data_disk(block_device):
+                self.sys_resolution.create_issue(
+                    IssueType.DISABLED_DATA_DISK,
+                    ContextType.SYSTEM,
+                    reference=block_device.device.as_posix(),
+                    suggestions=[
+                        SuggestionType.RENAME_DATA_DISK,
+                        SuggestionType.ADOPT_DATA_DISK,
+                    ],
+                )
+
+    async def approve_check(self, reference: str | None = None) -> bool:
+        """Approve check if it is affected by issue."""
+        resolved = await self.sys_dbus.udisks2.resolve_device(
+            DeviceSpecification(path=Path(reference))
+        )
+        return resolved and self._is_disabled_data_disk(resolved[0])
+
+    def _is_disabled_data_disk(self, block_device: UDisks2Block) -> bool:
+        """Return true if filesystem block device has name indicating it was disabled by OS."""
+        return (
+            block_device.filesystem
+            and block_device.id_label == FILESYSTEM_LABEL_DISABLED_DATA_DISK
+        )
+
+    @property
+    def issue(self) -> IssueType:
+        """Return a IssueType enum."""
+        return IssueType.DISABLED_DATA_DISK
+
+    @property
+    def context(self) -> ContextType:
+        """Return a ContextType enum."""
+        return ContextType.SYSTEM
+
+    @property
+    def states(self) -> list[CoreState]:
+        """Return a list of valid states when this check can run."""
+        return [CoreState.RUNNING, CoreState.SETUP]

supervisor/resolution/const.py

@@ -73,6 +73,7 @@ class IssueType(StrEnum):
     CORRUPT_DOCKER = "corrupt_docker"
     CORRUPT_REPOSITORY = "corrupt_repository"
     CORRUPT_FILESYSTEM = "corrupt_filesystem"
+    DISABLED_DATA_DISK = "disabled_data_disk"
     DNS_LOOP = "dns_loop"
     DNS_SERVER_FAILED = "dns_server_failed"
     DNS_SERVER_IPV6_ERROR = "dns_server_ipv6_error"

supervisor/resolution/fixups/system_adopt_data_disk.py

@@ -6,7 +6,7 @@ from pathlib import Path
 from ...coresys import CoreSys
 from ...dbus.udisks2.data import DeviceSpecification
 from ...exceptions import DBusError, HostError, ResolutionFixupError
-from ...os.const import FILESYSTEM_LABEL_OLD_DATA_DISK
+from ...os.const import FILESYSTEM_LABEL_DATA_DISK, FILESYSTEM_LABEL_OLD_DATA_DISK
 from ..const import ContextType, IssueType, SuggestionType
 from .base import FixupBase
 
@@ -23,8 +23,10 @@ class FixupSystemAdoptDataDisk(FixupBase):
 
     async def process_fixup(self, reference: str | None = None) -> None:
         """Initialize the fixup class."""
-        if not await self.sys_dbus.udisks2.resolve_device(
+        if not (
-            DeviceSpecification(path=Path(reference))
+            new_resolved := await self.sys_dbus.udisks2.resolve_device(
+                DeviceSpecification(path=Path(reference))
+            )
         ):
             _LOGGER.info(
                 "Data disk at %s with name conflict was removed, skipping adopt",
@@ -36,16 +38,30 @@ class FixupSystemAdoptDataDisk(FixupBase):
         if (
             not current
             or not (
-                resolved := await self.sys_dbus.udisks2.resolve_device(
+                current_resolved := await self.sys_dbus.udisks2.resolve_device(
                     DeviceSpecification(path=current)
                 )
             )
-            or not resolved[0].filesystem
+            or not current_resolved[0].filesystem
         ):
             raise ResolutionFixupError(
                 "Cannot resolve current data disk for rename", _LOGGER.error
             )
 
+        if new_resolved[0].id_label != FILESYSTEM_LABEL_DATA_DISK:
+            _LOGGER.info(
+                "Renaming disabled data disk at %s to %s to activate it",
+                reference,
+                FILESYSTEM_LABEL_DATA_DISK,
+            )
+            try:
+                await new_resolved[0].filesystem.set_label(FILESYSTEM_LABEL_DATA_DISK)
+            except DBusError as err:
+                raise ResolutionFixupError(
+                    f"Could not rename filesystem at {reference}: {err!s}",
+                    _LOGGER.error,
+                ) from err
+
         _LOGGER.info(
             "Renaming current data disk at %s to %s so new data disk at %s becomes primary ",
             self.sys_dbus.agent.datadisk.current_device,
@@ -53,7 +69,9 @@ class FixupSystemAdoptDataDisk(FixupBase):
             reference,
         )
         try:
-            await resolved[0].filesystem.set_label(FILESYSTEM_LABEL_OLD_DATA_DISK)
+            await current_resolved[0].filesystem.set_label(
+                FILESYSTEM_LABEL_OLD_DATA_DISK
+            )
         except DBusError as err:
             raise ResolutionFixupError(
                 f"Could not rename filesystem at {current.as_posix()}: {err!s}",
@@ -87,4 +105,4 @@ class FixupSystemAdoptDataDisk(FixupBase):
     @property
     def issues(self) -> list[IssueType]:
         """Return a IssueType enum list."""
-        return [IssueType.MULTIPLE_DATA_DISKS]
+        return [IssueType.DISABLED_DATA_DISK, IssueType.MULTIPLE_DATA_DISKS]

supervisor/resolution/fixups/system_rename_data_disk.py

@@ -66,4 +66,4 @@ class FixupSystemRenameDataDisk(FixupBase):
     @property
     def issues(self) -> list[IssueType]:
         """Return a IssueType enum list."""
-        return [IssueType.MULTIPLE_DATA_DISKS]
+        return [IssueType.DISABLED_DATA_DISK, IssueType.MULTIPLE_DATA_DISKS]

tests/dbus/udisks2/test_filesystem.py

@@ -144,5 +144,9 @@ async def test_set_label(
     filesystem_sda1_service.SetLabel.calls.clear()
     await sda1.set_label("test")
     assert filesystem_sda1_service.SetLabel.calls == [
-        ("test", {"auth.no_user_interaction": Variant("b", True)})
+        (
+            "/org/freedesktop/UDisks2/block_devices/sda1",
+            "test",
+            {"auth.no_user_interaction": Variant("b", True)},
+        )
     ]

tests/dbus_service_mocks/base.py

@@ -9,32 +9,6 @@ from dbus_fast.aio.message_bus import MessageBus
 from dbus_fast.service import ServiceInterface, method
 
 
-def dbus_method(name: str = None, disabled: bool = False):
-    """Make DBus method with call tracking.
-
-    Identical to dbus_fast.service.method wrapper except all calls to it are tracked.
-    Can then test that methods with no output were called or the right arguments were
-    used if the output is static.
-    """
-    orig_decorator = method(name=name, disabled=disabled)
-
-    @no_type_check_decorator
-    def decorator(func):
-        calls: list[list[Any]] = []
-
-        @wraps(func)
-        def track_calls(self, *args):
-            calls.append(args)
-            return func(self, *args)
-
-        wrapped = orig_decorator(track_calls)
-        wrapped.__dict__["calls"] = calls
-
-        return wrapped
-
-    return decorator
-
-
 class DBusServiceMock(ServiceInterface):
     """Base dbus service mock."""
 
@@ -66,3 +40,32 @@ class DBusServiceMock(ServiceInterface):
         # So in general we sleep(0) after to clear the new task
         if sleep:
             await asyncio.sleep(0)
+
+
+def dbus_method(name: str = None, disabled: bool = False, track_obj_path: bool = False):
+    """Make DBus method with call tracking.
+
+    Identical to dbus_fast.service.method wrapper except all calls to it are tracked.
+    Can then test that methods with no output were called or the right arguments were
+    used if the output is static.
+    """
+    orig_decorator = method(name=name, disabled=disabled)
+
+    @no_type_check_decorator
+    def decorator(func):
+        calls: list[list[Any]] = []
+
+        @wraps(func)
+        def track_calls(self: DBusServiceMock, *args):
+            if track_obj_path:
+                calls.append((self.object_path, *args))
+            else:
+                calls.append(args)
+            return func(self, *args)
+
+        wrapped = orig_decorator(track_calls)
+        wrapped.__dict__["calls"] = calls
+
+        return wrapped
+
+    return decorator

tests/dbus_service_mocks/udisks2_filesystem.py

@@ -83,7 +83,7 @@ class Filesystem(DBusServiceMock):
         """Get Size."""
         return self.fixture.Size
 
-    @dbus_method()
+    @dbus_method(track_obj_path=True)
     def SetLabel(self, label: "s", options: "a{sv}") -> None:
         """Do SetLabel method."""
 

tests/dbus_service_mocks/udisks2_manager.py

@@ -32,7 +32,9 @@ class UDisks2Manager(DBusServiceMock):
         "/org/freedesktop/UDisks2/block_devices/sdb1",
         "/org/freedesktop/UDisks2/block_devices/zram1",
     ]
-    resolved_devices = ["/org/freedesktop/UDisks2/block_devices/sda1"]
+    resolved_devices: list[list[str]] | list[str] = [
+        "/org/freedesktop/UDisks2/block_devices/sda1"
+    ]
 
     @dbus_property(access=PropertyAccess.READ)
     def Version(self) -> "s":
@@ -98,4 +100,8 @@ class UDisks2Manager(DBusServiceMock):
     @dbus_method()
     def ResolveDevice(self, devspec: "a{sv}", options: "a{sv}") -> "ao":
         """Do ResolveDevice method."""
+        if len(self.resolved_devices) > 0 and isinstance(
+            self.resolved_devices[0], list
+        ):
+            return self.resolved_devices.pop(0)
         return self.resolved_devices

tests/resolution/check/test_check_disabled_data_disk.py

@@ -0,0 +1,99 @@
+"""Test check for disabled data disk."""
+# pylint: disable=import-error
+from dataclasses import replace
+from unittest.mock import patch
+
+import pytest
+
+from supervisor.const import CoreState
+from supervisor.coresys import CoreSys
+from supervisor.resolution.checks.disabled_data_disk import CheckDisabledDataDisk
+from supervisor.resolution.const import ContextType, IssueType, SuggestionType
+from supervisor.resolution.data import Issue, Suggestion
+
+from tests.dbus_service_mocks.base import DBusServiceMock
+from tests.dbus_service_mocks.udisks2_block import Block as BlockService
+
+
+@pytest.fixture(name="sda1_block_service")
+async def fixture_sda1_block_service(
+    udisks2_services: dict[str, DBusServiceMock | dict[str, DBusServiceMock]],
+) -> BlockService:
+    """Return sda1 block service."""
+    yield udisks2_services["udisks2_block"][
+        "/org/freedesktop/UDisks2/block_devices/sda1"
+    ]
+
+
+async def test_base(coresys: CoreSys):
+    """Test check basics."""
+    disabled_data_disk = CheckDisabledDataDisk(coresys)
+    assert disabled_data_disk.slug == "disabled_data_disk"
+    assert disabled_data_disk.enabled
+
+
+async def test_check(coresys: CoreSys, sda1_block_service: BlockService):
+    """Test check."""
+    disabled_data_disk = CheckDisabledDataDisk(coresys)
+    coresys.core.state = CoreState.RUNNING
+
+    await disabled_data_disk.run_check()
+
+    assert len(coresys.resolution.issues) == 0
+    assert len(coresys.resolution.suggestions) == 0
+
+    sda1_block_service.emit_properties_changed({"IdLabel": "hassos-data-dis"})
+    await sda1_block_service.ping()
+
+    await disabled_data_disk.run_check()
+
+    assert coresys.resolution.issues == [
+        Issue(IssueType.DISABLED_DATA_DISK, ContextType.SYSTEM, reference="/dev/sda1")
+    ]
+    assert coresys.resolution.suggestions == [
+        Suggestion(
+            SuggestionType.RENAME_DATA_DISK, ContextType.SYSTEM, reference="/dev/sda1"
+        ),
+        Suggestion(
+            SuggestionType.ADOPT_DATA_DISK, ContextType.SYSTEM, reference="/dev/sda1"
+        ),
+    ]
+
+
+async def test_approve(coresys: CoreSys, sda1_block_service: BlockService):
+    """Test approve."""
+    disabled_data_disk = CheckDisabledDataDisk(coresys)
+    coresys.core.state = CoreState.RUNNING
+
+    assert not await disabled_data_disk.approve_check(reference="/dev/sda1")
+
+    sda1_block_service.fixture = replace(
+        sda1_block_service.fixture, IdLabel="hassos-data-dis"
+    )
+
+    assert await disabled_data_disk.approve_check(reference="/dev/sda1")
+
+
+async def test_did_run(coresys: CoreSys):
+    """Test that the check ran as expected."""
+    disabled_data_disk = CheckDisabledDataDisk(coresys)
+    should_run = disabled_data_disk.states
+    should_not_run = [state for state in CoreState if state not in should_run]
+    assert len(should_run) != 0
+    assert len(should_not_run) != 0
+
+    with patch(
+        "supervisor.resolution.checks.disabled_data_disk.CheckDisabledDataDisk.run_check",
+        return_value=None,
+    ) as check:
+        for state in should_run:
+            coresys.core.state = state
+            await disabled_data_disk()
+            check.assert_called_once()
+            check.reset_mock()
+
+        for state in should_not_run:
+            coresys.core.state = state
+            await disabled_data_disk()
+            check.assert_not_called()
+            check.reset_mock()

tests/resolution/fixup/test_system_adopt_data_disk.py

@@ -1,5 +1,7 @@
 """Test system fixup adopt data disk."""
 
+from dataclasses import dataclass, replace
+
 from dbus_fast import DBusError, ErrorType, Variant
 import pytest
 
@@ -10,20 +12,34 @@ from supervisor.resolution.fixups.system_adopt_data_disk import FixupSystemAdopt
 
 from tests.dbus_service_mocks.base import DBusServiceMock
 from tests.dbus_service_mocks.logind import Logind as LogindService
+from tests.dbus_service_mocks.udisks2_block import Block as BlockService
 from tests.dbus_service_mocks.udisks2_filesystem import Filesystem as FilesystemService
 from tests.dbus_service_mocks.udisks2_manager import (
     UDisks2Manager as UDisks2ManagerService,
 )
 
 
-@pytest.fixture(name="sda1_filesystem_service")
+@dataclass
-async def fixture_sda1_filesystem_service(
+class FSDevice:
+    """Filesystem device services."""
+
+    block_service: BlockService
+    filesystem_service: FilesystemService
+
+
+@pytest.fixture(name="sda1_device")
+async def fixture_sda1_device(
     udisks2_services: dict[str, DBusServiceMock | dict[str, DBusServiceMock]],
-) -> FilesystemService:
+) -> FSDevice:
-    """Return sda1 filesystem service."""
+    """Return sda1 services."""
-    return udisks2_services["udisks2_filesystem"][
+    return FSDevice(
-        "/org/freedesktop/UDisks2/block_devices/sda1"
+        udisks2_services["udisks2_block"][
-    ]
+            "/org/freedesktop/UDisks2/block_devices/sda1"
+        ],
+        udisks2_services["udisks2_filesystem"][
+            "/org/freedesktop/UDisks2/block_devices/sda1"
+        ],
+    )
 
 
 @pytest.fixture(name="mmcblk1p3_filesystem_service")
@@ -54,6 +70,7 @@ async def fixture_logind_service(
 
 async def test_fixup(
     coresys: CoreSys,
+    sda1_device: FSDevice,
     mmcblk1p3_filesystem_service: FilesystemService,
     udisks2_service: UDisks2ManagerService,
     logind_service: LogindService,
@@ -61,6 +78,9 @@ async def test_fixup(
     """Test fixup."""
     mmcblk1p3_filesystem_service.SetLabel.calls.clear()
     logind_service.Reboot.calls.clear()
+    sda1_device.block_service.fixture = replace(
+        sda1_device.block_service.fixture, IdLabel="hassos-data"
+    )
     system_adopt_data_disk = FixupSystemAdoptDataDisk(coresys)
 
     assert not system_adopt_data_disk.auto
@@ -72,13 +92,18 @@ async def test_fixup(
         IssueType.MULTIPLE_DATA_DISKS, ContextType.SYSTEM, reference="/dev/sda1"
     )
     udisks2_service.resolved_devices = [
-        "/org/freedesktop/UDisks2/block_devices/mmcblk1p3"
+        ["/org/freedesktop/UDisks2/block_devices/sda1"],
+        ["/org/freedesktop/UDisks2/block_devices/mmcblk1p3"],
     ]
 
     await system_adopt_data_disk()
 
     assert mmcblk1p3_filesystem_service.SetLabel.calls == [
-        ("hassos-data-old", {"auth.no_user_interaction": Variant("b", True)})
+        (
+            "/org/freedesktop/UDisks2/block_devices/mmcblk1p3",
+            "hassos-data-old",
+            {"auth.no_user_interaction": Variant("b", True)},
+        )
     ]
     assert len(coresys.resolution.suggestions) == 0
     assert len(coresys.resolution.issues) == 0
@@ -118,6 +143,7 @@ async def test_fixup_device_removed(
 
 async def test_fixup_reboot_failed(
     coresys: CoreSys,
+    sda1_device: FSDevice,
     mmcblk1p3_filesystem_service: FilesystemService,
     udisks2_service: UDisks2ManagerService,
     logind_service: LogindService,
@@ -126,6 +152,9 @@ async def test_fixup_reboot_failed(
     """Test fixup when reboot fails."""
     mmcblk1p3_filesystem_service.SetLabel.calls.clear()
     logind_service.side_effect_reboot = DBusError(ErrorType.SERVICE_ERROR, "error")
+    sda1_device.block_service.fixture = replace(
+        sda1_device.block_service.fixture, IdLabel="hassos-data"
+    )
     system_adopt_data_disk = FixupSystemAdoptDataDisk(coresys)
 
     assert not system_adopt_data_disk.auto
@@ -137,13 +166,18 @@ async def test_fixup_reboot_failed(
         IssueType.MULTIPLE_DATA_DISKS, ContextType.SYSTEM, reference="/dev/sda1"
     )
     udisks2_service.resolved_devices = [
-        "/org/freedesktop/UDisks2/block_devices/mmcblk1p3"
+        ["/org/freedesktop/UDisks2/block_devices/sda1"],
+        ["/org/freedesktop/UDisks2/block_devices/mmcblk1p3"],
     ]
 
     await system_adopt_data_disk()
 
     assert mmcblk1p3_filesystem_service.SetLabel.calls == [
-        ("hassos-data-old", {"auth.no_user_interaction": Variant("b", True)})
+        (
+            "/org/freedesktop/UDisks2/block_devices/mmcblk1p3",
+            "hassos-data-old",
+            {"auth.no_user_interaction": Variant("b", True)},
+        )
     ]
     assert len(coresys.resolution.suggestions) == 1
     assert (
@@ -156,3 +190,50 @@ async def test_fixup_reboot_failed(
         in coresys.resolution.issues
     )
     assert "Could not reboot host to finish data disk adoption" in caplog.text
+
+
+async def test_fixup_disabled_data_disk(
+    coresys: CoreSys,
+    sda1_device: FSDevice,
+    mmcblk1p3_filesystem_service: FilesystemService,
+    udisks2_service: UDisks2ManagerService,
+    logind_service: LogindService,
+):
+    """Test fixup for activating a disabled data disk."""
+    mmcblk1p3_filesystem_service.SetLabel.calls.clear()
+    logind_service.Reboot.calls.clear()
+    sda1_device.block_service.fixture = replace(
+        sda1_device.block_service.fixture, IdLabel="hassos-data-dis"
+    )
+    system_adopt_data_disk = FixupSystemAdoptDataDisk(coresys)
+
+    assert not system_adopt_data_disk.auto
+
+    coresys.resolution.suggestions = Suggestion(
+        SuggestionType.ADOPT_DATA_DISK, ContextType.SYSTEM, reference="/dev/sda1"
+    )
+    coresys.resolution.issues = Issue(
+        IssueType.DISABLED_DATA_DISK, ContextType.SYSTEM, reference="/dev/sda1"
+    )
+    udisks2_service.resolved_devices = [
+        ["/org/freedesktop/UDisks2/block_devices/sda1"],
+        ["/org/freedesktop/UDisks2/block_devices/mmcblk1p3"],
+    ]
+
+    await system_adopt_data_disk()
+
+    assert mmcblk1p3_filesystem_service.SetLabel.calls == [
+        (
+            "/org/freedesktop/UDisks2/block_devices/sda1",
+            "hassos-data",
+            {"auth.no_user_interaction": Variant("b", True)},
+        ),
+        (
+            "/org/freedesktop/UDisks2/block_devices/mmcblk1p3",
+            "hassos-data-old",
+            {"auth.no_user_interaction": Variant("b", True)},
+        ),
+    ]
+    assert len(coresys.resolution.suggestions) == 0
+    assert len(coresys.resolution.issues) == 0
+    assert logind_service.Reboot.calls == [(False,)]

tests/resolution/fixup/test_system_rename_data_disk.py

@@ -52,7 +52,11 @@ async def test_fixup(coresys: CoreSys, sda1_filesystem_service: FilesystemServic
     await system_rename_data_disk()
 
     assert sda1_filesystem_service.SetLabel.calls == [
-        ("hassos-data-old", {"auth.no_user_interaction": Variant("b", True)})
+        (
+            "/org/freedesktop/UDisks2/block_devices/sda1",
+            "hassos-data-old",
+            {"auth.no_user_interaction": Variant("b", True)},
+        )
     ]
     assert len(coresys.resolution.suggestions) == 0
     assert len(coresys.resolution.issues) == 0