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

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

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

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

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,

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": {

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": {

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

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