Mount the run directory into orchestrated containers - #356
Open
atnair-amd wants to merge 1 commit into
Open
Conversation
atnair-amd
force-pushed
the
worktree-aimvt-300-core
branch
from
August 20, 2026 06:05
824b932 to
9701e2e
Compare
atnair-amd
force-pushed
the
worktree-aimvt-300-orch-mount
branch
from
August 20, 2026 06:08
877a3b6 to
0ba4495
Compare
atnair-amd
force-pushed
the
worktree-aimvt-300-orch-mount
branch
from
August 20, 2026 06:14
0ba4495 to
4ebe7c0
Compare
atnair-amd
marked this pull request as ready for review
August 20, 2026 18:13
atnair-amd
force-pushed
the
worktree-aimvt-300-orch-mount
branch
3 times, most recently
from
August 21, 2026 03:13
d42ba24 to
e4ded76
Compare
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
force-pushed
the
worktree-aimvt-300-orch-mount
branch
from
August 21, 2026 04:01
e4ded76 to
7713e0f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test configs resolve
{run_dir}to a host path, but with the container orchestrator the tests execute inside the container viadocker 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
RunLayoutand is now merged; this branch is rebased onto main and stands alone.Ticket: AIMVT-300 (story AIMVT-297)
Change
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.RunLayout:cvs runresolves the layout before pytest, so the orchestrator always has one, and whether the run is scheduler-managed is already answered byis_managed_compute(). The production change is confined tocontainer.py; the rest is its unit tests and one line of docs.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 pinCVS_SCHEDULERso they do not depend on whether the host has scheduler binaries installed. TheCVS_SCHEDULERoverride itself iscvs.core.scheduler's contract and is tested there.The negative case matches on
layout.run_dirrather than a path literal. An earlier revision filtered on the pre-rename substringcvs/runs, which the path never contains, so the assertion could not fail; it has been checked by forcingis_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.