From 696dcf6149840f1217ab4c04406280d42219eee9 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Fri, 28 Feb 2025 18:01:55 +0100 Subject: [PATCH] Initialize Supervisor Core state in constructor (#5686) * Initialize Supervisor Core state in constructor Make sure the Supervisor Core state is set to a value early on. This makes sure that the state is always of type CoreState, and makes sure that any use of the state can rely on it being an actual value from the CoreState enum. This fixes Sentry filter during early startup, where the state previously was None. Because of that, the Sentry filter tried to collect more Context, which lead to an exception and not reporting errors. * Fix pytest It seems that with initializing the state early, the pytest actually runs a system evaluation with: Starting system evaluation with state initialize Before it did that with: Starting system evaluation with state None It detects that the container runs as privileged, and declares the system as unhealthy. It is unclear to me why coresys.core.healthy was checked in this context, it doesn't seem useful. Just remove the check, and validate the state through the getter instead. * Update supervisor/core.py Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Make sure Supervisor container is privileged in pytest With the Supervisor Core state being valid now, some evaluations now actually run when loading the resolution center. This leads to Supervisor getting declared unhealthy due to not running in a privileged container under pytest. Fake the host container to be privileged to make evaluations not causing the system to be declared unhealthy under pytest. * Avoid writing actual Supervisor run state file With the Supervisor Core state being valid from the very start, we end up writing a state everytime. Instead of actually writing a state file, simply validate the the necessary calls are being made. This is more conform to typical unit tests and avoids writing a file for every test. * Extend WebSocket client fixture and use it consistently Extend the ha_ws_client WebSocket client fixture to set Supervisor Core into run state and clear all pending messages. Currently only some tests use the ha_ws_client WebSocket client fixture. Use it consistently for all tests. --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --- supervisor/core.py | 41 ++++++++++++++------------- tests/api/middleware/test_security.py | 4 +-- tests/conftest.py | 26 +++++++++++------ tests/homeassistant/test_websocket.py | 30 ++++++++++---------- tests/jobs/test_job_decorator.py | 11 +++---- tests/jobs/test_job_manager.py | 16 +++++------ tests/test_core.py | 30 +++++++++++--------- tests/test_coresys.py | 2 +- 8 files changed, 86 insertions(+), 74 deletions(-) diff --git a/supervisor/core.py b/supervisor/core.py index 0602e831d..211cedfc5 100644 --- a/supervisor/core.py +++ b/supervisor/core.py @@ -38,7 +38,8 @@ class Core(CoreSysAttributes): def __init__(self, coresys: CoreSys): """Initialize Supervisor object.""" self.coresys: CoreSys = coresys - self._state: CoreState | None = None + self._state: CoreState = CoreState.INITIALIZE + self._write_run_state(self._state) self.exit_code: int = 0 @property @@ -56,34 +57,36 @@ class Core(CoreSysAttributes): """Return true if the installation is healthy.""" return len(self.sys_resolution.unhealthy) == 0 + def _write_run_state(self, new_state: CoreState): + """Write run state for s6 service supervisor.""" + try: + RUN_SUPERVISOR_STATE.write_text(str(new_state), encoding="utf-8") + except OSError as err: + _LOGGER.warning( + "Can't update the Supervisor state to %s: %s", new_state, err + ) + @state.setter def state(self, new_state: CoreState) -> None: """Set core into new state.""" if self._state == new_state: return - try: - RUN_SUPERVISOR_STATE.write_text(new_state, encoding="utf-8") - except OSError as err: - _LOGGER.warning( - "Can't update the Supervisor state to %s: %s", new_state, err - ) - finally: - self._state = new_state - # Don't attempt to notify anyone on CLOSE as we're about to stop the event loop - if new_state != CoreState.CLOSE: - self.sys_bus.fire_event(BusEvent.SUPERVISOR_STATE_CHANGE, new_state) + self._write_run_state(new_state) + self._state = new_state - # These will be received by HA after startup has completed which won't make sense - if new_state not in STARTING_STATES: - self.sys_homeassistant.websocket.supervisor_update_event( - "info", {"state": new_state} - ) + # Don't attempt to notify anyone on CLOSE as we're about to stop the event loop + if new_state != CoreState.CLOSE: + self.sys_bus.fire_event(BusEvent.SUPERVISOR_STATE_CHANGE, new_state) + + # These will be received by HA after startup has completed which won't make sense + if new_state not in STARTING_STATES: + self.sys_homeassistant.websocket.supervisor_update_event( + "info", {"state": new_state} + ) async def connect(self): """Connect Supervisor container.""" - self.state = CoreState.INITIALIZE - # Load information from container await self.sys_supervisor.load() diff --git a/tests/api/middleware/test_security.py b/tests/api/middleware/test_security.py index 1de2a3fd3..032bbf16c 100644 --- a/tests/api/middleware/test_security.py +++ b/tests/api/middleware/test_security.py @@ -23,7 +23,7 @@ async def mock_handler(request): @pytest.fixture -async def api_system(aiohttp_client, run_dir, coresys: CoreSys) -> TestClient: +async def api_system(aiohttp_client, coresys: CoreSys) -> TestClient: """Fixture for RestAPI client.""" api = RestAPI(coresys) api.webapp = web.Application() @@ -39,7 +39,7 @@ async def api_system(aiohttp_client, run_dir, coresys: CoreSys) -> TestClient: @pytest.fixture -async def api_token_validation(aiohttp_client, run_dir, coresys: CoreSys) -> TestClient: +async def api_token_validation(aiohttp_client, coresys: CoreSys) -> TestClient: """Fixture for RestAPI client with token validation middleware.""" api = RestAPI(coresys) api.webapp = web.Application() diff --git a/tests/conftest.py b/tests/conftest.py index 065158d63..42a54306a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -39,6 +39,7 @@ from supervisor.const import ( ATTR_TYPE, ATTR_VERSION, REQUEST_FROM, + CoreState, ) from supervisor.coresys import CoreSys from supervisor.dbus.network import NetworkManager @@ -307,7 +308,7 @@ async def coresys( dbus_session_bus, all_dbus_services, aiohttp_client, - run_dir, + run_supervisor_state, supervisor_name, ) -> CoreSys: """Create a CoreSys Mock.""" @@ -330,13 +331,17 @@ async def coresys( # Mock test client coresys_obj._supervisor.instance._meta = { - "Config": {"Labels": {"io.hass.arch": "amd64"}} + "Config": {"Labels": {"io.hass.arch": "amd64"}}, + "HostConfig": {"Privileged": True}, } coresys_obj.arch._default_arch = "amd64" coresys_obj.arch._supported_set = {"amd64"} coresys_obj._machine = "qemux86-64" coresys_obj._machine_id = uuid4() + # Load resolution center + await coresys_obj.resolution.load() + # Mock host communication with ( patch("supervisor.dbus.manager.MessageBus") as message_bus, @@ -384,9 +389,14 @@ async def coresys( @pytest.fixture -def ha_ws_client(coresys: CoreSys) -> AsyncMock: +async def ha_ws_client(coresys: CoreSys) -> AsyncMock: """Return HA WS client mock for assertions.""" - return coresys.homeassistant.websocket._client + # Set Supervisor Core state to RUNNING, otherwise WS events won't be delivered + coresys.core.state = CoreState.RUNNING + await asyncio.sleep(0) + client = coresys.homeassistant.websocket._client + client.async_send_command.reset_mock() + return client @pytest.fixture @@ -494,12 +504,10 @@ def store_manager(coresys: CoreSys): @pytest.fixture -def run_dir(tmp_path): - """Fixture to inject hassio env.""" +def run_supervisor_state() -> Generator[MagicMock]: + """Fixture to simulate Supervisor state file in /run/supervisor.""" with patch("supervisor.core.RUN_SUPERVISOR_STATE") as mock_run: - tmp_state = Path(tmp_path, "supervisor") - mock_run.write_text = tmp_state.write_text - yield tmp_state + yield mock_run @pytest.fixture diff --git a/tests/homeassistant/test_websocket.py b/tests/homeassistant/test_websocket.py index e44ba366b..06fa18c2e 100644 --- a/tests/homeassistant/test_websocket.py +++ b/tests/homeassistant/test_websocket.py @@ -1,8 +1,9 @@ """Test websocket.""" -# pylint: disable=protected-access, import-error +# pylint: disable=import-error import asyncio import logging +from unittest.mock import AsyncMock from awesomeversion import AwesomeVersion @@ -11,16 +12,15 @@ from supervisor.coresys import CoreSys from supervisor.homeassistant.const import WSEvent, WSType -async def test_send_command(coresys: CoreSys): +async def test_send_command(coresys: CoreSys, ha_ws_client: AsyncMock): """Test websocket error on listen.""" - client = coresys.homeassistant.websocket._client await coresys.homeassistant.websocket.async_send_command({"type": "test"}) - client.async_send_command.assert_called_with({"type": "test"}) + ha_ws_client.async_send_command.assert_called_with({"type": "test"}) await coresys.homeassistant.websocket.async_supervisor_update_event( "test", {"lorem": "ipsum"} ) - client.async_send_command.assert_called_with( + ha_ws_client.async_send_command.assert_called_with( { "type": WSType.SUPERVISOR_EVENT, "data": { @@ -32,11 +32,12 @@ async def test_send_command(coresys: CoreSys): ) -async def test_send_command_old_core_version(coresys: CoreSys, caplog): +async def test_send_command_old_core_version( + coresys: CoreSys, ha_ws_client: AsyncMock, caplog +): """Test websocket error on listen.""" caplog.set_level(logging.INFO) - client = coresys.homeassistant.websocket._client - client.ha_version = AwesomeVersion("1970.1.1") + ha_ws_client.ha_version = AwesomeVersion("1970.1.1") await coresys.homeassistant.websocket.async_send_command( {"type": "supervisor/event"} @@ -50,25 +51,24 @@ async def test_send_command_old_core_version(coresys: CoreSys, caplog): await coresys.homeassistant.websocket.async_supervisor_update_event( "test", {"lorem": "ipsum"} ) - client.async_send_command.assert_not_called() + ha_ws_client.async_send_command.assert_not_called() -async def test_send_message_during_startup(coresys: CoreSys): +async def test_send_message_during_startup(coresys: CoreSys, ha_ws_client: AsyncMock): """Test websocket messages queue during startup.""" - client = coresys.homeassistant.websocket._client await coresys.homeassistant.websocket.load() coresys.core.state = CoreState.SETUP await coresys.homeassistant.websocket.async_supervisor_update_event( "test", {"lorem": "ipsum"} ) - client.async_send_command.assert_not_called() + ha_ws_client.async_send_command.assert_not_called() coresys.core.state = CoreState.RUNNING await asyncio.sleep(0) - assert client.async_send_command.call_count == 2 - assert client.async_send_command.call_args_list[0][0][0] == { + assert ha_ws_client.async_send_command.call_count == 2 + assert ha_ws_client.async_send_command.call_args_list[0][0][0] == { "type": WSType.SUPERVISOR_EVENT, "data": { "event": WSEvent.SUPERVISOR_UPDATE, @@ -76,7 +76,7 @@ async def test_send_message_during_startup(coresys: CoreSys): "data": {"lorem": "ipsum"}, }, } - assert client.async_send_command.call_args_list[1][0][0] == { + assert ha_ws_client.async_send_command.call_args_list[1][0][0] == { "type": WSType.SUPERVISOR_EVENT, "data": { "event": WSEvent.SUPERVISOR_UPDATE, diff --git a/tests/jobs/test_job_decorator.py b/tests/jobs/test_job_decorator.py index 986234bc7..8e863c95c 100644 --- a/tests/jobs/test_job_decorator.py +++ b/tests/jobs/test_job_decorator.py @@ -951,7 +951,7 @@ async def test_execution_limit_group_throttle_rate_limit( assert test2.call == 3 -async def test_internal_jobs_no_notify(coresys: CoreSys): +async def test_internal_jobs_no_notify(coresys: CoreSys, ha_ws_client: AsyncMock): """Test internal jobs do not send any notifications.""" class TestClass: @@ -972,18 +972,15 @@ async def test_internal_jobs_no_notify(coresys: CoreSys): return True test1 = TestClass(coresys) - # pylint: disable-next=protected-access - client = coresys.homeassistant.websocket._client - client.async_send_command.reset_mock() await test1.execute_internal() await asyncio.sleep(0) - client.async_send_command.assert_not_called() + ha_ws_client.async_send_command.assert_not_called() await test1.execute_default() await asyncio.sleep(0) - assert client.async_send_command.call_count == 2 - client.async_send_command.assert_called_with( + assert ha_ws_client.async_send_command.call_count == 2 + ha_ws_client.async_send_command.assert_called_with( { "type": "supervisor/event", "data": { diff --git a/tests/jobs/test_job_manager.py b/tests/jobs/test_job_manager.py index 831e18443..4c584b5ce 100644 --- a/tests/jobs/test_job_manager.py +++ b/tests/jobs/test_job_manager.py @@ -1,7 +1,7 @@ """Test the condition decorators.""" import asyncio -from unittest.mock import ANY +from unittest.mock import ANY, AsyncMock import pytest @@ -83,14 +83,14 @@ async def test_update_job(coresys: CoreSys): job.progress = -10 -async def test_notify_on_change(coresys: CoreSys): +async def test_notify_on_change(coresys: CoreSys, ha_ws_client: AsyncMock): """Test jobs notify Home Assistant on changes.""" job = coresys.jobs.new_job(TEST_JOB) job.progress = 50 await asyncio.sleep(0) # pylint: disable=protected-access - coresys.homeassistant.websocket._client.async_send_command.assert_called_with( + ha_ws_client.async_send_command.assert_called_with( { "type": "supervisor/event", "data": { @@ -112,7 +112,7 @@ async def test_notify_on_change(coresys: CoreSys): job.stage = "test" await asyncio.sleep(0) - coresys.homeassistant.websocket._client.async_send_command.assert_called_with( + ha_ws_client.async_send_command.assert_called_with( { "type": "supervisor/event", "data": { @@ -134,7 +134,7 @@ async def test_notify_on_change(coresys: CoreSys): job.reference = "test" await asyncio.sleep(0) - coresys.homeassistant.websocket._client.async_send_command.assert_called_with( + ha_ws_client.async_send_command.assert_called_with( { "type": "supervisor/event", "data": { @@ -156,7 +156,7 @@ async def test_notify_on_change(coresys: CoreSys): with job.start(): await asyncio.sleep(0) - coresys.homeassistant.websocket._client.async_send_command.assert_called_with( + ha_ws_client.async_send_command.assert_called_with( { "type": "supervisor/event", "data": { @@ -178,7 +178,7 @@ async def test_notify_on_change(coresys: CoreSys): job.capture_error() await asyncio.sleep(0) - coresys.homeassistant.websocket._client.async_send_command.assert_called_with( + ha_ws_client.async_send_command.assert_called_with( { "type": "supervisor/event", "data": { @@ -204,7 +204,7 @@ async def test_notify_on_change(coresys: CoreSys): ) await asyncio.sleep(0) - coresys.homeassistant.websocket._client.async_send_command.assert_called_with( + ha_ws_client.async_send_command.assert_called_with( { "type": "supervisor/event", "data": { diff --git a/tests/test_core.py b/tests/test_core.py index 87985225a..05100a3bf 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -3,7 +3,7 @@ # pylint: disable=W0212 import datetime import errno -from unittest.mock import AsyncMock, PropertyMock, patch +from unittest.mock import AsyncMock, MagicMock, PropertyMock, patch from pytest import LogCaptureFixture @@ -16,15 +16,19 @@ from supervisor.supervisor import Supervisor from supervisor.utils.whoami import WhoamiData -def test_write_state(run_dir, coresys: CoreSys): +def test_write_state(run_supervisor_state, coresys: CoreSys): """Test write corestate to /run/supervisor.""" coresys.core.state = CoreState.RUNNING - assert run_dir.read_text() == CoreState.RUNNING + run_supervisor_state.write_text.assert_called_with( + str(CoreState.RUNNING), encoding="utf-8" + ) coresys.core.state = CoreState.SHUTDOWN - assert run_dir.read_text() == CoreState.SHUTDOWN + run_supervisor_state.write_text.assert_called_with( + str(CoreState.SHUTDOWN), encoding="utf-8" + ) async def test_adjust_system_datetime(coresys: CoreSys): @@ -83,14 +87,14 @@ async def test_adjust_system_datetime_if_time_behind(coresys: CoreSys): mock_check_connectivity.assert_called_once() -def test_write_state_failure(run_dir, coresys: CoreSys, caplog: LogCaptureFixture): +def test_write_state_failure( + run_supervisor_state: MagicMock, coresys: CoreSys, caplog: LogCaptureFixture +): """Test failure to write corestate to /run/supervisor.""" - with patch( - "supervisor.core.RUN_SUPERVISOR_STATE.write_text", - side_effect=(err := OSError()), - ): - err.errno = errno.EBADMSG - coresys.core.state = CoreState.RUNNING + err = OSError() + err.errno = errno.EBADMSG + run_supervisor_state.write_text.side_effect = err + coresys.core.state = CoreState.RUNNING - assert "Can't update the Supervisor state" in caplog.text - assert coresys.core.healthy is True + assert "Can't update the Supervisor state" in caplog.text + assert coresys.core.state == CoreState.RUNNING diff --git a/tests/test_coresys.py b/tests/test_coresys.py index 47d843f5d..cf3d1d5c7 100644 --- a/tests/test_coresys.py +++ b/tests/test_coresys.py @@ -9,7 +9,7 @@ from supervisor.dbus.timedate import TimeDate from supervisor.utils.dt import utcnow -async def test_timezone(run_dir, coresys: CoreSys): +async def test_timezone(coresys: CoreSys): """Test write corestate to /run/supervisor.""" # pylint: disable=protected-access coresys.host.sys_dbus._timedate = TimeDate()