From 52cc17fa3fa0809ee89423a07ad5961f24d0db33 Mon Sep 17 00:00:00 2001 From: Mike Degatano Date: Tue, 11 Feb 2025 07:22:33 -0500 Subject: [PATCH] Delay initial version fetch until there is connectivity (#5603) * Delay inital version fetch until there is connectivity * Add test * Only mock get not whole websession object * drive delayed fetch off of supervisor connectivity not host * Fix test to not rely on sleep guessing to track tasks * Use fixture to remove job throttle temporarily --- supervisor/const.py | 1 + supervisor/host/network.py | 4 +- supervisor/jobs/__init__.py | 4 +- supervisor/supervisor.py | 8 ++- supervisor/updater.py | 23 +++++++- tests/conftest.py | 15 ++--- tests/dbus_service_mocks/network_manager.py | 3 +- tests/jobs/test_job_decorator.py | 16 +++--- tests/misc/test_tasks.py | 1 + tests/os/test_manager.py | 2 +- tests/plugins/test_plugin_manager.py | 2 + tests/test_supervisor.py | 16 ++---- tests/test_updater.py | 62 ++++++++++++++++++++- 13 files changed, 121 insertions(+), 36 deletions(-) diff --git a/supervisor/const.py b/supervisor/const.py index 7d1c829e1..42a7fee4a 100644 --- a/supervisor/const.py +++ b/supervisor/const.py @@ -484,6 +484,7 @@ class BusEvent(StrEnum): DOCKER_CONTAINER_STATE_CHANGE = "docker_container_state_change" HARDWARE_NEW_DEVICE = "hardware_new_device" HARDWARE_REMOVE_DEVICE = "hardware_remove_device" + SUPERVISOR_CONNECTIVITY_CHANGE = "supervisor_connectivity_change" SUPERVISOR_JOB_END = "supervisor_job_end" SUPERVISOR_JOB_START = "supervisor_job_start" SUPERVISOR_STATE_CHANGE = "supervisor_state_change" diff --git a/supervisor/host/network.py b/supervisor/host/network.py index 5058c3168..b77c96434 100644 --- a/supervisor/host/network.py +++ b/supervisor/host/network.py @@ -67,6 +67,8 @@ class NetworkManager(CoreSysAttributes): self.sys_homeassistant.websocket.supervisor_update_event( "network", {ATTR_HOST_INTERNET: state} ) + if state and not self.sys_supervisor.connectivity: + self.sys_create_task(self.sys_supervisor.check_connectivity()) @property def interfaces(self) -> list[Interface]: @@ -148,7 +150,7 @@ class NetworkManager(CoreSysAttributes): return connectivity_check: bool | None = changed.get(DBUS_ATTR_CONNECTION_ENABLED) - connectivity: bool | None = changed.get(DBUS_ATTR_CONNECTIVITY) + connectivity: int | None = changed.get(DBUS_ATTR_CONNECTIVITY) # This potentially updated the DNS configuration. Make sure the DNS plug-in # picks up the latest settings. diff --git a/supervisor/jobs/__init__.py b/supervisor/jobs/__init__.py index f513b3f30..ca8c2d07b 100644 --- a/supervisor/jobs/__init__.py +++ b/supervisor/jobs/__init__.py @@ -213,9 +213,9 @@ class JobManager(FileConfiguration, CoreSysAttributes): if attribute.name == "done": if value is False: - self.sys_bus.fire_event(BusEvent.SUPERVISOR_JOB_START, job.uuid) + self.sys_bus.fire_event(BusEvent.SUPERVISOR_JOB_START, job) if value is True: - self.sys_bus.fire_event(BusEvent.SUPERVISOR_JOB_END, job.uuid) + self.sys_bus.fire_event(BusEvent.SUPERVISOR_JOB_END, job) def new_job( self, diff --git a/supervisor/supervisor.py b/supervisor/supervisor.py index e96a29fdc..617c328ce 100644 --- a/supervisor/supervisor.py +++ b/supervisor/supervisor.py @@ -13,7 +13,12 @@ import aiohttp from aiohttp.client_exceptions import ClientError from awesomeversion import AwesomeVersion, AwesomeVersionException -from .const import ATTR_SUPERVISOR_INTERNET, SUPERVISOR_VERSION, URL_HASSIO_APPARMOR +from .const import ( + ATTR_SUPERVISOR_INTERNET, + SUPERVISOR_VERSION, + URL_HASSIO_APPARMOR, + BusEvent, +) from .coresys import CoreSys, CoreSysAttributes from .docker.stats import DockerStats from .docker.supervisor import DockerSupervisor @@ -74,6 +79,7 @@ class Supervisor(CoreSysAttributes): if self._connectivity == state: return self._connectivity = state + self.sys_bus.fire_event(BusEvent.SUPERVISOR_CONNECTIVITY_CHANGE, state) self.sys_homeassistant.websocket.supervisor_update_event( "network", {ATTR_SUPERVISOR_INTERNET: state} ) diff --git a/supervisor/updater.py b/supervisor/updater.py index 949090ca3..425ff7ffd 100644 --- a/supervisor/updater.py +++ b/supervisor/updater.py @@ -8,6 +8,7 @@ import logging import aiohttp from awesomeversion import AwesomeVersion +from .bus import EventListener from .const import ( ATTR_AUDIO, ATTR_AUTO_UPDATE, @@ -23,6 +24,7 @@ from .const import ( ATTR_SUPERVISOR, FILE_HASSIO_UPDATER, URL_HASSIO_VERSION, + BusEvent, UpdateChannel, ) from .coresys import CoreSysAttributes @@ -47,11 +49,18 @@ class Updater(FileConfiguration, CoreSysAttributes): """Initialize updater.""" super().__init__(FILE_HASSIO_UPDATER, SCHEMA_UPDATER_CONFIG) self.coresys = coresys + self._connectivity_listener: EventListener | None = None async def load(self) -> None: """Update internal data.""" - with suppress(UpdaterError): - await self.fetch_data() + # If there's no connectivity, delay initial version fetch + if not self.sys_supervisor.connectivity: + self._connectivity_listener = self.sys_bus.register_event( + BusEvent.SUPERVISOR_CONNECTIVITY_CHANGE, self._check_connectivity + ) + return + + await self.reload() async def reload(self) -> None: """Update internal data.""" @@ -180,6 +189,11 @@ class Updater(FileConfiguration, CoreSysAttributes): """Set Supervisor auto updates enabled.""" self._data[ATTR_AUTO_UPDATE] = value + async def _check_connectivity(self, connectivity: bool): + """Fetch data once connectivity is true.""" + if connectivity: + await self.reload() + @Job( name="updater_fetch_data", conditions=[JobCondition.INTERNET_SYSTEM], @@ -214,6 +228,11 @@ class Updater(FileConfiguration, CoreSysAttributes): _LOGGER.warning, ) from err + # Fetch was successful. If there's a connectivity listener, time to remove it + if self._connectivity_listener: + self.sys_bus.remove_listener(self._connectivity_listener) + self._connectivity_listener = None + # Validate try: await self.sys_security.verify_own_content(calc_checksum(data)) diff --git a/tests/conftest.py b/tests/conftest.py index 1046773ff..aaccd8a84 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,8 +2,7 @@ import asyncio from collections.abc import AsyncGenerator, Generator -from functools import partial -from inspect import unwrap +from datetime import datetime import os from pathlib import Path import subprocess @@ -381,11 +380,6 @@ async def coresys( ha_version=AwesomeVersion("2021.2.4") ) - # Remove rate limiting decorator from fetch_data - coresys_obj.updater.fetch_data = partial( - unwrap(coresys_obj.updater.fetch_data), coresys_obj.updater - ) - # Don't remove files/folders related to addons and stores with patch("supervisor.store.git.GitRepo._remove"): yield coresys_obj @@ -765,3 +759,10 @@ def mock_is_mount() -> MagicMock: """Mock is_mount in mounts.""" with patch("supervisor.mounts.mount.Path.is_mount", return_value=True) as is_mount: yield is_mount + + +@pytest.fixture +def no_job_throttle(): + """Remove job throttle for tests.""" + with patch("supervisor.jobs.decorator.Job.last_call", return_value=datetime.min): + yield diff --git a/tests/dbus_service_mocks/network_manager.py b/tests/dbus_service_mocks/network_manager.py index c0ebaeab8..cc224dd04 100644 --- a/tests/dbus_service_mocks/network_manager.py +++ b/tests/dbus_service_mocks/network_manager.py @@ -22,6 +22,7 @@ class NetworkManager(DBusServiceMock): interface = "org.freedesktop.NetworkManager" object_path = "/org/freedesktop/NetworkManager" version = "1.22.10" + connectivity_check_enabled = True connectivity = 4 devices = [ "/org/freedesktop/NetworkManager/Devices/1", @@ -155,7 +156,7 @@ class NetworkManager(DBusServiceMock): @dbus_property() def ConnectivityCheckEnabled(self) -> "b": """Get ConnectivityCheckEnabled.""" - return True + return self.connectivity_check_enabled @ConnectivityCheckEnabled.setter def ConnectivityCheckEnabled(self, value: "b"): diff --git a/tests/jobs/test_job_decorator.py b/tests/jobs/test_job_decorator.py index cb8035dba..986234bc7 100644 --- a/tests/jobs/test_job_decorator.py +++ b/tests/jobs/test_job_decorator.py @@ -1144,13 +1144,13 @@ async def test_job_scheduled_delay(coresys: CoreSys): started = False ended = False - async def start_listener(job_id: str): + async def start_listener(evt_job: SupervisorJob): nonlocal started - started = started or job_id == job.uuid + started = started or evt_job.uuid == job.uuid - async def end_listener(job_id: str): + async def end_listener(evt_job: SupervisorJob): nonlocal ended - ended = ended or job_id == job.uuid + ended = ended or evt_job.uuid == job.uuid coresys.bus.register_event(BusEvent.SUPERVISOR_JOB_START, start_listener) coresys.bus.register_event(BusEvent.SUPERVISOR_JOB_END, end_listener) @@ -1196,13 +1196,13 @@ async def test_job_scheduled_at(coresys: CoreSys): started = False ended = False - async def start_listener(job_id: str): + async def start_listener(evt_job: SupervisorJob): nonlocal started - started = started or job_id == job.uuid + started = started or evt_job.uuid == job.uuid - async def end_listener(job_id: str): + async def end_listener(evt_job: SupervisorJob): nonlocal ended - ended = ended or job_id == job.uuid + ended = ended or evt_job.uuid == job.uuid coresys.bus.register_event(BusEvent.SUPERVISOR_JOB_START, start_listener) coresys.bus.register_event(BusEvent.SUPERVISOR_JOB_END, end_listener) diff --git a/tests/misc/test_tasks.py b/tests/misc/test_tasks.py index 40892dc4d..53a0fccf9 100644 --- a/tests/misc/test_tasks.py +++ b/tests/misc/test_tasks.py @@ -171,6 +171,7 @@ async def test_watchdog_homeassistant_api_reanimation_limit( rebuild.assert_not_called() +@pytest.mark.usefixtures("no_job_throttle") async def test_reload_updater_triggers_supervisor_update( tasks: Tasks, coresys: CoreSys ): diff --git a/tests/os/test_manager.py b/tests/os/test_manager.py index c35ff2a6b..2a0178186 100644 --- a/tests/os/test_manager.py +++ b/tests/os/test_manager.py @@ -16,7 +16,7 @@ from tests.dbus_service_mocks.rauc import Rauc as RaucService # pylint: disable=protected-access -@pytest.mark.asyncio +@pytest.mark.usefixtures("no_job_throttle") async def test_ota_url_generic_x86_64_rename(coresys: CoreSys) -> None: """Test download URL generated.""" coresys.os._board = "intel-nuc" diff --git a/tests/plugins/test_plugin_manager.py b/tests/plugins/test_plugin_manager.py index 0764c5111..fcd05ef6f 100644 --- a/tests/plugins/test_plugin_manager.py +++ b/tests/plugins/test_plugin_manager.py @@ -3,6 +3,7 @@ from unittest.mock import PropertyMock, patch from awesomeversion import AwesomeVersion +import pytest from supervisor.coresys import CoreSys from supervisor.docker.interface import DockerInterface @@ -35,6 +36,7 @@ async def test_repair(coresys: CoreSys): assert install.call_count == len(coresys.plugins.all_plugins) +@pytest.mark.usefixtures("no_job_throttle") async def test_load(coresys: CoreSys): """Test plugin manager load.""" coresys.hardware.disk.get_disk_free_space = lambda x: 5000 diff --git a/tests/test_supervisor.py b/tests/test_supervisor.py index 42ec8d5ba..15172d448 100644 --- a/tests/test_supervisor.py +++ b/tests/test_supervisor.py @@ -36,29 +36,23 @@ async def fixture_webession(coresys: CoreSys) -> AsyncMock: yield mock_websession -@pytest.fixture(name="supervisor_unthrottled") -async def fixture_supervisor_unthrottled(coresys: CoreSys) -> Supervisor: - """Get supervisor object with connectivity check throttle removed.""" - with patch("supervisor.jobs.decorator.Job.last_call", return_value=datetime.min): - yield coresys.supervisor - - @pytest.mark.parametrize( "side_effect,connectivity", [(ClientError(), False), (None, True)] ) +@pytest.mark.usefixtures("no_job_throttle") async def test_connectivity_check( - supervisor_unthrottled: Supervisor, + coresys: CoreSys, websession: AsyncMock, side_effect: Exception | None, connectivity: bool, ): """Test connectivity check.""" - assert supervisor_unthrottled.connectivity is True + assert coresys.supervisor.connectivity is True websession.head.side_effect = side_effect - await supervisor_unthrottled.check_connectivity() + await coresys.supervisor.check_connectivity() - assert supervisor_unthrottled.connectivity is connectivity + assert coresys.supervisor.connectivity is connectivity @pytest.mark.parametrize( diff --git a/tests/test_updater.py b/tests/test_updater.py index 208310ca4..7f14e726a 100644 --- a/tests/test_updater.py +++ b/tests/test_updater.py @@ -1,16 +1,25 @@ """Test updater files.""" -from unittest.mock import patch +import asyncio +from unittest.mock import AsyncMock, MagicMock, patch from awesomeversion import AwesomeVersion import pytest +from supervisor.const import BusEvent from supervisor.coresys import CoreSys +from supervisor.dbus.const import ConnectivityState +from supervisor.jobs import SupervisorJob + +from tests.common import load_binary_fixture +from tests.dbus_service_mocks.network_manager import ( + NetworkManager as NetworkManagerService, +) URL_TEST = "https://version.home-assistant.io/stable.json" -@pytest.mark.asyncio +@pytest.mark.usefixtures("no_job_throttle") async def test_fetch_versions(coresys: CoreSys) -> None: """Test download and sync version.""" @@ -53,6 +62,7 @@ async def test_fetch_versions(coresys: CoreSys) -> None: ) +@pytest.mark.usefixtures("no_job_throttle") @pytest.mark.parametrize( "version, expected", [ @@ -71,3 +81,51 @@ async def test_os_update_path(coresys: CoreSys, version: str, expected: str): await coresys.updater.fetch_data() assert coresys.updater.version_hassos == AwesomeVersion(expected) + + +@pytest.mark.usefixtures("no_job_throttle") +async def test_delayed_fetch_for_connectivity( + coresys: CoreSys, network_manager_service: NetworkManagerService +): + """Test initial version fetch waits for connectivity on load.""" + coresys.websession.get = MagicMock() + coresys.websession.get.return_value.__aenter__.return_value.status = 200 + coresys.websession.get.return_value.__aenter__.return_value.read.return_value = ( + load_binary_fixture("version_stable.json") + ) + coresys.websession.head = AsyncMock() + coresys.security.verify_own_content = AsyncMock() + + # Network connectivity change causes a series of async tasks to eventually do a version fetch + # Rather then use some kind of sleep loop, set up listener for start of fetch data job + event = asyncio.Event() + + async def find_fetch_data_job_start(job: SupervisorJob): + if job.name == "updater_fetch_data": + event.set() + + coresys.bus.register_event(BusEvent.SUPERVISOR_JOB_START, find_fetch_data_job_start) + + # Start with no connectivity and confirm there is no version fetch on load + coresys.supervisor.connectivity = False + network_manager_service.connectivity = ConnectivityState.CONNECTIVITY_NONE.value + await coresys.host.network.load() + await coresys.host.network.check_connectivity() + + await coresys.updater.load() + coresys.websession.get.assert_not_called() + + # Now signal host has connectivity and wait for fetch data to complete to assert + network_manager_service.emit_properties_changed( + {"Connectivity": ConnectivityState.CONNECTIVITY_FULL} + ) + await network_manager_service.ping() + async with asyncio.timeout(5): + await event.wait() + await asyncio.sleep(0) + + coresys.websession.get.assert_called_once() + assert ( + coresys.websession.get.call_args[0][0] + == "https://version.home-assistant.io/stable.json" + )