From 651fdd6c7194fad8d0592abd6b3e487874425286 Mon Sep 17 00:00:00 2001 From: Parfait Detchenou Date: Mon, 18 Aug 2025 16:52:33 +0100 Subject: [PATCH 1/4] testnode: fix conflicts with rebase on main --- ceph_devstack/resources/ceph/containers.py | 168 ++++++++++++--------- 1 file changed, 99 insertions(+), 69 deletions(-) diff --git a/ceph_devstack/resources/ceph/containers.py b/ceph_devstack/resources/ceph/containers.py index 5cd626a6..6ae1d754 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 +from ceph_devstack import config, DEFAULT_CONFIG_PATH from ceph_devstack.host import host from ceph_devstack.resources.container import Container @@ -154,7 +154,7 @@ class Pulpito(Container): class TestNode(Container): - cmd_vars: List[str] = ["name", "image", "loop_dev_name"] + cmd_vars: List[str] = ["name", "image"] capabilities = [ "SYS_ADMIN", "NET_ADMIN", @@ -170,49 +170,6 @@ class TestNode(Container): "AUDIT_WRITE", "AUDIT_CONTROL", ] - create_cmd = [ - "podman", - "container", - "create", - "--rm", - "-i", - "--network", - "ceph-devstack", - "--systemd=always", - "--cgroupns=host", - "--secret", - "id_rsa.pub", - "-p", - "22", - "--cap-add", - ",".join(capabilities), - "--security-opt", - "unmask=/sys/dev/block", - "-v", - "/sys/dev/block:/sys/dev/block", - "-v", - "/sys/fs/cgroup:/sys/fs/cgroup", - "-v", - "/dev/fuse:/dev/fuse", - "-v", - "/dev/disk:/dev/disk", - # cephadm tries to access these DMI-related files, and by default they - # have 600 permissions on the host. It appears to be ok if they are - # empty, though. - "-v", - "/dev/null:/sys/class/dmi/id/board_serial", - "-v", - "/dev/null:/sys/class/dmi/id/chassis_serial", - "-v", - "/dev/null:/sys/class/dmi/id/product_serial", - "--device", - "/dev/net/tun", - "--device", - "{loop_dev_name}", - "--name", - "{name}", - "{image}", - ] env_vars = { "SSH_PUBKEY": "", "CEPH_VOLUME_ALLOW_LOOP_DEVICES": "true", @@ -220,49 +177,118 @@ class TestNode(Container): def __init__(self, name: str = ""): super().__init__(name=name) - self.loop_index = 0 - self.loop_img_name = self.name + self.index = 0 if "_" in self.name: - self.loop_index = int(self.name.split("_")[-1]) - else: - self.loop_img_name += str(self.loop_index) - self.loop_dev_name = f"/dev/loop{self.loop_index}" + self.index = int(self.name.split("_")[-1]) + self.osd_count = config["containers"]["testnode"].get("osd_count", 1) + self.devices = [self.device_name(i) for i in range(self.osd_count)] @property def loop_img_dir(self): return (Path(config["data_dir"]) / "disk_images").expanduser() + @property + def create_cmd(self): + return [ + "podman", + "container", + "create", + "--rm", + "-i", + "--network", + "ceph-devstack", + "--systemd=always", + "--cgroupns=host", + "--secret", + "id_rsa.pub", + "-p", + "22", + "--cap-add", + ",".join(self.capabilities), + "--security-opt", + "unmask=/sys/dev/block", + "-v", + "/sys/dev/block:/sys/dev/block", + "-v", + "/sys/fs/cgroup:/sys/fs/cgroup", + "-v", + "/dev/fuse:/dev/fuse", + "-v", + "/dev/disk:/dev/disk", + # cephadm tries to access these DMI-related files, and by default they + # have 600 permissions on the host. It appears to be ok if they are + # empty, though. + # The below was bizarrely causing this error message: + # No such file or directory: OCI runtime attempted to invoke a command that was + # not found + # That was causing the container to fail to start up. + "-v", + "/dev/null:/sys/class/dmi/id/board_serial", + "-v", + "/dev/null:/sys/class/dmi/id/chassis_serial", + "-v", + "/dev/null:/sys/class/dmi/id/product_serial", + *self.additional_volumes, + "--device", + "/dev/net/tun", + *[f"--device={device}" for device in self.devices], + "--name", + "{name}", + "{image}", + ] + + @property + def additional_volumes(self): + volumes = [] + if DEFAULT_CONFIG_PATH.parent.joinpath("sshd_config").exists(): + volumes.extend( + [ + "-v", + f"{DEFAULT_CONFIG_PATH.parent.joinpath('sshd_config')}:/etc/ssh/sshd_config.d/teuthology.conf:z", + ] + ) + return volumes + async def create(self): if not await self.exists(): - await self.create_loop_device() + await self.create_loop_devices() await super().create() async def remove(self): await super().remove() - await self.remove_loop_device() + await self.remove_loop_devices() - async def create_loop_device(self): + async def create_loop_devices(self): + for device in self.devices: + await self.create_loop_device(device) + + async def remove_loop_devices(self): + for device in self.devices: + await self.remove_loop_device(device) + + async def create_loop_device(self, device: str): size = config["containers"]["testnode"]["loop_device_size"] os.makedirs(self.loop_img_dir, exist_ok=True) proc = await self.cmd(["lsmod", "|", "grep", "loop"]) if proc and await proc.wait() != 0: await self.cmd(["sudo", "modprobe", "loop"]) - loop_img_name = os.path.join(self.loop_img_dir, self.loop_img_name) - await self.remove_loop_device() + loop_img_name = os.path.join(self.loop_img_dir, self.device_image(device)) + await self.remove_loop_device(device) + device_pos = device.lstrip("/dev/loop") await self.cmd( [ "sudo", "mknod", "-m700", - self.loop_dev_name, + device, "b", "7", - str(self.loop_index), + device_pos, ], check=True, ) await self.cmd( - ["sudo", "chown", f"{os.getuid()}:{os.getgid()}", self.loop_dev_name], + ["sudo", "chown", f"{os.getuid()}:{os.getgid()}", device], check=True, ) await self.cmd( @@ -277,20 +303,24 @@ async def create_loop_device(self): ], check=True, ) - await self.cmd( - ["sudo", "losetup", self.loop_dev_name, loop_img_name], check=True - ) + await self.cmd(["sudo", "losetup", device, loop_img_name], check=True) - async def remove_loop_device(self): - loop_img_name = os.path.join(self.loop_img_dir, self.loop_img_name) - if os.path.ismount(self.loop_dev_name): - await self.cmd(["umount", self.loop_dev_name], check=True) - if host.path_exists(self.loop_dev_name): - await self.cmd(["sudo", "losetup", "-d", self.loop_dev_name]) - await self.cmd(["sudo", "rm", "-f", self.loop_dev_name], check=True) + async def remove_loop_device(self, device: str): + loop_img_name = os.path.join(self.loop_img_dir, self.device_image(device)) + if os.path.ismount(device): + await self.cmd(["umount", device], check=True) + if host.path_exists(device): + await self.cmd(["sudo", "losetup", "-d", device]) + await self.cmd(["sudo", "rm", "-f", device], check=True) if host.path_exists(loop_img_name): os.remove(loop_img_name) + def device_name(self, index: int): + return f"/dev/loop{self.osd_count * self.index + index}" + + def device_image(self, device: str): + return f"{self.name}-{device.lstrip('/dev/loop')}" + class Teuthology(Container): cmd_vars: List[str] = ["name", "image", "image_tag", "archive_dir"] From bf6b5820d12e4e1f54bc81c86d737c1ad26da07b Mon Sep 17 00:00:00 2001 From: Parfait Detchenou Date: Mon, 18 Aug 2025 23:55:03 +0100 Subject: [PATCH 2/4] loop devices: requires that all devices have correct label --- ceph_devstack/resources/ceph/containers.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/ceph_devstack/resources/ceph/containers.py b/ceph_devstack/resources/ceph/containers.py index 6ae1d754..6b2428b6 100644 --- a/ceph_devstack/resources/ceph/containers.py +++ b/ceph_devstack/resources/ceph/containers.py @@ -240,11 +240,15 @@ def create_cmd(self): @property def additional_volumes(self): volumes = [] - if DEFAULT_CONFIG_PATH.parent.joinpath("sshd_config").exists(): + if ( + sshd_config := DEFAULT_CONFIG_PATH.parent.joinpath( + "sshd_config" + ).expanduser() + ) and sshd_config.exists(): volumes.extend( [ "-v", - f"{DEFAULT_CONFIG_PATH.parent.joinpath('sshd_config')}:/etc/ssh/sshd_config.d/teuthology.conf:z", + f"{sshd_config}:/etc/ssh/sshd_config.d/teuthology.conf:z", ] ) return volumes @@ -304,6 +308,7 @@ async def create_loop_device(self, device: str): check=True, ) await self.cmd(["sudo", "losetup", device, loop_img_name], check=True) + await self.cmd(["chcon", "-t", "fixed_disk_device_t", device]) async def remove_loop_device(self, device: str): loop_img_name = os.path.join(self.loop_img_dir, self.device_image(device)) From d8c380c1078323ec996db6d4a7f3c2b344dad192 Mon Sep 17 00:00:00 2001 From: Parfait Detchenou Date: Fri, 22 Aug 2025 16:31:27 +0100 Subject: [PATCH 3/4] testnode: add tests for loop_device_count config --- ceph_devstack/resources/ceph/containers.py | 8 +++-- .../test/fixtures/testnode-config.toml | 2 ++ ceph_devstack/resources/test/test_testnode.py | 35 +++++++++++++++++++ 3 files changed, 42 insertions(+), 3 deletions(-) create mode 100644 ceph_devstack/resources/test/fixtures/testnode-config.toml create mode 100644 ceph_devstack/resources/test/test_testnode.py diff --git a/ceph_devstack/resources/ceph/containers.py b/ceph_devstack/resources/ceph/containers.py index 6b2428b6..3d2525b4 100644 --- a/ceph_devstack/resources/ceph/containers.py +++ b/ceph_devstack/resources/ceph/containers.py @@ -180,8 +180,10 @@ def __init__(self, name: str = ""): self.index = 0 if "_" in self.name: self.index = int(self.name.split("_")[-1]) - self.osd_count = config["containers"]["testnode"].get("osd_count", 1) - self.devices = [self.device_name(i) for i in range(self.osd_count)] + self.loop_device_count = config["containers"]["testnode"].get( + "loop_device_count", 1 + ) + self.devices = [self.device_name(i) for i in range(self.loop_device_count)] @property def loop_img_dir(self): @@ -321,7 +323,7 @@ async def remove_loop_device(self, device: str): os.remove(loop_img_name) def device_name(self, index: int): - return f"/dev/loop{self.osd_count * self.index + index}" + return f"/dev/loop{self.loop_device_count * self.index + index}" def device_image(self, device: str): return f"{self.name}-{device.lstrip('/dev/loop')}" diff --git a/ceph_devstack/resources/test/fixtures/testnode-config.toml b/ceph_devstack/resources/test/fixtures/testnode-config.toml new file mode 100644 index 00000000..0a53eecc --- /dev/null +++ b/ceph_devstack/resources/test/fixtures/testnode-config.toml @@ -0,0 +1,2 @@ +[containers.testnode] +loop_device_count = 4 diff --git a/ceph_devstack/resources/test/test_testnode.py b/ceph_devstack/resources/test/test_testnode.py new file mode 100644 index 00000000..62fe9d30 --- /dev/null +++ b/ceph_devstack/resources/test/test_testnode.py @@ -0,0 +1,35 @@ +from pathlib import Path + +import pytest + +from ceph_devstack.resources.ceph import TestNode +from ceph_devstack import config + + +class TestTestnode: + @pytest.fixture(scope="class") + def cls(self) -> type[TestNode]: + return TestNode + + def test_testnode_loop_device_count_default_to_one(self, cls): + testnode = cls("testnode_1") + assert testnode.loop_device_count == 1 + + def test_testnode_create_cmd_includes_related_devices(self, cls): + config.load(Path(__file__).parent.joinpath("fixtures", "testnode-config.toml")) + testnode = cls("testnode_1") + create_cmd = testnode.create_cmd + assert "--device=/dev/loop4" in create_cmd + assert "--device=/dev/loop5" in create_cmd + assert "--device=/dev/loop6" in create_cmd + assert "--device=/dev/loop7" in create_cmd + + def test_testnode_devices_is_based_on_loop_device_count_config(self, cls): + testnode = cls("testnode_1") + assert testnode.loop_device_count == 4 + assert testnode.devices == [ + "/dev/loop4", + "/dev/loop5", + "/dev/loop6", + "/dev/loop7", + ] From 0cb5fa10f11aefc32dc17a0c2f26535d00d0d30d Mon Sep 17 00:00:00 2001 From: Parfait Detchenou Date: Thu, 28 Aug 2025 08:53:35 +0100 Subject: [PATCH 4/4] testnode: replace lstrip by removeprefix --- ceph_devstack/resources/ceph/containers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ceph_devstack/resources/ceph/containers.py b/ceph_devstack/resources/ceph/containers.py index 3d2525b4..dab877a9 100644 --- a/ceph_devstack/resources/ceph/containers.py +++ b/ceph_devstack/resources/ceph/containers.py @@ -280,7 +280,7 @@ async def create_loop_device(self, device: str): await self.cmd(["sudo", "modprobe", "loop"]) loop_img_name = os.path.join(self.loop_img_dir, self.device_image(device)) await self.remove_loop_device(device) - device_pos = device.lstrip("/dev/loop") + device_pos = device.removeprefix("/dev/loop") await self.cmd( [ "sudo", @@ -326,7 +326,7 @@ def device_name(self, index: int): return f"/dev/loop{self.loop_device_count * self.index + index}" def device_image(self, device: str): - return f"{self.name}-{device.lstrip('/dev/loop')}" + return f"{self.name}-{device.removeprefix('/dev/loop')}" class Teuthology(Container):