fix: stack profile file mounts onto profile git-mounts to avoid masking
When a config profile declares both a git_mount and a mounts entry for the same directory (e.g. ~/.pi), the generated bind-mounts would mask each other inside the container. Instead, copy the static profile files into the instance-scoped git-mount source directory so the container sees both the cloned repo contents and the static files through a single bind-mount. - Add _stack_profile_mounts_with_git_mounts helper to merge overlapping profile mounts into git-mount sources. - Integrate stacking into start_tool_instance after resolving both mount types. - Add unit tests for exact, descendant, non-overlapping, and file cases.
This commit is contained in:
@@ -5,6 +5,7 @@ import contextlib
|
|||||||
import glob as glob_module
|
import glob as glob_module
|
||||||
import logging
|
import logging
|
||||||
import os
|
import os
|
||||||
|
import shutil
|
||||||
import subprocess
|
import subprocess
|
||||||
import uuid
|
import uuid
|
||||||
from datetime import datetime
|
from datetime import datetime
|
||||||
@@ -139,6 +140,79 @@ def _chown_staged_mounts(
|
|||||||
_chown_path(source, uid, gid)
|
_chown_path(source, uid, gid)
|
||||||
|
|
||||||
|
|
||||||
|
def _relative_under(parent: str, child: str) -> str | None:
|
||||||
|
"""Return the relative path of ``child`` under ``parent`` if it is inside.
|
||||||
|
|
||||||
|
Returns ``""`` when the paths are equal. Returns ``None`` when ``child``
|
||||||
|
is not under ``parent``.
|
||||||
|
"""
|
||||||
|
parent = os.path.normpath(parent)
|
||||||
|
child = os.path.normpath(child)
|
||||||
|
if child == parent:
|
||||||
|
return ""
|
||||||
|
prefix = parent + os.sep
|
||||||
|
if child.startswith(prefix):
|
||||||
|
return child[len(prefix) :]
|
||||||
|
return None
|
||||||
|
|
||||||
|
|
||||||
|
def _stack_profile_mounts_with_git_mounts(
|
||||||
|
profile_mounts: list[dict],
|
||||||
|
git_mount_volumes: list[dict],
|
||||||
|
) -> list[dict]:
|
||||||
|
"""Merge profile file mounts into overlapping git-mount sources.
|
||||||
|
|
||||||
|
When a config profile mounts static files to the same directory as a
|
||||||
|
git-mount (e.g. ``~/.pi``), a directory-level bind mount for the profile
|
||||||
|
would mask the cloned repository. Instead, copy the profile files into
|
||||||
|
the git-mount source directory so the container sees both sets of files
|
||||||
|
through a single bind mount.
|
||||||
|
|
||||||
|
Profile mounts whose target is a child of a git-mount target are copied
|
||||||
|
into the corresponding subdirectory. Mounts that do not overlap are
|
||||||
|
returned unchanged.
|
||||||
|
"""
|
||||||
|
remaining: list[dict] = []
|
||||||
|
for pvol in profile_mounts:
|
||||||
|
p_source = pvol.get("source", "")
|
||||||
|
p_target = pvol.get("target", "")
|
||||||
|
if not p_source or not os.path.exists(p_source):
|
||||||
|
remaining.append(pvol)
|
||||||
|
continue
|
||||||
|
|
||||||
|
merged = False
|
||||||
|
for gvol in git_mount_volumes:
|
||||||
|
g_source = gvol.get("source", "")
|
||||||
|
g_target = gvol.get("target", "")
|
||||||
|
if not g_source or not os.path.isdir(g_source):
|
||||||
|
continue
|
||||||
|
|
||||||
|
rel = _relative_under(g_target, p_target)
|
||||||
|
if rel is None:
|
||||||
|
continue
|
||||||
|
|
||||||
|
dst = os.path.join(g_source, rel) if rel else g_source
|
||||||
|
if os.path.isdir(p_source):
|
||||||
|
shutil.copytree(p_source, dst, dirs_exist_ok=True)
|
||||||
|
else:
|
||||||
|
os.makedirs(os.path.dirname(dst), exist_ok=True)
|
||||||
|
shutil.copy2(p_source, dst)
|
||||||
|
|
||||||
|
logger.debug(
|
||||||
|
"Stacked profile mount %s into git mount %s at %s",
|
||||||
|
p_target,
|
||||||
|
g_target,
|
||||||
|
dst,
|
||||||
|
)
|
||||||
|
merged = True
|
||||||
|
break
|
||||||
|
|
||||||
|
if not merged:
|
||||||
|
remaining.append(pvol)
|
||||||
|
|
||||||
|
return remaining
|
||||||
|
|
||||||
|
|
||||||
async def resolve_git_mounts(
|
async def resolve_git_mounts(
|
||||||
session: AsyncSession,
|
session: AsyncSession,
|
||||||
resolved: ResolvedProfile,
|
resolved: ResolvedProfile,
|
||||||
@@ -1323,19 +1397,25 @@ async def start_tool_instance(
|
|||||||
port_override = profile_hints["port_override"]
|
port_override = profile_hints["port_override"]
|
||||||
env_vars.update(profile_env)
|
env_vars.update(profile_env)
|
||||||
config_files.update(profile_files)
|
config_files.update(profile_files)
|
||||||
extra_volumes.extend(profile_mounts)
|
|
||||||
git_mount_volumes = await resolve_git_mounts(
|
git_mount_volumes = await resolve_git_mounts(
|
||||||
session, resolved, instance_dir, working_directory, home_dir
|
session, resolved, instance_dir, working_directory, home_dir
|
||||||
)
|
)
|
||||||
|
# Stack static file mounts on top of git repo mounts so they do
|
||||||
|
# not mask each other when they target the same directory.
|
||||||
|
stacked_profile_mounts = _stack_profile_mounts_with_git_mounts(
|
||||||
|
profile_mounts, git_mount_volumes
|
||||||
|
)
|
||||||
|
extra_volumes.extend(stacked_profile_mounts)
|
||||||
extra_volumes.extend(git_mount_volumes)
|
extra_volumes.extend(git_mount_volumes)
|
||||||
logger.debug(
|
logger.debug(
|
||||||
"Applied config profile %s to instance %s (env=%d, files=%d, mounts=%d, git_mounts=%d)",
|
"Applied config profile %s to instance %s (env=%d, files=%d, mounts=%d, git_mounts=%d, stacked=%d)",
|
||||||
resolved.profile_name,
|
resolved.profile_name,
|
||||||
instance.id,
|
instance.id,
|
||||||
len(profile_env),
|
len(profile_env),
|
||||||
len(profile_files),
|
len(profile_files),
|
||||||
len(profile_mounts),
|
len(profile_mounts),
|
||||||
len(git_mount_volumes),
|
len(git_mount_volumes),
|
||||||
|
len(profile_mounts) - len(stacked_profile_mounts),
|
||||||
)
|
)
|
||||||
except ConfigProfileCycleError as exc:
|
except ConfigProfileCycleError as exc:
|
||||||
logger.error(
|
logger.error(
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ import pytest
|
|||||||
|
|
||||||
from src.services.tool.instance_service import (
|
from src.services.tool.instance_service import (
|
||||||
_get_repository_mount_name,
|
_get_repository_mount_name,
|
||||||
|
_stack_profile_mounts_with_git_mounts,
|
||||||
modify_compose_file,
|
modify_compose_file,
|
||||||
prepare_manifest_instance,
|
prepare_manifest_instance,
|
||||||
)
|
)
|
||||||
@@ -85,6 +86,159 @@ class TestGetRepositoryMountName:
|
|||||||
assert _get_repository_mount_name(repo) == "my-cool-repo"
|
assert _get_repository_mount_name(repo) == "my-cool-repo"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
class TestStackProfileMountsWithGitMounts:
|
||||||
|
"""Tests for _stack_profile_mounts_with_git_mounts."""
|
||||||
|
|
||||||
|
def test_exact_overlap_merges_profile_files_into_git_source(
|
||||||
|
self, tmp_path
|
||||||
|
) -> None:
|
||||||
|
"""When a profile mount targets the same directory as a git mount,
|
||||||
|
the profile files should be copied into the git-mount source so the
|
||||||
|
container sees both sets of files through one bind mount."""
|
||||||
|
git_source = tmp_path / "git" / "repo-clone"
|
||||||
|
git_source.mkdir(parents=True)
|
||||||
|
(git_source / "existing.txt").write_text("from git")
|
||||||
|
|
||||||
|
profile_source = tmp_path / "profile" / "home_user_.pi"
|
||||||
|
profile_source.mkdir(parents=True)
|
||||||
|
(profile_source / "settings.json").write_text("{}")
|
||||||
|
|
||||||
|
profile_mounts = [
|
||||||
|
{
|
||||||
|
"source": str(profile_source),
|
||||||
|
"target": "/home/user/.pi",
|
||||||
|
"type": "bind",
|
||||||
|
"readonly": False,
|
||||||
|
}
|
||||||
|
]
|
||||||
|
git_mount_volumes = [
|
||||||
|
{"source": str(git_source), "target": "/home/user/.pi", "type": "bind"}
|
||||||
|
]
|
||||||
|
|
||||||
|
result = _stack_profile_mounts_with_git_mounts(
|
||||||
|
profile_mounts, git_mount_volumes
|
||||||
|
)
|
||||||
|
|
||||||
|
assert result == []
|
||||||
|
assert (git_source / "existing.txt").read_text() == "from git"
|
||||||
|
assert (git_source / "settings.json").read_text() == "{}"
|
||||||
|
|
||||||
|
def test_descendant_overlap_copies_into_subdirectory(self, tmp_path) -> None:
|
||||||
|
"""Profile mounts targeting a child directory are copied into the
|
||||||
|
corresponding subdirectory of the git-mount source."""
|
||||||
|
git_source = tmp_path / "git"
|
||||||
|
git_source.mkdir()
|
||||||
|
(git_source / "README").write_text("repo")
|
||||||
|
|
||||||
|
profile_source = tmp_path / "profile" / "agent"
|
||||||
|
profile_source.mkdir(parents=True)
|
||||||
|
(profile_source / "settings.json").write_text("x")
|
||||||
|
|
||||||
|
profile_mounts = [
|
||||||
|
{
|
||||||
|
"source": str(profile_source),
|
||||||
|
"target": "/home/user/.pi/agent",
|
||||||
|
"type": "bind",
|
||||||
|
}
|
||||||
|
]
|
||||||
|
git_mount_volumes = [
|
||||||
|
{"source": str(git_source), "target": "/home/user/.pi", "type": "bind"}
|
||||||
|
]
|
||||||
|
|
||||||
|
result = _stack_profile_mounts_with_git_mounts(
|
||||||
|
profile_mounts, git_mount_volumes
|
||||||
|
)
|
||||||
|
|
||||||
|
assert result == []
|
||||||
|
assert (git_source / "agent" / "settings.json").read_text() == "x"
|
||||||
|
assert (git_source / "README").read_text() == "repo"
|
||||||
|
|
||||||
|
def test_non_overlapping_mounts_left_untouched(self, tmp_path) -> None:
|
||||||
|
"""Profile mounts that do not overlap a git mount are returned as-is."""
|
||||||
|
git_source = tmp_path / "git"
|
||||||
|
git_source.mkdir()
|
||||||
|
|
||||||
|
profile_source = tmp_path / "profile"
|
||||||
|
profile_source.mkdir()
|
||||||
|
(profile_source / "config").write_text("c")
|
||||||
|
|
||||||
|
profile_mounts = [
|
||||||
|
{
|
||||||
|
"source": str(profile_source),
|
||||||
|
"target": "/home/user/.config",
|
||||||
|
"type": "bind",
|
||||||
|
}
|
||||||
|
]
|
||||||
|
git_mount_volumes = [
|
||||||
|
{"source": str(git_source), "target": "/home/user/.pi", "type": "bind"}
|
||||||
|
]
|
||||||
|
|
||||||
|
result = _stack_profile_mounts_with_git_mounts(
|
||||||
|
profile_mounts, git_mount_volumes
|
||||||
|
)
|
||||||
|
|
||||||
|
assert result == profile_mounts
|
||||||
|
|
||||||
|
def test_git_source_file_does_not_consume_profile_mount(self, tmp_path) -> None:
|
||||||
|
"""If the overlapping git-mount source is a file, the profile mount
|
||||||
|
cannot be merged and must be kept."""
|
||||||
|
git_source = tmp_path / "file.txt"
|
||||||
|
git_source.write_text("file")
|
||||||
|
|
||||||
|
profile_source = tmp_path / "profile"
|
||||||
|
profile_source.mkdir()
|
||||||
|
(profile_source / "settings.json").write_text("{}")
|
||||||
|
|
||||||
|
profile_mounts = [
|
||||||
|
{
|
||||||
|
"source": str(profile_source),
|
||||||
|
"target": "/home/user/.pi",
|
||||||
|
"type": "bind",
|
||||||
|
}
|
||||||
|
]
|
||||||
|
git_mount_volumes = [
|
||||||
|
{
|
||||||
|
"source": str(git_source),
|
||||||
|
"target": "/home/user/.pi/file.txt",
|
||||||
|
"type": "bind",
|
||||||
|
}
|
||||||
|
]
|
||||||
|
|
||||||
|
result = _stack_profile_mounts_with_git_mounts(
|
||||||
|
profile_mounts, git_mount_volumes
|
||||||
|
)
|
||||||
|
|
||||||
|
assert result == profile_mounts
|
||||||
|
|
||||||
|
def test_profile_source_file_copied_into_git_source(self, tmp_path) -> None:
|
||||||
|
"""A profile mount that supplies a single file is copied into the
|
||||||
|
git-mount source directory."""
|
||||||
|
git_source = tmp_path / "git"
|
||||||
|
git_source.mkdir()
|
||||||
|
|
||||||
|
profile_source = tmp_path / "settings.json"
|
||||||
|
profile_source.write_text("{}")
|
||||||
|
|
||||||
|
profile_mounts = [
|
||||||
|
{
|
||||||
|
"source": str(profile_source),
|
||||||
|
"target": "/home/user/.pi/settings.json",
|
||||||
|
"type": "bind",
|
||||||
|
}
|
||||||
|
]
|
||||||
|
git_mount_volumes = [
|
||||||
|
{"source": str(git_source), "target": "/home/user/.pi", "type": "bind"}
|
||||||
|
]
|
||||||
|
|
||||||
|
result = _stack_profile_mounts_with_git_mounts(
|
||||||
|
profile_mounts, git_mount_volumes
|
||||||
|
)
|
||||||
|
|
||||||
|
assert result == []
|
||||||
|
assert (git_source / "settings.json").read_text() == "{}"
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.unit
|
@pytest.mark.unit
|
||||||
async def test_prepare_manifest_instance_uses_workspace_path_basename():
|
async def test_prepare_manifest_instance_uses_workspace_path_basename():
|
||||||
"""WORKSPACE_NAME must be the repo-named workspace directory, not workspace.name."""
|
"""WORKSPACE_NAME must be the repo-named workspace directory, not workspace.name."""
|
||||||
|
|||||||
Reference in New Issue
Block a user