Ignore veth changes (#3955)

* Reduce log noise for unmanaged interfaces

* Ignore signals with veth changes only

* Fix test and add one
This commit is contained in:
Mike Degatano 2022-10-16 05:06:35 -04:00 committed by GitHub
parent 34afcef4f1
commit bde5c938a7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 134 additions and 18 deletions

View File

@ -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)

View File

@ -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

View File

@ -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,

View File

@ -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,

View File

@ -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

View File

@ -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()

View File

@ -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

View File

@ -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")