Skip to content

Commit f3e1e0f

Browse files
authored
Fix CID file handling to prevent directory creation (home-assistant#6225)
* Fix CID file handling to prevent directory creation It seems that under certain conditions Docker creates a directory instead of a file for the CID file. This change ensures that the CID file is always created as a file, and any existing directory is removed before creating the file. * Fix tests * Fix pytest
1 parent 5779b56 commit f3e1e0f

File tree

4 files changed

+62
-4
lines changed

4 files changed

+62
-4
lines changed

supervisor/docker/manager.py

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -326,11 +326,19 @@ def run(
326326
if name:
327327
cidfile_path = self.coresys.config.path_cid_files / f"{name}.cid"
328328

329-
# Remove the file if it exists e.g. as a leftover from unclean shutdown
330-
if cidfile_path.is_file():
331-
with suppress(OSError):
329+
# Remove the file/directory if it exists e.g. as a leftover from unclean shutdown
330+
# Note: Can be a directory if Docker auto-started container with restart policy
331+
# before Supervisor could write the CID file
332+
with suppress(OSError):
333+
if cidfile_path.is_dir():
334+
cidfile_path.rmdir()
335+
elif cidfile_path.is_file():
332336
cidfile_path.unlink(missing_ok=True)
333337

338+
# Create empty CID file before adding it to volumes to prevent Docker
339+
# from creating it as a directory if container auto-starts
340+
cidfile_path.touch()
341+
334342
extern_cidfile_path = (
335343
self.coresys.config.path_extern_cid_files / f"{name}.cid"
336344
)

tests/docker/test_addon.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,10 @@ def test_not_journald_addon(
318318

319319

320320
async def test_addon_run_docker_error(
321-
coresys: CoreSys, addonsdata_system: dict[str, Data], path_extern
321+
coresys: CoreSys,
322+
addonsdata_system: dict[str, Data],
323+
path_extern,
324+
tmp_supervisor_data: Path,
322325
):
323326
"""Test docker error when addon is run."""
324327
await coresys.dbus.timedate.connect(coresys.dbus.bus)

tests/docker/test_interface.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""Test Docker interface."""
22

33
import asyncio
4+
from pathlib import Path
45
from typing import Any
56
from unittest.mock import ANY, AsyncMock, MagicMock, Mock, PropertyMock, call, patch
67

@@ -281,6 +282,7 @@ async def test_run_missing_image(
281282
container: MagicMock,
282283
capture_exception: Mock,
283284
path_extern,
285+
tmp_supervisor_data: Path,
284286
):
285287
"""Test run captures the exception when image is missing."""
286288
coresys.docker.containers.create.side_effect = [NotFound("missing"), MagicMock()]

tests/docker/test_manager.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,8 @@ async def test_cidfile_cleanup_handles_oserror(
293293
# Mock the containers.get method and cidfile cleanup to raise OSError
294294
with (
295295
patch.object(docker.containers, "get", return_value=mock_container),
296+
patch("pathlib.Path.is_dir", return_value=False),
297+
patch("pathlib.Path.is_file", return_value=True),
296298
patch(
297299
"pathlib.Path.unlink", side_effect=OSError("File not found")
298300
) as mock_unlink,
@@ -306,3 +308,46 @@ async def test_cidfile_cleanup_handles_oserror(
306308

307309
# Verify cidfile cleanup was attempted
308310
mock_unlink.assert_called_once_with(missing_ok=True)
311+
312+
313+
async def test_run_container_with_leftover_cidfile_directory(
314+
coresys: CoreSys, docker: DockerAPI, path_extern, tmp_supervisor_data
315+
):
316+
"""Test container creation removes leftover cidfile directory before creating new one.
317+
318+
This can happen when Docker auto-starts a container with restart policy
319+
before Supervisor could write the CID file, causing Docker to create
320+
the bind mount source as a directory.
321+
"""
322+
# Mock container
323+
mock_container = MagicMock()
324+
mock_container.id = "test_container_id_new"
325+
326+
container_name = "test_container"
327+
cidfile_path = coresys.config.path_cid_files / f"{container_name}.cid"
328+
329+
# Create a leftover directory (simulating Docker's behavior)
330+
cidfile_path.mkdir()
331+
assert cidfile_path.is_dir()
332+
333+
# Mock container creation
334+
with patch.object(
335+
docker.containers, "create", return_value=mock_container
336+
) as create_mock:
337+
# Execute run with a container name
338+
loop = asyncio.get_event_loop()
339+
result = await loop.run_in_executor(
340+
None,
341+
lambda kwrgs: docker.run(**kwrgs),
342+
{"image": "test_image", "tag": "latest", "name": container_name},
343+
)
344+
345+
# Verify container was created
346+
create_mock.assert_called_once()
347+
348+
# Verify new cidfile was written as a file (not directory)
349+
assert cidfile_path.exists()
350+
assert cidfile_path.is_file()
351+
assert cidfile_path.read_text() == mock_container.id
352+
353+
assert result == mock_container

0 commit comments

Comments
 (0)