Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
"""Unit tests for profiler config_modifiers/protocol helpers."""

import copy
import errno
from pathlib import Path
from unittest.mock import AsyncMock, patch

import pytest
Expand Down Expand Up @@ -32,10 +34,17 @@
EngineType,
SearchStrategy,
)
from dynamo.profiler.utils.dgd_materialization import (
DGDMaterializationPurpose,
materialize_dgd,
)
from dynamo.profiler.utils.dgdr_v1beta1_types import (
DynamoGraphDeploymentRequestSpec,
OverridesSpec,
)
from dynamo.profiler.utils.model_info import (
model_ref_allows_implicit_trust_remote_code,
)
from dynamo.profiler.utils.profile_common import ProfilerOperationalConfig
except ImportError:
pytest.skip("dynamo.llm bindings not available", allow_module_level=True)
Expand Down Expand Up @@ -1735,6 +1744,82 @@ def test_materialize_dgd_shell_form_preserves_syntax() -> None:
assert result_args[0] == original_cmd + " --trust-remote-code"


@pytest.mark.parametrize(
("path_kind", "expected"),
[
("directory", True),
("symlink", True),
("file", False),
("missing", False),
("child_of_file", False),
],
)
def test_implicit_trust_requires_local_directory(tmp_path, path_kind, expected):
model_path = tmp_path / "model"
if path_kind == "directory":
model_path.mkdir()
elif path_kind == "symlink":
target = tmp_path / "snapshot"
target.mkdir()
model_path.symlink_to(target, target_is_directory=True)
elif path_kind in ("file", "child_of_file"):
model_path.touch()
if path_kind == "child_of_file":
model_path /= "child"

assert model_ref_allows_implicit_trust_remote_code(model_path) is expected


@pytest.mark.parametrize("explicit_trust", [False, True])
def test_materialize_dgd_inaccessible_model_path(
tmp_path, monkeypatch, caplog, explicit_trust
):
model_path = tmp_path / "model"
model_path.mkdir()
(model_path / "config.json").write_text("{}")
error = PermissionError(errno.EACCES, "Cannot inspect model", str(model_path))
original_stat = Path.stat

def stat(self, *args, **kwargs):
if self == model_path:
raise error
return original_stat(self, *args, **kwargs)

monkeypatch.setattr(Path, "stat", stat)
config = _make_dgd_with_workers("decode")
if explicit_trust:
_main_container(_components_by_name(config)["decode"])["args"].append(
"--trust-remote-code"
)
original_config = copy.deepcopy(config)

if explicit_trust:
result = materialize_dgd(
config,
purpose=DGDMaterializationPurpose.FINAL_OUTPUT,
runtime_backend="vllm",
model_name_or_path=str(model_path),
)
assert result == original_config
else:
with pytest.raises(RuntimeError, match="Cannot inspect model path") as exc:
materialize_dgd(
config,
purpose=DGDMaterializationPurpose.FINAL_OUTPUT,
runtime_backend="vllm",
model_name_or_path=str(model_path),
)
assert exc.value.__cause__ is error
assert str(model_path) in str(exc.value)
assert "symlink ownership" in str(exc.value)
assert "modelCache.pvcModelPath" in str(exc.value)
assert "mutable remote" not in str(exc.value)

assert config == original_config
assert "auto_map detection is inconclusive" in caplog.text
assert "injecting --trust-remote-code" not in caplog.text


def test_model_has_auto_map_returns_true_on_unexpected_error() -> None:
"""Unexpected errors (network, auth) must return True (conservative default)
rather than silently returning False and risking a missed injection."""
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@
"""Unit tests for resolve_model_path() and the rapid.py / thorough.py call
sites that feed its result into aiconfigurator."""

import asyncio
import copy
import errno
import os
from unittest.mock import AsyncMock, MagicMock, patch

import pandas as pd
import pytest
import yaml

pytestmark = [
pytest.mark.pre_merge,
Expand All @@ -18,6 +22,7 @@
]

