Skip to content

Commit

Permalink
Keep shared images on addon update
Browse files Browse the repository at this point in the history
  • Loading branch information
mdegat01 committed Aug 23, 2024
1 parent 5423d97 commit 19d0e51
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 17 deletions.
27 changes: 27 additions & 0 deletions supervisor/docker/addon.py
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,33 @@ async def import_image(self, tar_file: Path) -> None:
with suppress(DockerError):
await self.cleanup()

@Job(name="docker_addon_cleanup", limit=JobExecutionLimit.GROUP_WAIT)
async def cleanup(
self,
old_image: str | None = None,
image: str | None = None,
version: AwesomeVersion | None = None,
) -> None:
"""Check if old version exists and cleanup other versions of image not in use."""
if not image:
image = self.image
if not version:
version = self.version

await self.sys_run_in_executor(
self.sys_docker.cleanup_old_images,
image,
version,
{old_image} if old_image else None,
keep_images={
f"{addon.image}:{addon.version}"
for addon in self.sys_addons.installed
if addon.slug != self.addon.slug
and addon.image
and addon.image in {old_image, image}
},
)

@Job(
name="docker_addon_write_stdin",
limit=JobExecutionLimit.GROUP_ONCE,
Expand Down
6 changes: 3 additions & 3 deletions supervisor/docker/interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -512,14 +512,14 @@ async def logs(self) -> bytes:
return b""

@Job(name="docker_interface_cleanup", limit=JobExecutionLimit.GROUP_WAIT)
def cleanup(
async def cleanup(
self,
old_image: str | None = None,
image: str | None = None,
version: AwesomeVersion | None = None,
) -> Awaitable[None]:
) -> None:
"""Check if old version exists and cleanup."""
return self.sys_run_in_executor(
await self.sys_run_in_executor(
self.sys_docker.cleanup_old_images,
image or self.image,
version or self.version,
Expand Down
20 changes: 18 additions & 2 deletions supervisor/docker/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -548,10 +548,13 @@ def cleanup_old_images(
current_image: str,
current_version: AwesomeVersion,
old_images: set[str] | None = None,
*,
keep_images: set[str] | None = None,
) -> None:
"""Clean up old versions of an image."""
image = f"{current_image}:{current_version!s}"
try:
current: Image = self.images.get(f"{current_image}:{current_version!s}")
keep: set[str] = {self.images.get(image).id}
except ImageNotFound:
raise DockerNotFound(
f"{current_image} not found for cleanup", _LOGGER.warning
Expand All @@ -561,6 +564,19 @@ def cleanup_old_images(
f"Can't get {current_image} for cleanup", _LOGGER.warning
) from err

keep_images -= {image}
if keep_images:
try:
keep = keep | {self.images.get(image).id for image in keep_images}
except ImageNotFound:
# This one is already missing, no need to preserve it from getting removed
pass
except (DockerException, requests.RequestException) as err:
raise DockerError(
f"Failed to get one or more images from {keep} during cleanup",
_LOGGER.warning,
) from err

# Cleanup old and current
image_names = list(
old_images | {current_image} if old_images else {current_image}
Expand All @@ -573,7 +589,7 @@ def cleanup_old_images(
) from err

for image in images_list:
if current.id == image.id:
if image.id in keep:
continue

with suppress(DockerException, requests.RequestException):
Expand Down
30 changes: 18 additions & 12 deletions tests/addons/test_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,12 @@ async def fixture_remove_wait_boot(coresys: CoreSys) -> None:
coresys.config.wait_boot = 0


@pytest.fixture("install_addon_example_image")
@pytest.fixture(name="install_addon_example_image")
def fixture_install_addon_example_image(coresys: CoreSys, repository) -> Addon:
"""Install local_example add-on with image."""
store = coresys.addons.store["local_example_image"]
coresys.addons.data.install(store)
# pylint: disable-next=protected-access
coresys.addons.data._data = coresys.addons.data._schema(coresys.addons.data._data)

addon = Addon(coresys, store.slug)
Expand Down Expand Up @@ -506,7 +507,7 @@ async def test_shared_image_kept_on_uninstall(


async def test_shared_image_kept_on_update(
coresys: CoreSys, install_addon_example_image: Addon
coresys: CoreSys, install_addon_example_image: Addon, docker: DockerAPI
):
"""Test if two addons share an image it is not removed on update."""
# Clone example to a new mock copy so two share an image
Expand All @@ -529,13 +530,18 @@ async def test_shared_image_kept_on_update(
assert example_2.version == "1.2.0"
assert install_addon_example_image.version == "1.2.0"

with patch.object(DockerAPI, "cleanup_old_images") as cleanup:
await coresys.addons.update("local_example2")
cleanup.assert_not_called()
assert example_2.version == "1.3.0"

await coresys.addons.update("local_example_image")
cleanup.assert_called_once_with(
"example/with_image", AwesomeVersion("1.3.0"), {"example/with_image"}
)
assert install_addon_example_image.version == "1.3.0"
image_new = MagicMock()
image_new.id = "image_new"
image_old = MagicMock()
image_old.id = "image_old"
docker.images.get.side_effect = [image_new, image_old]
docker.images.list.return_value = [image_new, image_old]

await coresys.addons.update("local_example2")
docker.images.remove.assert_not_called()
assert example_2.version == "1.3.0"

docker.images.get.side_effect = [image_new]
await coresys.addons.update("local_example_image")
docker.images.remove.assert_called_once_with("image_old", force=True)
assert install_addon_example_image.version == "1.3.0"
1 change: 1 addition & 0 deletions tests/fixtures/addons/local/example_image/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,4 @@ ingress_port: 0
breaking_versions:
- test
- 1.0
image: example/with-image

0 comments on commit 19d0e51

Please sign in to comment.