Prep and fixes

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`
@@ -126,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:
 
@@ -192,4 +198,4 @@ ceph-devstack -v start
 ```bash
 ceph-devstack wait teuthology
 podman logs -f teuthology
-```
+```

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;

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__(

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 '<cmd>'" stay as one shell word
+        remote.extend(shlex.quote(arg) for arg in args)
         return remote
 
     def cmd(

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

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

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()
 

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
 
@@ -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",
@@ -218,6 +223,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",
@@ -376,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}",
@@ -424,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()

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,48 +48,71 @@ 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):
         proc = await self.host.arun(["sudo", "semodule", "-l"])
         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

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()