Skip to content

Mount the run directory into orchestrated containers - #356

Open
atnair-amd wants to merge 1 commit into
mainfrom
worktree-aimvt-300-orch-mount
Open

Mount the run directory into orchestrated containers#356
atnair-amd wants to merge 1 commit into
mainfrom
worktree-aimvt-300-orch-mount

Conversation

@atnair-amd

@atnair-amd atnair-amd commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

Test configs resolve {run_dir} to a host path, but with the container orchestrator the tests execute inside the container via docker exec. Without a passthrough that path resolves to nothing on the container side and the artifacts land in a disposable overlay.

Follows #355, which added RunLayout and is now merged; this branch is rebased onto main and stands alone.

Ticket: AIMVT-300 (story AIMVT-297)

Change

  • The run directory is bind-mounted as an identity mapping. Any other container path would leave the value already substituted into the config pointing somewhere that does not exist.
  • Scheduler-managed runs only, per is_managed_compute(). Elsewhere nothing guarantees the workspace is reachable from the node the container is launched on, and a bind mount of a path the node lacks quietly materializes a root-owned empty directory instead of failing.
  • Nothing is added to RunLayout: cvs run resolves the layout before pytest, so the orchestrator always has one, and whether the run is scheduler-managed is already answered by is_managed_compute(). The production change is confined to container.py; the rest is its unit tests and one line of docs.
  • Out of scope: mounting anything else into the container, and the container image's own venv location.

Tests

3 unit tests in cvs/core/orchestrators/unittests/test_container.py: the mount is present in a managed run, absent in an unmanaged one, and configured volumes still merge. They pin CVS_SCHEDULER so they do not depend on whether the host has scheduler binaries installed. The CVS_SCHEDULER override itself is cvs.core.scheduler's contract and is tested there.

The negative case matches on layout.run_dir rather than a path literal. An earlier revision filtered on the pre-rename substring cvs/runs, which the path never contains, so the assertion could not fail; it has been checked by forcing is_managed_compute() true and confirming the test then fails.

Gate: make fmt-check, make lint (pylint 10.00/10), make ut — 1246 tests, all passing.

@atnair-amd atnair-amd self-assigned this Aug 20, 2026
@atnair-amd
atnair-amd force-pushed the worktree-aimvt-300-core branch from 824b932 to 9701e2e Compare August 20, 2026 06:05
@atnair-amd
atnair-amd force-pushed the worktree-aimvt-300-orch-mount branch from 877a3b6 to 0ba4495 Compare August 20, 2026 06:08
@atnair-amd
atnair-amd force-pushed the worktree-aimvt-300-orch-mount branch from 0ba4495 to 4ebe7c0 Compare August 20, 2026 06:14
@atnair-amd
atnair-amd marked this pull request as ready for review August 20, 2026 18:13
@atnair-amd
atnair-amd force-pushed the worktree-aimvt-300-orch-mount branch 3 times, most recently from d42ba24 to e4ded76 Compare August 21, 2026 03:13
Test configs resolve {run_dir} to a host path, but tests execute inside
the container via docker exec, so without a passthrough the substituted
path resolves to nothing on the container side and artifacts land in a
disposable overlay.

Mount it as an identity mapping -- any other container path would leave
the already-substituted value pointing elsewhere.

Scheduler-managed runs only. Outside a managed job nothing guarantees the
workspace is reachable from the node the container is launched on, and
bind-mounting a path the node lacks quietly materializes a root-owned
empty directory instead of failing. is_managed_compute() already answers
that question, so nothing is added to RunLayout.
@atnair-amd
atnair-amd changed the base branch from worktree-aimvt-300-core to main August 21, 2026 04:01
@atnair-amd
atnair-amd force-pushed the worktree-aimvt-300-orch-mount branch from e4ded76 to 7713e0f Compare August 21, 2026 04:01
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.

1 participant