fix(update): kein Downgrade und keine parallelen Installs pro Lampe
This commit is contained in:
@@ -74,6 +74,7 @@ class NinlightUpdateEntity(NinlightEntity, UpdateEntity):
|
|||||||
self._offer: FirmwareImage | None = None
|
self._offer: FirmwareImage | None = None
|
||||||
self._evaluated_for: tuple[str, str, str] | None = None
|
self._evaluated_for: tuple[str, str, str] | None = None
|
||||||
self._eval_lock = asyncio.Lock()
|
self._eval_lock = asyncio.Lock()
|
||||||
|
self._install_lock = asyncio.Lock()
|
||||||
self._attr_in_progress = False
|
self._attr_in_progress = False
|
||||||
|
|
||||||
@property
|
@property
|
||||||
@@ -168,6 +169,16 @@ class NinlightUpdateEntity(NinlightEntity, UpdateEntity):
|
|||||||
)
|
)
|
||||||
|
|
||||||
async def async_install(self, version: str | None, backup: bool, **kwargs: Any) -> None:
|
async def async_install(self, version: str | None, backup: bool, **kwargs: Any) -> None:
|
||||||
|
# Only the call that owns this lock may touch in_progress/ota_in_progress below: without
|
||||||
|
# this check, a second concurrent call would sail through HA's own in-progress guard (it
|
||||||
|
# only rejects installed == latest) and its `finally` would clear the flags out from under
|
||||||
|
# the first call while that one is still flashing the lamp.
|
||||||
|
if self._install_lock.locked():
|
||||||
|
raise _error("update_busy")
|
||||||
|
async with self._install_lock:
|
||||||
|
await self._async_install_locked()
|
||||||
|
|
||||||
|
async def _async_install_locked(self) -> None:
|
||||||
shared = self.hass.data[DATA_SHARED]
|
shared = self.hass.data[DATA_SHARED]
|
||||||
image = self._offer
|
image = self._offer
|
||||||
if image is None:
|
if image is None:
|
||||||
@@ -176,8 +187,12 @@ class NinlightUpdateEntity(NinlightEntity, UpdateEntity):
|
|||||||
if not await self.hass.async_add_executor_job(shared.catalog.verify, image):
|
if not await self.hass.async_add_executor_job(shared.catalog.verify, image):
|
||||||
raise _error("image_invalid")
|
raise _error("image_invalid")
|
||||||
|
|
||||||
# Re-check right before sending: settings may have changed since the offer.
|
# Re-check right before sending: the lamp's firmware may have changed since the offer was
|
||||||
|
# computed (e.g. a stale offer surviving a failed evaluation - see _async_evaluate) and
|
||||||
|
# must never be flashed if it is no longer actually newer than what is running now.
|
||||||
info = await self.coordinator.async_call(lambda c: c.get_info())
|
info = await self.coordinator.async_call(lambda c: c.get_info())
|
||||||
|
if not _firmware_is_newer(image.version, str(info.get("fw_version"))):
|
||||||
|
raise _error("no_update")
|
||||||
settings = await self.coordinator.async_call(lambda c: c.get_settings())
|
settings = await self.coordinator.async_call(lambda c: c.get_settings())
|
||||||
if info.get("device_type") != image.device_type:
|
if info.get("device_type") != image.device_type:
|
||||||
raise _error("image_invalid")
|
raise _error("image_invalid")
|
||||||
|
|||||||
@@ -2,6 +2,8 @@
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import asyncio
|
||||||
|
from copy import deepcopy
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import Any
|
from typing import Any
|
||||||
from unittest.mock import MagicMock
|
from unittest.mock import MagicMock
|
||||||
@@ -38,6 +40,7 @@ BASE = "http://192.168.1.50:8123"
|
|||||||
INSTALLED = "0.3.8-36-gca9bb"
|
INSTALLED = "0.3.8-36-gca9bb"
|
||||||
NEWER = "0.3.8-47-g71a57"
|
NEWER = "0.3.8-47-g71a57"
|
||||||
NEWEST = "0.3.9-rc1"
|
NEWEST = "0.3.9-rc1"
|
||||||
|
EVEN_NEWER = "0.3.9-rc2" # newer than NEWEST
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
@@ -328,6 +331,98 @@ async def test_install_rejects_changed_file(
|
|||||||
mock_client.fw_update.assert_not_awaited()
|
mock_client.fw_update.assert_not_awaited()
|
||||||
|
|
||||||
|
|
||||||
|
async def test_install_rejects_downgrade(
|
||||||
|
hass: HomeAssistant,
|
||||||
|
init_integration,
|
||||||
|
mock_client: MagicMock,
|
||||||
|
lamp_info: dict[str, Any],
|
||||||
|
firmware_dir: Path,
|
||||||
|
) -> None:
|
||||||
|
"""The lamp may have been reflashed by another route since the offer was computed."""
|
||||||
|
put(firmware_dir, "a.img", version=NEWER, content=CONTENT_WITH_SSID)
|
||||||
|
await rescan(hass)
|
||||||
|
entity_id = entity_id_for(hass, "update", "firmware")
|
||||||
|
assert hass.states.get(entity_id).attributes[ATTR_LATEST_VERSION] == NEWER
|
||||||
|
|
||||||
|
# A newer image also appears on disk, and the lamp is reflashed by another route to a version
|
||||||
|
# newer than our stale offer - but the follow-up evaluation (triggered by the catalog scan)
|
||||||
|
# fails at get_settings and returns early, so the stale NEWER offer survives untouched.
|
||||||
|
put(firmware_dir, "b.img", version=EVEN_NEWER, content=CONTENT_WITH_SSID)
|
||||||
|
mock_client.get_settings.side_effect = NinlightConnectionError("down")
|
||||||
|
lamp_info["fw_version"] = NEWEST
|
||||||
|
await rescan(hass)
|
||||||
|
assert hass.states.get(entity_id).attributes[ATTR_LATEST_VERSION] == NEWER
|
||||||
|
|
||||||
|
with pytest.raises(HomeAssistantError) as raised:
|
||||||
|
await install(hass)
|
||||||
|
assert raised.value.translation_key == "no_update"
|
||||||
|
mock_client.fw_update.assert_not_awaited()
|
||||||
|
|
||||||
|
|
||||||
|
async def test_install_rejects_concurrent_calls(
|
||||||
|
hass: HomeAssistant,
|
||||||
|
init_integration,
|
||||||
|
mock_client: MagicMock,
|
||||||
|
lamp_info: dict[str, Any],
|
||||||
|
firmware_dir: Path,
|
||||||
|
) -> None:
|
||||||
|
"""A second call arriving in the pre-flag window (before in_progress is set) must be
|
||||||
|
rejected by our own lock rather than allowed to touch OTA state.
|
||||||
|
|
||||||
|
HA's own service-level "already in progress" guard only helps once `_attr_in_progress` is
|
||||||
|
already True (it is a `cached_property` that HA invalidates whenever `_attr_in_progress` is
|
||||||
|
set, so it is never stale) - by the time that guard would fire, a second concurrent call is
|
||||||
|
already blocked by HA itself with translation_key "update_in_progress", not "update_busy".
|
||||||
|
The real gap I2 describes is *before* that: `verify`, `get_info` and `get_settings` are all
|
||||||
|
awaited before we ever set the flags, and HA's guard does not protect that window. This test
|
||||||
|
blocks the first `get_info()` re-check inside that window to reach it deterministically.
|
||||||
|
"""
|
||||||
|
put(firmware_dir, "a.img", version=NEWER, content=CONTENT_WITH_SSID)
|
||||||
|
await rescan(hass)
|
||||||
|
entity_id = entity_id_for(hass, "update", "firmware")
|
||||||
|
|
||||||
|
entered = asyncio.Event()
|
||||||
|
release = asyncio.Event()
|
||||||
|
first_call = True
|
||||||
|
|
||||||
|
async def blocking_get_info() -> dict[str, Any]:
|
||||||
|
nonlocal first_call
|
||||||
|
if first_call:
|
||||||
|
first_call = False
|
||||||
|
entered.set()
|
||||||
|
await release.wait()
|
||||||
|
return deepcopy(lamp_info)
|
||||||
|
|
||||||
|
async def lamp_flashes(url: str) -> None:
|
||||||
|
lamp_info["fw_version"] = NEWER
|
||||||
|
lamp_info["uptime"] = 4
|
||||||
|
|
||||||
|
mock_client.get_info.side_effect = blocking_get_info
|
||||||
|
mock_client.fw_update.side_effect = lamp_flashes
|
||||||
|
|
||||||
|
task1 = hass.async_create_task(install(hass), name="install #1")
|
||||||
|
async with asyncio.timeout(5):
|
||||||
|
await entered.wait()
|
||||||
|
|
||||||
|
# Install #1 is blocked inside its pre-flag get_info() re-check: the lock is held, but
|
||||||
|
# in_progress/ota_in_progress are still untouched.
|
||||||
|
assert init_integration.runtime_data.ota_in_progress is False
|
||||||
|
assert hass.states.get(entity_id).attributes[ATTR_IN_PROGRESS] is False
|
||||||
|
|
||||||
|
with pytest.raises(HomeAssistantError) as raised:
|
||||||
|
await install(hass)
|
||||||
|
assert raised.value.translation_key == "update_busy"
|
||||||
|
assert init_integration.runtime_data.ota_in_progress is False
|
||||||
|
assert hass.states.get(entity_id).attributes[ATTR_IN_PROGRESS] is False
|
||||||
|
|
||||||
|
release.set()
|
||||||
|
async with asyncio.timeout(5):
|
||||||
|
await task1
|
||||||
|
assert init_integration.runtime_data.ota_in_progress is False
|
||||||
|
assert hass.states.get(entity_id).attributes[ATTR_IN_PROGRESS] is False
|
||||||
|
assert hass.states.get(entity_id).attributes[ATTR_INSTALLED_VERSION] == NEWER
|
||||||
|
|
||||||
|
|
||||||
async def test_removing_the_entry_clears_the_wifi_issue(
|
async def test_removing_the_entry_clears_the_wifi_issue(
|
||||||
hass: HomeAssistant, init_integration, firmware_dir: Path
|
hass: HomeAssistant, init_integration, firmware_dir: Path
|
||||||
) -> None:
|
) -> None:
|
||||||
|
|||||||
Reference in New Issue
Block a user