From 590674ba7cf8323c0daa358a6b9e263c7ee82f53 Mon Sep 17 00:00:00 2001 From: Mike Degatano Date: Thu, 12 Feb 2026 11:37:15 -0500 Subject: [PATCH] Remove blocking I/O added to import_image (#6557) * Remove blocking I/O added to import_image * Add scanned modules to extra blockbuster functions * Use same cast avoidance approach in export_image * Remove unnecessary local image_writer variable * Remove unnecessary local image_tar_stream variable --------- Co-authored-by: Stefan Agner --- supervisor/docker/manager.py | 32 +++++++++++++++++++++++++------- tests/conftest.py | 16 ++++++++++++---- tests/utils/test_apparmor.py | 19 ++++++++++++++++--- 3 files changed, 53 insertions(+), 14 deletions(-) diff --git a/supervisor/docker/manager.py b/supervisor/docker/manager.py index 5e4d23e41..6f58926a1 100644 --- a/supervisor/docker/manager.py +++ b/supervisor/docker/manager.py @@ -9,7 +9,7 @@ from dataclasses import dataclass import errno from functools import partial from http import HTTPStatus -from io import BufferedWriter +from io import BufferedReader, BufferedWriter from ipaddress import IPv4Address import json import logging @@ -1025,13 +1025,30 @@ class DockerAPI(CoreSysAttributes): async def import_image(self, tar_file: Path) -> dict[str, Any] | None: """Import a tar file as image.""" + image_tar_stream: BufferedReader | None = None try: - with tar_file.open("rb") as read_tar: - resp: list[dict[str, Any]] = await self.images.import_image(read_tar) - except (aiodocker.DockerError, OSError) as err: + # Lambda avoids need for a cast here. Since return type of open is based on mode + image_tar_stream = await self.sys_run_in_executor( + lambda: tar_file.open("rb") + ) + resp: list[dict[str, Any]] = await self.images.import_image( + image_tar_stream + ) + except aiodocker.DockerError as err: raise DockerError( f"Can't import image from tar: {err}", _LOGGER.error ) from err + except OSError as err: + if err.errno == errno.EBADMSG: + self.sys_resolution.add_unhealthy_reason( + UnhealthyReason.OSERROR_BAD_MESSAGE + ) + raise DockerError( + f"Can't read tar file {tar_file}: {err}", _LOGGER.error + ) from err + finally: + if image_tar_stream: + await self.sys_run_in_executor(image_tar_stream.close) docker_image_list: list[str] = [] for chunk in resp: @@ -1066,12 +1083,13 @@ class DockerAPI(CoreSysAttributes): image_tar_stream: BufferedWriter | None = None try: - image_tar_stream = image_writer = cast( - BufferedWriter, await self.sys_run_in_executor(tar_file.open, "wb") + # Lambda avoids need for a cast here. Since return type of open is based on mode + image_tar_stream = await self.sys_run_in_executor( + lambda: tar_file.open("wb") ) async with self.images.export_image(f"{image}:{version}") as content: async for chunk in content.iter_chunked(DEFAULT_CHUNK_SIZE): - await self.sys_run_in_executor(image_writer.write, chunk) + await self.sys_run_in_executor(image_tar_stream.write, chunk) except aiodocker.DockerError as err: raise DockerError( f"Can't fetch image {image}:{version}: {err}", _LOGGER.error diff --git a/tests/conftest.py b/tests/conftest.py index 28df8ac8f..64c8461c9 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -18,7 +18,7 @@ from aiodocker.system import DockerSystem from aiohttp import ClientSession, web from aiohttp.test_utils import TestClient from awesomeversion import AwesomeVersion -from blockbuster import BlockBuster, blockbuster_ctx +from blockbuster import BlockBuster, BlockBusterFunction from dbus_fast import BusType from dbus_fast.aio.message_bus import MessageBus import pytest @@ -94,9 +94,17 @@ def blockbuster(request: pytest.FixtureRequest) -> BlockBuster | None: # But it will ignore calls to libraries and such that do blocking I/O directly from tests # Removing that would be nice but a todo for the future - # pylint: disable-next=contextmanager-generator-missing-cleanup - with blockbuster_ctx(scanned_modules=["supervisor"]) as bb: - yield bb + SCANNED_MODULES = ["supervisor"] + blockbuster = BlockBuster(scanned_modules=SCANNED_MODULES) + blockbuster.functions["pathlib.Path.open"] = BlockBusterFunction( + Path, "open", scanned_modules=SCANNED_MODULES + ) + blockbuster.functions["pathlib.Path.close"] = BlockBusterFunction( + Path, "close", scanned_modules=SCANNED_MODULES + ) + blockbuster.activate() + yield blockbuster + blockbuster.deactivate() @pytest.fixture diff --git a/tests/utils/test_apparmor.py b/tests/utils/test_apparmor.py index ceb281f06..b6ed78a7c 100644 --- a/tests/utils/test_apparmor.py +++ b/tests/utils/test_apparmor.py @@ -1,5 +1,6 @@ """Tests for apparmor utility.""" +import asyncio from pathlib import Path import pytest @@ -31,13 +32,20 @@ profile test flags=(attach_disconnected,mediate_deleted) { async def test_valid_apparmor_file(): """Test a valid apparmor file.""" - assert validate_profile("example", get_fixture_path("apparmor_valid.txt")) + assert await asyncio.get_running_loop().run_in_executor( + None, validate_profile, "example", get_fixture_path("apparmor_valid.txt") + ) async def test_apparmor_missing_profile(caplog: pytest.LogCaptureFixture): """Test apparmor file missing profile.""" with pytest.raises(AppArmorInvalidError): - validate_profile("example", get_fixture_path("apparmor_no_profile.txt")) + await asyncio.get_running_loop().run_in_executor( + None, + validate_profile, + "example", + get_fixture_path("apparmor_no_profile.txt"), + ) assert ( "Missing AppArmor profile inside file: apparmor_no_profile.txt" in caplog.text @@ -47,7 +55,12 @@ async def test_apparmor_missing_profile(caplog: pytest.LogCaptureFixture): async def test_apparmor_multiple_profiles(caplog: pytest.LogCaptureFixture): """Test apparmor file with too many profiles.""" with pytest.raises(AppArmorInvalidError): - validate_profile("example", get_fixture_path("apparmor_multiple_profiles.txt")) + await asyncio.get_running_loop().run_in_executor( + None, + validate_profile, + "example", + get_fixture_path("apparmor_multiple_profiles.txt"), + ) assert ( "Too many AppArmor profiles inside file: apparmor_multiple_profiles.txt"