diff --git a/supervisor/addons/__init__.py b/supervisor/addons/__init__.py index 71b822789..4c615e69b 100644 --- a/supervisor/addons/__init__.py +++ b/supervisor/addons/__init__.py @@ -290,6 +290,8 @@ class AddonManager(CoreSysAttributes): # Update instance last_state: AddonState = addon.state old_image = addon.image + # Cache data to prevent races with other updates to global + store = store.clone() try: await addon.instance.update(store.version, store.image) except DockerError as err: @@ -300,7 +302,9 @@ class AddonManager(CoreSysAttributes): # Cleanup with suppress(DockerError): - await addon.instance.cleanup(old_image=old_image) + await addon.instance.cleanup( + old_image=old_image, image=store.image, version=store.version + ) # Setup/Fix AppArmor profile await addon.install_apparmor() diff --git a/supervisor/docker/interface.py b/supervisor/docker/interface.py index 406935ff5..af63b6d35 100644 --- a/supervisor/docker/interface.py +++ b/supervisor/docker/interface.py @@ -449,12 +449,17 @@ class DockerInterface(JobGroup): return b"" @Job(name="docker_interface_cleanup", limit=JobExecutionLimit.GROUP_WAIT) - def cleanup(self, old_image: str | None = None) -> Awaitable[None]: + def cleanup( + self, + old_image: str | None = None, + image: str | None = None, + version: AwesomeVersion | None = None, + ) -> Awaitable[None]: """Check if old version exists and cleanup.""" return self.sys_run_in_executor( self.sys_docker.cleanup_old_images, - self.image, - self.version, + image or self.image, + version or self.version, {old_image} if old_image else None, ) diff --git a/supervisor/store/addon.py b/supervisor/store/addon.py index cfa7f9a65..ea9d070e7 100644 --- a/supervisor/store/addon.py +++ b/supervisor/store/addon.py @@ -1,7 +1,11 @@ """Init file for Supervisor add-ons.""" + +from copy import deepcopy import logging +from typing import Self from ..addons.model import AddonModel, Data +from ..coresys import CoreSys _LOGGER: logging.Logger = logging.getLogger(__name__) @@ -9,6 +13,11 @@ _LOGGER: logging.Logger = logging.getLogger(__name__) class AddonStore(AddonModel): """Hold data for add-on inside Supervisor.""" + def __init__(self, coresys: CoreSys, slug: str, data: Data | None = None): + """Initialize object.""" + super().__init__(coresys, slug) + self._data: Data | None = data + def __repr__(self) -> str: """Return internal representation.""" return f"" @@ -16,7 +25,7 @@ class AddonStore(AddonModel): @property def data(self) -> Data: """Return add-on data/config.""" - return self.sys_store.data.addons[self.slug] + return self._data or self.sys_store.data.addons[self.slug] @property def is_installed(self) -> bool: @@ -27,3 +36,7 @@ class AddonStore(AddonModel): def is_detached(self) -> bool: """Return True if add-on is detached.""" return False + + def clone(self) -> Self: + """Return a copy that includes data and does not use global store data.""" + return type(self)(self.coresys, self.slug, deepcopy(self.data)) diff --git a/tests/addons/test_manager.py b/tests/addons/test_manager.py index c95246a51..2dbf30849 100644 --- a/tests/addons/test_manager.py +++ b/tests/addons/test_manager.py @@ -15,6 +15,7 @@ from supervisor.coresys import CoreSys from supervisor.docker.addon import DockerAddon from supervisor.docker.const import ContainerState from supervisor.docker.interface import DockerInterface +from supervisor.docker.manager import DockerAPI from supervisor.docker.monitor import DockerContainerStateEvent from supervisor.exceptions import ( AddonConfigurationError, @@ -23,6 +24,7 @@ from supervisor.exceptions import ( DockerNotFound, ) from supervisor.plugins.dns import PluginDns +from supervisor.store.repository import Repository from supervisor.utils import check_exception_chain from supervisor.utils.common import write_json_file @@ -364,3 +366,41 @@ async def test_repository_file_error( write_json_file(repo_file, {"invalid": "bad"}) await coresys.store.data.update() assert f"Repository parse error {repo_dir.as_posix()}" in caplog.text + + +async def test_store_data_changes_during_update( + coresys: CoreSys, install_addon_ssh: Addon +): + """Test store data changing for an addon during an update does not cause errors.""" + event = asyncio.Event() + coresys.store.data.addons["local_ssh"]["image"] = "test_image" + coresys.store.data.addons["local_ssh"]["version"] = AwesomeVersion("1.1.1") + + async def simulate_update(): + async def mock_update(_, version, image, *args, **kwargs): + assert version == AwesomeVersion("1.1.1") + assert image == "test_image" + await event.wait() + + with patch.object(DockerAddon, "update", new=mock_update), patch.object( + DockerAPI, "cleanup_old_images" + ) as cleanup: + await coresys.addons.update("local_ssh") + cleanup.assert_called_once_with( + "test_image", AwesomeVersion("1.1.1"), {"local/amd64-addon-ssh"} + ) + + update_task = coresys.create_task(simulate_update()) + await asyncio.sleep(0) + + with patch.object(Repository, "update"): + await coresys.store.reload() + + assert "image" not in coresys.store.data.addons["local_ssh"] + assert coresys.store.data.addons["local_ssh"]["version"] == AwesomeVersion("9.2.1") + + event.set() + await update_task + + assert install_addon_ssh.image == "test_image" + assert install_addon_ssh.version == AwesomeVersion("1.1.1")