Skip to content

Cluster Local Scheduler Tests#314

Open
bennoschoenstein wants to merge 20 commits into
queens-py:mainfrom
bennoschoenstein:cluster-local-scheduler-tests
Open

Cluster Local Scheduler Tests#314
bennoschoenstein wants to merge 20 commits into
queens-py:mainfrom
bennoschoenstein:cluster-local-scheduler-tests

Conversation

@bennoschoenstein

@bennoschoenstein bennoschoenstein commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

What & why?

Provides the test coverage that #291 (ClusterLocal scheduler, @reginabuehler) still needs, so it can be merged:

  • Mock unit test - fast check of the scheduler logic, no real cluster required.
  • SLURM integration test - runs the scheduler against a real single-node SLURM in Docker.
  • CI builds that image with pixi (matching the repo's conda→pixi switch), and
    GitHub builds the image itself.

Also carries a small fix for a shared-list bug in _cluster_base.py.

How it works

A small, self-contained SLURM inside a Docker container and testing against it.

The Docker image

The Dockerfile builds a single machine that acts as its own tiny cluster: it installs SLURM, installs QUEENS via pixi, and on startup runs start-slurm.sh, which boots the SLURM services and waits until the node reports "ready" before any test runs. The result is a throwaway cluster that exists only for the length of one test run.

Why Ubuntu 22.04 (and not the newest)

This is the part that cost the most time, so it's worth stating plainly. SLURM normally wants to use cgroups - a Linux kernel feature for tracking and limiting what a job may use. On the newest Ubuntu (24.04, SLURM 23.11), SLURM insists on setting up the modern cgroup machinery at startup, and that setup needs system services that simply aren't there in a plain container. So SLURM refuses to start. Ubuntu 22.04, which ships the older SLURM 21.08, is the last series that can run completely cgroup-free.

How Docker fits into the CI pipeline

The GitHub Actions workflow (docker_tests.yml) does just two steps on every pull request:

  • Build the image from the Dockerfile.
  • Run the container, which boots SLURM and executes the integration test inside it. GitHub builds and runs the image itself on the CI runner. The workflow is named "Docker tests" (rather than something cluster-specific) so other container-based tests can be added to it later.

Related

Companion to #291 — merge alongside it or fold these commits in, whichever is preferred.

cc

@reginabuehler @sbrandstaeter

reginabuehler and others added 9 commits February 17, 2026 17:41
Co-authored-by: Sebastian Brandstäter <45557303+sbrandstaeter@users.noreply.github.com>
Mock-based unit tests for ClusterLocal plus an end-to-end Monte Carlo integration test against real SLURM.
Single-node, container-safe SLURM image (Dockerfile, slurm.conf,
start-slurm.sh) and a GitHub Actions workflow that builds it and runs
the cluster_local integration tests against real SLURM.
…uler-tests

# Conflicts:
#	src/queens/schedulers/cluster.py
Match main's tuple-unpacking style in timedelta_to_str. No functional change.
Replace the conda-based image with an ubuntu:24.04 + pixi build that installs the dev environment from the lockfile and runs the cluster_local integration tests via 'pixi run --frozen -e dev pytest'. GitHub builds the image itself (docker build + docker run), so no registry or maintainer action is required.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds the missing test coverage and CI environment needed to validate the new ClusterLocal scheduler (from #291), including a real SLURM-in-Docker integration test, while also refactoring shared cluster scheduler logic into a common base.

Changes:

  • Add unit tests for ClusterLocal using a mocked dask-jobqueue layer.
  • Add a SLURM-in-Docker integration test suite and a dedicated GitHub Actions workflow to run it.
  • Refactor shared cluster scheduling logic into src/queens/schedulers/_cluster_base.py and reuse it from both remote (Cluster) and local (ClusterLocal) schedulers.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/unit_tests/schedulers/test_cluster_local.py Adds mocked unit coverage for ClusterLocal cluster kwargs/adapt/jobscript behavior and the skip-directives regression.
tests/integration_tests/cluster_local/test_cluster_local.py Adds end-to-end ClusterLocal Monte Carlo test targeting a real SLURM runtime.
tests/integration_tests/cluster_local/docker/start-slurm.sh Bootstraps munge + single-node SLURM daemons inside the test container and executes pytest as a non-root submitter.
tests/integration_tests/cluster_local/docker/slurm.conf Provides a container-safe SLURM config for single-node CI execution.
tests/integration_tests/cluster_local/docker/Dockerfile Builds the SLURM + pixi + QUEENS test image used by CI.
tests/integration_tests/cluster_local/init.py Declares the new integration test package.
tests/conftest.py Excludes cluster_local integration tests from generic integration marker handling (tests self-mark).
src/queens/utils/start_dask_cluster.py Reuses _initialize_dask_cluster from the new cluster base module.
src/queens/schedulers/cluster.py Refactors remote Cluster scheduler to inherit shared logic from _BaseCluster.
src/queens/schedulers/cluster_local.py Introduces ClusterLocal scheduler implementation built on _BaseCluster.
src/queens/schedulers/_cluster_base.py Introduces shared cluster scheduler base + shared dask-jobqueue initialization helper.
src/queens/schedulers/init.py Enables lazy import exposure for _BaseCluster / ClusterLocal.
pyproject.toml Registers the cluster_local pytest marker.
.github/workflows/tests_cluster_local.yml Adds CI job to build the SLURM image and run the cluster_local integration tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/queens/schedulers/cluster_local.py
Comment thread src/queens/schedulers/cluster_local.py Outdated
Comment thread src/queens/schedulers/cluster_local.py Outdated
Comment thread src/queens/schedulers/cluster_local.py
Comment thread src/queens/schedulers/_cluster_base.py Outdated
From Copilot review on queens-py#314.

Return remote_port_dashboard (where the dashboard actually binds) instead of the unused local_port_dashboard, so the logged dashboard URL is correct.

restart_worker passed list(worker), which splits an address string into characters; wrap a str in a list instead.

Add messages to the re-raised RuntimeError/OSError so failures are diagnosable.
slurmd logs to a file and set -e killed the script before the diagnostics block ran, so CI showed no cause. On slurmd failure, dump slurmd.log/slurmctld.log and a foreground 'slurmd -Dvvv' run before exiting.
GitHub Action containers have no systemd/dbus, so slurmd's cgroup/v2 plugin cannot attach to a scope and aborts with 'Unable to initialize cgroup plugin'. Add cgroup.conf with CgroupPlugin=disabled; process tracking uses proctrack/linuxproc and task/none, so cgroup enforcement is not needed.
slurm 23.11 has no CgroupPlugin=disabled (slurmd then looks for a plugin literally named 'disabled' and fails to init). The supported way to run cgroup/v2 without systemd/dbus is IgnoreSystemd=yes, which makes slurmd skip the dbus scope call and mkdir the cgroup dirs directly. See https://slurm.schedmd.com/cgroup.conf.html
slurm 23.11 (Ubuntu 24.04) forces cgroup/v2 initialization, which fails in a non-privileged container without systemd/dbus or a writable /sys/fs/cgroup (CI rounds 3-4). slurm 21.08 (Ubuntu 22.04) does not load the cgroup plugin with proctrack/linuxproc + task/none, matching dask-jobqueue's working SLURM CI. Drop the now-unneeded cgroup.conf.
Shorten the long comments/docstrings across the cluster_local test infrastructure (Dockerfile, slurm.conf, start-slurm.sh, tests) and the scheduler code. License headers and Args/Returns blocks kept; no code or values changed.
Workflow is now 'Docker tests' with job 'local cluster scheduler' (check shows 'Docker tests / local cluster scheduler'), file renamed to docker_tests.yml, so future container-based tests can be added as further jobs.

@reginabuehler reginabuehler left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @bennoschoenstein for tackling the challenge of creating a testing infrastructure. I really appreciate that you found a practical way to make this feature testable. Nice work! 🚀
I only added a small comment that can be discussed.

On another note: I am not sure how strongly the community feels about the need to squash commits into single one. So that is another point we might want to discuss on how to handle.

Comment thread tests/unit_tests/schedulers/test_cluster_local.py Outdated
reginabuehler
reginabuehler previously approved these changes Jun 29, 2026

@danielwolff1 danielwolff1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one small remark from my side.

Thanks a lot for creating the test so that we can merge the ClusterLocal scheduler hopefully soon! 🚀

Comment thread tests/integration_tests/cluster_local/docker/Dockerfile Outdated
danielwolff1
danielwolff1 previously approved these changes Jul 2, 2026

@danielwolff1 danielwolff1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks for taking care! no more comments from my side 🚀

reginabuehler
reginabuehler previously approved these changes Jul 2, 2026

@reginabuehler reginabuehler left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for finishing up the comments. I am fine with merging this as it is, but it might be worth waiting until the next developers meeting to discuss the squashing of commits. Also I am not sure if it was decided to keep the 24 hours waiting period before merging.

Either way, thanks for the effort and the setup of the testing.

It might also be worth opening an issue regarding the use of the older Ubuntu version to possibly update this in the future.

@sbrandstaeter sbrandstaeter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks for adding the tests!
The local cluster functionality is becoming a really nice collaborative effort 🙌

I only have a few minor remarks.

Comment thread .github/workflows/docker_tests.yml
Comment thread tests/integration_tests/cluster_local/docker/Dockerfile
Comment thread tests/integration_tests/cluster_local/docker/slurm.conf Outdated
Comment thread tests/integration_tests/cluster_local/docker/slurm.conf Outdated
Comment thread tests/integration_tests/cluster_local/test_cluster_local.py Outdated
Comment thread tests/unit_tests/schedulers/test_cluster_local.py Outdated
Comment thread tests/unit_tests/schedulers/test_cluster_local.py Outdated
Comment thread tests/unit_tests/schedulers/test_cluster_local.py Outdated
Comment thread tests/unit_tests/schedulers/test_cluster_local.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants