Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/srtctl/cli/mixins/frontend_stage.py
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ def _start_nginx(self, topology: FrontendTopology) -> ManagedProcess:
# Install nginx and run it (daemon off keeps nginx in foreground so srun can manage it)
# Use container path (/logs) since log_dir is mounted there
container_config_path = "/logs/nginx.conf"
# Optional ulimit: use_bash_wrapper=False bypasses default_bash_preamble;
# some clusters reject raising nofile inside the nginx container.
# Optional nginx-specific ulimit; some clusters reject raising nofile
# inside the nginx container.
fe = self.config.frontend
inner = (
f"ulimit -n 1048576 && nginx -c {container_config_path} -g 'daemon off;'"
Expand Down
3 changes: 1 addition & 2 deletions src/srtctl/core/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,7 @@ class ClusterConfig:
default_mounts: dict[str, str] | None = None
# Shell snippet prepended to every container srun (after env exports, before
# the main command). Useful for cluster-wide ulimits, e.g.
# ``"ulimit -n 1048576 -s unlimited -u 1048576"``. Silently dropped for
# sruns that bypass the bash wrapper (distroless containers).
# ``"ulimit -n 1048576 -s unlimited -u 1048576"``.
default_bash_preamble: str | None = None
reporting: ReportingConfig | None = None
telemetry: dict | None = None # opaque dict, parsed by try_start_snapshotter
Expand Down
10 changes: 4 additions & 6 deletions src/srtctl/core/slurm.py
Original file line number Diff line number Diff line change
Expand Up @@ -314,12 +314,10 @@ def start_srun_process(
else:
cluster_preamble = _get_cluster_bash_preamble()
if cluster_preamble:
logger.warning(
"Cluster default_bash_preamble is set but this srun bypasses the bash wrapper "
"(use_bash_wrapper=False); preamble will not be applied. command=%s",
shlex.join(command),
)
srun_cmd.extend(command)
bash_command = " && ".join([cluster_preamble, shlex.join(command)])
srun_cmd.extend(["bash", "-c", bash_command])
else:
srun_cmd.extend(command)

# Demoted to debug — every worker srun line is multi-KB once the
# fingerprint heredoc is inlined (see core/fingerprint.generate_capture_script),
Expand Down
13 changes: 6 additions & 7 deletions tests/test_slurm.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

"""Tests for SLURM command construction."""

import shlex
import subprocess
from pathlib import Path
from types import SimpleNamespace
Expand Down Expand Up @@ -77,23 +78,21 @@ def test_cluster_bash_preamble_applied_when_only_cluster_set() -> None:
assert bash_cmd.startswith("ulimit -n 1048576 && python3 -m server")


def test_cluster_bash_preamble_warns_when_bash_wrapper_disabled(caplog) -> None:
def test_cluster_bash_preamble_wraps_command_when_bash_wrapper_disabled() -> None:
with (
patch("srtctl.core.slurm.get_slurm_job_id", return_value="12345"),
patch(
"srtctl.core.slurm._get_cluster_bash_preamble",
return_value="ulimit -n 1048576",
),
patch("subprocess.Popen") as mock_popen,
caplog.at_level("WARNING", logger="srtctl.core.slurm"),
):
mock_popen.return_value = MagicMock()
start_srun_process(["/bin/node_exporter"], use_bash_wrapper=False)
command = ["bash", "-c", "nginx -c /logs/nginx.conf -g 'daemon off;'"]
start_srun_process(command, use_bash_wrapper=False)

srun_cmd = mock_popen.call_args.args[0]
# Distroless path runs the binary directly; preamble cannot apply.
assert "bash" not in srun_cmd
assert any("default_bash_preamble" in record.message for record in caplog.records)
bash_cmd = _built_bash_command(mock_popen)
assert bash_cmd == " && ".join(["ulimit -n 1048576", shlex.join(command)])


def test_srun_options_use_equals_separator() -> None:
Expand Down