try:
from dynamo.profiler.profile_sla import run_profile
from dynamo.profiler.rapid import (
_generate_dgd_from_pick,
_run_autoscale_sim,
Expand All @@ -28,7 +33,10 @@
run_thorough,
)
from dynamo.profiler.utils.config_modifiers import CONFIG_MODIFIERS
from dynamo.profiler.utils.dgd_materialization import DGDMaterializationPurpose
from dynamo.profiler.utils.dgd_materialization import (
DGDMaterializationPurpose,
materialize_dgd,
)
from dynamo.profiler.utils.dgdr_v1beta1_types import (
DynamoGraphDeploymentRequestSpec,
HardwareSpec,
Expand Down Expand Up @@ -167,6 +175,85 @@ def test_returns_hf_id_when_dir_has_no_config_json(self, tmp_path):
dgdr = _make_dgdr(modelCache=_pvc_model_cache(str(tmp_path), "model"))
assert resolve_model_path(dgdr) == _HF_ID

def test_inaccessible_pvc_config_during_materialization(
self, tmp_path, monkeypatch
):
local_dir = tmp_path / "model"
_make_model_dir(local_dir)
config_path = local_dir / "config.json"
dgdr = _make_dgdr(
backend="vllm", modelCache=_pvc_model_cache(str(tmp_path), "model")
)
error = PermissionError(errno.EACCES, "Permission denied", str(config_path))
original_stat = os.stat

def stat(path, *args, **kwargs):
if os.fspath(path) == str(config_path):
raise error
return original_stat(path, *args, **kwargs)

monkeypatch.setattr(os, "stat", stat)
Comment thread
tedzhouhk marked this conversation as resolved.
blueprint = {
"spec": {
"components": [
{
"name": "worker",
"type": "worker",
"podTemplate": {
"spec": {"containers": [{"name": "main", "args": []}]}
},
}
]
}
}
with (
patch("dynamo.profiler.utils.model_info.hf_hub_download") as download,
pytest.raises(RuntimeError, match="Cannot inspect PVC model config") as exc,
):
materialize_dgd(
blueprint,
purpose=DGDMaterializationPurpose.FINAL_OUTPUT,
runtime_backend=dgdr.backend,
model_name_or_path=resolve_model_path(dgdr),
)
assert exc.value.__cause__ is error
assert str(config_path) in str(exc.value)
assert "symlink ownership" in str(exc.value)
assert "modelCache.pvcModelPath" in str(exc.value)
download.assert_not_called()

def test_run_profile_records_pvc_inspection_failure(self, tmp_path, monkeypatch):
local_dir = tmp_path / "model"
_make_model_dir(local_dir)
config_path = local_dir / "config.json"
dgdr = _make_dgdr(modelCache=_pvc_model_cache(str(tmp_path), "model"))
error = PermissionError(errno.EACCES, "Permission denied", str(config_path))
original_stat = os.stat

def stat(path, *args, **kwargs):
if os.fspath(path) == str(config_path):
raise error
return original_stat(path, *args, **kwargs)

monkeypatch.setattr(os, "stat", stat)
output_dir = tmp_path / "output"
with (
patch(
"dynamo.profiler.profile_sla.check_model_hardware_support",
side_effect=AssertionError("Must not fall back to the Hub model"),
),
pytest.raises(RuntimeError, match="Cannot inspect PVC model config") as exc,
):
asyncio.run(
run_profile(dgdr, ProfilerOperationalConfig(output_dir=str(output_dir)))
)
status = yaml.safe_load((output_dir / "profiler_status.yaml").read_text())
assert status["status"] == "failed"
assert status["error"] == str(exc.value)
assert str(config_path) in status["error"]
assert "modelCache.pvcModelPath" in status["error"]
assert exc.value.__cause__ is error


# ---------------------------------------------------------------------------
# rapid.py call sites
Expand Down
29 changes: 23 additions & 6 deletions components/src/dynamo/profiler/utils/model_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import json
import logging
from pathlib import Path
from stat import S_ISDIR
from typing import Optional, Union

from huggingface_hub import hf_hub_download, model_info
Expand Down Expand Up @@ -144,8 +145,9 @@ def model_has_auto_map(

Works for both local directories and Hub model IDs. Reads ``config.json``
directly (no ``AutoConfig`` load) so it works even for architectures
that ``transformers`` doesn't know about. Returns False on any read
error so callers can treat detection as best-effort.
that ``transformers`` doesn't know about. Returns False for missing or
malformed configs. Unexpected read errors return True so callers apply
their trust policy; this does not itself authorize custom code execution.
"""
path = Path(model_name_or_path)
try:
Expand Down Expand Up @@ -184,11 +186,12 @@ def model_has_auto_map(
return False
except Exception as e:
# Unexpected failure (network, auth, I/O). We cannot determine whether
# the model needs trust_remote_code, so conservatively return True to
# avoid workers crashing at load time.
# the model needs trust_remote_code, so return True and let the caller
# decide whether custom code execution is allowed.
logger.warning(
"model_has_auto_map: unexpected error reading config.json for %s: %s "
"— defaulting to True (injecting --trust-remote-code).",
"— auto_map detection is inconclusive; returning True for the "
"caller's trust policy check.",
model_name_or_path,
e,
)
Expand All @@ -205,9 +208,23 @@ def model_ref_allows_implicit_trust_remote_code(
remote HF model IDs are treated as mutable and must opt in explicitly.
Only local directories (including PVC-resolved snapshots) qualify for
implicit ``--trust-remote-code`` injection.

Raises RuntimeError when the path cannot be inspected, rather than
treating an inaccessible local path as a remote model ID.
"""
path = Path(model_name_or_path)
return path.exists() and path.is_dir()
try:
Comment thread
tedzhouhk marked this conversation as resolved.
return S_ISDIR(path.stat().st_mode)
except (FileNotFoundError, NotADirectoryError):
return False
except OSError as e:
raise RuntimeError(
f"Cannot inspect model path {str(path)!r} to determine whether "
f"--trust-remote-code may be enabled automatically: {e}. "
"Check directory permissions and symlink ownership for the profiler "
"user. For PVC models, set modelCache.pvcModelPath to the actual "
"snapshot directory or create the symlink with the profiler's UID."
) from e


class ModelInfo(BaseModel):
Expand Down
16 changes: 14 additions & 2 deletions components/src/dynamo/profiler/utils/profile_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import logging
import os
from dataclasses import dataclass, field
from stat import S_ISREG

import pandas as pd

Expand Down Expand Up @@ -164,8 +165,19 @@ def resolve_model_path(dgdr: DynamoGraphDeploymentRequestSpec) -> str:
dgdr.modelCache.pvcMountPath,
dgdr.modelCache.pvcModelPath,
)
if os.path.isfile(os.path.join(local_path, "config.json")):
return local_path
config_path = os.path.join(local_path, "config.json")
try:
if S_ISREG(os.stat(config_path).st_mode):
return local_path
except (FileNotFoundError, NotADirectoryError):
pass
except OSError as e:
raise RuntimeError(
f"Cannot inspect PVC model config {config_path!r}: {e}. "
"Check directory permissions and symlink ownership for the "
"profiler user, or set modelCache.pvcModelPath to an accessible "
"snapshot directory."
) from e
return dgdr.model


Expand Down
Loading