Skip to content

Commit f09b279

Browse files
committed
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.
1 parent 3d62c9a commit f09b279

File tree

7 files changed

+207
-1
lines changed

7 files changed

+207
-1
lines changed

supervisor/bootstrap.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,10 @@ def initialize_system(coresys: CoreSys) -> None:
226226
)
227227
config.path_addon_configs.mkdir()
228228

229+
if not config.path_cidfiles.is_dir():
230+
_LOGGER.debug("Creating Docker cidfiles folder at '%s'", config.path_cidfiles)
231+
config.path_cidfiles.mkdir()
232+
229233

230234
def warning_handler(message, category, filename, lineno, file=None, line=None):
231235
"""Warning handler which logs warnings using the logging module."""

supervisor/config.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
EMERGENCY_DATA = PurePath("emergency")
5555
ADDON_CONFIGS = PurePath("addon_configs")
5656
CORE_BACKUP_DATA = PurePath("core/backup")
57+
CIDFILES = PurePath("cidfiles")
5758

5859
DEFAULT_BOOT_TIME = datetime.fromtimestamp(0, UTC).isoformat()
5960

@@ -399,6 +400,11 @@ def path_extern_media(self) -> PurePath:
399400
"""Return root media data folder external for Docker."""
400401
return PurePath(self.path_extern_supervisor, MEDIA_DATA)
401402

403+
@property
404+
def path_cidfiles(self) -> Path:
405+
"""Return CID files folder."""
406+
return self.path_supervisor / CIDFILES
407+
402408
@property
403409
def addons_repositories(self) -> list[str]:
404410
"""Return list of custom Add-on repositories."""

