diff --git a/supervisor/dbus/interface.py b/supervisor/dbus/interface.py index 910c760c5..244483ce4 100644 --- a/supervisor/dbus/interface.py +++ b/supervisor/dbus/interface.py @@ -5,6 +5,8 @@ from typing import Any from dbus_fast.aio.message_bus import MessageBus +from supervisor.exceptions import DBusInterfaceError + from ..utils.dbus import DBus from .utils import dbus_connected @@ -70,8 +72,14 @@ class DBusInterfaceProxy(DBusInterface): async def connect(self, bus: MessageBus) -> None: """Connect to D-Bus.""" await super().connect(bus) - await self.update() + if not self.dbus.properties: + self.disconnect() + raise DBusInterfaceError( + f"D-Bus object {self.object_path} is not usable, introspection is missing required properties interface" + ) + + await self.update() if self.sync_properties and self.is_connected: self.dbus.sync_property_changes(self.properties_interface, self.update) diff --git a/supervisor/dbus/network/__init__.py b/supervisor/dbus/network/__init__.py index 49606cb0b..eadbcf4bb 100644 --- a/supervisor/dbus/network/__init__.py +++ b/supervisor/dbus/network/__init__.py @@ -10,7 +10,6 @@ from ...exceptions import ( DBusError, DBusFatalError, DBusInterfaceError, - DBusInterfaceMethodError, HostNotSupportedError, ) from ..const import ( @@ -163,7 +162,16 @@ class NetworkManager(DBusInterfaceProxy): if not changed and self.dns.is_connected: await self.dns.update() - if changed and DBUS_ATTR_DEVICES not in changed: + if changed and ( + DBUS_ATTR_DEVICES not in changed + or { + intr.object_path for intr in self.interfaces.values() if intr.managed + }.issubset(set(changed[DBUS_ATTR_DEVICES])) + ): + # If none of our managed devices were removed then most likely this is just veths changing. + # We don't care about veths and reprocessing all their changes can swamp a system when + # docker is having issues. This does mean we may miss activation of a new managed device + # in rare occaisions but we'll catch it on the next host update scheduled task. return interfaces = {} @@ -178,14 +186,14 @@ class NetworkManager(DBusInterfaceProxy): # Connect to interface try: await interface.connect(self.dbus.bus) - except (DBusFatalError, DBusInterfaceMethodError) as err: + except (DBusFatalError, DBusInterfaceError) as err: # Docker creates and deletes interfaces quite often, sometimes # this causes a race condition: A device disappears while we # try to query it. Ignore those cases. - _LOGGER.warning("Can't process %s: %s", device, err) + _LOGGER.debug("Can't process %s: %s", device, err) continue except Exception as err: # pylint: disable=broad-except - _LOGGER.exception("Error while processing interface: %s", err) + _LOGGER.exception("Error while processing %s: %s", device, err) sentry_sdk.capture_exception(err) continue diff --git a/supervisor/dbus/network/setting/generate.py b/supervisor/dbus/network/setting/generate.py index 30a2c4706..920635cd0 100644 --- a/supervisor/dbus/network/setting/generate.py +++ b/supervisor/dbus/network/setting/generate.py @@ -5,7 +5,7 @@ import socket from typing import TYPE_CHECKING, Any from uuid import uuid4 -from dbus_fast.signature import Variant +from dbus_fast import Variant from . import ( ATTR_ASSIGNED_MAC, diff --git a/supervisor/utils/dbus.py b/supervisor/utils/dbus.py index d32e28ecb..3cf46e57d 100644 --- a/supervisor/utils/dbus.py +++ b/supervisor/utils/dbus.py @@ -5,12 +5,17 @@ import asyncio import logging from typing import Any, Awaitable, Callable, Coroutine -from dbus_fast import ErrorType, InvalidIntrospectionError, Message, MessageType +from dbus_fast import ( + ErrorType, + InvalidIntrospectionError, + Message, + MessageType, + Variant, +) from dbus_fast.aio.message_bus import MessageBus from dbus_fast.aio.proxy_object import ProxyInterface, ProxyObject from dbus_fast.errors import DBusError from dbus_fast.introspection import Node -from dbus_fast.signature import Variant from ..exceptions import ( DBusFatalError, @@ -143,15 +148,19 @@ class DBus: return self._bus @property - def properties(self) -> ProxyInterface: + def properties(self) -> DBusCallWrapper | None: """Get properties proxy interface.""" + if DBUS_INTERFACE_PROPERTIES not in self._proxies: + return None return DBusCallWrapper(self, DBUS_INTERFACE_PROPERTIES) async def get_properties(self, interface: str) -> dict[str, Any]: """Read all properties from interface.""" - return await DBusCallWrapper(self, DBUS_INTERFACE_PROPERTIES).call_get_all( - interface - ) + if not self.properties: + raise DBusInterfaceError( + f"DBus Object does not have interface {DBUS_INTERFACE_PROPERTIES}" + ) + return await self.properties.call_get_all(interface) def sync_property_changes( self, diff --git a/tests/conftest.py b/tests/conftest.py index 1c246371e..44b7fdf99 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -253,7 +253,6 @@ async def network_manager(dbus, dbus_bus: MessageBus) -> NetworkManager: # Init await nm_obj.connect(dbus_bus) - await nm_obj.update() yield nm_obj diff --git a/tests/dbus/network/test_network_manager.py b/tests/dbus/network/test_network_manager.py index 4aab26c46..d29293fed 100644 --- a/tests/dbus/network/test_network_manager.py +++ b/tests/dbus/network/test_network_manager.py @@ -1,12 +1,16 @@ """Test NetworkInterface.""" import asyncio -from unittest.mock import AsyncMock +import logging +from unittest.mock import AsyncMock, PropertyMock, patch +from dbus_fast.aio.message_bus import MessageBus import pytest from supervisor.dbus.const import ConnectionStateType from supervisor.dbus.network import NetworkManager -from supervisor.exceptions import HostNotSupportedError +from supervisor.dbus.network.interface import NetworkInterface +from supervisor.exceptions import DBusFatalError, DBusParseError, HostNotSupportedError +from supervisor.utils.dbus import DBus from .setting.test_init import SETTINGS_WITH_SIGNATURE @@ -107,3 +111,77 @@ async def test_removed_devices_disconnect(network_manager: NetworkManager): assert TEST_INTERFACE_WLAN not in network_manager.interfaces assert wlan.is_connected is False + + +async def test_handling_bad_devices( + network_manager: NetworkManager, caplog: pytest.LogCaptureFixture +): + """Test handling of bad and disappearing devices.""" + caplog.clear() + caplog.set_level(logging.INFO, "supervisor.dbus.network") + + with patch.object(DBus, "_init_proxy", side_effect=DBusFatalError()): + await network_manager.update( + {"Devices": ["/org/freedesktop/NetworkManager/Devices/100"]} + ) + assert not caplog.text + + await network_manager.update() + with patch.object(DBus, "properties", new=PropertyMock(return_value=None)): + await network_manager.update( + {"Devices": ["/org/freedesktop/NetworkManager/Devices/101"]} + ) + assert not caplog.text + + # Unparseable introspections shouldn't happen, this one is logged and captured + await network_manager.update() + with patch.object( + DBus, "_init_proxy", side_effect=(err := DBusParseError()) + ), patch("supervisor.dbus.network.sentry_sdk.capture_exception") as capture: + await network_manager.update( + {"Devices": [device := "/org/freedesktop/NetworkManager/Devices/102"]} + ) + assert f"Error while processing {device}" in caplog.text + capture.assert_called_once_with(err) + + # We should be able to debug these situations if necessary + caplog.set_level(logging.DEBUG, "supervisor.dbus.network") + await network_manager.update() + with patch.object(DBus, "_init_proxy", side_effect=DBusFatalError()): + await network_manager.update( + {"Devices": [device := "/org/freedesktop/NetworkManager/Devices/103"]} + ) + assert f"Can't process {device}" in caplog.text + + await network_manager.update() + with patch.object(DBus, "properties", new=PropertyMock(return_value=None)): + await network_manager.update( + {"Devices": [device := "/org/freedesktop/NetworkManager/Devices/104"]} + ) + assert f"Can't process {device}" in caplog.text + + +async def test_ignore_veth_only_changes( + network_manager: NetworkManager, dbus_bus: MessageBus +): + """Changes to list of devices is ignored unless it changes managed devices.""" + assert network_manager.properties["Devices"] == [ + "/org/freedesktop/NetworkManager/Devices/1", + "/org/freedesktop/NetworkManager/Devices/3", + ] + with patch.object(NetworkInterface, "update") as update: + await network_manager.update( + { + "Devices": [ + "/org/freedesktop/NetworkManager/Devices/1", + "/org/freedesktop/NetworkManager/Devices/3", + "/org/freedesktop/NetworkManager/Devices/35", + ] + } + ) + update.assert_not_called() + + await network_manager.update( + {"Devices": ["/org/freedesktop/NetworkManager/Devices/35"]} + ) + update.assert_called_once() diff --git a/tests/dbus/test_interface.py b/tests/dbus/test_interface.py index 93ca88065..232234829 100644 --- a/tests/dbus/test_interface.py +++ b/tests/dbus/test_interface.py @@ -7,7 +7,9 @@ from unittest.mock import MagicMock from dbus_fast.aio.message_bus import MessageBus import pytest +from supervisor.dbus.const import DBUS_OBJECT_BASE from supervisor.dbus.interface import DBusInterfaceProxy +from supervisor.exceptions import DBusInterfaceError from tests.common import fire_property_change_signal, fire_watched_signal @@ -106,3 +108,15 @@ async def test_dbus_proxy_shutdown_pending_task(proxy: DBusInterfaceProxyMock): proxy.obj.shutdown() await asyncio.sleep(0) assert device == "/test/obj/1" + + +async def test_proxy_missing_properties_interface(dbus_bus: MessageBus): + """Test proxy instance disconnects and errors when missing properties interface.""" + proxy = DBusInterfaceProxy() + proxy.bus_name = "test.no.properties.interface" + proxy.object_path = DBUS_OBJECT_BASE + proxy.properties_interface = "test.no.properties.interface" + + with pytest.raises(DBusInterfaceError): + await proxy.connect(dbus_bus) + assert proxy.is_connected is False diff --git a/tests/utils/test_dbus.py b/tests/utils/test_dbus.py index 648b68dcc..996054e7e 100644 --- a/tests/utils/test_dbus.py +++ b/tests/utils/test_dbus.py @@ -3,7 +3,7 @@ from dbus_fast.aio.message_bus import MessageBus import pytest from supervisor.dbus.const import DBUS_OBJECT_BASE -from supervisor.exceptions import DBusInterfaceMethodError +from supervisor.exceptions import DBusInterfaceError from supervisor.utils.dbus import DBus @@ -12,5 +12,5 @@ async def test_missing_properties_interface(dbus_bus: MessageBus, dbus: list[str service = await DBus.connect( dbus_bus, "test.no.properties.interface", DBUS_OBJECT_BASE ) - with pytest.raises(DBusInterfaceMethodError): + with pytest.raises(DBusInterfaceError): await service.get_properties("test.no.properties.interface")