mirror of
https://github.com/home-assistant/supervisor.git
synced 2026-07-03 03:45:45 +01:00
a973d22e35
* Derive App state from container state
The App.state setter mixed two responsibilities: it both mutated a
private `_state` field and dispatched side effects (WebSocket events,
issue dismissal, startup_event signaling). On top of that, an installed
but never-started app stayed in AppState.UNKNOWN forever, because the
attach() image-only fallback never fires a container state-change event
and the AppState therefore kept its constructor default. Conceptually,
ContainerState.UNKNOWN ("container does not exist") and AppState.UNKNOWN
("nothing observed yet") happened to share a name but meant different
things, which made the distinction easy to lose.
Make App.state a pure derived property. The source of truth is the last
observed ContainerState (cached on the App), plus a sticky operation-
error flag for start/stop failures that the docker event stream cannot
reflect. When no container has been observed yet, the derivation falls
back to install signals: an attached instance (image present) is
STOPPED, otherwise UNKNOWN. As a side effect, an installed-but-never-
started app now correctly reports STOPPED instead of UNKNOWN.
container_state_changed updates the cached container state and routes
all side effects through a single _emit_state_change helper that diffs
old vs new derived state. The two start/stop failure paths route
through _set_operation_error. Uninstall resets the cached signals so
the derivation naturally returns UNKNOWN.
Tests use a new tests/common.force_app_state helper that pokes the
underlying signals directly; the production class no longer carries
test-only setters.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Fix App state drive to AppState.UNKNOWN
* Unify state mutation through _update_state
Previously, state-driving signal changes were spread across two helpers
(_set_operation_error, _emit_state_change(old_state)) and required each
caller to capture self.state before mutating a private field — leaking
implementation details to call sites and raising the "why am I emitting
the old state?" question pointed out in code review.
Replace both helpers with a single _update_state(*, container_state=,
operation_error=) entry point. Callers describe what changed via
keyword arguments (None leaves a signal untouched); the helper captures
the previous state, applies the updates, recomputes the derived state
and emits side effects if anything changed.
Diff against a tracked _last_state instead of a freshly derived
"current" state, so that an out-of-band mutation between updates does
not silently shift the comparison baseline. The concrete case is
App.uninstall: instance.remove() clears the docker meta mid-flow, which
would otherwise reshape the derivation (RUNNING with no healthcheck
becomes STARTED instead of STARTUP) and suppress the STARTUP transition
that resolves the start-wait task. As a side effect, the initial
UNKNOWN -> STOPPED transition on attach is also now reliably emitted.
Switch the uninstall path to ContainerState.UNKNOWN ("we know there is
no container") rather than the constructor sentinel None.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Cache app state instead of deriving on every read
Building on the previous commit, make App.state a plain read of a
cached _state field rather than re-deriving on every property access.
The derivation moves to _derive_state(), and _update_state() is the
sole place that recomputes and assigns _state, so the value consumers
read always matches what was last emitted to listeners.
This removes the _last_state bookkeeping introduced previously: with a
single cached value there is no longer a separate "derived now" vs
"last emitted" distinction to reconcile, and out-of-band mutations
(e.g. instance.remove() clearing _meta during uninstall) can no longer
silently shift what state returns between updates.
Call _update_state() at the end of load() so the cached state settles
once attach() has run. Image-only attaches do not fire a docker event,
so without this an installed app would stay in the constructor-default
UNKNOWN until first start; this also makes the initial transition on
attach observable to listeners.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Pass operation error to _derive_state instead of storing it
The two state-driving signals were not symmetric. _container_state is
genuinely persisted state ("the last thing docker told us") that
re-derivation legitimately reads across calls. _operation_error, on the
other hand, is a momentary "force ERROR for this transition" signal; the
persistence of an error condition already lives in the cached _state.
Storing it as an instance attribute implied a sticky cross-call behavior
that no call path actually exercised: every caller either set it
explicitly right before deriving (start/stop failures, container events)
or ran argless only at load time, where no failure has occurred.
Drop the _operation_error field and pass operation_error as a parameter
to _derive_state(), defaulting to False in _update_state(). A container
observation now supersedes a prior error implicitly via the default,
which lets the container-event and uninstall call sites drop their
explicit operation_error=False.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* Settle load state synchronously from current_state
The argless _update_state() settle at the end of load() raced attach()'s
container-state event. attach() fires DOCKER_CONTAINER_STATE_CHANGE via
the bus, which schedules the container_state_changed listener as a task
rather than running it inline. In the deprecated-arch early-return path
there is no await between attach() and the settle, so the listener had
not run yet: _container_state was still None and the settle derived
STOPPED (instance attached) — emitting a transient UNKNOWN->STOPPED even
for a running container before the listener corrected it. The main path
only avoided this incidentally, by having awaits (check_image,
save_persist) in between for the listener to run.
Derive the load-time state synchronously from instance.current_state()
instead of relying on the asynchronously delivered event. current_state()
returns the real container state, or UNKNOWN when only an image is
present (which derives to STOPPED), so both paths settle correctly
without racing the event.
Add a regression test that loading a running container settles to
STARTED, and mock current_state() in the state-listener test which
relies on a clean UNKNOWN baseline.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
202 lines
6.6 KiB
Python
202 lines
6.6 KiB
Python
"""Test discovery API."""
|
|
|
|
import logging
|
|
from unittest.mock import ANY, AsyncMock, MagicMock, patch
|
|
|
|
from aiohttp.test_utils import TestClient
|
|
import pytest
|
|
|
|
from supervisor.apps.app import App
|
|
from supervisor.const import AppState
|
|
from supervisor.coresys import CoreSys
|
|
from supervisor.discovery import Message
|
|
|
|
from tests.common import force_app_state, load_json_fixture
|
|
|
|
|
|
async def test_api_discovery_forbidden(
|
|
app_api_client_with_prefix: tuple[TestClient, str],
|
|
caplog: pytest.LogCaptureFixture,
|
|
):
|
|
"""Test app sending discovery message for an unregistered service."""
|
|
api_client, prefix = app_api_client_with_prefix
|
|
caplog.clear()
|
|
|
|
with caplog.at_level(logging.ERROR):
|
|
resp = await api_client.post(
|
|
f"{prefix}/discovery", json={"service": "mqtt", "config": {}}
|
|
)
|
|
|
|
assert resp.status == 403
|
|
result = await resp.json()
|
|
assert result["result"] == "error"
|
|
assert (
|
|
result["message"]
|
|
== "Apps must list services they provide via discovery in their config!"
|
|
)
|
|
assert "Please report this to the maintainer of the app" in caplog.text
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"skip_state", [AppState.ERROR, AppState.STOPPED, AppState.STARTUP]
|
|
)
|
|
async def test_api_list_discovery(
|
|
api_client_with_prefix: tuple[TestClient, str],
|
|
coresys: CoreSys,
|
|
install_app_ssh: App,
|
|
skip_state: AppState,
|
|
):
|
|
"""Test listing discovery messages only returns ones for healthy services."""
|
|
api_client, prefix = api_client_with_prefix
|
|
with (
|
|
patch(
|
|
"supervisor.utils.common.read_json_or_yaml_file",
|
|
return_value=load_json_fixture("discovery.json"),
|
|
),
|
|
patch("supervisor.utils.common.Path.is_file", return_value=True),
|
|
):
|
|
await coresys.discovery.read_data()
|
|
|
|
await coresys.discovery.load()
|
|
assert coresys.discovery.list_messages == [
|
|
Message(app="core_mosquitto", service="mqtt", config=ANY, uuid=ANY),
|
|
Message(app="local_ssh", service="adguard", config=ANY, uuid=ANY),
|
|
]
|
|
|
|
force_app_state(install_app_ssh, AppState.STARTED)
|
|
resp = await api_client.get(f"{prefix}/discovery")
|
|
assert resp.status == 200
|
|
result = await resp.json()
|
|
app_key = "app" if prefix == "/v2" else "addon"
|
|
assert result["data"]["discovery"] == [
|
|
{
|
|
app_key: "local_ssh",
|
|
"service": "adguard",
|
|
"config": ANY,
|
|
"uuid": ANY,
|
|
}
|
|
]
|
|
|
|
force_app_state(install_app_ssh, skip_state)
|
|
resp = await api_client.get(f"{prefix}/discovery")
|
|
assert resp.status == 200
|
|
result = await resp.json()
|
|
assert result["data"]["discovery"] == []
|
|
|
|
|
|
async def test_api_send_del_discovery(
|
|
app_api_client_with_prefix: tuple[TestClient, str],
|
|
coresys: CoreSys,
|
|
install_app_ssh: App,
|
|
websession: MagicMock,
|
|
):
|
|
"""Test adding and removing discovery."""
|
|
api_client, prefix = app_api_client_with_prefix
|
|
install_app_ssh.data["discovery"] = ["test"]
|
|
coresys.homeassistant.api._ensure_access_token = AsyncMock() # pylint: disable=protected-access
|
|
|
|
resp = await api_client.post(
|
|
f"{prefix}/discovery", json={"service": "test", "config": {}}
|
|
)
|
|
assert resp.status == 200
|
|
result = await resp.json()
|
|
uuid = result["data"]["uuid"]
|
|
coresys.websession.request.assert_called_once()
|
|
assert coresys.websession.request.call_args.args[0] == "post"
|
|
assert (
|
|
coresys.websession.request.call_args.args[1]
|
|
== f"http://172.30.32.1:8123/api/hassio_push/discovery/{uuid}"
|
|
)
|
|
assert coresys.websession.request.call_args.kwargs["json"] == {
|
|
"addon": install_app_ssh.slug,
|
|
"service": "test",
|
|
"uuid": uuid,
|
|
}
|
|
|
|
message = coresys.discovery.get(uuid)
|
|
assert message.app == install_app_ssh.slug
|
|
assert message.service == "test"
|
|
assert message.config == {}
|
|
|
|
coresys.websession.request.reset_mock()
|
|
resp = await api_client.delete(f"{prefix}/discovery/{uuid}")
|
|
assert resp.status == 200
|
|
coresys.websession.request.assert_called_once()
|
|
assert coresys.websession.request.call_args.args[0] == "delete"
|
|
assert (
|
|
coresys.websession.request.call_args.args[1]
|
|
== f"http://172.30.32.1:8123/api/hassio_push/discovery/{uuid}"
|
|
)
|
|
assert coresys.websession.request.call_args.kwargs["json"] == {
|
|
"addon": install_app_ssh.slug,
|
|
"service": "test",
|
|
"uuid": uuid,
|
|
}
|
|
|
|
assert coresys.discovery.get(uuid) is None
|
|
|
|
|
|
async def test_api_invalid_discovery(
|
|
app_api_client_with_prefix: tuple[TestClient, str],
|
|
install_app_ssh: App,
|
|
):
|
|
"""Test invalid discovery messages."""
|
|
api_client, prefix = app_api_client_with_prefix
|
|
install_app_ssh.data["discovery"] = ["test"]
|
|
|
|
resp = await api_client.post(f"{prefix}/discovery", json={"service": "test"})
|
|
assert resp.status == 400
|
|
|
|
resp = await api_client.post(
|
|
f"{prefix}/discovery", json={"service": "test", "config": None}
|
|
)
|
|
assert resp.status == 400
|
|
|
|
|
|
async def test_discovery_not_found_get(
|
|
api_client_with_prefix: tuple[TestClient, str],
|
|
):
|
|
"""Test GET /discovery/{uuid} returns 404 for an unknown uuid."""
|
|
api_client, prefix = api_client_with_prefix
|
|
resp = await api_client.get(f"{prefix}/discovery/bad")
|
|
assert resp.status == 404
|
|
body = await resp.json()
|
|
assert body["message"] == "Discovery message not found"
|
|
|
|
|
|
async def test_discovery_not_found_delete(
|
|
app_api_client_with_prefix: tuple[TestClient, str],
|
|
):
|
|
"""Test DELETE /discovery/{uuid} returns 404 for an unknown uuid."""
|
|
api_client, prefix = app_api_client_with_prefix
|
|
resp = await api_client.delete(f"{prefix}/discovery/bad")
|
|
assert resp.status == 404
|
|
body = await resp.json()
|
|
assert body["message"] == "Discovery message not found"
|
|
|
|
|
|
async def test_get_discovery_v1_v2_keys(
|
|
api_client_with_prefix: tuple[TestClient, str],
|
|
coresys: CoreSys,
|
|
install_app_ssh: App,
|
|
):
|
|
"""Test GET /discovery/{uuid} returns 'addon' key on V1 and 'app' key on V2."""
|
|
api_client, prefix = api_client_with_prefix
|
|
|
|
# Seed a discovery message directly (bypass the HA push)
|
|
message = await coresys.discovery.send(
|
|
install_app_ssh, "adguard", {"host": "127.0.0.1", "port": 3000}
|
|
)
|
|
uuid = message.uuid
|
|
|
|
resp = await api_client.get(f"{prefix}/discovery/{uuid}")
|
|
assert resp.status == 200
|
|
result = await resp.json()
|
|
|
|
app_key = "app" if prefix == "/v2" else "addon"
|
|
absent_key = "addon" if prefix == "/v2" else "app"
|
|
assert result["data"][app_key] == install_app_ssh.slug
|
|
assert absent_key not in result["data"]
|
|
assert result["data"]["service"] == "adguard"
|
|
assert result["data"]["uuid"] == uuid
|