diff --git a/supervisor/core.py b/supervisor/core.py index ec53960a5..000047db4 100644 --- a/supervisor/core.py +++ b/supervisor/core.py @@ -5,9 +5,7 @@ from collections.abc import Awaitable from contextlib import suppress from datetime import timedelta import logging -from typing import Final, Self - -from dbus_fast import Variant +from typing import Self from .const import ( ATTR_STARTUP, @@ -18,8 +16,7 @@ from .const import ( CoreState, ) from .coresys import CoreSys, CoreSysAttributes -from .dbus.const import StartUnitMode, StopUnitMode, UnitActiveState -from .dbus.systemd import ExecStartEntry +from .dbus.const import StopUnitMode, UnitActiveState from .exceptions import ( AppFileReadError, HassioError, @@ -37,25 +34,11 @@ from .utils.whoami import retrieve_whoami _LOGGER: logging.Logger = logging.getLogger(__name__) + # Transient systemd units used to hold Core's host port(s) during boot. The # socket is paired with a no-op oneshot service (RemainAfterExit) so systemd # always has a valid activation target -- otherwise it would tear the socket # down on the first connection attempt before Core is ready. -_PORT_RESERVE_UNIT: Final = "homeassistant-core-port-reserve.socket" -_PORT_RESERVE_SERVICE: Final = "homeassistant-core-port-reserve.service" -_PORT_RESERVE_TIMEOUT: Final = 10 -_TERMINAL_STATES: Final = {UnitActiveState.INACTIVE, UnitActiveState.FAILED} - - -def _format_bind_address(host: str, port: int) -> str: - """Format a host/port pair for a systemd ``Listen`` directive. - - IPv6 addresses must be bracketed (e.g. ``[::]:8123``) or systemd rejects - them; plain ``host:port`` is used for IPv4 addresses/hostnames. - """ - return f"[{host}]:{port}" if ":" in host else f"{host}:{port}" - - class Core(CoreSysAttributes): """Main object of Supervisor.""" @@ -269,13 +252,12 @@ class Core(CoreSysAttributes): # Reserve Core's TCP port before booting other apps, since Core runs # with --network=host and competes with them for it. Released again - # just before Core starts so it can claim the port itself. - core_port_reserved = False + # just before Core starts so it can claim the port itself. Any other + # path that starts Core in the meantime (API, watchdog, restore) + # releases it too: HomeAssistantCore.start()/restart() own that. try: if not await self.sys_homeassistant.core.is_running(): - hosts = self.sys_homeassistant.http_server_host or ["0.0.0.0", "::"] - port = self.sys_homeassistant.api_port - core_port_reserved = await self._reserve_core_port(hosts, port) + await self.sys_homeassistant.core.reserve_port() # Start app mark as initialize await self.sys_apps.boot(AppStartup.INITIALIZE) @@ -298,8 +280,7 @@ class Core(CoreSysAttributes): # best-effort only: Core's port is just its API/frontend bind, so # Core must be started regardless -- if it can't bind yet it just # logs and keeps retrying, which beats not starting Core at all. - if core_port_reserved: - core_port_reserved = not await self._release_core_port() + await self.sys_homeassistant.core.release_port() # run HomeAssistant if ( @@ -336,8 +317,7 @@ class Core(CoreSysAttributes): finally: # Ensure the port reservation is always released - if core_port_reserved: - await self._release_core_port() + await self.sys_homeassistant.core.release_port() # Add core tasks into scheduler await self.sys_tasks.load() @@ -485,161 +465,6 @@ class Core(CoreSysAttributes): self.sys_config.last_boot = last_boot await self.sys_config.save_data() - async def _reserve_core_port(self, hosts: list[str], port: int) -> bool: - """Reserve Core's host TCP port(s) using a transient systemd socket. - - Core runs with --network=host, so Supervisor (on the ``hassio`` - Docker bridge network) can't bind the port itself -- instead systemd - is asked to bind a transient ``.socket`` unit for each host, paired - atomically (via ``aux``) with a no-op oneshot service so it has a - valid activation target. - - Returns True if reserved (caller must later call - ``_release_core_port``), or False if it couldn't be reserved - (non-fatal, boot continues without the protection). - """ - if not self.sys_dbus.systemd.is_connected: - _LOGGER.warning( - "Cannot reserve Core port(s) %s:%d: systemd D-Bus not connected", - hosts, - port, - ) - return False - - # Clean up any unit left behind by a crashed Supervisor boot -- systemd - # refuses to redefine a transient unit that's still loaded under the - # same name, even with mode=replace. - if not await self._release_core_port(): - _LOGGER.warning( - "Cannot reserve Core port(s) %s:%d: a previous reservation " - "could not be confirmed released", - hosts, - port, - ) - return False - - service_properties: list[tuple[str, Variant]] = [ - ( - "Description", - Variant("s", "Home Assistant Core port reservation holder"), - ), - ("Type", Variant("s", "oneshot")), - ("RemainAfterExit", Variant("b", True)), - ( - "ExecStart", - Variant( - "a(sasb)", - [ExecStartEntry("/bin/sh", ["/bin/sh", "-c", ":"], False)], - ), - ), - ] - socket_properties: list[tuple[str, Variant]] = [ - ("Description", Variant("s", "Home Assistant Core port reservation")), - ( - "Listen", - Variant( - "a(ss)", - [("Stream", _format_bind_address(host, port)) for host in hosts], - ), - ), - ("BindIPv6Only", Variant("s", "ipv6-only")), - ] - - try: - await self.sys_dbus.systemd.start_transient_unit( - _PORT_RESERVE_UNIT, - StartUnitMode.REPLACE, - socket_properties, - aux=[(_PORT_RESERVE_SERVICE, service_properties)], - ) - unit = await self.sys_dbus.systemd.get_unit(_PORT_RESERVE_UNIT) - async with asyncio.timeout(_PORT_RESERVE_TIMEOUT): - state = await unit.wait_for_active_state( - {UnitActiveState.ACTIVE, UnitActiveState.FAILED} - ) - if state != UnitActiveState.ACTIVE: - raise HassioError(f"unit entered state {state}") - except (HassioError, TimeoutError) as err: - _LOGGER.warning( - "Could not reserve Home Assistant Core port(s) %s:%d: %s", - hosts, - port, - err, - ) - await self._release_core_port() - return False - - _LOGGER.debug( - "Reserved Home Assistant Core port(s) %s:%d during app startup", - hosts, - port, - ) - return True - - async def _stop_unit_confirmed(self, unit_name: str) -> bool: - """Stop a transient unit and confirm it actually reached INACTIVE. - - Retries once. Returns False if this can't be confirmed, meaning - systemd may still be holding whatever resource the unit represents. - """ - try: - unit = await self.sys_dbus.systemd.get_unit(unit_name) - except HassioError: - # No such unit (or couldn't even ask) -- nothing to release. - return True - - for _attempt in range(2): - # A stop error doesn't necessarily mean it didn't happen on the - # systemd side -- always re-check the real state below instead. - with suppress(HassioError): - await self.sys_dbus.systemd.stop_unit(unit_name, StopUnitMode.REPLACE) - - try: - with suppress(TimeoutError): - async with asyncio.timeout(_PORT_RESERVE_TIMEOUT): - await unit.wait_for_active_state(_TERMINAL_STATES) - - state = await unit.get_active_state() - except HassioError: - return True # unit disappeared -- nothing left to hold it - - if state == UnitActiveState.INACTIVE: - return True - - if state == UnitActiveState.FAILED: - # FAILED units stick around until explicitly reset. - with suppress(HassioError): - await self.sys_dbus.systemd.reset_failed_unit(unit_name) - with suppress(HassioError): - if await unit.get_active_state() == UnitActiveState.INACTIVE: - return True - - _LOGGER.error( - "Could not confirm systemd unit %s was released; it may still be " - "holding Home Assistant Core's port", - unit_name, - ) - return False - - async def _release_core_port(self) -> bool: - """Stop the port reservation units if they exist, releasing the port. - - Safe to call as a no-op when nothing exists yet. Returns True once - both units are confirmed gone/INACTIVE, or False otherwise. - """ - if not self.sys_dbus.systemd.is_connected: - return True - - # Stop both units -- leaving either loaded would make systemd refuse - # to redefine it under the same name next time. - socket_released = await self._stop_unit_confirmed(_PORT_RESERVE_UNIT) - service_released = await self._stop_unit_confirmed(_PORT_RESERVE_SERVICE) - - if socket_released and service_released: - _LOGGER.debug("Stopped Home Assistant Core port reservation unit") - - return socket_released and service_released - async def _adjust_system_datetime(self) -> None: """Adjust system time/date on startup.""" # Ensure host system timezone matches supervisor timezone configuration diff --git a/supervisor/homeassistant/core.py b/supervisor/homeassistant/core.py index f798efb1c..2e4e64c35 100644 --- a/supervisor/homeassistant/core.py +++ b/supervisor/homeassistant/core.py @@ -12,12 +12,15 @@ import shutil from typing import Final from awesomeversion import AwesomeVersion +from dbus_fast import Variant from supervisor.utils import remove_colors from ..bus import EventListener from ..const import ATTR_HOMEASSISTANT, BusEvent, CoreState from ..coresys import CoreSys +from ..dbus.const import StartUnitMode, StopUnitMode, UnitActiveState +from ..dbus.systemd import ExecStartEntry from ..docker.const import ContainerState from ..docker.homeassistant import HASS_DOCKER_NAME, DockerHomeAssistant from ..docker.monitor import DockerContainerStateEvent @@ -28,6 +31,7 @@ from ..exceptions import ( DockerError, DockerRegistryRateLimitExceeded, DockerStatsTimeoutError, + HassioError, HomeAssistantCrashError, HomeAssistantError, HomeAssistantJobError, @@ -71,6 +75,20 @@ DATABASE_MIGRATION_TIMEOUT: Final[timedelta] = timedelta( ) RE_YAML_ERROR = re.compile(r"homeassistant\.util\.yaml") +_PORT_RESERVE_UNIT: Final = "homeassistant-core-port-reserve.socket" +_PORT_RESERVE_SERVICE: Final = "homeassistant-core-port-reserve.service" +_PORT_RESERVE_TIMEOUT: Final = 10 +_TERMINAL_STATES: Final = {UnitActiveState.INACTIVE, UnitActiveState.FAILED} + + +def _format_bind_address(host: str, port: int) -> str: + """Format a host/port pair for a systemd ``Listen`` directive. + + IPv6 addresses must be bracketed (e.g. ``[::]:8123``) or systemd rejects + them; plain ``host:port`` is used for IPv4 addresses/hostnames. + """ + return f"[{host}]:{port}" if ":" in host else f"{host}:{port}" + @dataclass class ConfigResult: @@ -89,6 +107,7 @@ class HomeAssistantCore(JobGroup): self.instance: DockerHomeAssistant = DockerHomeAssistant(coresys) self._error_state: bool = False self._watchdog_listener: EventListener | None = None + self._port_reserved: bool = False @property def error_state(self) -> bool: @@ -505,6 +524,10 @@ class HomeAssistantCore(JobGroup): _LOGGER.warning("Home Assistant is already running!") return + # Give the port back before the container needs it, in case the boot + # flow still holds it (e.g. Core started via the API during app boot) + await self.release_port() + # Instance/Container exists, simple start if await self.instance.is_initialize(): try: @@ -555,6 +578,9 @@ class HomeAssistantCore(JobGroup): (self.sys_config.path_homeassistant / SAFE_MODE_FILENAME).touch ) + # See start(): Core rebinds its port on restart + await self.release_port() + try: await self.instance.restart() except DockerError as err: @@ -613,6 +639,187 @@ class HomeAssistantCore(JobGroup): """Return True if a task is in progress.""" return self.instance.in_progress or self.active_job is not None + async def reserve_port(self) -> bool: + """Reserve Core's host TCP port(s) using a transient systemd socket. + + Used by the Supervisor boot flow to keep host-network apps from + grabbing Core's port while they boot ahead of Core. Core runs with + --network=host, so Supervisor (on the ``hassio`` Docker bridge + network) can't bind the port itself -- instead systemd is asked to + bind a transient ``.socket`` unit for each host, paired atomically + (via ``aux``) with a no-op oneshot service so it has a valid + activation target. + + The reservation is released by ``release_port``, which ``start`` and + ``restart`` call themselves before touching the container, so it is + safe no matter which path ends up starting Core first. + + Returns True if reserved, or False if it couldn't be reserved + (non-fatal, boot continues without the protection). + """ + hosts = self.sys_homeassistant.http_server_host or ["0.0.0.0", "::"] + port = self.sys_homeassistant.api_port + + if not self.sys_dbus.systemd.is_connected: + _LOGGER.warning( + "Cannot reserve Core port(s) %s:%d: systemd D-Bus not connected", + hosts, + port, + ) + return False + + # Clean up any unit left behind by a crashed Supervisor boot -- systemd + # refuses to redefine a transient unit that's still loaded under the + # same name, even with mode=replace. + if not await self._stop_port_reserve_units(): + _LOGGER.warning( + "Cannot reserve Core port(s) %s:%d: a previous reservation " + "could not be confirmed released", + hosts, + port, + ) + return False + + service_properties: list[tuple[str, Variant]] = [ + ( + "Description", + Variant("s", "Home Assistant Core port reservation holder"), + ), + ("Type", Variant("s", "oneshot")), + ("RemainAfterExit", Variant("b", True)), + ( + "ExecStart", + Variant( + "a(sasb)", + [ExecStartEntry("/bin/sh", ["/bin/sh", "-c", ":"], False)], + ), + ), + ] + socket_properties: list[tuple[str, Variant]] = [ + ("Description", Variant("s", "Home Assistant Core port reservation")), + ( + "Listen", + Variant( + "a(ss)", + [("Stream", _format_bind_address(host, port)) for host in hosts], + ), + ), + ("BindIPv6Only", Variant("s", "ipv6-only")), + ] + + try: + await self.sys_dbus.systemd.start_transient_unit( + _PORT_RESERVE_UNIT, + StartUnitMode.REPLACE, + socket_properties, + aux=[(_PORT_RESERVE_SERVICE, service_properties)], + ) + unit = await self.sys_dbus.systemd.get_unit(_PORT_RESERVE_UNIT) + async with asyncio.timeout(_PORT_RESERVE_TIMEOUT): + state = await unit.wait_for_active_state( + {UnitActiveState.ACTIVE, UnitActiveState.FAILED} + ) + if state != UnitActiveState.ACTIVE: + raise HassioError(f"unit entered state {state}") + except (HassioError, TimeoutError) as err: + _LOGGER.warning( + "Could not reserve Home Assistant Core port(s) %s:%d: %s", + hosts, + port, + err, + ) + await self._stop_port_reserve_units() + return False + + _LOGGER.debug( + "Reserved Home Assistant Core port(s) %s:%d during app startup", + hosts, + port, + ) + self._port_reserved = True + return True + + async def release_port(self) -> bool: + """Release the port reservation made by ``reserve_port``, if any. + + Cheap no-op when nothing is reserved. Returns True once the port is + confirmed free (or was never held), False if the release could not + be confirmed -- in which case the reservation stays flagged so a + later call retries. + """ + if not self._port_reserved: + return True + + released = await self._stop_port_reserve_units() + self._port_reserved = not released + return released + + async def _stop_unit_confirmed(self, unit_name: str) -> bool: + """Stop a transient unit and confirm it actually reached INACTIVE. + + Retries once. Returns False if this can't be confirmed, meaning + systemd may still be holding whatever resource the unit represents. + """ + try: + unit = await self.sys_dbus.systemd.get_unit(unit_name) + except HassioError: + # No such unit (or couldn't even ask) -- nothing to release. + return True + + for _attempt in range(2): + # A stop error doesn't necessarily mean it didn't happen on the + # systemd side -- always re-check the real state below instead. + with suppress(HassioError): + await self.sys_dbus.systemd.stop_unit(unit_name, StopUnitMode.REPLACE) + + try: + with suppress(TimeoutError): + async with asyncio.timeout(_PORT_RESERVE_TIMEOUT): + await unit.wait_for_active_state(_TERMINAL_STATES) + + state = await unit.get_active_state() + except HassioError: + return True # unit disappeared -- nothing left to hold it + + if state == UnitActiveState.INACTIVE: + return True + + if state == UnitActiveState.FAILED: + # FAILED units stick around until explicitly reset. + with suppress(HassioError): + await self.sys_dbus.systemd.reset_failed_unit(unit_name) + with suppress(HassioError): + if await unit.get_active_state() == UnitActiveState.INACTIVE: + return True + + _LOGGER.error( + "Could not confirm systemd unit %s was released; it may still be " + "holding Home Assistant Core's port", + unit_name, + ) + return False + + async def _stop_port_reserve_units(self) -> bool: + """Stop the port reservation units if they exist, releasing the port. + + Unconditional: also used to clean up units left behind by a crashed + Supervisor before reserving again. Safe to call as a no-op when + nothing exists yet. Returns True once both units are confirmed + gone/INACTIVE, or False otherwise. + """ + if not self.sys_dbus.systemd.is_connected: + return True + + # Stop both units -- leaving either loaded would make systemd refuse + # to redefine it under the same name next time. + socket_released = await self._stop_unit_confirmed(_PORT_RESERVE_UNIT) + service_released = await self._stop_unit_confirmed(_PORT_RESERVE_SERVICE) + + if socket_released and service_released: + _LOGGER.debug("Stopped Home Assistant Core port reservation unit") + + return socket_released and service_released + async def check_config(self) -> ConfigResult: """Run Home Assistant config check.""" try: diff --git a/tests/homeassistant/test_core.py b/tests/homeassistant/test_core.py index 57d875eb4..a2b3d483a 100644 --- a/tests/homeassistant/test_core.py +++ b/tests/homeassistant/test_core.py @@ -8,16 +8,19 @@ from unittest.mock import ANY, AsyncMock, MagicMock, Mock, PropertyMock, call, p import aiodocker from aiodocker.containers import DockerContainer from awesomeversion import AwesomeVersion +from dbus_fast import DBusError import pytest from time_machine import travel from supervisor.const import CpuArch from supervisor.coresys import CoreSys +from supervisor.dbus.const import DBUS_ERR_SYSTEMD_NO_SUCH_UNIT from supervisor.docker.homeassistant import DockerHomeAssistant from supervisor.docker.interface import DockerInterface from supervisor.docker.manager import DockerAPI from supervisor.exceptions import ( AudioUpdateError, + DBusNotConnectedError, DockerAPIError, DockerContainerNotFoundError, DockerContainerNotRunningError, @@ -33,7 +36,7 @@ from supervisor.exceptions import ( ) from supervisor.homeassistant.api import APIState from supervisor.homeassistant.const import LANDINGPAGE, WSEvent -from supervisor.homeassistant.core import HomeAssistantCore +from supervisor.homeassistant.core import HomeAssistantCore, _format_bind_address from supervisor.homeassistant.module import HomeAssistant from supervisor.jobs.const import JobCondition from supervisor.resolution.const import ContextType, IssueType @@ -41,6 +44,9 @@ from supervisor.resolution.data import Issue from supervisor.updater import Updater from tests.common import AsyncIterator, load_json_fixture +from tests.dbus_service_mocks.base import DBusServiceMock +from tests.dbus_service_mocks.systemd import Systemd as SystemdService +from tests.dbus_service_mocks.systemd_unit import SystemdUnit as SystemdUnitService async def test_update_fails_if_out_of_date(coresys: CoreSys): @@ -1244,3 +1250,338 @@ async def test_load_missing_image_reinstall_retries_on_ratelimit( assert [c.args[2] for c in pull_image.call_args_list] == ["2026.2.3", "2026.2.3"] assert coresys.homeassistant.version == AwesomeVersion("2026.2.3") assert "Could not reinstall" not in caplog.text + + +# ---- Core port reservation ---- + + +@pytest.mark.parametrize( + ("host", "expected"), + [ + ("0.0.0.0", "0.0.0.0:8123"), + ("::", "[::]:8123"), + ("192.0.2.1", "192.0.2.1:8123"), + ("2001:db8::1", "[2001:db8::1]:8123"), + ], +) +def test_format_bind_address(host: str, expected: str) -> None: + """IPv6 addresses must be bracketed for systemd's Listen directive.""" + assert _format_bind_address(host, 8123) == expected + + +@pytest.fixture(name="held_port_reservation") +async def fixture_held_port_reservation( + coresys: CoreSys, + all_dbus_services: dict[str, DBusServiceMock | dict[str, DBusServiceMock]], +) -> SystemdService: + """Take a real port reservation through the systemd mock and hand back the service.""" + systemd_service: SystemdService = all_dbus_services["systemd"] + systemd_unit_service: SystemdUnitService = all_dbus_services["systemd_unit"] + # Fresh boot: neither unit exists during pre-cleanup, afterwards GetUnit + # finds the created socket unit and the linked unit mock tracks state + systemd_service.response_get_unit = [ + DBusError(DBUS_ERR_SYSTEMD_NO_SUCH_UNIT, "no such unit"), + DBusError(DBUS_ERR_SYSTEMD_NO_SUCH_UNIT, "no such unit"), + ] + [SystemdService.response_get_unit] * 20 + systemd_service.mock_systemd_unit = systemd_unit_service + coresys.homeassistant.http_server_host = None + + assert await coresys.homeassistant.core.reserve_port() is True + systemd_service.StopUnit.calls.clear() + return systemd_service + + +async def test_start_releases_held_port_reservation( + coresys: CoreSys, + container: DockerContainer, + held_port_reservation: SystemdService, +): + """Starting Core releases the boot-time port reservation before the container runs. + + Regression test for #7189: Core started through any path other than the + Supervisor boot flow (API, watchdog, restore) while the reservation was + still held came up unable to bind its port. + """ + coresys.docker.images.inspect.return_value = {"Id": "123"} + container.show.return_value["Image"] = "123" + container.show.return_value["State"]["Status"] = "exited" + container.show.return_value["State"]["Running"] = False + + stop_calls_at_start: list[int] = [] + container.start.side_effect = lambda: stop_calls_at_start.append( + len(held_port_reservation.StopUnit.calls) + ) + + with ( + patch.object( + HomeAssistant, + "version", + new=PropertyMock(return_value=AwesomeVersion("2023.7.0")), + ), + patch.object(HomeAssistantCore, "_block_till_run"), + ): + await coresys.homeassistant.core.start() + + container.start.assert_called_once() + # Both units were stopped, and before the container was started + assert ("homeassistant-core-port-reserve.socket", "replace") in ( + held_port_reservation.StopUnit.calls + ) + assert ("homeassistant-core-port-reserve.service", "replace") in ( + held_port_reservation.StopUnit.calls + ) + assert stop_calls_at_start == [2] + # Flag cleared: a later release is a no-op + held_port_reservation.StopUnit.calls.clear() + assert await coresys.homeassistant.core.release_port() is True + assert held_port_reservation.StopUnit.calls == [] + + +async def test_restart_releases_held_port_reservation( + coresys: CoreSys, + container: DockerContainer, + held_port_reservation: SystemdService, +): + """Restarting Core releases the port reservation before the container restarts.""" + stop_calls_at_restart: list[int] = [] + container.restart.side_effect = lambda **_: stop_calls_at_restart.append( + len(held_port_reservation.StopUnit.calls) + ) + + with patch.object(HomeAssistantCore, "_block_till_run"): + await coresys.homeassistant.core.restart() + + container.restart.assert_called_once_with(t=260) + assert stop_calls_at_restart == [2] + + +async def test_start_without_reservation_skips_systemd( + coresys: CoreSys, + container: DockerContainer, + all_dbus_services: dict[str, DBusServiceMock | dict[str, DBusServiceMock]], +): + """Starting Core with no reservation held never touches systemd.""" + systemd_service: SystemdService = all_dbus_services["systemd"] + systemd_service.StopUnit.calls.clear() + coresys.docker.images.inspect.return_value = {"Id": "123"} + container.show.return_value["Image"] = "123" + container.show.return_value["State"]["Status"] = "exited" + container.show.return_value["State"]["Running"] = False + + with ( + patch.object( + HomeAssistant, + "version", + new=PropertyMock(return_value=AwesomeVersion("2023.7.0")), + ), + patch.object(HomeAssistantCore, "_block_till_run"), + ): + await coresys.homeassistant.core.start() + + container.start.assert_called_once() + assert systemd_service.StopUnit.calls == [] + + +async def test_start_proceeds_when_port_release_not_confirmed( + coresys: CoreSys, + container: DockerContainer, + held_port_reservation: SystemdService, +): + """Core is still started when the reservation release can't be confirmed. + + The port is only Core's API/frontend bind, so a stuck unit must never + keep Core down; Core logs and retries the bind itself. + """ + coresys.docker.images.inspect.return_value = {"Id": "123"} + container.show.return_value["Image"] = "123" + container.show.return_value["State"]["Status"] = "exited" + container.show.return_value["State"]["Running"] = False + + with ( + patch.object( + HomeAssistant, + "version", + new=PropertyMock(return_value=AwesomeVersion("2023.7.0")), + ), + patch.object(HomeAssistantCore, "_block_till_run"), + patch.object( + coresys.homeassistant.core, + "_stop_port_reserve_units", + new=AsyncMock(return_value=False), + ), + ): + await coresys.homeassistant.core.start() + + container.start.assert_called_once() + # Still flagged as held so the boot flow's final release retries + assert coresys.homeassistant.core._port_reserved is True + + +async def test_release_port_resets_failed_unit( + coresys: CoreSys, + all_dbus_services: dict[str, DBusServiceMock | dict[str, DBusServiceMock]], +): + """Releasing a unit that ended up FAILED (not cleanly INACTIVE) resets it. + + Systemd keeps a FAILED unit around until it is explicitly reset, unlike a + unit that cleanly stops to INACTIVE and is garbage collected on its own. + """ + systemd_service: SystemdService = all_dbus_services["systemd"] + systemd_unit_service: SystemdUnitService = all_dbus_services["systemd_unit"] + + # Simulate a unit that is FAILED and stays that way: StopUnit's mock + # unconditionally flips a *linked* unit to INACTIVE, so leave it unlinked + # and set the state directly instead, matching a real FAILED unit that + # ignores a stop request. ResetFailedUnit is also a no-op when unlinked, + # so the unit never actually clears -- release can't be confirmed. + systemd_service.response_get_unit = SystemdService.response_get_unit + systemd_unit_service.active_state = "failed" + + coresys.homeassistant.core._port_reserved = True + result = await coresys.homeassistant.core.release_port() + + assert result is False + assert ( + "homeassistant-core-port-reserve.socket", + "replace", + ) in systemd_service.StopUnit.calls + assert ( + "homeassistant-core-port-reserve.socket", + ) in systemd_service.ResetFailedUnit.calls + + +async def test_release_port_confirms_success_when_unit_goes_inactive( + coresys: CoreSys, + all_dbus_services: dict[str, DBusServiceMock | dict[str, DBusServiceMock]], +): + """Releasing returns True once both units are confirmed INACTIVE.""" + systemd_service: SystemdService = all_dbus_services["systemd"] + systemd_unit_service: SystemdUnitService = all_dbus_services["systemd_unit"] + + systemd_service.response_get_unit = SystemdService.response_get_unit + systemd_service.mock_systemd_unit = systemd_unit_service + systemd_unit_service.active_state = "active" + + coresys.homeassistant.core._port_reserved = True + result = await coresys.homeassistant.core.release_port() + + assert result is True + + +async def test_reserve_port_skips_when_dbus_not_connected( + coresys: CoreSys, + all_dbus_services: dict[str, DBusServiceMock | dict[str, DBusServiceMock]], +): + """No reservation is attempted if systemd D-Bus is not connected.""" + systemd_service: SystemdService = all_dbus_services["systemd"] + systemd_service.StartTransientUnit.calls.clear() + + with patch.object(coresys.dbus.systemd, "dbus", None): + coresys.homeassistant.http_server_host = ["0.0.0.0"] + result = await coresys.homeassistant.core.reserve_port() + + assert result is False + assert systemd_service.StartTransientUnit.calls == [] + + +async def test_release_port_skips_when_dbus_not_connected( + coresys: CoreSys, + all_dbus_services: dict[str, DBusServiceMock | dict[str, DBusServiceMock]], +): + """Releasing is a cheap no-op (confirmed released) if systemd D-Bus is not connected.""" + systemd_service: SystemdService = all_dbus_services["systemd"] + systemd_service.StopUnit.calls.clear() + + with patch.object(coresys.dbus.systemd, "dbus", None): + coresys.homeassistant.core._port_reserved = True + result = await coresys.homeassistant.core.release_port() + + assert result is True + assert systemd_service.StopUnit.calls == [] + + +async def test_reserve_port_returns_false_when_unit_ends_up_failed( + coresys: CoreSys, + all_dbus_services: dict[str, DBusServiceMock | dict[str, DBusServiceMock]], +): + """If the unit is created but ends up FAILED rather than ACTIVE, fail cleanly. + + This can happen if the port turns out to already be bound by something + else outside Supervisor's knowledge: systemd creates the unit but it + immediately fails to actually claim the socket. + """ + systemd_service: SystemdService = all_dbus_services["systemd"] + systemd_unit_service: SystemdUnitService = all_dbus_services["systemd_unit"] + + # First two GetUnit calls are the pre-cleanup check (one per unit) on a + # clean/fresh boot -- neither exists yet. The next GetUnit call is the + # post-creation poll for the newly created socket unit, which ends up + # FAILED rather than ACTIVE. Leave mock_systemd_unit unlinked so + # StartTransientUnit's mock doesn't overwrite ActiveState back to + # "active" after we set it. + systemd_service.response_get_unit = [ + DBusError(DBUS_ERR_SYSTEMD_NO_SUCH_UNIT, "no such unit"), + DBusError(DBUS_ERR_SYSTEMD_NO_SUCH_UNIT, "no such unit"), + ] + [SystemdService.response_get_unit] * 5 + systemd_unit_service.active_state = "failed" + systemd_service.StartTransientUnit.calls.clear() + + coresys.homeassistant.http_server_host = ["0.0.0.0"] + result = await coresys.homeassistant.core.reserve_port() + + assert result is False + assert len(systemd_service.StartTransientUnit.calls) == 1 + + +async def test_reserve_port_handles_dbus_disconnect_mid_call( + coresys: CoreSys, + all_dbus_services: dict[str, DBusServiceMock | dict[str, DBusServiceMock]], +): + """A mid-call D-Bus disconnect is treated as a non-fatal reservation failure. + + DBusNotConnectedError is not a DBusError subclass -- it's raised directly + by the dbus_connected decorator when the bus drops after the initial + is_connected check passed -- so it must be caught via the broader + HassioError, or it would propagate and abort Supervisor startup. + """ + systemd_service: SystemdService = all_dbus_services["systemd"] + # Pre-cleanup finds nothing on both units (clean/fresh boot); the + # disconnect happens on the create attempt itself. + systemd_service.response_get_unit = DBusError( + DBUS_ERR_SYSTEMD_NO_SUCH_UNIT, "no such unit" + ) + + with patch.object( + coresys.dbus.systemd, + "start_transient_unit", + side_effect=DBusNotConnectedError(), + ): + coresys.homeassistant.http_server_host = ["0.0.0.0"] + result = await coresys.homeassistant.core.reserve_port() + + assert result is False + + +async def test_release_port_handles_dbus_disconnect_mid_call( + coresys: CoreSys, + all_dbus_services: dict[str, DBusServiceMock | dict[str, DBusServiceMock]], +): + """A mid-call D-Bus disconnect while stopping a unit does not propagate.""" + systemd_service: SystemdService = all_dbus_services["systemd"] + systemd_service.response_get_unit = SystemdService.response_get_unit + + # mock_systemd_unit is left unlinked, so the unit's ActiveState never + # actually moves off the "active" default -- shorten the wait so the + # test doesn't block for the real _PORT_RESERVE_TIMEOUT (10s). + with ( + patch("supervisor.homeassistant.core._PORT_RESERVE_TIMEOUT", 0.01), + patch.object( + coresys.dbus.systemd, + "stop_unit", + side_effect=DBusNotConnectedError(), + ), + ): + coresys.homeassistant.core._port_reserved = True + result = await coresys.homeassistant.core.release_port() + + assert result is False diff --git a/tests/test_core.py b/tests/test_core.py index 4690c40ec..ecf27b1ba 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -12,15 +12,9 @@ from dbus_fast import DBusError, ErrorType, Variant import pytest from supervisor.const import AppStartup, CoreState -from supervisor.core import _format_bind_address from supervisor.coresys import CoreSys from supervisor.dbus.const import DBUS_ERR_SYSTEMD_NO_SUCH_UNIT -from supervisor.exceptions import ( - AppFileReadError, - DBusNotConnectedError, - HassioError, - WhoamiSSLError, -) +from supervisor.exceptions import AppFileReadError, HassioError, WhoamiSSLError from supervisor.hardware.helper import HwHelper from supervisor.homeassistant.core import HomeAssistantCore from supervisor.host.control import SystemControl @@ -491,20 +485,6 @@ def core_start_base_mocks( yield -@pytest.mark.parametrize( - ("host", "expected"), - [ - ("0.0.0.0", "0.0.0.0:8123"), - ("192.0.2.1", "192.0.2.1:8123"), - ("::", "[::]:8123"), - ("2001:db8::1", "[2001:db8::1]:8123"), - ], -) -def test_format_bind_address(host: str, expected: str) -> None: - """IPv6 addresses must be bracketed for systemd's Listen directive.""" - assert _format_bind_address(host, 8123) == expected - - @pytest.mark.usefixtures("core_start_base_mocks") @pytest.mark.parametrize( ("http_server_host", "expected_listen"), @@ -588,11 +568,11 @@ async def test_start_port_held_during_app_boot_released_before_core_start( call_sequence: list[str] = [] - orig_reserve = coresys.core._reserve_core_port - orig_release = coresys.core._release_core_port + orig_reserve = coresys.homeassistant.core.reserve_port + orig_release = coresys.homeassistant.core.release_port - async def tracking_reserve(hosts: list[str], port: int) -> bool: - result = await orig_reserve(hosts, port) + async def tracking_reserve() -> bool: + result = await orig_reserve() call_sequence.append("port_reserved") return result @@ -608,8 +588,12 @@ async def test_start_port_held_during_app_boot_released_before_core_start( call_sequence.append("core_start") with ( - patch.object(coresys.core, "_reserve_core_port", side_effect=tracking_reserve), - patch.object(coresys.core, "_release_core_port", side_effect=tracking_release), + patch.object( + coresys.homeassistant.core, "reserve_port", side_effect=tracking_reserve + ), + patch.object( + coresys.homeassistant.core, "release_port", side_effect=tracking_release + ), patch.object(coresys.apps, "boot", side_effect=tracking_boot), patch.object( coresys.homeassistant.core, "start", side_effect=tracking_core_start @@ -686,19 +670,21 @@ async def test_start_still_starts_core_when_release_not_confirmed( """ coresys.homeassistant.http_server_host = None - orig_release = coresys.core._release_core_port - release_calls = 0 + orig_stop = coresys.homeassistant.core._stop_port_reserve_units + stop_calls = 0 - async def flaky_release() -> bool: - nonlocal release_calls - release_calls += 1 - if release_calls == 1: - # Pre-cleanup release inside _reserve_core_port succeeds normally - return await orig_release() + async def flaky_stop() -> bool: + nonlocal stop_calls + stop_calls += 1 + if stop_calls == 1: + # Pre-cleanup inside reserve_port succeeds normally + return await orig_stop() # The real release before Core starts can't be confirmed return False - with patch.object(coresys.core, "_release_core_port", side_effect=flaky_release): + with patch.object( + coresys.homeassistant.core, "_stop_port_reserve_units", side_effect=flaky_stop + ): await coresys.core.start() coresys.homeassistant.core.start.assert_awaited_once() @@ -742,166 +728,3 @@ async def test_start_cleans_up_stale_active_unit_before_reserving( # It cleanly reached INACTIVE, so there was nothing to reset assert systemd_service.ResetFailedUnit.calls == [] coresys.homeassistant.core.start.assert_awaited_once() - - -async def test_release_core_port_resets_failed_unit( - coresys: CoreSys, - all_dbus_services: dict[str, DBusServiceMock | dict[str, DBusServiceMock]], -): - """Releasing a unit that ended up FAILED (not cleanly INACTIVE) resets it. - - Systemd keeps a FAILED unit around until it is explicitly reset, unlike a - unit that cleanly stops to INACTIVE and is garbage collected on its own. - """ - systemd_service: SystemdService = all_dbus_services["systemd"] - systemd_unit_service: SystemdUnitService = all_dbus_services["systemd_unit"] - - # Simulate a unit that is FAILED and stays that way: StopUnit's mock - # unconditionally flips a *linked* unit to INACTIVE, so leave it unlinked - # and set the state directly instead, matching a real FAILED unit that - # ignores a stop request. ResetFailedUnit is also a no-op when unlinked, - # so the unit never actually clears -- release can't be confirmed. - systemd_service.response_get_unit = SystemdService.response_get_unit - systemd_unit_service.active_state = "failed" - - result = await coresys.core._release_core_port() - - assert result is False - assert ( - "homeassistant-core-port-reserve.socket", - "replace", - ) in systemd_service.StopUnit.calls - assert ( - "homeassistant-core-port-reserve.socket", - ) in systemd_service.ResetFailedUnit.calls - - -async def test_release_core_port_confirms_success_when_unit_goes_inactive( - coresys: CoreSys, - all_dbus_services: dict[str, DBusServiceMock | dict[str, DBusServiceMock]], -): - """Releasing returns True once both units are confirmed INACTIVE.""" - systemd_service: SystemdService = all_dbus_services["systemd"] - systemd_unit_service: SystemdUnitService = all_dbus_services["systemd_unit"] - - systemd_service.response_get_unit = SystemdService.response_get_unit - systemd_service.mock_systemd_unit = systemd_unit_service - systemd_unit_service.active_state = "active" - - result = await coresys.core._release_core_port() - - assert result is True - - -async def test_reserve_core_port_skips_when_dbus_not_connected( - coresys: CoreSys, - all_dbus_services: dict[str, DBusServiceMock | dict[str, DBusServiceMock]], -): - """No reservation is attempted if systemd D-Bus is not connected.""" - systemd_service: SystemdService = all_dbus_services["systemd"] - systemd_service.StartTransientUnit.calls.clear() - - with patch.object(coresys.dbus.systemd, "dbus", None): - result = await coresys.core._reserve_core_port(["0.0.0.0"], 8123) - - assert result is False - assert systemd_service.StartTransientUnit.calls == [] - - -async def test_release_core_port_skips_when_dbus_not_connected( - coresys: CoreSys, - all_dbus_services: dict[str, DBusServiceMock | dict[str, DBusServiceMock]], -): - """Releasing is a cheap no-op (confirmed released) if systemd D-Bus is not connected.""" - systemd_service: SystemdService = all_dbus_services["systemd"] - systemd_service.StopUnit.calls.clear() - - with patch.object(coresys.dbus.systemd, "dbus", None): - result = await coresys.core._release_core_port() - - assert result is True - assert systemd_service.StopUnit.calls == [] - - -async def test_reserve_core_port_returns_false_when_unit_ends_up_failed( - coresys: CoreSys, - all_dbus_services: dict[str, DBusServiceMock | dict[str, DBusServiceMock]], -): - """If the unit is created but ends up FAILED rather than ACTIVE, fail cleanly. - - This can happen if the port turns out to already be bound by something - else outside Supervisor's knowledge: systemd creates the unit but it - immediately fails to actually claim the socket. - """ - systemd_service: SystemdService = all_dbus_services["systemd"] - systemd_unit_service: SystemdUnitService = all_dbus_services["systemd_unit"] - - # First two GetUnit calls are the pre-cleanup check (one per unit) on a - # clean/fresh boot -- neither exists yet. The next GetUnit call is the - # post-creation poll for the newly created socket unit, which ends up - # FAILED rather than ACTIVE. Leave mock_systemd_unit unlinked so - # StartTransientUnit's mock doesn't overwrite ActiveState back to - # "active" after we set it. - systemd_service.response_get_unit = [ - DBusError(DBUS_ERR_SYSTEMD_NO_SUCH_UNIT, "no such unit"), - DBusError(DBUS_ERR_SYSTEMD_NO_SUCH_UNIT, "no such unit"), - ] + [SystemdService.response_get_unit] * 5 - systemd_unit_service.active_state = "failed" - systemd_service.StartTransientUnit.calls.clear() - - result = await coresys.core._reserve_core_port(["0.0.0.0"], 8123) - - assert result is False - assert len(systemd_service.StartTransientUnit.calls) == 1 - - -async def test_reserve_core_port_handles_dbus_disconnect_mid_call( - coresys: CoreSys, - all_dbus_services: dict[str, DBusServiceMock | dict[str, DBusServiceMock]], -): - """A mid-call D-Bus disconnect is treated as a non-fatal reservation failure. - - DBusNotConnectedError is not a DBusError subclass -- it's raised directly - by the dbus_connected decorator when the bus drops after the initial - is_connected check passed -- so it must be caught via the broader - HassioError, or it would propagate and abort Supervisor startup. - """ - systemd_service: SystemdService = all_dbus_services["systemd"] - # Pre-cleanup finds nothing on both units (clean/fresh boot); the - # disconnect happens on the create attempt itself. - systemd_service.response_get_unit = DBusError( - DBUS_ERR_SYSTEMD_NO_SUCH_UNIT, "no such unit" - ) - - with patch.object( - coresys.dbus.systemd, - "start_transient_unit", - side_effect=DBusNotConnectedError(), - ): - result = await coresys.core._reserve_core_port(["0.0.0.0"], 8123) - - assert result is False - - -async def test_release_core_port_handles_dbus_disconnect_mid_call( - coresys: CoreSys, - all_dbus_services: dict[str, DBusServiceMock | dict[str, DBusServiceMock]], -): - """A mid-call D-Bus disconnect while stopping a unit does not propagate.""" - systemd_service: SystemdService = all_dbus_services["systemd"] - systemd_service.response_get_unit = SystemdService.response_get_unit - - # mock_systemd_unit is left unlinked, so the unit's ActiveState never - # actually moves off the "active" default -- shorten the wait so the - # test doesn't block for the real _PORT_RESERVE_TIMEOUT (10s). - with ( - patch("supervisor.core._PORT_RESERVE_TIMEOUT", 0.01), - patch.object( - coresys.dbus.systemd, - "stop_unit", - side_effect=DBusNotConnectedError(), - ), - ): - result = await coresys.core._release_core_port() - - assert result is False