Skip to content

fix(swebench-service): let srun find its config and honour proxy policy - #452

Open
leopck wants to merge 1 commit into
mlcommons:mainfrom
leopck:fix/pyxis-srun-env
Open

fix(swebench-service): let srun find its config and honour proxy policy#452
leopck wants to merge 1 commit into
mlcommons:mainfrom
leopck:fix/pyxis-srun-env

Conversation

@leopck

@leopck leopck commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

The Pyxis runtime builds each container step's environment from an explicit allow-list, and two variables that srun and enroot genuinely need were missing. Neither failure is visible in a unit test, because both live in the environment handed to a subprocess we mock.

SLURM_CONF. Without it the child srun falls back to /etc/slurm/slurm.conf. On a configless site that file does not exist and srun aborts with "Could not establish a configuration source"; on a multi-cluster site it exists but is a different cluster's file whose plugins are not installed locally, and srun aborts with "failed to initialize cli_filter plugin". Either way every step dies before a container is created, and the run reports only the generic "Pyxis infrastructure failure before the command completed". The remaining SLURM_* variables stay out of the allow-list deliberately: inheriting SLURM_JOB_ID / SLURM_STEP_ID is exactly what breaks a nested srun, which is why the allow-list exists.

Proxy policy. enroot performs the registry pull inside the step, so it needs the same proxy configuration as the caller. A site that pins a container-cache proxy system-wide will 403 the CONNECT for any registry outside that cache's allow-list, and every per-instance image import fails with "curl: (56) CONNECT tunnel failed, response 403" -- including the SWE-bench task images this runtime is built to pull.

Verified on a GB200 cluster whose enroot pins a container cache: before the change no sweb.eval.arm64 image could be imported from any node; after it, the image imports and the container starts.

What does this PR do?

Type of change

  • Bug fix
  • New feature
  • Documentation update
  • Refactor/cleanup

@leopck
leopck requested a review from a team August 19, 2026 13:39
@github-actions

github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

@github-actions github-actions Bot added the size/normal PR Review Policy: <=500 non-test lines & <=20 files label Aug 19, 2026
@leopck
leopck force-pushed the fix/pyxis-srun-env branch from d8d8f73 to 3b9ccf5 Compare August 19, 2026 13:41
@codecov-commenter

codecov-commenter commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@c30860a). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #452   +/-   ##
=======================================
  Coverage        ?   80.98%           
=======================================
  Files           ?      150           
  Lines           ?    20208           
  Branches        ?        0           
=======================================
  Hits            ?    16366           
  Misses          ?     3842           
  Partials        ?        0           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

The Pyxis runtime builds each container step's environment from an explicit
allow-list, and two variables that srun and enroot genuinely need were missing.
Neither failure is visible in a unit test, because both live in the environment
handed to a subprocess we mock.

SLURM_CONF. Without it the child srun falls back to /etc/slurm/slurm.conf. On a
configless site that file does not exist and srun aborts with "Could not
establish a configuration source"; on a multi-cluster site it exists but is a
different cluster's file whose plugins are not installed locally, and srun
aborts with "failed to initialize cli_filter plugin". Either way every step
dies before a container is created, and the run reports only the generic
"Pyxis infrastructure failure before the command completed". The remaining
SLURM_* variables stay out of the allow-list deliberately: inheriting
SLURM_JOB_ID / SLURM_STEP_ID is exactly what breaks a nested srun, which is why
the allow-list exists.

Proxy policy. enroot performs the registry pull inside the step, so it needs
the same proxy configuration as the caller. A site that pins a container-cache
proxy system-wide will 403 the CONNECT for any registry outside that cache's
allow-list, and every per-instance image import fails with
"curl: (56) CONNECT tunnel failed, response 403" -- including the SWE-bench
task images this runtime is built to pull.

Verified on a GB200 cluster whose enroot pins a container cache: before the
change no sweb.eval.arm64 image could be imported from any node; after it, the
image imports and the container starts.

@tianmu-li tianmu-li left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Comments can be trimmed a bit.

@arekay-nv arekay-nv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

One aspect to think about is not passing the variables to the SWEBench environment which can further harden it.
Also, can you add some tests to ensure that the env-vars are being passed as expected. And if we want to isolate the SWEBench environment, the corresponding tests there too.

"no_proxy",
"HTTP_PROXY",
"HTTPS_PROXY",
"NO_PROXY",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

minor : missing all_proxy/ALL_PROXY which can be set in some envs.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

oh thanks, forgot about them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/normal PR Review Policy: <=500 non-test lines & <=20 files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants