diff --git a/supervisor/bootstrap.py b/supervisor/bootstrap.py index a77587bfb62..c1f46ec260f 100644 --- a/supervisor/bootstrap.py +++ b/supervisor/bootstrap.py @@ -226,6 +226,10 @@ def initialize_system(coresys: CoreSys) -> None: ) config.path_addon_configs.mkdir() + if not config.path_cid_files.is_dir(): + _LOGGER.debug("Creating Docker cidfiles folder at '%s'", config.path_cid_files) + config.path_cid_files.mkdir() + def warning_handler(message, category, filename, lineno, file=None, line=None): """Warning handler which logs warnings using the logging module.""" diff --git a/supervisor/config.py b/supervisor/config.py index 83c9e9380dc..03abb91fbc4 100644 --- a/supervisor/config.py +++ b/supervisor/config.py @@ -54,6 +54,7 @@ EMERGENCY_DATA = PurePath("emergency") ADDON_CONFIGS = PurePath("addon_configs") CORE_BACKUP_DATA = PurePath("core/backup") +CID_FILES = PurePath("cid_files") DEFAULT_BOOT_TIME = datetime.fromtimestamp(0, UTC).isoformat() @@ -399,6 +400,16 @@ def path_extern_media(self) -> PurePath: """Return root media data folder external for Docker.""" return PurePath(self.path_extern_supervisor, MEDIA_DATA) + @property + def path_cid_files(self) -> Path: + """Return CID files folder.""" + return self.path_supervisor / CID_FILES + + @property + def path_extern_cid_files(self) -> PurePath: + """Return CID files folder.""" + return PurePath(self.path_extern_supervisor, CID_FILES) + @property def addons_repositories(self) -> list[str]: """Return list of custom Add-on repositories.""" diff --git a/supervisor/docker/manager.py b/supervisor/docker/manager.py index e67bbc9348c..b44fc386f1e 100644 --- a/supervisor/docker/manager.py +++ b/supervisor/docker/manager.py @@ -321,11 +321,36 @@ def run( if not network_mode: kwargs["network"] = None + # Setup cidfile and bind mount it + cidfile_path = None + if name: + cidfile_path = self.coresys.config.path_cid_files / f"{name}.cid" + + # Remove the file if it exists e.g. as a leftover from unclean shutdown + if cidfile_path.is_file(): + with suppress(OSError): + cidfile_path.unlink(missing_ok=True) + + extern_cidfile_path = ( + self.coresys.config.path_extern_cid_files / f"{name}.cid" + ) + + # Bind mount to /run/cid in container + if "volumes" not in kwargs: + kwargs["volumes"] = {} + kwargs["volumes"][str(extern_cidfile_path)] = { + "bind": "/run/cid", + "mode": "ro", + } + # Create container try: container = self.containers.create( f"{image}:{tag}", use_config_proxy=False, **kwargs ) + if cidfile_path: + with cidfile_path.open("w", encoding="ascii") as cidfile: + cidfile.write(str(container.id)) except docker_errors.NotFound as err: raise DockerNotFound( f"Image {image}:{tag} does not exist for {name}", _LOGGER.error @@ -563,6 +588,10 @@ def stop_container( _LOGGER.info("Cleaning %s application", name) docker_container.remove(force=True, v=True) + cidfile_path = self.coresys.config.path_cid_files / f"{name}.cid" + with suppress(OSError): + cidfile_path.unlink(missing_ok=True) + def start_container(self, name: str) -> None: """Start Docker container.""" try: diff --git a/tests/conftest.py b/tests/conftest.py index f14fbf09e38..0bab5ea4e7c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -488,6 +488,7 @@ async def tmp_supervisor_data(coresys: CoreSys, tmp_path: Path) -> Path: coresys.config.path_addon_configs.mkdir(parents=True) coresys.config.path_ssl.mkdir() coresys.config.path_core_backup.mkdir(parents=True) + coresys.config.path_cid_files.mkdir() yield tmp_path diff --git a/tests/docker/test_addon.py b/tests/docker/test_addon.py index 1b710e2a73f..37700bf9348 100644 --- a/tests/docker/test_addon.py +++ b/tests/docker/test_addon.py @@ -347,6 +347,7 @@ async def test_addon_run_add_host_error( addonsdata_system: dict[str, Data], capture_exception: Mock, path_extern, + tmp_supervisor_data: Path, ): """Test error adding host when addon is run.""" await coresys.dbus.timedate.connect(coresys.dbus.bus) @@ -433,6 +434,7 @@ async def test_addon_new_device( dev_path: str, cgroup: str, is_os: bool, + tmp_supervisor_data: Path, ): """Test new device that is listed in static devices.""" coresys.hardware.disk.get_disk_free_space = lambda x: 5000 @@ -463,6 +465,7 @@ async def test_addon_new_device_no_haos( install_addon_ssh: Addon, docker: DockerAPI, dev_path: str, + tmp_supervisor_data: Path, ): """Test new device that is listed in static devices on non HAOS system with CGroup V2.""" coresys.hardware.disk.get_disk_free_space = lambda x: 5000 diff --git a/tests/docker/test_manager.py b/tests/docker/test_manager.py index cd6e2b4a44c..70c4371d217 100644 --- a/tests/docker/test_manager.py +++ b/tests/docker/test_manager.py @@ -1,11 +1,13 @@ """Test Docker manager.""" -from unittest.mock import MagicMock +import asyncio +from unittest.mock import MagicMock, patch from docker.errors import DockerException import pytest from requests import RequestException +from supervisor.coresys import CoreSys from supervisor.docker.manager import CommandReturn, DockerAPI from supervisor.exceptions import DockerError @@ -134,3 +136,173 @@ async def test_run_command_custom_stdout_stderr(docker: DockerAPI): # Verify the result assert result.exit_code == 0 assert result.output == b"output" + + +async def test_run_container_with_cidfile( + coresys: CoreSys, docker: DockerAPI, path_extern, tmp_supervisor_data +): + """Test container creation with cidfile and bind mount.""" + # Mock container + mock_container = MagicMock() + mock_container.id = "test_container_id_12345" + + container_name = "test_container" + cidfile_path = coresys.config.path_cid_files / f"{container_name}.cid" + extern_cidfile_path = coresys.config.path_extern_cid_files / f"{container_name}.cid" + + docker.docker.containers.run.return_value = mock_container + + # Mock container creation + with patch.object( + docker.containers, "create", return_value=mock_container + ) as create_mock: + # Execute run with a container name + loop = asyncio.get_event_loop() + result = await loop.run_in_executor( + None, + lambda kwrgs: docker.run(**kwrgs), + {"image": "test_image", "tag": "latest", "name": container_name}, + ) + + # Check the container creation parameters + create_mock.assert_called_once() + kwargs = create_mock.call_args[1] + + assert "volumes" in kwargs + assert str(extern_cidfile_path) in kwargs["volumes"] + assert kwargs["volumes"][str(extern_cidfile_path)]["bind"] == "/run/cid" + assert kwargs["volumes"][str(extern_cidfile_path)]["mode"] == "ro" + + # Verify container start was called + mock_container.start.assert_called_once() + + # Verify cidfile was written with container ID + assert cidfile_path.exists() + assert cidfile_path.read_text() == mock_container.id + + assert result == mock_container + + +async def test_run_container_with_leftover_cidfile( + coresys: CoreSys, docker: DockerAPI, path_extern, tmp_supervisor_data +): + """Test container creation removes leftover cidfile before creating new one.""" + # Mock container + mock_container = MagicMock() + mock_container.id = "test_container_id_new" + + container_name = "test_container" + cidfile_path = coresys.config.path_cid_files / f"{container_name}.cid" + + # Create a leftover cidfile + cidfile_path.touch() + + # Mock container creation + with patch.object( + docker.containers, "create", return_value=mock_container + ) as create_mock: + # Execute run with a container name + loop = asyncio.get_event_loop() + result = await loop.run_in_executor( + None, + lambda kwrgs: docker.run(**kwrgs), + {"image": "test_image", "tag": "latest", "name": container_name}, + ) + + # Verify container was created + create_mock.assert_called_once() + + # Verify new cidfile was written with container ID + assert cidfile_path.exists() + assert cidfile_path.read_text() == mock_container.id + + assert result == mock_container + + +async def test_stop_container_with_cidfile_cleanup( + coresys: CoreSys, docker: DockerAPI, path_extern, tmp_supervisor_data +): + """Test container stop with cidfile cleanup.""" + # Mock container + mock_container = MagicMock() + mock_container.status = "running" + + container_name = "test_container" + cidfile_path = coresys.config.path_cid_files / f"{container_name}.cid" + + # Create a cidfile + cidfile_path.touch() + + # Mock the containers.get method and cidfile cleanup + with ( + patch.object(docker.containers, "get", return_value=mock_container), + ): + # Call stop_container with remove_container=True + loop = asyncio.get_event_loop() + await loop.run_in_executor( + None, + lambda kwrgs: docker.stop_container(**kwrgs), + {"timeout": 10, "remove_container": True, "name": container_name}, + ) + + # Verify container operations + mock_container.stop.assert_called_once_with(timeout=10) + mock_container.remove.assert_called_once_with(force=True, v=True) + + assert not cidfile_path.exists() + + +async def test_stop_container_without_removal_no_cidfile_cleanup(docker: DockerAPI): + """Test container stop without removal doesn't clean up cidfile.""" + # Mock container + mock_container = MagicMock() + mock_container.status = "running" + + container_name = "test_container" + + # Mock the containers.get method and cidfile cleanup + with ( + patch.object(docker.containers, "get", return_value=mock_container), + patch("pathlib.Path.unlink") as mock_unlink, + ): + # Call stop_container with remove_container=False + docker.stop_container(container_name, timeout=10, remove_container=False) + + # Verify container operations + mock_container.stop.assert_called_once_with(timeout=10) + mock_container.remove.assert_not_called() + + # Verify cidfile cleanup was NOT called + mock_unlink.assert_not_called() + + +async def test_cidfile_cleanup_handles_oserror( + coresys: CoreSys, docker: DockerAPI, path_extern, tmp_supervisor_data +): + """Test that cidfile cleanup handles OSError gracefully.""" + # Mock container + mock_container = MagicMock() + mock_container.status = "running" + + container_name = "test_container" + cidfile_path = coresys.config.path_cid_files / f"{container_name}.cid" + + # Create a cidfile + cidfile_path.touch() + + # Mock the containers.get method and cidfile cleanup to raise OSError + with ( + patch.object(docker.containers, "get", return_value=mock_container), + patch( + "pathlib.Path.unlink", side_effect=OSError("File not found") + ) as mock_unlink, + ): + # Call stop_container - should not raise exception + docker.stop_container(container_name, timeout=10, remove_container=True) + + # Verify container operations completed + mock_container.stop.assert_called_once_with(timeout=10) + mock_container.remove.assert_called_once_with(force=True, v=True) + + # Verify cidfile cleanup was attempted + mock_unlink.assert_called_once_with(missing_ok=True) diff --git a/tests/plugins/test_plugin_base.py b/tests/plugins/test_plugin_base.py index ae0df926537..c47144a5b57 100644 --- a/tests/plugins/test_plugin_base.py +++ b/tests/plugins/test_plugin_base.py @@ -1,6 +1,7 @@ """Test base plugin functionality.""" import asyncio +from pathlib import Path from unittest.mock import ANY, MagicMock, Mock, PropertyMock, patch from awesomeversion import AwesomeVersion @@ -165,6 +166,8 @@ async def test_plugin_watchdog_max_failed_attempts( error: PluginError, container: MagicMock, caplog: pytest.LogCaptureFixture, + tmp_supervisor_data: Path, + path_extern, ) -> None: """Test plugin watchdog gives up after max failed attempts.""" with patch.object(type(plugin.instance), "attach"):