Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delay initial version fetch until there is connectivity #5603

Merged
merged 6 commits into from
Feb 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions supervisor/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
4 changes: 3 additions & 1 deletion supervisor/host/network.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ def connectivity(self, state: bool | None) -> None:
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]:
Expand Down Expand Up @@ -148,7 +150,7 @@ async def _check_connectivity_changed(
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.
Expand Down
4 changes: 2 additions & 2 deletions supervisor/jobs/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,9 @@ def _notify_on_job_change(

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)
Comment on lines -216 to +218
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize this feels unrelated. As it turns out, trying to use these in test made me realize they are entirely useless. The listener only receives a uuid and it can't actually count on the job still being around to look up more info about it, so there's really no way to reliably know what job started/ended. Passing the actual job in the event makes these useful (and it was needed in the test)


def new_job(
self,
Expand Down
8 changes: 7 additions & 1 deletion supervisor/supervisor.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,12 @@
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
Expand Down Expand Up @@ -74,6 +79,7 @@ def connectivity(self, state: bool) -> None:
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}
)
Expand Down
23 changes: 21 additions & 2 deletions supervisor/updater.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import aiohttp
from awesomeversion import AwesomeVersion

from .bus import EventListener
from .const import (
ATTR_AUDIO,
ATTR_AUTO_UPDATE,
Expand All @@ -23,6 +24,7 @@
ATTR_SUPERVISOR,
FILE_HASSIO_UPDATER,
URL_HASSIO_VERSION,
BusEvent,
UpdateChannel,
)
from .coresys import CoreSysAttributes
Expand All @@ -47,11 +49,18 @@ def __init__(self, coresys):
"""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."""
Expand Down Expand Up @@ -180,6 +189,11 @@ def auto_update(self, value: bool) -> None:
"""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],
Expand Down Expand Up @@ -214,6 +228,11 @@ async def fetch_data(self):
_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))
Expand Down
15 changes: 8 additions & 7 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
3 changes: 2 additions & 1 deletion tests/dbus_service_mocks/network_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -155,7 +156,7 @@ def ConnectivityCheckAvailable(self) -> "b":
@dbus_property()
def ConnectivityCheckEnabled(self) -> "b":
"""Get ConnectivityCheckEnabled."""
return True
return self.connectivity_check_enabled

@ConnectivityCheckEnabled.setter
def ConnectivityCheckEnabled(self, value: "b"):
Expand Down
16 changes: 8 additions & 8 deletions tests/jobs/test_job_decorator.py
Original file line number Diff line number Diff line change
Expand Up @@ -1144,13 +1144,13 @@ async def job_task(self) -> None:
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)
Expand Down Expand Up @@ -1196,13 +1196,13 @@ async def job_task(self) -> None:
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)
Expand Down
1 change: 1 addition & 0 deletions tests/misc/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
):
Expand Down
2 changes: 1 addition & 1 deletion tests/os/test_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
# 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"
Expand Down
2 changes: 2 additions & 0 deletions tests/plugins/test_plugin_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
16 changes: 5 additions & 11 deletions tests/test_supervisor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
62 changes: 60 additions & 2 deletions tests/test_updater.py
Original file line number Diff line number Diff line change
@@ -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."""

Expand Down Expand Up @@ -53,6 +62,7 @@ async def test_fetch_versions(coresys: CoreSys) -> None:
)


@pytest.mark.usefixtures("no_job_throttle")
@pytest.mark.parametrize(
"version, expected",
[
Expand All @@ -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"
)