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 <stefan@agner.ch>

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

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

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"