From 217f0e85fe07686078cee9f26cb1bdc47bb54ec6 Mon Sep 17 00:00:00 2001 From: Zack Cerza Date: Wed, 21 Oct 2026 14:31:36 -0600 Subject: [PATCH 1/2] Let resources take data_dir and containers config Add a StackResource protocol describing what CephDevStack needs from a service, and let PodmanResource and Container take data_dir and containers_config, so a stack can give each service its own on-disk state and its own view of the [containers] config. Signed-off-by: Zack Cerza --- ceph_devstack/resources/__init__.py | 56 +++++++++++++++++++++++++- ceph_devstack/resources/container.py | 8 ++-- tests/resources/test_container.py | 5 +++ tests/resources/test_podmanresource.py | 5 +++ 4 files changed, 68 insertions(+), 6 deletions(-) diff --git a/ceph_devstack/resources/__init__.py b/ceph_devstack/resources/__init__.py index b9607191..39bed8ae 100644 --- a/ceph_devstack/resources/__init__.py +++ b/ceph_devstack/resources/__init__.py @@ -6,12 +6,42 @@ from pathlib import Path from subprocess import CalledProcessError -from typing import List, Dict, Set +from typing import List, Dict, Optional, Protocol, Set +from ceph_devstack import config from ceph_devstack.exec import Subprocess from ceph_devstack.host import host, local_host +class StackResource(Protocol): + """Interface that CephDevStack expects from every service it manages.""" + + def __init__( + self, name: str = "", *, data_dir: Path, containers_config: dict + ) -> None: ... + + @property + def name(self) -> str: ... + + async def pull(self) -> None: ... + + async def build(self) -> None: ... + + async def create(self) -> None: ... + + async def start(self) -> None: ... + + async def stop(self) -> None: ... + + async def remove(self) -> None: ... + + async def exists(self) -> bool: ... + + async def is_running(self) -> bool: ... + + async def wait(self) -> int | None: ... + + class DevStack: def __init__(self, args: argparse.Namespace): self.args = args @@ -40,9 +70,17 @@ class PodmanResource: log: Dict[str, Set[str]] = {} _name: str | None = None - def __init__(self, name: str = ""): + def __init__( + self, + name: str = "", + *, + data_dir: Optional[Path] = None, + containers_config: Optional[dict] = None, + ): if name: self._name = name + self._data_dir = data_dir + self._containers_config = containers_config @property def name(self) -> str: @@ -50,6 +88,20 @@ def name(self) -> str: return self._name return self.__class__.__name__.lower() + @property + def data_dir(self) -> Path: + """Where this resource keeps its on-disk state; set by its stack.""" + if self._data_dir is None: + raise ValueError(f"{self.name}: no data_dir was given") + return self._data_dir + + @property + def containers_config(self) -> dict: + """The stack's merged [containers] config, else the global one.""" + if self._containers_config is None: + return config["containers"] + return self._containers_config + async def cmd( self, args: List[str], diff --git a/ceph_devstack/resources/container.py b/ceph_devstack/resources/container.py index 49662b3c..15fb53e3 100644 --- a/ceph_devstack/resources/container.py +++ b/ceph_devstack/resources/container.py @@ -4,7 +4,7 @@ from typing import Dict, List, Optional -from ceph_devstack import config, logger +from ceph_devstack import logger from ceph_devstack.resources import PodmanResource @@ -29,8 +29,8 @@ class Container(PodmanResource): env_vars: Dict[str, Optional[str]] = {} _image_name: str | None = None - def __init__(self, name: str = ""): - super().__init__(name) + def __init__(self, name: str = "", **kwargs): + super().__init__(name, **kwargs) self.env_vars = {**self.__class__.env_vars} for key in self.env_vars: if os.environ.get(key): @@ -47,7 +47,7 @@ def add_env_to_args(self, args: List): @property def config(self): - return config["containers"].get(self.__class__.__name__.lower(), {}) + return self.containers_config.get(self.__class__.__name__.lower(), {}) @property def image_name(self) -> str: diff --git a/tests/resources/test_container.py b/tests/resources/test_container.py index 3ac95c6b..29d5f5ed 100644 --- a/tests/resources/test_container.py +++ b/tests/resources/test_container.py @@ -32,6 +32,11 @@ def cls(self): def action(self, request): return request.param + def test_containers_config_overrides_global_config(self, cls): + obj = cls(containers_config={"container": {"image": "stack/image:v1"}}) + assert obj.image == "stack/image:v1" + assert cls().image == "example.com/image:latest" + async def test_action_calls_cmd_with_correct_args(self, cls, action): if action == "build": config["containers"][cls.__name__.lower()]["repo"] = "/repo_path" diff --git a/tests/resources/test_podmanresource.py b/tests/resources/test_podmanresource.py index 2d181cdd..4504bbbb 100644 --- a/tests/resources/test_podmanresource.py +++ b/tests/resources/test_podmanresource.py @@ -25,6 +25,11 @@ def test_name(self, cls): obj = cls(name="foo") assert obj.name == "foo" + def test_data_dir_must_be_given(self, cls, tmp_path): + assert cls(data_dir=tmp_path).data_dir == tmp_path + with pytest.raises(ValueError, match="no data_dir"): + _ = cls().data_dir + def test_format_cmd(self, cls): obj = cls(name="pr") assert "name" in obj.cmd_vars From 9508e94c4ef1a52b5c0996a0570af6e4e50aad5c Mon Sep 17 00:00:00 2001 From: Zack Cerza Date: Wed, 21 Oct 2026 14:32:32 -0600 Subject: [PATCH 2/2] Add configurable stacks Stacks are named sets of services defined in config.toml. The existing services become the "teuthology" stack, which stays the default. - Each stack keeps its state in data_dir/ and can override [containers] settings for its own services. Existing archives and disk images directly under data_dir are not moved. - --stack picks a stack, and a stack's depends run before it on start. - Loop allocation accounts for every stack's backing files. - Loop and block-pool requirements are only checked for stacks that deploy testnodes. Signed-off-by: Zack Cerza --- .github/workflows/ci.yml | 2 +- README.md | 5 +- ceph_devstack/__init__.py | 10 + ceph_devstack/cli.py | 77 ++-- ceph_devstack/config.toml | 13 + ceph_devstack/resources/ceph/__init__.py | 215 +++++++----- ceph_devstack/resources/ceph/block_devices.py | 24 ++ ceph_devstack/resources/ceph/containers.py | 51 +-- ceph_devstack/resources/ceph/host_loops.py | 13 +- tests/conftest.py | 16 + .../resources/ceph/test_cephdevstack_core.py | 328 ++++++++---------- tests/resources/ceph/test_devstack.py | 31 +- tests/resources/ceph/test_host_loops.py | 21 ++ .../resources/ceph/test_requirements_ceph.py | 10 +- tests/resources/ceph/test_testnode.py | 21 +- tests/resources/ceph/test_teuthology.py | 16 +- tests/test_config.py | 15 + tests/test_stacks.py | 56 +++ 18 files changed, 561 insertions(+), 363 deletions(-) create mode 100644 tests/test_stacks.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d09cdbca..5f46607d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -79,7 +79,7 @@ jobs: - name: Create tarball of log archive if: success() || failure() run: | - tar -czf /tmp/artifacts/archive.tar ~/.local/share/ceph-devstack/archive/ + tar -czf /tmp/artifacts/archive.tar ~/.local/share/ceph-devstack/teuthology/archive/ - name: Upload log archive if: success() || failure() uses: actions/upload-artifact@v7.0.1 diff --git a/README.md b/README.md index 77b75619..8d5175a1 100644 --- a/README.md +++ b/README.md @@ -105,7 +105,8 @@ pool state file and its lock always stay on the local machine. Each container picks host loop devices at create time: it needs `loop_device_count` devices of `loop_device_size`, reuses any existing backing files for that container name, then takes the lowest free loop numbers after -scanning what is already attached or claimed in the `disk_images` directory. +scanning what is already attached or claimed in any stack's `disk_images` +directory. If `block_pool.json` is lost but the on-disk marker remains, start again to reclaim the marker; the device is not reformatted. The old allocations are @@ -146,7 +147,7 @@ Once everything is started, a message similar to this will be logged: `View test results at http://smithi065.front.sepia.ceph.com:8081/` -This link points to the running Pulpito instance. Test archives are also stored in the `--data-dir` (default: `~/.local/share/ceph-devstack`). +This link points to the running Pulpito instance. Test archives are also stored in `/teuthology/archive` (`data_dir` defaults to `~/.local/share/ceph-devstack`). To watch teuthology's output, you can: diff --git a/ceph_devstack/__init__.py b/ceph_devstack/__init__.py index 71612111..8bdeb99c 100644 --- a/ceph_devstack/__init__.py +++ b/ceph_devstack/__init__.py @@ -42,6 +42,11 @@ def parse_args(args: List[str]) -> argparse.Namespace: default=DEFAULT_CONFIG_PATH, help="Path to the ceph-devstack config file", ) + parser.add_argument( + "--stack", + default=None, + help="Stack to deploy (default: value of 'stack' in config)", + ) subparsers = parser.add_subparsers(dest="command") parser_config = subparsers.add_parser("config", help="Get or set config items") subparsers_config = parser_config.add_subparsers(dest="config_op") @@ -216,3 +221,8 @@ def unset_value(self, name: str) -> None: config = Config() config.load() + + +def base_data_dir() -> Path: + """The configured data_dir; each stack keeps its state in a subdirectory.""" + return Path(config.get("data_dir", DEFAULT_DATA_DIR)).expanduser().absolute() diff --git a/ceph_devstack/cli.py b/ceph_devstack/cli.py index 760b5490..2c34a846 100644 --- a/ceph_devstack/cli.py +++ b/ceph_devstack/cli.py @@ -2,7 +2,8 @@ import logging import sys -from pathlib import Path +from argparse import Namespace +from subprocess import CalledProcessError from ceph_devstack import config, logger, parse_args, VERBOSE from ceph_devstack.block_pool import BlockPoolError, format_size, shared_pool @@ -16,22 +17,6 @@ "unset": lambda config, args: config.unset_value(args.name), } -COMMAND_HANDLERS = { - "doctor": None, - "apply": lambda args, obj: obj.apply(args.command), - "pull": lambda _, obj: obj.pull(), - "build": lambda _, obj: obj.build(), - "create": lambda _, obj: obj.create(), - "remove": lambda _, obj: obj.remove(), - "start": lambda _, obj: obj.start(), - "stop": lambda _, obj: obj.stop(), - "watch": lambda _, obj: obj.watch(), - "wait": lambda args, obj: obj.wait(container_name=args.container), - "logs": lambda args, obj: obj.logs( - run_name=args.run_name, job_id=args.job_id, locate=args.locate - ), -} - def _configure_console_logging(verbose: bool) -> None: if not verbose: @@ -67,6 +52,36 @@ def block_pool_status() -> int: return 0 +def _action_kwargs(args: Namespace) -> dict: + match args.command: + case "wait": + return {"container_name": args.container} + case "logs": + return { + "run_name": args.run_name, + "job_id": args.job_id, + "locate": args.locate, + } + case _: + return {} + + +async def _apply(stack: CephDevStack, action: str, **kwargs) -> int: + """Check requirements, then run the action (if any) on the stack.""" + if not all( + await asyncio.gather( + check_requirements(), + stack.check_requirements(), + ) + ): + logger.error("Requirements not met!") + return 1 + if action == "doctor": + return 0 + result = await stack.apply(action, **kwargs) + return result if result is not None else 0 + + def main() -> int: args = parse_args(sys.argv[1:]) config.load(args.config_file) @@ -80,25 +95,17 @@ def main() -> int: return 2 return block_pool_status() config["args"] = vars(args) - data_path = Path(config["data_dir"]).expanduser() - data_path.mkdir(parents=True, exist_ok=True) - obj = CephDevStack() - - async def run(): - if not all( - await asyncio.gather( - check_requirements(), - obj.check_requirements(), - ) - ): - logger.error("Requirements not met!") - return 1 - handler = COMMAND_HANDLERS.get(args.command) - if handler: - return await handler(args, obj) - try: - sys.exit(asyncio.run(run())) + stack = CephDevStack(stack_name=args.stack) + except ValueError as exc: + logger.error(str(exc)) + return 1 + stack.data_dir.mkdir(parents=True, exist_ok=True) + action = args.command + try: + return asyncio.run(_apply(stack, action, **_action_kwargs(args))) + except CalledProcessError as exc: + return exc.returncode if exc.returncode is not None else 1 except KeyboardInterrupt: logger.debug("Exiting!") return 130 # 128 + SIGINT diff --git a/ceph_devstack/config.toml b/ceph_devstack/config.toml index 41f24b1d..e1c0807e 100644 --- a/ceph_devstack/config.toml +++ b/ceph_devstack/config.toml @@ -1,3 +1,4 @@ +stack = "teuthology" data_dir = "~/.local/share/ceph-devstack" [block_pool] @@ -14,6 +15,18 @@ data_dir = "~/.local/share/ceph-devstack" # allow_enroll = true # state_dir = "/path" # where block_pool.json lives; defaults to data_dir +[stacks.teuthology] +services = [ + "postgres", + "paddles", + "beanstalk", + "pulpito", + "teuthology", + "testnode", + "archive", +] +secrets = ["ssh_keypair"] + [containers.archive] image = "python:alpine" diff --git a/ceph_devstack/resources/ceph/__init__.py b/ceph_devstack/resources/ceph/__init__.py index bbe10451..3da1df35 100644 --- a/ceph_devstack/resources/ceph/__init__.py +++ b/ceph_devstack/resources/ceph/__init__.py @@ -5,9 +5,11 @@ import tempfile from subprocess import CalledProcessError +from typing import Iterator, Type, TypeVar, cast -from ceph_devstack import config, logger +from ceph_devstack import base_data_dir, config, deep_merge, logger from ceph_devstack.host import host +from ceph_devstack.resources import StackResource from ceph_devstack.resources.misc import Secret, Network from ceph_devstack.resources.ceph.containers import ( Postgres, @@ -88,39 +90,88 @@ class CephDevStackNetwork(Network): _name = "ceph-devstack" +SERVICES: dict[str, Type[StackResource]] = { + "postgres": Postgres, + "paddles": Paddles, + "beanstalk": Beanstalk, + "pulpito": Pulpito, + "teuthology": Teuthology, + "testnode": TestNode, + "archive": Archive, +} + +SECRETS = { + "ssh_keypair": SSHKeyPair, +} + + +R = TypeVar("R", bound=StackResource) + + class CephDevStack: - networks = [CephDevStackNetwork] - secrets = [SSHKeyPair] - - def __init__(self): - services = [ - Postgres, - Paddles, - Beanstalk, - Pulpito, - Teuthology, - TestNode, - Archive, - ] + def __init__(self, stack_name: str | None = None): + if stack_name is None: + stack_name = config.get("stack", "teuthology") + stacks = config.get("stacks", {}) + if stack_name not in stacks: + known = ", ".join(sorted(stacks)) or "(none)" + raise ValueError(f"Unknown stack {stack_name!r}; known stacks: {known}") + + self.stack_name = stack_name + self.stack_def = stacks[stack_name] + self.active_services = list(self.stack_def.get("services", [])) + self.data_dir = base_data_dir() / stack_name + # The stack's own view of [containers]; the global config is untouched + # so that other stacks (e.g. dependencies) cannot leak into this one. + self.containers_config = deep_merge( + config.get("containers", {}), self.stack_def.get("containers", {}) + ) + self.service_specs = {} - for service in services: - name = service.__name__.lower() - count = config["containers"][name].get("count", 1) + for name in self.active_services: + service = SERVICES.get(name) + if service is None: + logger.warning(f"Unknown service {name!r} in stack {self.stack_name!r}") + continue + count = self.containers_config.get(name, {}).get("count", 1) if count == 0: continue + names = [""] if count == 1 else [f"{name}_{i}" for i in range(count)] self.service_specs[name] = { - "obj": service, - "count": count, + "objects": [self.make_resource(service, n) for n in names] } - if count == 1: - self.service_specs[name]["objects"] = [service()] - elif count > 1: - self.service_specs[name]["objects"] = [ - service(name=f"{name}_{i}") for i in range(count) - ] - if postgres_spec := self.service_specs.get("postgres"): - postgres_obj = postgres_spec["objects"][0] - paddles_obj = self.service_specs["paddles"]["objects"][0] + self._wire_services() + self.secrets = [] + for secret_name in self.stack_def.get("secrets", []): + if secret_name not in SECRETS: + logger.warning( + f"Unknown secret {secret_name!r} in stack {self.stack_name!r}" + ) + continue + self.secrets.append(SECRETS[secret_name]) + + def make_resource(self, cls: Type[R], name: str = "") -> R: + """Instantiate a resource with this stack's data_dir and config.""" + return cls( + name, data_dir=self.data_dir, containers_config=self.containers_config + ) + + def _objects(self) -> Iterator[StackResource]: + for spec in self.service_specs.values(): + yield from spec["objects"] + + def _first(self, name: str, cls: Type[R]) -> R | None: + """The (first) resource of service ``name``, if the stack deploys it. + + ``cls`` is the class SERVICES maps ``name`` to; it only types the result. + """ + spec = self.service_specs.get(name) + return cast(R, spec["objects"][0]) if spec else None + + def _wire_services(self) -> None: + postgres_obj = self._first("postgres", Postgres) + paddles_obj = self._first("paddles", Paddles) + if postgres_obj and paddles_obj: paddles_obj.env_vars["PADDLES_SQLALCHEMY_URL"] = ( postgres_obj.paddles_sqla_url ) @@ -129,10 +180,11 @@ async def check_requirements(self): result = True result = has_sudo = await HasSudo().evaluate() - result = result and await LoopControlDeviceExists().evaluate() - result = result and await LoopControlDeviceWriteable().evaluate() - result = result and await BlockPoolDiskGroup().evaluate() - result = result and await BlockPoolParentAccessible().evaluate() + if "testnode" in self.service_specs: + result = result and await LoopControlDeviceExists().evaluate() + result = result and await LoopControlDeviceWriteable().evaluate() + result = result and await BlockPoolDiskGroup().evaluate() + result = result and await BlockPoolParentAccessible().evaluate() # Check for SELinux being enabled and Enforcing; then check for the # presence of our module. If necessary, inform the user and instruct @@ -140,14 +192,17 @@ async def check_requirements(self): if has_sudo and await host.selinux_enforcing(): result = result and await SELinuxModule().evaluate() - for name, obj in config["containers"].items(): - if (repo := obj.get("repo")) and not host.path_exists(repo): + for name in self.service_specs: + obj = self.containers_config.get(name, {}) + if (repo := obj.get("repo")) and not host.path_exists( + os.path.expanduser(str(repo)) + ): result = False logger.error(f"Repo for {name} not found at {repo}") return result - async def apply(self, action): - return await getattr(self, action)() + async def apply(self, action: str, **kwargs) -> int | None: + return await getattr(self, action)(**kwargs) async def pull(self): logger.info("Pulling images...") @@ -160,63 +215,64 @@ async def build(self): await spec["objects"][0].build() async def create(self): + args = config.get("args", {}) + if args.get("build"): + await self.build() logger.info("Creating containers...") await CephDevStackNetwork().create() - await SSHKeyPair().create() + for secret in self.secrets: + await secret().create() # Pre-allocate loop device numbers sequentially so concurrent creates # don't race on the same device numbers. - for spec in self.service_specs.values(): - for obj in spec["objects"]: - if isinstance(obj, LoopDeviceMixin): - obj.allocate_devices() - containers = [] - for spec in self.service_specs.values(): - for object in spec["objects"]: - containers.append(object.create()) - await asyncio.gather(*containers) + for obj in self._objects(): + if isinstance(obj, LoopDeviceMixin): + obj.allocate_devices() + await self._gather_action("create") + + async def _run_depends(self): + for dep_name in self.stack_def.get("depends", []): + logger.info(f"Running dependency stack: {dep_name}") + dep = CephDevStack(stack_name=dep_name) + await dep.start() async def start(self): + await self._run_depends() await self.create() logger.info("Starting containers...") - for spec in self.service_specs.values(): - for object in spec["objects"]: - await object.start() - logger.info( - "All containers are running. To monitor teuthology, try running: podman " - "logs -f teuthology" - ) - hostname = host.hostname() - logger.info(f"View test results at http://{hostname}:8081/") + for obj in self._objects(): + await obj.start() + if "teuthology" in self.service_specs: + logger.info( + "All containers are running. To monitor teuthology, try running: " + "podman logs -f teuthology" + ) + else: + logger.info("All containers are running.") + if pulpito := self._first("pulpito", Pulpito): + logger.info( + f"View test results at http://{host.hostname()}:{pulpito.port}/" + ) + + async def _gather_action(self, action: str): + await asyncio.gather(*(getattr(obj, action)() for obj in self._objects())) async def stop(self): logger.info("Stopping containers...") - containers = [] - for spec in self.service_specs.values(): - for object in spec["objects"]: - containers.append(object.stop()) - await asyncio.gather(*containers) + await self._gather_action("stop") async def remove(self): logger.info("Removing containers...") - containers = [] - for spec in self.service_specs.values(): - for object in spec["objects"]: - containers.append(object.remove()) - await asyncio.gather(*containers) + await self._gather_action("remove") await CephDevStackNetwork().remove() - await SSHKeyPair().remove() + for secret in self.secrets: + await secret().remove() async def watch(self): logger.info( - "Watching containers; will replace any that are removed and, " - "except for teuthology, restart any that are stopped" + "Entering watch mode: containers will be recreated if removed " + "and, except for teuthology, restarted if stopped." ) - containers = [] - for spec in self.service_specs.values(): - if not spec["count"] > 0: - continue - for object in spec["objects"]: - containers.append(object) + containers = list(self._objects()) logger.info(f"Watching {containers}") while True: for container in containers: @@ -234,10 +290,9 @@ async def watch(self): await container.start() async def wait(self, container_name: str): - for spec in self.service_specs.values(): - for object in spec["objects"]: - if object.name == container_name: - return await object.wait() + for obj in self._objects(): + if obj.name == container_name: + return await obj.wait() logger.error(f"Could not find container {container_name}") return 1 @@ -248,7 +303,7 @@ async def logs(self, run_name: str = "", job_id: str = "", locate: bool = False) logger.error("No log file found") else: if locate: - print(str(log_file).replace(str(pathlib.Path.home()), "~")) + print(str(log_file.resolve())) else: buffer_size = 8 * 1024 with open(log_file) as f: @@ -256,7 +311,7 @@ async def logs(self, run_name: str = "", job_id: str = "", locate: bool = False) print(chunk, end="") def get_log_file(self, run_name: str = "", job_id: str = "") -> pathlib.Path: - archive_dir = Teuthology().archive_dir + archive_dir = self.data_dir / "archive" if not run_name: runs = get_runs(archive_dir) diff --git a/ceph_devstack/resources/ceph/block_devices.py b/ceph_devstack/resources/ceph/block_devices.py index 9cf2e3ef..de0a4e60 100644 --- a/ceph_devstack/resources/ceph/block_devices.py +++ b/ceph_devstack/resources/ceph/block_devices.py @@ -71,6 +71,30 @@ def backing_files(image_dir: Path) -> list[tuple[str, int]]: ] +def stack_image_dirs(base: Path) -> list[Path]: + """Every stack's disk_images directory, plus the pre-stack one in base. + + One host command finds them, so other entries in base (block_pool.json, + stacks without disk images) cost no further host round trips. + """ + proc = host.output( + [ + "find", + str(base), + "-mindepth", + "2", + "-maxdepth", + "2", + "-type", + "d", + "-name", + "disk_images", + ] + ) + found = proc.stdout.decode().splitlines() if proc.returncode == 0 else [] + return [base / "disk_images", *sorted(Path(line) for line in found if line)] + + class CommandRunner(Protocol): async def __call__( self, diff --git a/ceph_devstack/resources/ceph/containers.py b/ceph_devstack/resources/ceph/containers.py index 6a5f3ac3..61a368ae 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, logger +from ceph_devstack import DEFAULT_CONFIG_PATH, logger from ceph_devstack.host import host from ceph_devstack.resources.ceph.host_loops import LoopDeviceMixin from ceph_devstack.resources.container import Container @@ -43,8 +43,8 @@ class Postgres(Container): "APP_DB_NAME": "paddles", } - def __init__(self, name: str = ""): - super().__init__(name) + def __init__(self, name: str = "", **kwargs): + super().__init__(name, **kwargs) username = self.env_vars["APP_DB_USER"] password = self.env_vars["APP_DB_PASS"] db_name = self.env_vars["APP_DB_NAME"] @@ -124,10 +124,12 @@ class Archive(Container): @property def archive_dir(self): - return (Path(config["data_dir"]) / "archive").expanduser() + return self.data_dir / "archive" class Pulpito(Container): + port = 8081 + cmd_vars = Container.cmd_vars + ["port"] create_cmd = [ "podman", "container", @@ -136,9 +138,9 @@ class Pulpito(Container): "--network", "ceph-devstack", "-p", - "8081:8081", + "{port}:{port}", "--health-cmd", - "CMD curl -f http://0.0.0.0:8081", + "CMD curl -f http://0.0.0.0:{port}", "--health-interval", "10s", "--health-retries", @@ -177,13 +179,13 @@ class TestNode(LoopDeviceMixin, Container): "CEPH_VOLUME_ALLOW_LOOP_DEVICES": "true", } - def __init__(self, name: str = ""): - super().__init__(name=name) + def __init__(self, name: str = "", **kwargs): + super().__init__(name=name, **kwargs) self._init_loop_devices() @property def loop_img_dir(self): - return (Path(config["data_dir"]) / "disk_images").expanduser() + return self.data_dir / "disk_images" @property def create_cmd(self): @@ -234,9 +236,9 @@ def create_cmd(self): def additional_volumes(self): volumes = [] if ( - sshd_config := DEFAULT_CONFIG_PATH.parent.joinpath( - "sshd_config" - ).expanduser() + sshd_config := DEFAULT_CONFIG_PATH.parent.joinpath("sshd_config") + .expanduser() + .absolute() ) and sshd_config.exists(): volumes.extend( [ @@ -297,7 +299,7 @@ def create_cmd(self): "create", "-i", "--label", - f"testnode_count={config['containers']['testnode']['count']}", + f"testnode_count={self.containers_config['testnode']['count']}", "--network", "ceph-devstack", "--secret", @@ -307,6 +309,7 @@ def create_cmd(self): ] ansible_inv = os.environ.get("ANSIBLE_INVENTORY_PATH") if ansible_inv: + ansible_inv = Path(ansible_inv).expanduser().absolute() cmd += [ "-v", f"{ansible_inv}/inventory:/etc/ansible/hosts", @@ -314,15 +317,18 @@ 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 host.path_exists(ssh_auth_socket): - cmd += [ - "-v", - f"{ssh_auth_socket}:{ssh_auth_socket}", - "-e", - f"SSH_AUTH_SOCK={ssh_auth_socket}", - ] + if ssh_auth_socket: + ssh_auth_socket = Path(ssh_auth_socket).expanduser().absolute() + if host.path_exists(ssh_auth_socket): + cmd += [ + "-v", + f"{ssh_auth_socket}:{ssh_auth_socket}", + "-e", + f"SSH_AUTH_SOCK={ssh_auth_socket}", + ] custom_conf = os.environ.get("TEUTHOLOGY_CONF") if custom_conf: + custom_conf = Path(custom_conf).expanduser().absolute() cmd += [ "-v", f"{custom_conf}:/tmp/conf.yaml", @@ -331,6 +337,7 @@ def create_cmd(self): ] teuthology_yaml = os.environ.get("TEUTHOLOGY_YAML") if teuthology_yaml: + teuthology_yaml = Path(teuthology_yaml).expanduser().absolute() cmd += [ "-v", f"{teuthology_yaml}:/root/.teuthology.yaml", @@ -358,10 +365,10 @@ def create_cmd(self): @property def archive_dir(self) -> Path: - return (Path(config["data_dir"]) / "archive").expanduser() + return self.data_dir / "archive" async def create(self): - self.archive_dir.expanduser().resolve().mkdir(parents=True, exist_ok=True) + self.archive_dir.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, diff --git a/ceph_devstack/resources/ceph/host_loops.py b/ceph_devstack/resources/ceph/host_loops.py index 51f1e4b7..dd072170 100644 --- a/ceph_devstack/resources/ceph/host_loops.py +++ b/ceph_devstack/resources/ceph/host_loops.py @@ -3,7 +3,7 @@ from pathlib import Path from typing import Any, ClassVar -from ceph_devstack import logger +from ceph_devstack import base_data_dir, logger from ceph_devstack.block_pool import shared_pool from ceph_devstack.resources.ceph.block_devices import ( BlockDeviceProvisioner, @@ -11,6 +11,7 @@ backing_files, loop_number, loop_path, + stack_image_dirs, ) @@ -33,9 +34,15 @@ def loop_img_dir(self) -> Path: @classmethod def discover_used_loop_numbers(cls, image_dir: Path) -> set[int]: - """Loop numbers already attached on the host or claimed by backing files.""" + """Loop numbers attached on the host or claimed by backing files. + + Backing files of every stack count, not just image_dir's: after a + reboot the loops are detached, but another stack's files still claim + their numbers. + """ attached = {number for number, _, _ in attached_loops()} - return attached | {number for _, number in backing_files(image_dir)} + image_dirs = {image_dir, *stack_image_dirs(base_data_dir())} + return attached | {n for d in image_dirs for _, n in backing_files(d)} @classmethod def owner_loop_numbers(cls, owner: str, image_dir: Path) -> list[int]: diff --git a/tests/conftest.py b/tests/conftest.py index 40fc5b91..02d9e69b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -70,6 +70,20 @@ def _children(self, path: str) -> list[str]: def path_exists(self, path) -> bool: return str(path) in self.files or bool(self._children(str(path))) + def _find(self, rest: list[str]) -> tuple[int, bytes]: + """``find BASE -mindepth 2 -maxdepth 2 -type d -name NAME``.""" + base, want = rest[0].rstrip("/"), rest[-1] + if not self.path_exists(base): + return 1, b"" + dirs = { + f"{base}/{parts[0]}/{want}" + for path in self.files + if path.startswith(base + "/") + and len(parts := path[len(base) + 1 :].split("/")) > 2 + and parts[1] == want + } + return 0, "".join(f"{d}\n" for d in sorted(dirs)).encode() + def output(self, args, input=None) -> subprocess.CompletedProcess: self.commands.append(args) name, *rest = args @@ -81,6 +95,8 @@ def output(self, args, input=None) -> subprocess.CompletedProcess: elif name == "ls": children = self._children(rest[-1]) rc, out = (0, "".join(f"{c}\n" for c in children).encode()) + elif name == "find": + rc, out = self._find(rest) elif name == "realpath": rc, out = 0, f"{rest[0]}\n".encode() elif name == "losetup" and rest[0] == "--list": diff --git a/tests/resources/ceph/test_cephdevstack_core.py b/tests/resources/ceph/test_cephdevstack_core.py index ddf2d889..15caf4a2 100644 --- a/tests/resources/ceph/test_cephdevstack_core.py +++ b/tests/resources/ceph/test_cephdevstack_core.py @@ -3,8 +3,8 @@ import pytest -from ceph_devstack import config -from ceph_devstack.resources.ceph import CephDevStack +from ceph_devstack import config, deep_merge +from ceph_devstack.resources.ceph import SERVICES, CephDevStack from ceph_devstack.resources.ceph.containers import ( Archive, Beanstalk, @@ -17,6 +17,20 @@ class TestCephDevStackServiceSpecs: + @pytest.mark.parametrize("name", sorted(SERVICES)) + def test_registry_key_is_the_service_config_section(self, name, tmp_path): + containers = deep_merge(config["containers"], {name: {"marker": True}}) + obj = SERVICES[name](data_dir=tmp_path, containers_config=containers) + assert obj.config["marker"] is True + + def test_unknown_service_and_secret_warn(self, tmp_path, caplog): + config["stacks"]["odd"] = {"services": ["nope"], "secrets": ["nada"]} + devstack = CephDevStack("odd") + assert devstack.service_specs == {} + assert devstack.secrets == [] + assert "Unknown service 'nope'" in caplog.text + assert "Unknown secret 'nada'" in caplog.text + def test_service_specs_includes_all_services(self): devstack = CephDevStack() assert "postgres" in devstack.service_specs @@ -87,66 +101,36 @@ def test_service_specs_does_not_set_postgres_url_when_no_postgres(self): paddles_obj = devstack.service_specs["paddles"]["objects"][0] assert "PADDLES_SQLALCHEMY_URL" not in paddles_obj.env_vars - def test_service_specs_count_attribute(self): - devstack = CephDevStack() - assert devstack.service_specs["postgres"]["count"] == 1 - assert devstack.service_specs["testnode"]["count"] == 3 - class TestCephDevStackApply: - async def test_apply_calls_correct_method(self): + @pytest.mark.parametrize("action", ["pull", "create", "start"]) + async def test_apply_dispatches_to_method(self, action): devstack = CephDevStack() - with patch.object(devstack, "pull", new_callable=AsyncMock) as mock_pull: - await devstack.apply("pull") - assert mock_pull.called is True + with patch.object(devstack, action, new_callable=AsyncMock) as mock: + await devstack.apply(action) + assert mock.called is True - async def test_apply_calls_create(self): - devstack = CephDevStack() - with patch.object(devstack, "create", new_callable=AsyncMock) as mock_create: - await devstack.apply("create") - assert mock_create.called is True - async def test_apply_calls_start(self): +@pytest.mark.parametrize("action", ["pull", "build", "stop"]) +class TestCephDevStackServiceDispatch: + async def test_action_dispatches_to_all_services(self, action): devstack = CephDevStack() - with patch.object(devstack, "start", new_callable=AsyncMock) as mock_start: - await devstack.apply("start") - assert mock_start.called is True - - -class TestCephDevStackPull: - async def test_pull_calls_pull_on_all_services(self): - devstack = CephDevStack() - # Override service_specs to control the objects mock_postgres = AsyncMock() mock_paddles = AsyncMock() devstack.service_specs = { - "postgres": {"count": 1, "objects": [mock_postgres]}, - "paddles": {"count": 1, "objects": [mock_paddles]}, + "postgres": {"objects": [mock_postgres]}, + "paddles": {"objects": [mock_paddles]}, } with patch("ceph_devstack.logger.info"): - await devstack.pull() - mock_postgres.pull.assert_called_once() - mock_paddles.pull.assert_called_once() - - -class TestCephDevStackBuild: - async def test_build_calls_build_on_all_services(self): - devstack = CephDevStack() - mock_postgres = AsyncMock() - mock_paddles = AsyncMock() - devstack.service_specs = { - "postgres": {"count": 1, "objects": [mock_postgres]}, - "paddles": {"count": 1, "objects": [mock_paddles]}, - } - with patch("ceph_devstack.logger.info"): - await devstack.build() - mock_postgres.build.assert_called_once() - mock_paddles.build.assert_called_once() + await getattr(devstack, action)() + getattr(mock_postgres, action).assert_called_once() + getattr(mock_paddles, action).assert_called_once() class TestCephDevStackGetLogFile: def test_get_log_file_with_run_name_and_job_id(self, tmp_path): devstack = CephDevStack() + devstack.data_dir = tmp_path archive_dir = tmp_path / "archive" archive_dir.mkdir() run_name = "root-2025-01-01_00:00:00-orch:cephadm:smoke-small-main-distro-default-testnode" @@ -157,16 +141,12 @@ def test_get_log_file_with_run_name_and_job_id(self, tmp_path): log_file = job_dir / "teuthology.log" log_file.write_text("test log content") - # Mock Teuthology to return our test archive_dir - with patch("ceph_devstack.resources.ceph.Teuthology") as MockTeuthology: - mock_teuthology = MagicMock() - mock_teuthology.archive_dir = archive_dir - MockTeuthology.return_value = mock_teuthology - result = devstack.get_log_file(run_name, "42") - assert str(result) == str(log_file) + result = devstack.get_log_file(run_name, "42") + assert str(result) == str(log_file) def test_get_log_file_with_run_name_only(self, tmp_path): devstack = CephDevStack() + devstack.data_dir = tmp_path archive_dir = tmp_path / "archive" archive_dir.mkdir() run_name = "root-2025-01-01_00:00:00-orch:cephadm:smoke-small-main-distro-default-testnode" @@ -177,30 +157,24 @@ def test_get_log_file_with_run_name_only(self, tmp_path): log_file = job_dir / "teuthology.log" log_file.write_text("test log content") - with patch("ceph_devstack.resources.ceph.Teuthology") as MockTeuthology: - mock_teuthology = MagicMock() - mock_teuthology.archive_dir = archive_dir - MockTeuthology.return_value = mock_teuthology - result = devstack.get_log_file(run_name, "") - assert str(result) == str(log_file) + result = devstack.get_log_file(run_name, "") + assert str(result) == str(log_file) def test_get_log_file_raises_file_not_found_for_missing_job(self, tmp_path): devstack = CephDevStack() + devstack.data_dir = tmp_path archive_dir = tmp_path / "archive" archive_dir.mkdir() run_name = "root-2025-01-01_00:00:00-orch:cephadm:smoke-small-main-distro-default-testnode" run_dir = archive_dir / run_name run_dir.mkdir() - with patch("ceph_devstack.resources.ceph.Teuthology") as MockTeuthology: - mock_teuthology = MagicMock() - mock_teuthology.archive_dir = archive_dir - MockTeuthology.return_value = mock_teuthology - with pytest.raises(FileNotFoundError): - devstack.get_log_file(run_name, "99") + with pytest.raises(FileNotFoundError): + devstack.get_log_file(run_name, "99") def test_get_log_file_raises_file_not_found_for_missing_log(self, tmp_path): devstack = CephDevStack() + devstack.data_dir = tmp_path archive_dir = tmp_path / "archive" archive_dir.mkdir() run_name = "root-2025-01-01_00:00:00-orch:cephadm:smoke-small-main-distro-default-testnode" @@ -209,24 +183,24 @@ def test_get_log_file_raises_file_not_found_for_missing_log(self, tmp_path): job_dir = run_dir / "1" job_dir.mkdir() - with patch("ceph_devstack.resources.ceph.Teuthology") as MockTeuthology: - mock_teuthology = MagicMock() - mock_teuthology.archive_dir = archive_dir - MockTeuthology.return_value = mock_teuthology - with pytest.raises(FileNotFoundError): - devstack.get_log_file(run_name, "1") + with pytest.raises(FileNotFoundError): + devstack.get_log_file(run_name, "1") def test_get_log_file_uses_most_recent_when_no_run_name( self, tmp_path, create_log_file ): config["data_dir"] = str(tmp_path) + devstack = CephDevStack() create_log_file( - tmp_path, timestamp=datetime(year=2024, month=1, day=1), content="old log" + devstack.data_dir, + timestamp=datetime(year=2024, month=1, day=1), + content="old log", ) new_log_file = create_log_file( - tmp_path, timestamp=datetime(year=2025, month=1, day=1), content="new log" + devstack.data_dir, + timestamp=datetime(year=2025, month=1, day=1), + content="new log", ) - devstack = CephDevStack() result = devstack.get_log_file("", "") assert str(result) == str(new_log_file) @@ -234,6 +208,7 @@ def test_get_log_file_returns_latest_job_log_when_multiple_and_no_job_id( self, tmp_path ): devstack = CephDevStack() + devstack.data_dir = tmp_path archive_dir = tmp_path / "archive" archive_dir.mkdir() @@ -251,11 +226,7 @@ def test_get_log_file_returns_latest_job_log_when_multiple_and_no_job_id( job2_log = job2_dir / "teuthology.log" job2_log.write_text("job 2 log") - with patch("ceph_devstack.resources.ceph.Teuthology") as MockTeuthology: - mock_teuthology = MagicMock() - mock_teuthology.archive_dir = archive_dir - MockTeuthology.return_value = mock_teuthology - assert devstack.get_log_file(run_name, "").parent.name == "2" + assert devstack.get_log_file(run_name, "").parent.name == "2" class TestCephDevStackRemove: @@ -264,38 +235,22 @@ async def test_remove_calls_remove_on_all_containers(self): mock_postgres = AsyncMock() mock_paddles = AsyncMock() devstack.service_specs = { - "postgres": {"count": 1, "objects": [mock_postgres]}, - "paddles": {"count": 1, "objects": [mock_paddles]}, + "postgres": {"objects": [mock_postgres]}, + "paddles": {"objects": [mock_paddles]}, } with patch("ceph_devstack.resources.ceph.CephDevStackNetwork") as MockNetwork: mock_network_instance = MagicMock() mock_network_instance.remove = AsyncMock() MockNetwork.return_value = mock_network_instance - with patch("ceph_devstack.resources.ceph.SSHKeyPair") as MockSecret: - mock_secret_instance = MagicMock() - mock_secret_instance.remove = AsyncMock() - MockSecret.return_value = mock_secret_instance - with patch("ceph_devstack.logger.info"): - await devstack.remove() - mock_postgres.remove.assert_called_once() - mock_paddles.remove.assert_called_once() - mock_network_instance.remove.assert_called_once() - mock_secret_instance.remove.assert_called_once() - - -class TestCephDevStackStop: - async def test_stop_calls_stop_on_all_containers(self): - devstack = CephDevStack() - mock_postgres = AsyncMock() - mock_paddles = AsyncMock() - devstack.service_specs = { - "postgres": {"count": 1, "objects": [mock_postgres]}, - "paddles": {"count": 1, "objects": [mock_paddles]}, - } - with patch("ceph_devstack.logger.info"): - await devstack.stop() - mock_postgres.stop.assert_called_once() - mock_paddles.stop.assert_called_once() + mock_secret = MagicMock() + mock_secret.remove = AsyncMock() + devstack.secrets = [MagicMock(return_value=mock_secret)] + with patch("ceph_devstack.logger.info"): + await devstack.remove() + mock_postgres.remove.assert_called_once() + mock_paddles.remove.assert_called_once() + mock_network_instance.remove.assert_called_once() + mock_secret.remove.assert_called_once() class _StopWatching(Exception): @@ -313,8 +268,8 @@ async def test_restarts_stopped_containers_except_teuthology(self): obj.is_running.return_value = False paddles.start.side_effect = _StopWatching devstack.service_specs = { - "teuthology": {"count": 1, "objects": [teuth]}, - "paddles": {"count": 1, "objects": [paddles]}, + "teuthology": {"objects": [teuth]}, + "paddles": {"objects": [paddles]}, } with pytest.raises(_StopWatching): await devstack.watch() @@ -329,7 +284,7 @@ async def test_wait_returns_process_id(self): mock_container.name = "teuthology" mock_container.wait = AsyncMock(return_value=42) devstack.service_specs = { - "teuthology": {"count": 1, "objects": [mock_container]}, + "teuthology": {"objects": [mock_container]}, } result = await devstack.wait("teuthology") assert result == 42 @@ -339,7 +294,7 @@ async def test_wait_returns_one_for_nonexistent_container(self): mock_container = AsyncMock() mock_container.name = "teuthology" devstack.service_specs = { - "teuthology": {"count": 1, "objects": [mock_container]}, + "teuthology": {"objects": [mock_container]}, } result = await devstack.wait("nonexistent") assert result == 1 @@ -348,6 +303,7 @@ async def test_wait_returns_one_for_nonexistent_container(self): class TestCephDevStackLogs: async def test_logs_with_locate_true(self, tmp_path): devstack = CephDevStack() + devstack.data_dir = tmp_path archive_dir = tmp_path / "archive" archive_dir.mkdir() run_name = "root-2025-01-01_00:00:00-orch:cephadm:smoke-small-main-distro-default-testnode" @@ -362,28 +318,14 @@ async def test_logs_with_locate_true(self, tmp_path): import io f = io.StringIO() - with patch("ceph_devstack.resources.ceph.Teuthology") as MockTeuthology: - mock_teuthology = MagicMock() - mock_teuthology.archive_dir = archive_dir - MockTeuthology.return_value = mock_teuthology - - def mock_listdir(path): - if str(path) == str(archive_dir): - return [run_name] - if str(path) == str(run_dir): - return ["1"] - return [] - - with ( - patch("os.listdir", side_effect=mock_listdir), - contextlib.redirect_stdout(f), - ): - await devstack.logs(locate=True) + with contextlib.redirect_stdout(f): + await devstack.logs(locate=True) output = f.getvalue() assert str(log_file) in output async def test_logs_with_locate_false(self, tmp_path): devstack = CephDevStack() + devstack.data_dir = tmp_path archive_dir = tmp_path / "archive" archive_dir.mkdir() run_name = "root-2025-01-01_00:00:00-orch:cephadm:smoke-small-main-distro-default-testnode" @@ -398,66 +340,96 @@ async def test_logs_with_locate_false(self, tmp_path): import io f = io.StringIO() - with patch("ceph_devstack.resources.ceph.Teuthology") as MockTeuthology: - mock_teuthology = MagicMock() - mock_teuthology.archive_dir = archive_dir - MockTeuthology.return_value = mock_teuthology - - def mock_listdir(path): - if str(path) == str(archive_dir): - return [run_name] - if str(path) == str(run_dir): - return ["1"] - return [] - - with ( - patch("os.listdir", side_effect=mock_listdir), - contextlib.redirect_stdout(f), - ): - await devstack.logs(locate=False) + with contextlib.redirect_stdout(f): + await devstack.logs(locate=False) output = f.getvalue() assert "test log content" in output async def test_logs_with_missing_file_shows_error(self, tmp_path, caplog): devstack = CephDevStack() + devstack.data_dir = tmp_path archive_dir = tmp_path / "archive" archive_dir.mkdir() - run_name = "root-2025-01-01_00:00:00-orch:cephadm:smoke-small-main-distro-default-testnode" - run_dir = archive_dir / run_name - run_dir.mkdir() - - with patch("ceph_devstack.resources.ceph.Teuthology") as MockTeuthology: - mock_teuthology = MagicMock() - mock_teuthology.archive_dir = archive_dir - MockTeuthology.return_value = mock_teuthology - def mock_listdir(path): - if str(path) == str(archive_dir): - return [run_name] - if str(path) == str(run_dir): - return ["1"] - return [] - - with patch("os.listdir", side_effect=mock_listdir): - await devstack.logs() + await devstack.logs() assert "No log file found" in caplog.text -class TestCephDevStackInit: - def test_init_without_postgres(self): - config["containers"] = { - "postgres": {"image": "postgres:latest", "count": 0}, - "paddles": {"image": "paddles:latest", "count": 1}, - "beanstalk": {"image": "beanstalk:latest", "count": 1}, - "pulpito": {"image": "pulpito:latest", "count": 1}, - "testnode": { - "image": "testnode:latest", - "count": 3, - "loop_device_count": 1, - }, - "teuthology": {"image": "teuthology:latest", "count": 1}, - "archive": {"image": "archive:latest", "count": 1}, +class TestCephDevStackStacks: + def test_custom_stack_limits_services(self, tmp_path): + config_file = tmp_path / "config.toml" + config_file.write_text( + """ +[stacks.minimal] +services = ["postgres", "paddles"] +""" + ) + config.load(config_file) + devstack = CephDevStack(stack_name="minimal") + assert devstack.stack_name == "minimal" + assert set(devstack.service_specs) == {"postgres", "paddles"} + + +@pytest.fixture +def depends_config(tmp_path): + config_file = tmp_path / "config.toml" + config_file.write_text( + """ +[stacks.dep] +services = ["postgres"] + +[stacks.dep.containers.postgres] +image = "dep/postgres" + +[stacks.main] +services = ["postgres"] +depends = ["dep"] + +[stacks.main.containers.postgres] +image = "main/postgres" +""" + ) + config.load(config_file) + + +@pytest.mark.usefixtures("depends_config") +class TestCephDevStackDepends: + async def test_run_depends_runs_dependency_stack(self): + devstack = CephDevStack(stack_name="main") + started_stacks = [] + + async def tracking_start(dep_self): + started_stacks.append(dep_self.stack_name) + + with patch.object(CephDevStack, "start", tracking_start): + await devstack._run_depends() + assert started_stacks == ["dep"] + + async def test_dependency_overrides_do_not_leak_into_parent(self): + default_image = config["containers"]["postgres"]["image"] + devstack = CephDevStack(stack_name="main") + with patch.object(CephDevStack, "start", new_callable=AsyncMock): + await devstack._run_depends() + postgres = devstack.service_specs["postgres"]["objects"][0] + assert postgres.image == "main/postgres" + assert config["containers"]["postgres"]["image"] == default_image + + async def test_stop_and_remove_do_not_cascade_depends(self): + devstack = CephDevStack(stack_name="main") + mock_service = AsyncMock() + devstack.service_specs = { + "postgres": {"objects": [mock_service]}, } - devstack = CephDevStack() - assert "archive" in devstack.service_specs - assert "postgres" not in devstack.service_specs + with ( + patch("ceph_devstack.resources.ceph.CephDevStackNetwork") as MockNetwork, + patch("ceph_devstack.logger.info"), + patch.object(CephDevStack, "__init__") as mock_init, + ): + mock_network = MagicMock() + mock_network.remove = AsyncMock() + MockNetwork.return_value = mock_network + await devstack.stop() + await devstack.remove() + mock_init.assert_not_called() + mock_service.stop.assert_awaited_once() + mock_service.remove.assert_awaited_once() diff --git a/tests/resources/ceph/test_devstack.py b/tests/resources/ceph/test_devstack.py index 86630d96..2e9b7643 100644 --- a/tests/resources/ceph/test_devstack.py +++ b/tests/resources/ceph/test_devstack.py @@ -37,17 +37,18 @@ async def test_logs_command_display_log_file_of_latest_run( self, tmp_path, create_log_file ): config["data_dir"] = str(tmp_path) + devstack = CephDevStack() + stack_dir = devstack.data_dir f = io.StringIO() content = "custom log content" create_log_file( - tmp_path, + stack_dir, timestamp=datetime.now() - timedelta(days=40), ) - create_log_file(tmp_path, timestamp=datetime.now(), content=content) + create_log_file(stack_dir, timestamp=datetime.now(), content=content) with contextlib.redirect_stdout(f): - devstack = CephDevStack() await devstack.logs() assert content in f.getvalue() @@ -55,19 +56,20 @@ async def test_logs_display_roughly_contents_of_log_file( self, tmp_path, create_log_file ): config["data_dir"] = str(tmp_path) + devstack = CephDevStack() + stack_dir = devstack.data_dir f = io.StringIO() content = "".join( secrets.choice(string.ascii_letters + string.digits) for _ in range(6 * 8 * 1024) ) create_log_file( - tmp_path, + stack_dir, timestamp=datetime.now(), content=content, ) with contextlib.redirect_stdout(f): - devstack = CephDevStack() await devstack.logs() assert content == f.getvalue() @@ -75,19 +77,21 @@ async def test_logs_command_display_log_file_of_given_job_id( self, tmp_path, create_log_file ): config["data_dir"] = str(tmp_path) + devstack = CephDevStack() + stack_dir = devstack.data_dir f = io.StringIO() content = "custom log message" now = datetime.now() create_log_file( - tmp_path, + stack_dir, timestamp=now, test_type="ceph", job_id="1", content="another log", ) create_log_file( - tmp_path, + stack_dir, timestamp=now, test_type="ceph", job_id="2", @@ -95,7 +99,6 @@ async def test_logs_command_display_log_file_of_given_job_id( ) with contextlib.redirect_stdout(f): - devstack = CephDevStack() await devstack.logs(job_id="2") assert content in f.getvalue() @@ -103,20 +106,21 @@ async def test_logs_display_content_of_provided_run_name( self, tmp_path, create_log_file ): config["data_dir"] = str(tmp_path) + devstack = CephDevStack() + stack_dir = devstack.data_dir f = io.StringIO() content = "custom content" create_log_file( - tmp_path, + stack_dir, timestamp=datetime.now(), ) run_name: pathlib.Path = create_log_file( - tmp_path, + stack_dir, timestamp=datetime.now() - timedelta(days=3), content=content, ).parent.parent with contextlib.redirect_stdout(f): - devstack = CephDevStack() await devstack.logs(run_name=run_name) assert content in f.getvalue() @@ -124,9 +128,10 @@ async def test_logs_locate_display_file_path_instead_of_config( self, tmp_path, create_log_file ): config["data_dir"] = str(tmp_path) + devstack = CephDevStack() + stack_dir = devstack.data_dir f = io.StringIO() - log_file = create_log_file(tmp_path) + log_file = create_log_file(stack_dir) with contextlib.redirect_stdout(f): - devstack = CephDevStack() await devstack.logs(locate=True) assert str(log_file) in f.getvalue() diff --git a/tests/resources/ceph/test_host_loops.py b/tests/resources/ceph/test_host_loops.py index d2b46966..5a0d0a98 100644 --- a/tests/resources/ceph/test_host_loops.py +++ b/tests/resources/ceph/test_host_loops.py @@ -2,11 +2,13 @@ import pytest +from ceph_devstack import config from ceph_devstack.block_pool import BlockPoolError from ceph_devstack.resources.ceph.block_devices import ( BlockDeviceProvisioner, loop_number, loop_path, + stack_image_dirs, ) from ceph_devstack.resources.ceph.host_loops import LoopDeviceMixin @@ -39,6 +41,25 @@ def test_discover_used_loop_numbers_from_backing_files(self, fake_host, tmp_path make_backing(fake_host, tmp_path, "testnode_0-1", "ceph_node-4", "ignore-me") assert LoopDeviceMixin.discover_used_loop_numbers(tmp_path) == {1, 4} + def test_discover_used_loop_numbers_across_stacks(self, fake_host, tmp_path): + config["data_dir"] = str(tmp_path) + make_backing(fake_host, tmp_path / "teuthology/disk_images", "testnode_0-1") + make_backing(fake_host, tmp_path / "ceph/disk_images", "ceph_node-2") + make_backing(fake_host, tmp_path / "disk_images", "testnode_1-3") # pre-stack + image_dir = tmp_path / "ceph/disk_images" + assert LoopDeviceMixin.discover_used_loop_numbers(image_dir) == {1, 2, 3} + + def test_stack_image_dirs_takes_one_host_command(self, fake_host, tmp_path): + fake_host.add_file(tmp_path / "block_pool.json") + make_backing(fake_host, tmp_path / "teuthology/disk_images", "testnode_0-1") + make_backing(fake_host, tmp_path / "ceph/cluster", "ceph.conf") + fake_host.commands.clear() + assert stack_image_dirs(tmp_path) == [ + tmp_path / "disk_images", + tmp_path / "teuthology/disk_images", + ] + assert [c[0] for c in fake_host.commands] == ["find"] + def test_discover_used_loop_numbers_includes_attached_loops( self, fake_host, tmp_path ): diff --git a/tests/resources/ceph/test_requirements_ceph.py b/tests/resources/ceph/test_requirements_ceph.py index 756b5aeb..f7c1bec8 100644 --- a/tests/resources/ceph/test_requirements_ceph.py +++ b/tests/resources/ceph/test_requirements_ceph.py @@ -294,8 +294,8 @@ async def test_check_requirements_fails_if_any_requirement_fails( self, requirement_results, failing ): devstack = CephDevStack() - devstack.service_specs = {} - config["containers"] = {} + devstack.service_specs = {"testnode": {}} + config["containers"] = {"testnode": {}} if failing: requirement_results[failing] = False assert await devstack.check_requirements() is (failing is None) @@ -304,9 +304,7 @@ async def test_check_requirements_returns_false_when_repo_missing( self, requirement_results ): devstack = CephDevStack() - devstack.service_specs = {} - config["containers"] = { - "custom": {"repo": "/nonexistent/path"}, - } + devstack.containers_config = {"custom": {"repo": "/nonexistent/path"}} + devstack.service_specs = {"custom": {}} with patch("ceph_devstack.host.host.path_exists", return_value=False): assert await devstack.check_requirements() is False diff --git a/tests/resources/ceph/test_testnode.py b/tests/resources/ceph/test_testnode.py index 53bfe2c7..1f06d306 100644 --- a/tests/resources/ceph/test_testnode.py +++ b/tests/resources/ceph/test_testnode.py @@ -12,8 +12,7 @@ def test_testnode_loop_device_count_default_to_one(self): def test_testnode_create_cmd_includes_related_devices(self, tmp_path): config.load(Path(__file__).parent.joinpath("fixtures", "testnode-config.toml")) - config["data_dir"] = str(tmp_path) - testnode = _TestNode("testnode_1") + testnode = _TestNode("testnode_1", data_dir=tmp_path) create_cmd = testnode.create_cmd assert "--device=/dev/loop0" in create_cmd assert "--device=/dev/loop1" in create_cmd @@ -21,21 +20,18 @@ def test_testnode_create_cmd_includes_related_devices(self, tmp_path): assert "--device=/dev/loop3" in create_cmd def test_testnode_create_cmd_mounts_run_udev_read_only(self, tmp_path): - config["data_dir"] = str(tmp_path) - create_cmd = _TestNode("testnode_1").create_cmd + create_cmd = _TestNode("testnode_1", data_dir=tmp_path).create_cmd 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, tmp_path): - config["data_dir"] = str(tmp_path) - create_cmd = _TestNode("testnode_1").create_cmd + create_cmd = _TestNode("testnode_1", data_dir=tmp_path).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, tmp_path): config.load(Path(__file__).parent.joinpath("fixtures", "testnode-config.toml")) - config["data_dir"] = str(tmp_path) - testnode = _TestNode("testnode_1") + testnode = _TestNode("testnode_1", data_dir=tmp_path) assert testnode.loop_device_count == 4 assert testnode.devices == [ "/dev/loop0", @@ -45,12 +41,11 @@ def test_testnode_devices_is_based_on_loop_device_count_config(self, tmp_path): ] def test_testnode_reuses_existing_backing_files(self, tmp_path, fake_host): - config["data_dir"] = str(tmp_path) image_dir = tmp_path / "disk_images" fake_host.add_file(image_dir / "testnode_1-4") fake_host.add_file(image_dir / "testnode_1-1") config["containers"]["testnode"]["loop_device_count"] = 3 - testnode = _TestNode("testnode_1") + testnode = _TestNode("testnode_1", data_dir=tmp_path) assert testnode.devices == ["/dev/loop1", "/dev/loop4", "/dev/loop0"] def test_get_available_dmi_mounts_with_all_files_present(self): @@ -93,8 +88,7 @@ def test_get_available_dmi_mounts_with_partial_files_present(self): def test_create_cmd_includes_dmi_mounts_when_available(self, tmp_path): config.load(Path(__file__).parent.joinpath("fixtures", "testnode-config.toml")) - config["data_dir"] = str(tmp_path) - testnode = _TestNode("testnode_1") + testnode = _TestNode("testnode_1", data_dir=tmp_path) with patch( "ceph_devstack.resources.ceph.containers.host.path_exists" ) as mock_exists: @@ -106,8 +100,7 @@ def test_create_cmd_includes_dmi_mounts_when_available(self, tmp_path): def test_create_cmd_excludes_dmi_mounts_when_unavailable(self, tmp_path): config.load(Path(__file__).parent.joinpath("fixtures", "testnode-config.toml")) - config["data_dir"] = str(tmp_path) - testnode = _TestNode("testnode_1") + testnode = _TestNode("testnode_1", data_dir=tmp_path) with patch( "ceph_devstack.resources.ceph.containers.host.path_exists" ) as mock_exists: diff --git a/tests/resources/ceph/test_teuthology.py b/tests/resources/ceph/test_teuthology.py index 69674e6c..a6b9e6ec 100644 --- a/tests/resources/ceph/test_teuthology.py +++ b/tests/resources/ceph/test_teuthology.py @@ -2,15 +2,13 @@ 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 = Teuthology(data_dir=tmp_path) teuth.remove = AsyncMock() return teuth @@ -44,19 +42,19 @@ async def test_replaces_only_a_stopped_container( class TestTeuthologySSHAuthSock: SOCK = "/tmp/ssh-XXXX/agent.123" - def create_cmd(self, monkeypatch, sock_exists: bool) -> list[str]: + def create_cmd(self, tmp_path, 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 + return Teuthology(data_dir=tmp_path).create_cmd - def test_mounts_socket_present_on_container_host(self, monkeypatch): - cmd = self.create_cmd(monkeypatch, sock_exists=True) + def test_mounts_socket_present_on_container_host(self, tmp_path, monkeypatch): + cmd = self.create_cmd(tmp_path, 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) + def test_skips_socket_missing_on_container_host(self, tmp_path, monkeypatch): + cmd = self.create_cmd(tmp_path, monkeypatch, sock_exists=False) assert not any("SSH_AUTH_SOCK" in arg or self.SOCK in arg for arg in cmd) diff --git a/tests/test_config.py b/tests/test_config.py index b399c430..439ee423 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -103,8 +103,23 @@ def test_unset_value_simple_key(self, test_config): class TestConfigDefaults: def test_config_defaults(self): assert config == { + "stack": "teuthology", "data_dir": "~/.local/share/ceph-devstack", "block_pool": {}, + "stacks": { + "teuthology": { + "services": [ + "postgres", + "paddles", + "beanstalk", + "pulpito", + "teuthology", + "testnode", + "archive", + ], + "secrets": ["ssh_keypair"], + }, + }, "containers": { "archive": {"image": "python:alpine"}, "beanstalk": {"image": "quay.io/ceph-infra/teuthology-beanstalkd:main"}, diff --git a/tests/test_stacks.py b/tests/test_stacks.py new file mode 100644 index 00000000..3ef6230d --- /dev/null +++ b/tests/test_stacks.py @@ -0,0 +1,56 @@ +import pytest + +from ceph_devstack import config, parse_args +from ceph_devstack.resources.ceph import CephDevStack + + +class TestCephDevStackInit: + def test_init_sets_active_services(self): + devstack = CephDevStack("teuthology") + assert "postgres" in devstack.active_services + assert "teuthology" in devstack.active_services + + def test_init_defaults_to_config_stack(self): + devstack = CephDevStack() + assert devstack.stack_name == "teuthology" + + def test_init_unknown_stack_raises(self): + with pytest.raises(ValueError, match="Unknown stack"): + CephDevStack("nonexistent") + + def test_init_merges_container_overrides(self, tmp_path): + config_file = tmp_path / "config.toml" + config_file.write_text( + """ +[stacks.minimal] +services = ["teuthology", "testnode"] + +[stacks.minimal.containers.testnode] +count = 1 +""" + ) + config.load(config_file) + devstack = CephDevStack("minimal") + assert devstack.containers_config["testnode"]["count"] == 1 + assert config["containers"]["testnode"]["count"] == 3 + assert len(devstack.service_specs["testnode"]["objects"]) == 1 + teuthology = devstack.service_specs["teuthology"]["objects"][0] + assert "testnode_count=1" in teuthology.create_cmd + assert devstack.active_services == ["teuthology", "testnode"] + + +class TestStackDataDir: + def test_data_dir_reflects_stack(self): + devstack = CephDevStack("teuthology") + assert str(devstack.data_dir).endswith("/teuthology") + + +class TestParseArgsStack: + def test_parse_args_stack(self): + args = parse_args(["--stack", "ceph", "start"]) + assert args.stack == "ceph" + assert args.command == "start" + + def test_parse_args_stack_default_none(self): + args = parse_args(["start"]) + assert args.stack is None