From 9264c0387af5cccf07f15095c08d7cb689c9f185 Mon Sep 17 00:00:00 2001 From: Zack Cerza Date: Fri, 16 Oct 2026 17:43:42 -0600 Subject: [PATCH 1/9] README: Add missing trailing newline --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index bcd0302e..26a53833 100644 --- a/README.md +++ b/README.md @@ -192,4 +192,4 @@ ceph-devstack -v start ```bash ceph-devstack wait teuthology podman logs -f teuthology -``` \ No newline at end of file +``` From 985e143d91a6381eb2c544f9dd87970cb221a6c8 Mon Sep 17 00:00:00 2001 From: Zack Cerza Date: Wed, 21 Oct 2026 14:25:32 -0600 Subject: [PATCH 2/9] PodmanResource: Log output of failed commands cmd(check=True) raised CalledProcessError with none of the command's output, which made failures hard to diagnose. Log stdout and stderr on failure, and attach them to the exception. Signed-off-by: Zack Cerza --- ceph_devstack/exec.py | 21 ++++++++++++++++- ceph_devstack/resources/__init__.py | 10 ++++++--- tests/resources/test_podmanresource.py | 31 +++++++++++++++++++++----- 3 files changed, 52 insertions(+), 10 deletions(-) diff --git a/ceph_devstack/exec.py b/ceph_devstack/exec.py index 9cc27def..1bddb991 100644 --- a/ceph_devstack/exec.py +++ b/ceph_devstack/exec.py @@ -8,7 +8,7 @@ import signal import subprocess -from typing import Dict, List, Optional +from typing import Dict, List, Optional, Tuple from ceph_devstack import logger, VERBOSE @@ -70,6 +70,25 @@ def signal_children(self, signal: signal.Signals, recursive=True): with contextlib.suppress(ProcessLookupError): os.killpg(pid, signal) + async def collect_output(self) -> Tuple[str, str]: + stdout = b"" + stderr = b"" + if self.stdout is not None: + stdout = await self.stdout.read() + if self.stderr is not None: + stderr = await self.stderr.read() + return stdout.decode(), stderr.decode() + + async def log_failure(self, cmd: List[str]) -> Tuple[str, str]: + stdout, stderr = await self.collect_output() + returncode = self.returncode if self.returncode is not None else -1 + logger.error(f"Command failed ({returncode}): {' '.join(cmd)}") + for line in stderr.rstrip("\n").splitlines(): + logger.error(line) + for line in stdout.rstrip("\n").splitlines(): + logger.error(line) + return stdout, stderr + class Command: def __init__( diff --git a/ceph_devstack/resources/__init__.py b/ceph_devstack/resources/__init__.py index 0a3c2a10..b9607191 100644 --- a/ceph_devstack/resources/__init__.py +++ b/ceph_devstack/resources/__init__.py @@ -65,9 +65,13 @@ async def cmd( ) returncode = await proc.wait() if check and returncode != 0: - # out = await proc.stderr.read() - # logger.error(out.decode()) - raise CalledProcessError(cmd=args, returncode=returncode) + stdout, stderr = await proc.log_failure(args) + raise CalledProcessError( + returncode, + args, + output=stdout or None, + stderr=stderr or None, + ) return proc def format_cmd(self, args: List): diff --git a/tests/resources/test_podmanresource.py b/tests/resources/test_podmanresource.py index 3ee8b6e4..2d181cdd 100644 --- a/tests/resources/test_podmanresource.py +++ b/tests/resources/test_podmanresource.py @@ -1,10 +1,11 @@ +import logging import pytest from pathlib import Path from subprocess import CalledProcessError -from unittest.mock import patch - +from unittest.mock import AsyncMock, patch +from ceph_devstack.exec import Subprocess from ceph_devstack.resources import PodmanResource @@ -54,7 +55,25 @@ async def test_cmd(self, cls): print(m_arun.await_args_list) m_arun.assert_awaited_once_with(["0"], cwd=Path("."), stream_output=False) - async def test_cmd_failed(self, cls): - obj = cls() - with pytest.raises(CalledProcessError): - await obj.cmd(["false"], check=True) + async def test_cmd_failed(self, cls, caplog): + class FakeProc: + returncode = 1 + collect_output = Subprocess.collect_output + log_failure = Subprocess.log_failure + + def __init__(self): + self.stdout = AsyncMock() + self.stdout.read = AsyncMock(return_value=b"") + self.stderr = AsyncMock() + self.stderr.read = AsyncMock(return_value=b"podman-failure\n") + + async def wait(self): + return 1 + + with patch("ceph_devstack.host.host.arun", return_value=FakeProc()) as m_arun: + obj = cls() + with caplog.at_level(logging.ERROR), pytest.raises(CalledProcessError): + await obj.cmd(["podman", "fail"], check=True) + m_arun.assert_awaited_once() + assert "Command failed (1)" in caplog.text + assert "podman-failure" in caplog.text From 74c4e19c8420ff16bf237a2b9831bf3c7d707042 Mon Sep 17 00:00:00 2001 From: Zack Cerza Date: Wed, 21 Oct 2026 14:25:58 -0600 Subject: [PATCH 3/9] RemoteHost: Quote arguments for podman machine ssh podman machine ssh joins its arguments into one remote shell line, so an argument like a bash -c script was split into words. Quote each one. Signed-off-by: Zack Cerza --- ceph_devstack/host.py | 4 +++- tests/test_host.py | 21 ++++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/ceph_devstack/host.py b/ceph_devstack/host.py index 3be43902..8c744acc 100644 --- a/ceph_devstack/host.py +++ b/ceph_devstack/host.py @@ -2,6 +2,7 @@ import logging import os import pathlib +import shlex import socket import sys import yaml @@ -149,7 +150,8 @@ class RemoteHost(Host): def _remote_args(self, args: List[str], stream_output: bool) -> List[str]: remote = list(self.base_args) remote.append("--") - remote.extend(args) + # Quote each arg so compound arguments like "bash -c ''" stay as one shell word + remote.extend(shlex.quote(arg) for arg in args) return remote def cmd( diff --git a/tests/test_host.py b/tests/test_host.py index 997dd3c6..6588576e 100644 --- a/tests/test_host.py +++ b/tests/test_host.py @@ -1,6 +1,6 @@ import pytest -from ceph_devstack.host import LocalHost +from ceph_devstack.host import LocalHost, RemoteHost class TestLocalHost: @@ -11,3 +11,22 @@ def host(self): def test_path_exists_expands_tilde(self, host): assert host.path_exists("~") is True assert host.path_exists("~/nonexistent_dir_12345") is False + + +SSH = ["podman", "machine", "ssh", "--"] + + +class TestRemoteHost: + @pytest.mark.parametrize( + "args,expected", + [ + (["podman", "ps"], ["podman", "ps"]), + ( + ["sudo", "losetup", "/dev/loop0"], + SSH + ["sudo", "losetup", "/dev/loop0"], + ), + (["bash", "-c", "echo hi; ls"], SSH + ["bash", "-c", "'echo hi; ls'"]), + ], + ) + def test_cmd_runs_non_podman_commands_in_machine(self, args, expected): + assert RemoteHost().cmd(args).args == expected From 447441c932b62b5267a87b25699e27a9fcf86686 Mon Sep 17 00:00:00 2001 From: Zack Cerza Date: Wed, 21 Oct 2026 14:26:34 -0600 Subject: [PATCH 4/9] doctor: Detect an outdated SELinux module doctor only checked that a ceph_devstack module was loaded, so an outdated one went unnoticed and could block containers. Compare the installed module's hash with the shipped ceph_devstack.pp, falling back to the presence check if the installed one can't be extracted. Signed-off-by: Zack Cerza --- ceph_devstack/resources/ceph/requirements.py | 86 ++++++++++++------- .../resources/ceph/test_requirements_ceph.py | 84 +++++++++++------- 2 files changed, 112 insertions(+), 58 deletions(-) diff --git a/ceph_devstack/resources/ceph/requirements.py b/ceph_devstack/resources/ceph/requirements.py index 00d79678..7a19d3e9 100644 --- a/ceph_devstack/resources/ceph/requirements.py +++ b/ceph_devstack/resources/ceph/requirements.py @@ -1,3 +1,8 @@ +import hashlib +import re + +from subprocess import CalledProcessError + from ceph_devstack import logger, PROJECT_ROOT from ceph_devstack.requirements import Requirement, FixableRequirement @@ -43,43 +48,20 @@ async def suggest(self): class SELinuxModule(FixableRequirement): + MODULE_NAME = "ceph_devstack" + SHIPPED_PP = PROJECT_ROOT / f"{MODULE_NAME}.pp" + def __init__(self): fix_cmd = self.fix_cmd_prebuilt if self.host.type == "remote": fix_cmd = ["podman", "machine", "ssh", "--"] + fix_cmd self.fix_cmd = fix_cmd - fix_cmd_build = [ - "(sudo", - "dnf", - "install", - "-y", - "policycoreutils-devel", - "selinux-policy-devel", - "&&", - "cd", - str(PROJECT_ROOT), - "&&", - "make", - "-f", - "/usr/share/selinux/devel/Makefile", - "ceph_devstack.pp", - "&&", - "sudo", - "semodule", - "-i", - "ceph_devstack.pp)", - ] - fix_cmd_prebuilt = [ - "sudo", - "semodule", - "-i", - str(PROJECT_ROOT / "ceph_devstack.pp"), - ] + fix_cmd_prebuilt = ["sudo", "semodule", "-i", str(SHIPPED_PP)] suggest_msg = ( "SELinux is in Enforcing mode. To run nested rootless podman " "containers, it is necessary to install ceph-devstack's SELinux " - "module" + "module (module may be missing or outdated)" ) async def check(self): @@ -87,4 +69,50 @@ async def check(self): assert proc.stdout is not None await proc.wait() out = (await proc.stdout.read()).decode() - return "ceph_devstack" in out.split("\n") + if self.MODULE_NAME not in out.split("\n"): + return False + if not self.SHIPPED_PP.exists(): + logger.warning( + f"Shipped SELinux module not found at {self.SHIPPED_PP}; " + "skipping hash check" + ) + return True + installed_hash = await self._installed_module_hash() + if installed_hash is None: + logger.warning( + "Could not extract the installed SELinux module; " + "falling back to presence-only check" + ) + return True + shipped_hash = hashlib.sha256(self.SHIPPED_PP.read_bytes()).hexdigest() + if installed_hash != shipped_hash: + logger.warning( + f"SELinux module hash mismatch: shipped={shipped_hash}, " + f"installed={installed_hash}" + ) + return False + return True + + async def _installed_module_hash(self) -> str | None: + """sha256 of the installed module, or None if it can't be determined.""" + # semodule -E writes a byte-identical .pp into the cwd (and chatters + # on stdout, hence the redirect); always clean up the temp dir. + script = ( + "d=$(mktemp -d) || exit 1; " + "trap 'sudo rm -rf \"$d\"' EXIT; " + 'cd "$d" && ' + f"sudo semodule -E {self.MODULE_NAME} >/dev/null 2>&1 && " + f"sha256sum {self.MODULE_NAME}.pp" + ) + try: + proc = await self.host.arun(["bash", "-c", script]) + assert proc.stdout is not None + if await proc.wait() != 0: + return None + out = (await proc.stdout.read()).decode().split() + except (OSError, CalledProcessError): + return None + # Output is "hash filename"; anything else is unparseable. + if out and re.fullmatch(r"[0-9a-f]{64}", out[0]): + return out[0] + return None diff --git a/tests/resources/ceph/test_requirements_ceph.py b/tests/resources/ceph/test_requirements_ceph.py index cc4b58a7..3e93261f 100644 --- a/tests/resources/ceph/test_requirements_ceph.py +++ b/tests/resources/ceph/test_requirements_ceph.py @@ -1,3 +1,5 @@ +import hashlib +import pytest from unittest.mock import AsyncMock, MagicMock, patch from ceph_devstack import config @@ -126,38 +128,62 @@ async def side_effect_arun(args): assert "chgrp" in " ".join(self.req.fix_cmd) -class TestSELinuxModule: - def setup_method(self): - self.req = SELinuxModule() +SHIPPED = b"shipped module" +SHIPPED_HASH = hashlib.sha256(SHIPPED).hexdigest().encode() +LISTED = b"ceph_devstack\nother_module\n" - async def test_selinux_module_check_true(self): - mock_proc = AsyncMock() - mock_proc.stdout = AsyncMock() - mock_proc.stdout.read = AsyncMock(return_value=b"ceph_devstack\nother_module\n") - mock_proc.wait = AsyncMock(return_value=0) - with patch.object(self.req.host, "arun", return_value=mock_proc): - result = await self.req.check() - assert result is True - async def test_selinux_module_check_false(self): - mock_proc = AsyncMock() - mock_proc.stdout = AsyncMock() - mock_proc.stdout.read = AsyncMock( - return_value=b"other_module\nanother_module\n" - ) - mock_proc.wait = AsyncMock(return_value=0) - with patch.object(self.req.host, "arun", return_value=mock_proc): - result = await self.req.check() - assert result is False +def _proc(returncode: int, stdout: bytes) -> AsyncMock: + proc = AsyncMock() + proc.stdout.read = AsyncMock(return_value=stdout) + proc.wait = AsyncMock(return_value=returncode) + return proc - async def test_selinux_module_check_empty_output(self): - mock_proc = AsyncMock() - mock_proc.stdout = AsyncMock() - mock_proc.stdout.read = AsyncMock(return_value=b"") - mock_proc.wait = AsyncMock(return_value=0) - with patch.object(self.req.host, "arun", return_value=mock_proc): - result = await self.req.check() - assert result is False + +class TestSELinuxModule: + @pytest.fixture + def req(self, tmp_path): + shipped_pp = tmp_path / "ceph_devstack.pp" + shipped_pp.write_bytes(SHIPPED) + with patch.object(SELinuxModule, "SHIPPED_PP", shipped_pp): + yield SELinuxModule() + + @pytest.mark.parametrize( + "listed, extract_rc, extract_out, expected", + [ + pytest.param(b"other_module\n", 0, b"", False, id="not-listed"), + pytest.param( + LISTED, 0, SHIPPED_HASH + b" ceph_devstack.pp\n", True, id="current" + ), + pytest.param( + LISTED, 0, b"0" * 64 + b" ceph_devstack.pp\n", False, id="stale" + ), + # Any failure to read the installed module must not read as + # "stale", otherwise doctor --fix would reinstall forever. + pytest.param(LISTED, 1, b"", True, id="extract-failed"), + pytest.param( + LISTED, + 0, + b"Extracting at highest existing priority '400'.\n", + True, + id="unparseable", + ), + ], + ) + async def test_check(self, req, listed, extract_rc, extract_out, expected): + procs = [_proc(0, listed), _proc(extract_rc, extract_out)] + with patch.object(req.host, "arun", side_effect=procs): + assert await req.check() is expected + + async def test_check_falls_back_when_extraction_cannot_run(self, req): + procs = [_proc(0, LISTED), OSError("bash not found")] + with patch.object(req.host, "arun", side_effect=procs): + assert await req.check() is True + + async def test_check_without_shipped_module_is_presence_only(self, req): + req.SHIPPED_PP.unlink() + with patch.object(req.host, "arun", side_effect=[_proc(0, LISTED)]): + assert await req.check() is True class TestSELinuxModuleFixCmd: From 2ec984afdaeddde238bda250e17b3d32ffe57df0 Mon Sep 17 00:00:00 2001 From: Zack Cerza Date: Wed, 21 Oct 2026 14:26:41 -0600 Subject: [PATCH 5/9] doctor: Check kernel keyring quota sysctls Every nested container gets a session keyring charged to the rootless user. The default quota (200 keys, 20000 bytes) ran out with four concurrent orch:cephadm:smoke-small jobs, failing cephadm commands at random with "crun: join keyctl ...: Disk quota exceeded". Require kernel.keys.maxkeys >= 20000 and kernel.keys.maxbytes >= 2000000. Signed-off-by: Zack Cerza --- README.md | 2 ++ ceph_devstack/requirements.py | 5 +++++ tests/test_requirements_core.py | 3 +++ 3 files changed, 10 insertions(+) diff --git a/README.md b/README.md index 26a53833..9fe5657c 100644 --- a/README.md +++ b/README.md @@ -37,6 +37,8 @@ It is currently under active development and has not yet had a formal release. * The following sysctl settings: * `fs.aio-max-nr=1048576` * `kernel.pid_max=4194304` + * `kernel.keys.maxkeys=20000` and `kernel.keys.maxbytes=2000000`, so that + nested containers do not exhaust the rootless user's keyring quota * If using SELinux in enforcing mode: * `setsebool -P container_manage_cgroup=true` * `setsebool -P container_use_devices=true` diff --git a/ceph_devstack/requirements.py b/ceph_devstack/requirements.py index 1291dbea..5fbaf8c7 100644 --- a/ceph_devstack/requirements.py +++ b/ceph_devstack/requirements.py @@ -296,5 +296,10 @@ async def check_requirements(): # sysctl settings for OSD result = result and await SysctlValue("fs.aio-max-nr", 2097152).evaluate() result = result and await SysctlValue("kernel.pid_max", 4194304).evaluate() + # Every nested container gets a session keyring charged to the rootless + # user; the defaults (200 keys, 20000 bytes) run out with a few + # cephadm clusters ("crun: join keyctl ...: Disk quota exceeded"). + result = result and await SysctlValue("kernel.keys.maxkeys", 20000).evaluate() + result = result and await SysctlValue("kernel.keys.maxbytes", 2000000).evaluate() return result diff --git a/tests/test_requirements_core.py b/tests/test_requirements_core.py index 0d1a325a..63937706 100644 --- a/tests/test_requirements_core.py +++ b/tests/test_requirements_core.py @@ -414,6 +414,9 @@ async def test_check_requirements_returns_true_when_all_pass(self): MockSysctl.return_value = AsyncMock(evaluate=AsyncMock(return_value=True)) result = await requirements.check_requirements() assert result is True + checked = [c.args for c in MockSysctl.call_args_list] + assert ("kernel.keys.maxkeys", 20000) in checked + assert ("kernel.keys.maxbytes", 2000000) in checked async def test_check_requirements_returns_false_on_runtime_failure(self): with ( From 030e9788ea3fbf771ecb48685360afdfd49466f3 Mon Sep 17 00:00:00 2001 From: Zack Cerza Date: Wed, 21 Oct 2026 14:27:57 -0600 Subject: [PATCH 6/9] TestNode: Mount /run/udev from the host cephadm reads device properties from the udev database. Mount it read-only, and allow that in the SELinux policy. Signed-off-by: Zack Cerza --- ceph_devstack/ceph_devstack.pp | Bin 14537 -> 14967 bytes ceph_devstack/ceph_devstack.te | 10 ++++++++-- ceph_devstack/resources/ceph/containers.py | 2 ++ tests/resources/ceph/test_testnode.py | 5 +++++ 4 files changed, 15 insertions(+), 2 deletions(-) diff --git a/ceph_devstack/ceph_devstack.pp b/ceph_devstack/ceph_devstack.pp index 04433ecef86d4a68b9b12e652fea257810972f09..16b6045d68b24a345564fc6db387238f655b3966 100644 GIT binary patch delta 452 zcmX?E_`O7^|9{O-Mg|53CLk68VppY!Li5~MfGiLOfwat=RCXW@Qk9-sl2}qw1mc1; z{6r4(3lVN1qoG-Q*qF+zaXL1(@&tyGu3r2y-@gmZb zgScEkUSSZJ{9aU=k$1AZxI0khKbQDqK1q(rDVze6b(wi4?~)Xpyj)V9QGW7y$$m!u z$?;O(lT>W*?}~OpPOHrSCXGM@!MG` zWF^J<$=UHBb*vzDo8K@tGxDkcnIOyo#N3lFGD~ls&Rog7*^Ye!6Qkzl_nh7=ldmuf zOzz`0om|h?3}bJ0=ND!IbL<4nHZKqqf^g!5%{TuP_GARh{Nps49LKFNSx>-ia)6i| zSPZCO@>@|^FuP7HWU`ic#AYpVABgUMoII2DBsC`2$%sroCn+%b2&cg0IB|i=e$u>? z&85{D`6oLtOHcM^<^khL(lL|eWF#j00kNEn2N0`F7GU9k%fr+mXpzYuWb8KU$nr5V Qs%*BF^I_V&RN;~&0DIJ6C;$Ke diff --git a/ceph_devstack/ceph_devstack.te b/ceph_devstack/ceph_devstack.te index f661669b..7439fd2a 100644 --- a/ceph_devstack/ceph_devstack.te +++ b/ceph_devstack/ceph_devstack.te @@ -22,7 +22,7 @@ require { class dir mounton; type proc_kcore_t; - class file mounton; + class file { getattr mounton open read }; type device_t; class filesystem remount; @@ -78,8 +78,10 @@ require { type tun_tap_device_t; - class sock_file write; + class sock_file { getattr write }; class unix_stream_socket connectto; + + type udev_var_run_t; } #============= container_init_t ============== @@ -125,3 +127,7 @@ allow container_init_t tun_tap_device_t:chr_file mounton; allow container_t user_tmp_t:sock_file write; allow container_t unconfined_t:unix_stream_socket connectto; + +allow container_init_t udev_var_run_t:dir { ioctl mounton read }; +allow container_init_t udev_var_run_t:file { getattr open read }; +allow container_init_t udev_var_run_t:sock_file getattr; diff --git a/ceph_devstack/resources/ceph/containers.py b/ceph_devstack/resources/ceph/containers.py index 934afa37..a03bec17 100644 --- a/ceph_devstack/resources/ceph/containers.py +++ b/ceph_devstack/resources/ceph/containers.py @@ -218,6 +218,8 @@ def create_cmd(self): "/dev/fuse:/dev/fuse", "-v", "/dev/disk:/dev/disk", + "-v", + "/run/udev:/run/udev:ro", *self._get_available_dmi_mounts(), *self.additional_volumes, "--device", diff --git a/tests/resources/ceph/test_testnode.py b/tests/resources/ceph/test_testnode.py index 69271fb1..24bf115a 100644 --- a/tests/resources/ceph/test_testnode.py +++ b/tests/resources/ceph/test_testnode.py @@ -26,6 +26,11 @@ def test_testnode_create_cmd_includes_related_devices(self, cls): assert "--device=/dev/loop6" in create_cmd assert "--device=/dev/loop7" in create_cmd + def test_testnode_create_cmd_mounts_run_udev_read_only(self, cls): + create_cmd = cls("testnode_1").create_cmd + index = create_cmd.index("/run/udev:/run/udev:ro") + assert create_cmd[index - 1] == "-v" + def test_testnode_devices_is_based_on_loop_device_count_config(self, cls): config.load(Path(__file__).parent.joinpath("fixtures", "testnode-config.toml")) testnode = cls("testnode_1") From 7433d4b778099babf14610f841a1851a825135f6 Mon Sep 17 00:00:00 2001 From: Zack Cerza Date: Wed, 21 Oct 2026 14:28:07 -0600 Subject: [PATCH 7/9] TestNode: Unmask /proc for nested containers With podman 5.8 and crun 1.29 on the host, containers that cephadm started inside a testnode failed with "crun: mount `tmpfs` to `proc/acpi`: Permission denied". Unmasking /proc in the testnode lets nested containers mask it themselves. Signed-off-by: Zack Cerza --- ceph_devstack/resources/ceph/containers.py | 5 +++++ tests/resources/ceph/test_testnode.py | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/ceph_devstack/resources/ceph/containers.py b/ceph_devstack/resources/ceph/containers.py index a03bec17..292142db 100644 --- a/ceph_devstack/resources/ceph/containers.py +++ b/ceph_devstack/resources/ceph/containers.py @@ -210,6 +210,11 @@ def create_cmd(self): ",".join(self.capabilities), "--security-opt", "unmask=/sys/dev/block", + # With this container's /proc paths masked, crun inside it fails + # to mask them for nested containers (e.g. "mount `tmpfs` to + # `proc/acpi`: Permission denied", seen with podman 5.8/crun 1.29). + "--security-opt", + "unmask=/proc/*", "-v", "/sys/dev/block:/sys/dev/block", "-v", diff --git a/tests/resources/ceph/test_testnode.py b/tests/resources/ceph/test_testnode.py index 24bf115a..6971afc5 100644 --- a/tests/resources/ceph/test_testnode.py +++ b/tests/resources/ceph/test_testnode.py @@ -31,6 +31,11 @@ def test_testnode_create_cmd_mounts_run_udev_read_only(self, cls): index = create_cmd.index("/run/udev:/run/udev:ro") assert create_cmd[index - 1] == "-v" + def test_testnode_create_cmd_unmasks_proc_for_nested_containers(self, cls): + create_cmd = cls("testnode_1").create_cmd + index = create_cmd.index("unmask=/proc/*") + assert create_cmd[index - 1] == "--security-opt" + def test_testnode_devices_is_based_on_loop_device_count_config(self, cls): config.load(Path(__file__).parent.joinpath("fixtures", "testnode-config.toml")) testnode = cls("testnode_1") From cee3138583dd07a1546e28f3ddcce578ea63bc4b Mon Sep 17 00:00:00 2001 From: Zack Cerza Date: Wed, 21 Oct 2026 14:28:43 -0600 Subject: [PATCH 8/9] Teuthology: Recreate a stopped container on create The teuthology container keeps the environment it was created with, so a later start ignored a newly exported TEUTHOLOGY_SUITE, and failed outright if the SSH agent socket it mounted was gone (as when create and start run in different SSH sessions): crun: cannot stat `/tmp/ssh-XXXX.../agent.NNN`: No such file or directory A stopped teuthology container holds nothing the archive doesn't, so replace it unless it is running. Also check for the agent socket on the container host rather than the local machine. Signed-off-by: Zack Cerza --- README.md | 4 ++ ceph_devstack/resources/ceph/containers.py | 11 +++- tests/resources/ceph/test_teuthology.py | 62 ++++++++++++++++++++++ 3 files changed, 75 insertions(+), 2 deletions(-) create mode 100644 tests/resources/ceph/test_teuthology.py diff --git a/README.md b/README.md index 9fe5657c..488d973c 100644 --- a/README.md +++ b/README.md @@ -128,6 +128,10 @@ It's possible to skip the automatic suite-scheduling behavior: export TEUTHOLOGY_SUITE=none ``` +These variables, like `SSH_AUTH_SOCK`, are read when the teuthology container +is created. Unless it is still running, `create` and `start` replace it, so +the environment of the current shell applies. + ### Using testnodes from an existing lab If you need to use "real" testnodes and have access to a lab, there are a few additonal steps to take. We will use the Sepia lab as an example below: diff --git a/ceph_devstack/resources/ceph/containers.py b/ceph_devstack/resources/ceph/containers.py index 292142db..1d3807c7 100644 --- a/ceph_devstack/resources/ceph/containers.py +++ b/ceph_devstack/resources/ceph/containers.py @@ -4,7 +4,7 @@ from pathlib import Path from typing import List -from ceph_devstack import config, DEFAULT_CONFIG_PATH +from ceph_devstack import config, DEFAULT_CONFIG_PATH, logger from ceph_devstack.host import host from ceph_devstack.resources.container import Container @@ -383,7 +383,7 @@ def create_cmd(self): f"{ansible_inv}/secrets:/etc/ansible/secrets", ] ssh_auth_socket = os.environ.get("SSH_AUTH_SOCK") - if ssh_auth_socket and Path(ssh_auth_socket).exists(): + if ssh_auth_socket and host.path_exists(ssh_auth_socket): cmd += [ "-v", f"{ssh_auth_socket}:{ssh_auth_socket}", @@ -431,4 +431,11 @@ def archive_dir(self) -> Path: async def create(self): self.archive_dir.expanduser().resolve().mkdir(parents=True, exist_ok=True) + # The container keeps the environment it was created with: the + # TEUTHOLOGY_* settings and an SSH agent socket that may be gone by + # its next start. A stopped one holds nothing the archive does not, + # so replace it with one built from the current environment. + if await self.exists() and not await self.is_running(): + logger.info(f"{self.name}: recreating from the current environment") + await self.remove() await super().create() diff --git a/tests/resources/ceph/test_teuthology.py b/tests/resources/ceph/test_teuthology.py new file mode 100644 index 00000000..69674e6c --- /dev/null +++ b/tests/resources/ceph/test_teuthology.py @@ -0,0 +1,62 @@ +from unittest.mock import AsyncMock, patch + +import pytest + +from ceph_devstack import config +from ceph_devstack.resources.ceph.containers import Teuthology + + +class TestTeuthologyCreate: + @pytest.fixture + def teuth(self, tmp_path): + config["data_dir"] = str(tmp_path) + teuth = Teuthology() + teuth.remove = AsyncMock() + return teuth + + @pytest.fixture + def podman_create(self): + with patch( + "ceph_devstack.resources.container.Container.create", new=AsyncMock() + ) as podman_create: + yield podman_create + + @pytest.mark.parametrize( + "exists,running,replaced", + [ + (False, False, False), + (True, True, False), + (True, False, True), + ], + ids=["missing", "running", "stopped"], + ) + async def test_replaces_only_a_stopped_container( + self, teuth, podman_create, exists, running, replaced + ): + teuth.exists = AsyncMock(return_value=exists) + teuth.is_running = AsyncMock(return_value=running) + await teuth.create() + assert teuth.remove.await_count == int(replaced) + podman_create.assert_awaited_once() + assert teuth.archive_dir.is_dir() + + +class TestTeuthologySSHAuthSock: + SOCK = "/tmp/ssh-XXXX/agent.123" + + def create_cmd(self, monkeypatch, sock_exists: bool) -> list[str]: + monkeypatch.setenv("SSH_AUTH_SOCK", self.SOCK) + with patch( + "ceph_devstack.resources.ceph.containers.host.path_exists", + side_effect=lambda path: sock_exists and str(path) == self.SOCK, + ): + return Teuthology().create_cmd + + def test_mounts_socket_present_on_container_host(self, monkeypatch): + cmd = self.create_cmd(monkeypatch, sock_exists=True) + assert f"{self.SOCK}:{self.SOCK}" in cmd + assert f"SSH_AUTH_SOCK={self.SOCK}" in cmd + + def test_skips_socket_missing_on_container_host(self, monkeypatch): + cmd = self.create_cmd(monkeypatch, sock_exists=False) + assert not any("SSH_AUTH_SOCK" in arg or self.SOCK in arg for arg in cmd) From d5c550e0e28813692149d03afcab34cb7d139ad4 Mon Sep 17 00:00:00 2001 From: Zack Cerza Date: Wed, 21 Oct 2026 14:28:56 -0600 Subject: [PATCH 9/9] Don't restart teuthology in watch mode Teuthology is expected to exit when its run completes, so watch mode should not restart it. Signed-off-by: Zack Cerza --- ceph_devstack/resources/ceph/__init__.py | 9 +++++-- .../resources/ceph/test_cephdevstack_core.py | 24 +++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/ceph_devstack/resources/ceph/__init__.py b/ceph_devstack/resources/ceph/__init__.py index 17cc5641..5864c1b6 100644 --- a/ceph_devstack/resources/ceph/__init__.py +++ b/ceph_devstack/resources/ceph/__init__.py @@ -196,7 +196,10 @@ async def remove(self): await SSHKeyPair().remove() async def watch(self): - logger.info("Watching containers; will replace any that are stopped") + logger.info( + "Watching containers; will replace any that are removed and, " + "except for teuthology, restart any that are stopped" + ) containers = [] for spec in self.service_specs.values(): if not spec["count"] > 0: @@ -213,7 +216,9 @@ async def watch(self): ) await container.create() await container.start() - elif not await container.is_running(): + elif not await container.is_running() and not isinstance( + container, Teuthology + ): logger.info(f"Container {container.name} stopped; restarting") await container.start() diff --git a/tests/resources/ceph/test_cephdevstack_core.py b/tests/resources/ceph/test_cephdevstack_core.py index d30a1625..fa22590e 100644 --- a/tests/resources/ceph/test_cephdevstack_core.py +++ b/tests/resources/ceph/test_cephdevstack_core.py @@ -287,6 +287,30 @@ async def test_stop_calls_stop_on_all_containers(self): mock_paddles.stop.assert_called_once() +class _StopWatching(Exception): + pass + + +class TestCephDevStackWatch: + async def test_restarts_stopped_containers_except_teuthology(self): + devstack = CephDevStack() + teuth = AsyncMock(spec=Teuthology) + paddles = AsyncMock(spec=Paddles) + for obj, name in ((teuth, "teuthology"), (paddles, "paddles")): + obj.name = name + obj.exists.return_value = True + obj.is_running.return_value = False + paddles.start.side_effect = _StopWatching + devstack.service_specs = { + "teuthology": {"count": 1, "objects": [teuth]}, + "paddles": {"count": 1, "objects": [paddles]}, + } + with pytest.raises(_StopWatching): + await devstack.watch() + teuth.start.assert_not_awaited() + paddles.start.assert_awaited_once() + + class TestCephDevStackWait: async def test_wait_returns_process_id(self): devstack = CephDevStack()