Skip to content

fix(aorta): container /dev/kfd access and --override argument packing - #327

Merged
speriaswamy-amd merged 4 commits into
surya/aorta-mn-01-teardownfrom
surya/aorta-mn-02-single-node-fixes
Sep 11, 2026
Merged

speriaswamy-amd merged 4 commits into
surya/aorta-mn-01-teardownfrom
surya/aorta-mn-02-single-node-fixes

Conversation

@speriaswamy-amd

Copy link
Copy Markdown
Collaborator

Stack 2/6 — splits #171. Base: #326.

Why

Two defects on the existing single-node path, both surfaced by live 2-node validation but neither multi-node specific.

  1. _launch_container() never passed user, so the container ran as the image's default UID (jenkins on the validation cluster) and could not open /dev/kfd despite --privileged. The render group was also missing. The function's own comment already claimed "Run as root so the container can access GPUs" — the code now matches it.
  2. Training overrides were emitted as --override k=v --override k2=v2. Aorta's train.py declares --override with nargs="*", so argparse keeps only the last group — every preceding override was silently dropped.

What changed

  • cvs/runners/aorta.py — user="root", group_add=["video", "render"]; all key=value tokens packed behind a single --override.
  • Extracted the env-building block out of run() into _build_base_env() so the override packing is unit-testable without a container.

Test

ruff clean. Unit tests 589 → 595 (6 new: env construction, single---override invariant, container user/groups).

Comment thread cvs/runners/aorta.py Outdated
Comment thread cvs/runners/aorta.py Outdated
speriaswamy-amd and others added 4 commits September 10, 2026 19:09
Two defects on the existing single-node path, both surfaced by live cluster
validation:

- _launch_container() never passed `user`, so the container ran as the image's
  default UID (jenkins on the validation cluster) and could not open /dev/kfd
  despite --privileged. The `render` group was also missing. The function's own
  comment already claimed "Run as root so the container can access GPUs" - the
  code now matches it.

- Training overrides were emitted as `--override k=v --override k2=v2`. Aorta's
  train.py declares `--override` with `nargs="*"`, so argparse keeps only the
  last group and every preceding override was silently dropped. All key=value
  tokens now sit behind a single `--override`.

Extracts the env-building block out of run() into _build_base_env() so the
override packing is unit-testable without a container.

Co-Authored-By: Claude <noreply@anthropic.com>
group_add=["video", "render"] failed containers.run() outright on hosts
without a "render" group. user="root" + privileged already grants
/dev/kfd access; probe for "render" over SSH (same pattern as the
existing UID/GID lookup) and only request it when present.

AORTA_OVERRIDE_ARGS embedded literal quotes around each value
(key="value"). Nothing downstream re-parses this as a shell, so the
quote characters reached argparse as part of the value itself
(training.max_steps="15" instead of 15). Emit bare key=value tokens.
…mmand

AORTA_OVERRIDE_ARGS was computed and logged but never consumed -- exp_cmd
never appended the --override tokens, so config-driven training overrides
were silently dropped in script mode (AIMVT-292).
Docker resolves group_add names against the container image's
/etc/group, not the host's, so a "render" group found on the host can
fail to resolve inside the image even though the host device node
needs that GID. Pass the numeric GID instead so it applies directly.
@speriaswamy-amd
speriaswamy-amd force-pushed the surya/aorta-mn-02-single-node-fixes branch from 0feffb5 to 1b24fe6 Compare September 10, 2026 23:13

@amd-droy amd-droy 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.

lgtm. thanks.

@speriaswamy-amd
speriaswamy-amd removed this pull request from stack #333 September 11, 2026 00:23
@speriaswamy-amd
speriaswamy-amd added this pull request to stack #414 September 11, 2026 00:23
@speriaswamy-amd
speriaswamy-amd merged commit 519fb88 into main Sep 11, 2026
4 checks passed
@cijohnson
cijohnson deleted the surya/aorta-mn-02-single-node-fixes branch September 15, 2026 00:08
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.

2 participants