Write cidfiles of Docker containers and mount them individually to /run/cid (#6154)

* Write cidfiles of Docker containers and mount them individually to /run/cid

There is no standard way to get the container ID in the container
itself, which can be needed for instance for #6006. The usual pattern is
to use the --cidfile argument of Docker CLI and mount the generated file
to the container. However, this is feature of Docker CLI and we can't
use it when creating the containers via API. To get container ID to
implement native logging in e.g. Core as well, we need the help of the
Supervisor.

This change implements similar feature fully in Supervisor's DockerAPI
class that orchestrates lifetime of all containers managed by
Supervisor. The files are created in the SUPERVISOR_DATA directory, as
it needs to be persisted between reboots, just as the instances of
Docker containers are.

Supervisor's cidfile must be created when starting the Supervisor
itself, for that see home-assistant/operating-system#4276.

* Address review comments, fix mounting of the cidfile

Author: sairon

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."""

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."""

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:

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
 
 

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

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)

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"):