Address review comments, fix mounting of the cidfile

Author: sairon

supervisor/bootstrap.py

@@ -226,9 +226,9 @@ def initialize_system(coresys: CoreSys) -> None:
         )
         config.path_addon_configs.mkdir()
 
-    if not config.path_cidfiles.is_dir():
-        _LOGGER.debug("Creating Docker cidfiles folder at '%s'", config.path_cidfiles)
-        config.path_cidfiles.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):

supervisor/config.py

@@ -54,7 +54,7 @@
 EMERGENCY_DATA = PurePath("emergency")
 ADDON_CONFIGS = PurePath("addon_configs")
 CORE_BACKUP_DATA = PurePath("core/backup")
-CIDFILES = PurePath("cidfiles")
+CID_FILES = PurePath("cid_files")
 
 DEFAULT_BOOT_TIME = datetime.fromtimestamp(0, UTC).isoformat()
 
@@ -401,9 +401,14 @@ def path_extern_media(self) -> PurePath:
         return PurePath(self.path_extern_supervisor, MEDIA_DATA)
 
     @property
-    def path_cidfiles(self) -> Path:
+    def path_cid_files(self) -> Path:
         """Return CID files folder."""
-        return self.path_supervisor / CIDFILES
+        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]:

supervisor/docker/manager.py

@@ -324,17 +324,24 @@ def run(
         # Setup cidfile and bind mount it
         cidfile_path = None
         if name:
-            cidfile_path = self.coresys.config.path_cidfiles / f"{name}.cid"
+            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"]["/run/cid"] = {"bind": str(cidfile_path), "mode": "ro"}
+            kwargs["volumes"][str(extern_cidfile_path)] = {
+                "bind": "/run/cid",
+                "mode": "ro",
+            }
 
         # Create container
         try:
@@ -581,7 +588,7 @@ def stop_container(
                 _LOGGER.info("Cleaning %s application", name)
                 docker_container.remove(force=True, v=True)
 
-            cidfile_path = self.coresys.config.path_cidfiles / f"{name}.cid"
+            cidfile_path = self.coresys.config.path_cid_files / f"{name}.cid"
             with suppress(OSError):
                 cidfile_path.unlink(missing_ok=True)
 

tests/conftest.py

@@ -488,7 +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_cidfiles.mkdir()
+        coresys.config.path_cid_files.mkdir()
         yield tmp_path
 
 

tests/docker/test_manager.py

@@ -7,7 +7,7 @@
 import pytest
 from requests import RequestException
 
-from supervisor.config import CIDFILES
+from supervisor.coresys import CoreSys
 from supervisor.docker.manager import CommandReturn, DockerAPI
 from supervisor.exceptions import DockerError
 
@@ -138,14 +138,17 @@ async def test_run_command_custom_stdout_stderr(docker: DockerAPI):
     assert result.output == b"output"
 
 
-async def test_run_container_with_cidfile(docker: DockerAPI, tmp_supervisor_data):
+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 = tmp_supervisor_data / CIDFILES / f"{container_name}.cid"
+    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
 
@@ -166,9 +169,9 @@ async def test_run_container_with_cidfile(docker: DockerAPI, tmp_supervisor_data
         kwargs = create_mock.call_args[1]
 
         assert "volumes" in kwargs
-        assert "/run/cid" in kwargs["volumes"]
-        assert kwargs["volumes"]["/run/cid"]["bind"] == str(cidfile_path)
-        assert kwargs["volumes"]["/run/cid"]["mode"] == "ro"
+        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()
@@ -181,15 +184,15 @@ async def test_run_container_with_cidfile(docker: DockerAPI, tmp_supervisor_data
 
 
 async def test_run_container_with_leftover_cidfile(
-    docker: DockerAPI, tmp_supervisor_data
+    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 = tmp_supervisor_data / "cidfiles" / f"{container_name}.cid"
+    cidfile_path = coresys.config.path_cid_files / f"{container_name}.cid"
 
     # Create a leftover cidfile
     cidfile_path.touch()
@@ -217,15 +220,15 @@ async def test_run_container_with_leftover_cidfile(
 
 
 async def test_stop_container_with_cidfile_cleanup(
-    docker: DockerAPI, tmp_supervisor_data
+    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 = tmp_supervisor_data / "cidfiles" / f"{container_name}.cid"
+    cidfile_path = coresys.config.path_cid_files / f"{container_name}.cid"
 
     # Create a cidfile
     cidfile_path.touch()
@@ -273,14 +276,16 @@ async def test_stop_container_without_removal_no_cidfile_cleanup(docker: DockerA
         mock_unlink.assert_not_called()
 
 
-async def test_cidfile_cleanup_handles_oserror(docker: DockerAPI, tmp_supervisor_data):
+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 = tmp_supervisor_data / "cidfiles" / f"{container_name}.cid"
+    cidfile_path = coresys.config.path_cid_files / f"{container_name}.cid"
 
     # Create a cidfile
     cidfile_path.touch()