supervisor/docker/manager.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,11 +321,29 @@ def run(
321321
if not network_mode:
322322
kwargs["network"] = None
323323

324+
# Setup cidfile and bind mount it
325+
cidfile_path = None
326+
if name:
327+
cidfile_path = self.coresys.config.path_cidfiles / f"{name}.cid"
328+
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):
332+
cidfile_path.unlink(missing_ok=True)
333+
334+
# Bind mount to /run/cid in container
335+
if "volumes" not in kwargs:
336+
kwargs["volumes"] = {}
337+
kwargs["volumes"]["/run/cid"] = {"bind": str(cidfile_path), "mode": "ro"}
338+
324339
# Create container
325340
try:
326341
container = self.containers.create(
327342
f"{image}:{tag}", use_config_proxy=False, **kwargs
328343
)
344+
if cidfile_path:
345+
with cidfile_path.open("w", encoding="ascii") as cidfile:
346+
cidfile.write(str(container.id))
329347
except docker_errors.NotFound as err:
330348
raise DockerNotFound(
331349
f"Image {image}:{tag} does not exist for {name}", _LOGGER.error
@@ -563,6 +581,10 @@ def stop_container(
563581
_LOGGER.info("Cleaning %s application", name)
564582
docker_container.remove(force=True, v=True)
565583

584+
cidfile_path = self.coresys.config.path_cidfiles / f"{name}.cid"
585+
with suppress(OSError):
586+
cidfile_path.unlink(missing_ok=True)
587+
566588
def start_container(self, name: str) -> None:
567589
"""Start Docker container."""
568590
try:

tests/conftest.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,7 @@ async def tmp_supervisor_data(coresys: CoreSys, tmp_path: Path) -> Path:
488488
coresys.config.path_addon_configs.mkdir(parents=True)
489489
coresys.config.path_ssl.mkdir()
490490
coresys.config.path_core_backup.mkdir(parents=True)
491+
coresys.config.path_cidfiles.mkdir()
491492
yield tmp_path
492493

493494

tests/docker/test_addon.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,6 +347,7 @@ async def test_addon_run_add_host_error(
347347
addonsdata_system: dict[str, Data],
348348
capture_exception: Mock,
349349
path_extern,
350+
tmp_supervisor_data: Path,
350351
):
351352
"""Test error adding host when addon is run."""
352353
await coresys.dbus.timedate.connect(coresys.dbus.bus)
@@ -433,6 +434,7 @@ async def test_addon_new_device(
433434
dev_path: str,
434435
cgroup: str,
435436
is_os: bool,
437+
tmp_supervisor_data: Path,
436438
):
437439
"""Test new device that is listed in static devices."""
438440
coresys.hardware.disk.get_disk_free_space = lambda x: 5000
@@ -463,6 +465,7 @@ async def test_addon_new_device_no_haos(
463465
install_addon_ssh: Addon,
464466
docker: DockerAPI,
465467
dev_path: str,
468+
tmp_supervisor_data: Path,
466469
):
467470
"""Test new device that is listed in static devices on non HAOS system with CGroup V2."""
468471
coresys.hardware.disk.get_disk_free_space = lambda x: 5000

tests/docker/test_manager.py

Lines changed: 168 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
"""Test Docker manager."""
22

3-
from unittest.mock import MagicMock
3+
import asyncio
4+
from unittest.mock import MagicMock, patch
45

56
from docker.errors import DockerException
67
import pytest
78
from requests import RequestException
89

10+
from supervisor.config import CIDFILES
911
from supervisor.docker.manager import CommandReturn, DockerAPI
1012
from supervisor.exceptions import DockerError
1113

@@ -134,3 +136,168 @@ async def test_run_command_custom_stdout_stderr(docker: DockerAPI):
134136
# Verify the result
135137
assert result.exit_code == 0
136138
assert result.output == b"output"
139+
140+
141+
async def test_run_container_with_cidfile(docker: DockerAPI, tmp_supervisor_data):
142+
"""Test container creation with cidfile and bind mount."""
143+
# Mock container
144+
mock_container = MagicMock()
145+
mock_container.id = "test_container_id_12345"
146+
147+
container_name = "test_container"
148+
cidfile_path = tmp_supervisor_data / CIDFILES / f"{container_name}.cid"
149+
150+
docker.docker.containers.run.return_value = mock_container
151+
152+
# Mock container creation
153+
with patch.object(
154+
docker.containers, "create", return_value=mock_container
155+
) as create_mock:
156+
# Execute run with a container name
157+
loop = asyncio.get_event_loop()
158+
result = await loop.run_in_executor(
159+
None,
160+
lambda kwrgs: docker.run(**kwrgs),
161+
{"image": "test_image", "tag": "latest", "name": container_name},
162+
)
163+
164+
# Check the container creation parameters
165+
create_mock.assert_called_once()
166+
kwargs = create_mock.call_args[1]
167+
168+
assert "volumes" in kwargs
169+
assert "/run/cid" in kwargs["volumes"]
170+
assert kwargs["volumes"]["/run/cid"]["bind"] == str(cidfile_path)
171+
assert kwargs["volumes"]["/run/cid"]["mode"] == "ro"
172+
173+
# Verify container start was called
174+
mock_container.start.assert_called_once()
175+
176+
# Verify cidfile was written with container ID
177+
assert cidfile_path.exists()
178+
assert cidfile_path.read_text() == mock_container.id
179+
180+
assert result == mock_container
181+
182+
183+
async def test_run_container_with_leftover_cidfile(
184+
docker: DockerAPI, tmp_supervisor_data
185+
):
186+
"""Test container creation removes leftover cidfile before creating new one."""
187+
# Mock container
188+
mock_container = MagicMock()
189+
mock_container.id = "test_container_id_new"
190+
191+
container_name = "test_container"
192+
cidfile_path = tmp_supervisor_data / "cidfiles" / f"{container_name}.cid"
193+
194+
# Create a leftover cidfile
195+
cidfile_path.touch()
196+
197+
# Mock container creation
198+
with patch.object(
199+
docker.containers, "create", return_value=mock_container
200+
) as create_mock:
201+
# Execute run with a container name
202+
loop = asyncio.get_event_loop()
203+
result = await loop.run_in_executor(
204+
None,
205+
lambda kwrgs: docker.run(**kwrgs),
206+
{"image": "test_image", "tag": "latest", "name": container_name},
207+
)
208+
209+
# Verify container was created
210+
create_mock.assert_called_once()
211+
212+
# Verify new cidfile was written with container ID
213+
assert cidfile_path.exists()
214+
assert cidfile_path.read_text() == mock_container.id
215+
216+
assert result == mock_container
217+
218+
219+
async def test_stop_container_with_cidfile_cleanup(
220+
docker: DockerAPI, tmp_supervisor_data
221+
):
222+
"""Test container stop with cidfile cleanup."""
223+
# Mock container
224+
mock_container = MagicMock()
225+
mock_container.status = "running"
226+
227+
container_name = "test_container"
228+
cidfile_path = tmp_supervisor_data / "cidfiles" / f"{container_name}.cid"
229+
230+
# Create a cidfile
231+
cidfile_path.touch()
232+
233+
# Mock the containers.get method and cidfile cleanup
234+
with (
235+
patch.object(docker.containers, "get", return_value=mock_container),
236+
):
237+
# Call stop_container with remove_container=True
238+
loop = asyncio.get_event_loop()
239+
await loop.run_in_executor(
240+
None,
241+
lambda kwrgs: docker.stop_container(**kwrgs),
242+
{"timeout": 10, "remove_container": True, "name": container_name},
243+
)
244+
245+
# Verify container operations
246+
mock_container.stop.assert_called_once_with(timeout=10)
247+
mock_container.remove.assert_called_once_with(force=True, v=True)
248+
249+
assert not cidfile_path.exists()
250+
251+
252+
async def test_stop_container_without_removal_no_cidfile_cleanup(docker: DockerAPI):
253+
"""Test container stop without removal doesn't clean up cidfile."""
254+
# Mock container
255+
mock_container = MagicMock()
256+
mock_container.status = "running"
257+
258+
container_name = "test_container"
259+
260+
# Mock the containers.get method and cidfile cleanup
261+
with (
262+
patch.object(docker.containers, "get", return_value=mock_container),
263+
patch("pathlib.Path.unlink") as mock_unlink,
264+
):
265+
# Call stop_container with remove_container=False
266+
docker.stop_container(container_name, timeout=10, remove_container=False)
267+
268+
# Verify container operations
269+
mock_container.stop.assert_called_once_with(timeout=10)
270+
mock_container.remove.assert_not_called()
271+
272+
# Verify cidfile cleanup was NOT called
273+
mock_unlink.assert_not_called()
274+
275+
276+
async def test_cidfile_cleanup_handles_oserror(docker: DockerAPI, tmp_supervisor_data):
277+
"""Test that cidfile cleanup handles OSError gracefully."""
278+
# Mock container
279+
mock_container = MagicMock()
280+
mock_container.status = "running"
281+
282+
container_name = "test_container"
283+
cidfile_path = tmp_supervisor_data / "cidfiles" / f"{container_name}.cid"
284+
285+
# Create a cidfile
286+
cidfile_path.touch()
287+
288+
# Mock the containers.get method and cidfile cleanup to raise OSError
289+
with (
290+
patch.object(docker.containers, "get", return_value=mock_container),
291+
patch(
292+
"pathlib.Path.unlink", side_effect=OSError("File not found")
293+
) as mock_unlink,
294+
):
295+
# Call stop_container - should not raise exception
296+
docker.stop_container(container_name, timeout=10, remove_container=True)
297+
298+
# Verify container operations completed
299+
mock_container.stop.assert_called_once_with(timeout=10)
300+
mock_container.remove.assert_called_once_with(force=True, v=True)
301+
302+
# Verify cidfile cleanup was attempted
303+
mock_unlink.assert_called_once_with(missing_ok=True)

tests/plugins/test_plugin_base.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""Test base plugin functionality."""
22

33
import asyncio
4+
from pathlib import Path
45
from unittest.mock import ANY, MagicMock, Mock, PropertyMock, patch
56

67
from awesomeversion import AwesomeVersion
@@ -165,6 +166,8 @@ async def test_plugin_watchdog_max_failed_attempts(
165166
error: PluginError,
166167
container: MagicMock,
167168
caplog: pytest.LogCaptureFixture,
169+
tmp_supervisor_data: Path,
170+
path_extern,
168171
) -> None:
169172
"""Test plugin watchdog gives up after max failed attempts."""
170173
with patch.object(type(plugin.instance), "attach"):

0 commit comments

Comments
 (0)