From fa41c47250ffa50288fb019564a99a8f3d149687 Mon Sep 17 00:00:00 2001 From: Joy Liu Date: Wed, 10 Jun 2026 17:25:30 -0400 Subject: [PATCH 1/3] :new: Add ModalProvider for running environments in Modal sandboxes --- pyproject.toml | 4 + .../core/containers/runtime/modal_provider.py | 489 ++++++++++++++++++ tests/test_core/test_modal_provider.py | 414 +++++++++++++++ 3 files changed, 907 insertions(+) create mode 100644 src/openenv/core/containers/runtime/modal_provider.py create mode 100644 tests/test_core/test_modal_provider.py diff --git a/pyproject.toml b/pyproject.toml index 656d2c2bf..44c22521f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -65,6 +65,10 @@ aca = [ "azure-containerapps-sandbox>=0.1.0b2,<0.2.0", "azure-identity>=1.23.0", ] +modal = [ + "modal>=1.4.0", + "pyyaml>=6.0", +] inspect = [ "inspect-ai>=0.3.0", ] diff --git a/src/openenv/core/containers/runtime/modal_provider.py b/src/openenv/core/containers/runtime/modal_provider.py new file mode 100644 index 000000000..ca7285bd1 --- /dev/null +++ b/src/openenv/core/containers/runtime/modal_provider.py @@ -0,0 +1,489 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. + +""" +Modal container provider for running OpenEnv environments in Modal sandboxes. + +Requires the ``modal`` SDK: ``pip install modal>=1.4`` + +Supports both the stable Sandbox API (``modal.Sandbox.create``) and the beta +Sandbox v2 API (``modal.Sandbox._experimental_create``). Sandbox v2 is opt-in +via ``use_sandbox_v2=True`` because it is an experimental feature; see +https://modal.com/docs/guide/sandbox-v2 . +""" + +from __future__ import annotations + +import json +import logging +import shlex +import time +from typing import Any, Dict, Optional + +import yaml + +from .providers import ContainerProvider + +logger = logging.getLogger(__name__) + + +class ModalProvider(ContainerProvider): + """ + Container provider that runs environments in Modal sandboxes. + + Example: + >>> provider = ModalProvider(app_name="openenv") + >>> image = ModalProvider.image_from_dockerfile("envs/echo_env/server/Dockerfile") + >>> base_url = provider.start_container(image) + >>> provider.wait_for_ready(base_url) + >>> provider.stop_container() + + Sandbox v2 (beta) is opt-in: + >>> provider = ModalProvider(app_name="openenv", use_sandbox_v2=True) + """ + + _dockerfile_registry: Dict[str, Dict[str, Any]] = {} + + def __init__( + self, + *, + app_name: str = "openenv", + use_sandbox_v2: bool = False, + cpu: Optional[float] = None, + memory: Optional[int] = None, + timeout: int = 300, + cmd: Optional[str] = None, + ): + """ + Args: + app_name (`str`, *optional*, defaults to `"openenv"`): + Modal app name the sandbox is created under. Looked up (and + created if missing) via ``modal.App.lookup``. + use_sandbox_v2 (`bool`, *optional*, defaults to `False`): + When `True`, sandboxes are created via the beta + ``modal.Sandbox._experimental_create`` API (Sandbox v2). This + is an experimental feature and is therefore off by default; see + https://modal.com/docs/guide/sandbox-v2 . + cpu (`float`, *optional*): + Number of CPU cores to request for the sandbox. + memory (`int`, *optional*): + Memory in MiB to request. Ignored when `use_sandbox_v2` is + `True` (Sandbox v2 does not accept a memory request). + timeout (`int`, *optional*, defaults to `300`): + Maximum sandbox lifetime in seconds. + cmd (`str`, *optional*): + Shell command to start the server inside the sandbox. When + omitted, the command is auto-discovered from ``openenv.yaml`` + (falling back to the Dockerfile ``CMD``). + """ + # Import eagerly so configuration errors surface at construction time. + import modal + + self._modal = modal + self._app_name = app_name + self._use_sandbox_v2 = use_sandbox_v2 + self._cpu = cpu + self._memory = memory + self._timeout = timeout + self._cmd = cmd + self._sandbox: Any = None + self._base_url: Optional[str] = None + + if use_sandbox_v2: + logger.info( + "Using Modal Sandbox v2 (experimental). This feature must be enabled." + ) + + def _exec_capture(self, cmd: str, timeout: int = 10) -> str: + """Run *cmd* inside the sandbox and return its captured stdout.""" + proc = self._sandbox.exec("bash", "-c", cmd, timeout=timeout) + try: + out = proc.stdout.read() + except Exception: + out = "" + proc.wait() + return out or "" + + def _discover_server_cmd(self, port: int = 8000) -> str: + """Discover the server command from ``openenv.yaml`` inside the sandbox. + + Finds the file, reads the ``app`` field, and constructs a command + of the form ``cd && python -m uvicorn --host 0.0.0.0 --port ``. + + Raises: + ValueError: If ``openenv.yaml`` is not found or lacks an ``app`` field. + """ + yaml_path = self._find_openenv_yaml() + if yaml_path is None: + raise ValueError( + "Could not find openenv.yaml inside the sandbox. " + "Pass an explicit cmd= to ModalProvider or start_container()." + ) + + content = self._exec_capture(f"cat {shlex.quote(yaml_path)}") + app = self._parse_app_field(content) + if app is None: + raise ValueError( + f"openenv.yaml at {yaml_path} does not contain an 'app' field. " + "Pass an explicit cmd= to ModalProvider or start_container()." + ) + + # The directory containing openenv.yaml is the env root + env_root = yaml_path.rsplit("/", 1)[0] + return ( + f"cd {shlex.quote(env_root)} && " + f"python -m uvicorn {shlex.quote(app)} --host 0.0.0.0 --port {port}" + ) + + def _find_openenv_yaml(self) -> Optional[str]: + """Locate ``openenv.yaml`` inside the sandbox. + + Tries the modern layout path ``/app/env/openenv.yaml`` first, + then falls back to a ``find`` command for the old layout. + """ + # Fast path: modern Dockerfile layout + out = self._exec_capture("test -f /app/env/openenv.yaml && echo found") + if "found" in (out or ""): + return "/app/env/openenv.yaml" + + # Fallback: search for it (redirect stderr so error messages + # like "No such file or directory" don't get mistaken for paths). + path = self._exec_capture( + "find /app -maxdepth 4 -name openenv.yaml -print -quit 2>/dev/null" + ).strip() + if path and path.startswith("/"): + return path + + return None + + @staticmethod + def _parse_app_field(yaml_content: str) -> Optional[str]: + """Extract the ``app`` value from raw openenv.yaml content. + + Uses PyYAML to handle comments, quotes, and nested keys correctly. + """ + try: + data = yaml.safe_load(yaml_content) or {} + except Exception: + return None + + if not isinstance(data, dict): + return None + + value = data.get("app") + if isinstance(value, str): + value = value.strip() + return value if value else None + return None + + @staticmethod + def _parse_dockerfile_cmd(dockerfile_content: str) -> Optional[str]: + """Extract the server command from the last ``CMD`` in a Dockerfile. + + Handles exec form (``CMD ["prog", "arg"]``) and shell form + (``CMD prog arg``). When a Dockerfile has multiple ``CMD`` + instructions (e.g. multi-stage builds), the last one wins - same + semantics as Docker itself. Lines where ``CMD`` appears inside a + comment are ignored. + + Returns: + The command as a single string, or ``None`` if no ``CMD`` found. + """ + import re + + last_cmd: Optional[str] = None + for line in dockerfile_content.splitlines(): + stripped = line.strip() + # Skip comments + if stripped.startswith("#"): + continue + match = re.match(r"CMD\s+(.+)", stripped, flags=re.IGNORECASE) + if match: + last_cmd = match.group(1).strip() + + if last_cmd is None: + return None + + # Exec form: CMD ["executable", "param1", ...] + if last_cmd.startswith("["): + try: + parts = json.loads(last_cmd) + if isinstance(parts, list) and all(isinstance(p, str) for p in parts): + return " ".join(parts) + except (json.JSONDecodeError, TypeError): + pass + + # Shell form: CMD executable param1 ... + return last_cmd if last_cmd else None + + @classmethod + def image_from_dockerfile( + cls, + dockerfile_path: str, + context_dir: str | None = None, + ) -> str: + """Validate a Dockerfile and return a ``dockerfile:`` URI for + :meth:`start_container`. + + Eagerly validates the Dockerfile (existence, COPY sources) and stores + its content in an internal registry. The actual ``modal.Image`` is + created later inside ``start_container``. + + Args: + dockerfile_path (`str`): + Path to the Dockerfile on disk. + context_dir (`str`, *optional*): + Build context directory. Defaults to the Dockerfile's + grandparent directory, matching the ``openenv init`` + convention where Dockerfiles live in + ``/server/Dockerfile`` and the build context is + ``/``. Pass explicitly for non-standard layouts + (e.g. ``context_dir="."`` for repo-root contexts). + + Returns: + `str`: A ``"dockerfile:"`` string to pass to + ``start_container``. + + Raises: + FileNotFoundError: If *dockerfile_path* does not exist. + ValueError: If *context_dir* is given but does not exist, + or if COPY sources in the Dockerfile cannot be found + under the resolved context directory. + """ + import pathlib + import re + + src = pathlib.Path(dockerfile_path).resolve() + if not src.is_file(): + raise FileNotFoundError(f"Dockerfile not found: {dockerfile_path}") + + if context_dir is not None: + ctx = pathlib.Path(context_dir) + if not ctx.is_dir(): + raise ValueError(f"context_dir does not exist: {context_dir}") + else: + # Default: grandparent of the Dockerfile, matching the + # openenv init layout (/server/Dockerfile -> /). + ctx = src.parent.parent + + content = src.read_text() + + # Validate that COPY sources exist under the context directory. + # This catches mismatches early (e.g. a Dockerfile expecting repo + # root as context when we defaulted to the env directory). + for line in content.splitlines(): + m = re.match(r"^\s*COPY\s+(?!--from=)(\S+)\s+", line, re.IGNORECASE) + if not m: + continue + copy_src = m.group(1) + if copy_src.startswith("/"): + continue + resolved = ctx / copy_src + if not resolved.exists() and not any(ctx.glob(copy_src)): + raise ValueError( + f"Dockerfile COPY source '{copy_src}' not found " + f"under context_dir '{ctx}'. This Dockerfile may " + f"expect a different build context (e.g. the repo " + f"root). Pass context_dir explicitly." + ) + + # Parse CMD from the Dockerfile so start_container can use it as a + # fallback when openenv.yaml is unavailable. + parsed_cmd = cls._parse_dockerfile_cmd(content) + + cls._dockerfile_registry[str(src)] = { + "dockerfile_path": str(src), + "context_dir": str(ctx), + "server_cmd": parsed_cmd, + } + + return f"dockerfile:{src}" + + def _build_image(self, image: str) -> Any: + """Build the ``modal.Image`` for *image* (registry tag or dockerfile:).""" + modal = self._modal + + if image.startswith("dockerfile:"): + dockerfile_path = image[len("dockerfile:") :] + meta = self._dockerfile_registry.get(dockerfile_path) + if meta is None: + raise ValueError( + f"No registered Dockerfile metadata for {dockerfile_path}. " + "Call ModalProvider.image_from_dockerfile() first." + ) + return modal.Image.from_dockerfile( + meta["dockerfile_path"], context_dir=meta["context_dir"] + ) + + # Plain registry tag (e.g. "echo-env:latest"). + return modal.Image.from_registry(image) + + def start_container( + self, + image: str, + port: Optional[int] = None, + env_vars: Optional[Dict[str, str]] = None, + **kwargs: Any, + ) -> str: + """ + Create a Modal sandbox from a Docker image or Dockerfile. + + The sandbox is started with a keep-alive process and the server + command is launched via ``exec`` afterwards, mirroring the discovery + flow used by other cloud providers. The server command is resolved in + order: + + 1. Explicit ``cmd`` passed to the constructor. + 2. ``cmd`` key in ``**kwargs`` (popped before forwarding). + 3. Auto-discovered from ``openenv.yaml`` inside the sandbox. + 4. ``CMD`` parsed from the Dockerfile (when *image* came from + ``image_from_dockerfile``). + + Args: + image (`str`): + Registry image tag (e.g. ``"echo-env:latest"``) or + ``"dockerfile:"`` returned by + :meth:`image_from_dockerfile`. + port (`int`, *optional*): + Must be ``None`` or ``8000``. Modal exposes port 8000 via an + encrypted tunnel; other ports raise ``ValueError``. + env_vars (`dict`, *optional*): + Environment variables forwarded to the sandbox. + **kwargs: + ``cmd`` (`str`) to override the server command. + + Returns: + `str`: HTTPS tunnel URL for the sandbox (base_url). + """ + if port is not None and port != 8000: + raise ValueError( + f"ModalProvider only supports port 8000 (got {port}). " + "The Modal tunnel routes to port 8000 inside the sandbox." + ) + + # Resolve the server command (may be None; discovery happens after + # sandbox creation when we can inspect the filesystem). + cmd = kwargs.pop("cmd", None) or self._cmd + + # CMD parsed from Dockerfile (populated for "dockerfile:" images). + parsed_cmd: Optional[str] = None + if image.startswith("dockerfile:"): + meta = self._dockerfile_registry.get(image[len("dockerfile:") :]) + if meta is not None: + parsed_cmd = meta.get("server_cmd") + + modal = self._modal + app = modal.App.lookup(self._app_name, create_if_missing=True) + modal_image = self._build_image(image) + + # Common creation kwargs shared by both APIs. + create_kwargs: Dict[str, Any] = { + "app": app, + "image": modal_image, + "timeout": self._timeout, + "encrypted_ports": [8000], + } + if env_vars: + create_kwargs["env"] = dict(env_vars) + if self._cpu is not None: + create_kwargs["cpu"] = self._cpu + + if self._use_sandbox_v2: + # Sandbox v2 (beta) does not accept a memory request. + self._sandbox = modal.Sandbox._experimental_create( + "sleep", "infinity", **create_kwargs + ) + else: + if self._memory is not None: + create_kwargs["memory"] = self._memory + self._sandbox = modal.Sandbox.create("sleep", "infinity", **create_kwargs) + + try: + # Discover server command from openenv.yaml if not explicitly set. + if cmd is None: + try: + cmd = self._discover_server_cmd() + except ValueError: + # Fall back to CMD parsed from Dockerfile (if available). + if parsed_cmd: + cmd = parsed_cmd + else: + raise + + # Launch the server in the background. Write the PID so we can + # check whether the process crashed in wait_for_ready(). + escaped_cmd = shlex.quote(cmd) + self._exec_capture( + f"nohup bash -c {escaped_cmd} > /tmp/openenv-server.log 2>&1 &" + " echo $! > /tmp/openenv-server.pid" + ) + + # Resolve the public tunnel URL for port 8000. + tunnels = self._sandbox.tunnels() + self._base_url = tunnels[8000].url + except Exception: + self.stop_container() + raise + + return self._base_url + + def stop_container(self) -> None: + """Terminate the Modal sandbox.""" + if self._sandbox is None: + return + + try: + self._sandbox.terminate() + finally: + self._sandbox = None + self._base_url = None + + def wait_for_ready(self, base_url: str, timeout_s: float = 120.0) -> None: + """ + Poll the /health endpoint until the sandbox is ready. + + Uses a longer default timeout (120s) than local Docker providers + because Modal sandboxes may have cold-start latency. + + Args: + base_url (`str`): + Tunnel URL returned by ``start_container()``. + timeout_s (`float`, *optional*, defaults to `120.0`): + Maximum seconds to wait. + + Raises: + TimeoutError: If the sandbox doesn't become ready in time. + RuntimeError: If the server process died (detected via PID check). + """ + import requests + + health_url = f"{base_url}/health" + + deadline = time.time() + timeout_s + while time.time() < deadline: + try: + response = requests.get(health_url, timeout=5.0) + if response.status_code == 200: + return + except requests.RequestException: + pass + + # Early exit: if the server process died, raise immediately + # instead of waiting for the full health-check timeout. + if self._sandbox is not None: + out = self._exec_capture( + "kill -0 $(cat /tmp/openenv-server.pid) 2>/dev/null" + " && echo RUNNING || echo DEAD" + ) + if "DEAD" in (out or ""): + log = self._exec_capture("cat /tmp/openenv-server.log 2>/dev/null") + raise RuntimeError(f"Server process died.\nLog:\n{log}") + + time.sleep(1.0) + + raise TimeoutError( + f"Modal sandbox at {base_url} did not become ready within {timeout_s}s" + ) diff --git a/tests/test_core/test_modal_provider.py b/tests/test_core/test_modal_provider.py new file mode 100644 index 000000000..1102de5d8 --- /dev/null +++ b/tests/test_core/test_modal_provider.py @@ -0,0 +1,414 @@ +# Copyright (c) Meta Platforms, Inc. and affiliates. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. + +"""Unit tests for ModalProvider. All tests mock the modal SDK.""" + +from __future__ import annotations + +import sys +import types +from unittest.mock import MagicMock, patch + +import pytest + + +# --------------------------------------------------------------------------- +# Fake modal SDK module (so tests run without ``pip install modal``) +# --------------------------------------------------------------------------- +def _install_fake_modal(): + """Install a minimal fake ``modal`` package into sys.modules.""" + modal_mod = types.ModuleType("modal") + + class _FakeProcess: + """Mimics modal ContainerProcess: .stdout.read() + .wait().""" + + def __init__(self, output: str = ""): + self.stdout = MagicMock() + self.stdout.read = MagicMock(return_value=output) + + def wait(self): + return 0 + + class _FakeTunnel: + def __init__(self, url: str): + self.url = url + + def _default_exec_output(cmd: str) -> str: + if "test -f /app/env/openenv.yaml" in cmd: + return "found" + if cmd.startswith("cat /app/env/openenv.yaml"): + return "spec_version: 1\nname: test\napp: server.app:app\nport: 8000\n" + if "kill -0" in cmd: + return "RUNNING" + return "" + + class _FakeSandbox: + # Records the (args, kwargs) of the most recent create call. + last_create: tuple = () + last_create_v2: tuple = () + + def __init__(self): + self._exec_fn = _default_exec_output + self.terminated = False + self.exec_calls: list[str] = [] + + def exec(self, *args, **kwargs): + # Provider always invokes ("bash", "-c", ). + cmd = args[2] if len(args) >= 3 else " ".join(args) + self.exec_calls.append(cmd) + return _FakeProcess(self._exec_fn(cmd)) + + def tunnels(self, timeout: int = 50): + return {8000: _FakeTunnel("https://sb-abc-8000.modal.host")} + + def terminate(self, *, wait: bool = False): + self.terminated = True + return 0 + + class _FakeSandboxCls: + @staticmethod + def create(*args, **kwargs): + sb = _FakeSandbox() + _FakeSandboxCls.last_create = (args, kwargs) + return sb + + @staticmethod + def _experimental_create(*args, **kwargs): + sb = _FakeSandbox() + _FakeSandboxCls.last_create_v2 = (args, kwargs) + return sb + + class _FakeApp: + def __init__(self, name): + self.name = name + + class _FakeAppCls: + @staticmethod + def lookup(name, *, create_if_missing=False, **kwargs): + return _FakeApp(name) + + class _FakeImage: + def __init__(self, kind, ref, **kwargs): + self.kind = kind + self.ref = ref + self.kwargs = kwargs + + @staticmethod + def from_dockerfile(path, **kwargs): + return _FakeImage("dockerfile", str(path), **kwargs) + + @staticmethod + def from_registry(tag, *args, **kwargs): + return _FakeImage("registry", tag, **kwargs) + + modal_mod.Sandbox = _FakeSandboxCls + modal_mod.App = _FakeAppCls + modal_mod.Image = _FakeImage + # Exposed for tests that need to introspect fakes. + modal_mod._FakeSandbox = _FakeSandbox + + sys.modules["modal"] = modal_mod + return modal_mod + + +_fake_modal = _install_fake_modal() + +# Now safe to import the provider +from openenv.core.containers.runtime.modal_provider import ModalProvider + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- +@pytest.fixture() +def provider(): + """Return a ModalProvider with default settings.""" + return ModalProvider(app_name="test-app") + + +@pytest.fixture(autouse=True) +def _fast_provider_sleep(): + """Avoid real sleeps in ModalProvider (wait_for_ready).""" + with patch("openenv.core.containers.runtime.modal_provider.time.sleep"): + yield + + +@pytest.fixture(autouse=True) +def _clean_dockerfile_registry(): + """Clear the Dockerfile registry between tests.""" + ModalProvider._dockerfile_registry.clear() + yield + ModalProvider._dockerfile_registry.clear() + + +def _write_dockerfile(tmp_path, body): + server = tmp_path / "server" + server.mkdir(exist_ok=True) + df = server / "Dockerfile" + df.write_text(body) + return df + + +# --------------------------------------------------------------------------- +# Tests: use_sandbox_v2 flag +# --------------------------------------------------------------------------- +class TestSandboxV2Flag: + def test_default_is_off(self): + p = ModalProvider(app_name="a") + assert p._use_sandbox_v2 is False + + def test_v2_emits_log(self, caplog): + import logging + + with caplog.at_level(logging.INFO): + ModalProvider(app_name="a", use_sandbox_v2=True) + assert any( + "Sandbox v2" in r.message and "must be enabled" in r.message + for r in caplog.records + ) + + def test_off_emits_no_log(self, caplog): + import logging + + with caplog.at_level(logging.INFO): + ModalProvider(app_name="a") + assert not any("Sandbox v2" in r.message for r in caplog.records) + + def test_v2_uses_experimental_create(self): + p = ModalProvider(app_name="a", use_sandbox_v2=True) + p.start_container("echo-env:latest") + # The v2 path must have been taken. + assert _fake_modal.Sandbox.last_create_v2 + args, kwargs = _fake_modal.Sandbox.last_create_v2 + assert args[:2] == ("sleep", "infinity") + assert kwargs["encrypted_ports"] == [8000] + + def test_v1_uses_create(self): + p = ModalProvider(app_name="a", use_sandbox_v2=False) + p.start_container("echo-env:latest") + args, kwargs = _fake_modal.Sandbox.last_create + assert args[:2] == ("sleep", "infinity") + assert kwargs["encrypted_ports"] == [8000] + + def test_v2_drops_memory(self): + """Sandbox v2 does not accept a memory request.""" + p = ModalProvider(app_name="a", use_sandbox_v2=True, memory=2048) + p.start_container("echo-env:latest") + _, kwargs = _fake_modal.Sandbox.last_create_v2 + assert "memory" not in kwargs + + def test_v1_forwards_memory(self): + p = ModalProvider(app_name="a", memory=2048) + p.start_container("echo-env:latest") + _, kwargs = _fake_modal.Sandbox.last_create + assert kwargs["memory"] == 2048 + + +# --------------------------------------------------------------------------- +# Tests: start_container +# --------------------------------------------------------------------------- +class TestStartContainer: + def test_returns_tunnel_url(self, provider): + url = provider.start_container("echo-env:latest") + assert url == "https://sb-abc-8000.modal.host" + + def test_rejects_non_8000_port(self, provider): + with pytest.raises(ValueError, match="only supports port 8000"): + provider.start_container("echo-env:latest", port=9000) + + def test_port_8000_allowed(self, provider): + url = provider.start_container("echo-env:latest", port=8000) + assert url.startswith("https://") + + def test_registry_image_used(self, provider): + provider.start_container("echo-env:latest") + _, kwargs = _fake_modal.Sandbox.last_create + assert kwargs["image"].kind == "registry" + assert kwargs["image"].ref == "echo-env:latest" + + def test_env_vars_forwarded(self, provider): + provider.start_container("echo-env:latest", env_vars={"FOO": "bar"}) + _, kwargs = _fake_modal.Sandbox.last_create + assert kwargs["env"] == {"FOO": "bar"} + + def test_cpu_forwarded(self): + p = ModalProvider(app_name="a", cpu=4.0) + p.start_container("echo-env:latest") + _, kwargs = _fake_modal.Sandbox.last_create + assert kwargs["cpu"] == 4.0 + + def test_server_started_in_background(self, provider): + provider.start_container("echo-env:latest") + assert any( + "nohup" in c and "uvicorn server.app:app" in c + for c in provider._sandbox.exec_calls + ) + + +# --------------------------------------------------------------------------- +# Tests: server command resolution +# --------------------------------------------------------------------------- +class TestServerCmd: + def test_explicit_cmd_constructor(self): + p = ModalProvider(app_name="a", cmd="python -m myserver") + p.start_container("echo-env:latest") + assert any("python -m myserver" in c for c in p._sandbox.exec_calls) + + def test_kwarg_cmd_overrides(self, provider): + provider.start_container("echo-env:latest", cmd="custom-cmd") + assert any("custom-cmd" in c for c in provider._sandbox.exec_calls) + + def test_discovers_from_yaml(self, provider): + provider.start_container("echo-env:latest") + assert any( + "cd /app/env && python -m uvicorn server.app:app" in c + for c in provider._sandbox.exec_calls + ) + + def test_no_yaml_raises(self, provider): + def _no_yaml(cmd): + return "" + + original = _fake_modal.Sandbox.create + + def patched(*args, **kwargs): + sb = original(*args, **kwargs) + sb._exec_fn = _no_yaml + return sb + + with patch.object(_fake_modal.Sandbox, "create", staticmethod(patched)): + with pytest.raises(ValueError, match="Could not find openenv.yaml"): + provider.start_container("no-yaml:latest") + + def test_falls_back_to_dockerfile_cmd(self, provider, tmp_path): + df = _write_dockerfile( + tmp_path, + 'FROM python:3.11\nCOPY . /app\nCMD ["uvicorn", "fallback:app"]\n', + ) + image = ModalProvider.image_from_dockerfile(str(df)) + + def _no_yaml(cmd): + return "" + + original = _fake_modal.Sandbox.create + + def patched(*args, **kwargs): + sb = original(*args, **kwargs) + sb._exec_fn = _no_yaml + return sb + + with patch.object(_fake_modal.Sandbox, "create", staticmethod(patched)): + provider.start_container(image) + assert any("uvicorn fallback:app" in c for c in provider._sandbox.exec_calls) + + +# --------------------------------------------------------------------------- +# Tests: image_from_dockerfile +# --------------------------------------------------------------------------- +class TestImageFromDockerfile: + def test_returns_dockerfile_uri(self, tmp_path): + df = _write_dockerfile(tmp_path, "FROM python:3.11\nCMD uvicorn app:app\n") + result = ModalProvider.image_from_dockerfile(str(df)) + assert result.startswith("dockerfile:") + assert str(df.resolve()) in result + + def test_missing_dockerfile_raises(self, tmp_path): + with pytest.raises(FileNotFoundError): + ModalProvider.image_from_dockerfile(str(tmp_path / "nope")) + + def test_bad_context_dir_raises(self, tmp_path): + df = _write_dockerfile(tmp_path, "FROM python:3.11\n") + with pytest.raises(ValueError, match="context_dir does not exist"): + ModalProvider.image_from_dockerfile(str(df), context_dir="/no/such/dir") + + def test_parses_cmd_into_registry(self, tmp_path): + df = _write_dockerfile( + tmp_path, 'FROM python:3.11\nCMD ["uvicorn", "app:app"]\n' + ) + ModalProvider.image_from_dockerfile(str(df)) + key = str(df.resolve()) + assert ( + ModalProvider._dockerfile_registry[key]["server_cmd"] == "uvicorn app:app" + ) + + def test_dockerfile_image_built_from_dockerfile(self, provider, tmp_path): + df = _write_dockerfile( + tmp_path, "FROM python:3.11\nCOPY . /app\nCMD uvicorn app:app\n" + ) + image = ModalProvider.image_from_dockerfile(str(df)) + provider.start_container(image) + _, kwargs = _fake_modal.Sandbox.last_create + assert kwargs["image"].kind == "dockerfile" + + def test_unregistered_dockerfile_raises(self, provider): + with pytest.raises(ValueError, match="No registered Dockerfile metadata"): + provider.start_container("dockerfile:/tmp/never-registered") + + +# --------------------------------------------------------------------------- +# Tests: _parse_app_field +# --------------------------------------------------------------------------- +class TestParseAppField: + def test_simple(self): + assert ModalProvider._parse_app_field("app: server.app:app") == "server.app:app" + + def test_missing(self): + assert ModalProvider._parse_app_field("name: test\nport: 8000") is None + + def test_invalid_yaml(self): + assert ModalProvider._parse_app_field("::: not yaml :::") is None + + +# --------------------------------------------------------------------------- +# Tests: stop_container +# --------------------------------------------------------------------------- +class TestStopContainer: + def test_terminates_sandbox(self, provider): + provider.start_container("echo-env:latest") + sb = provider._sandbox + provider.stop_container() + assert sb.terminated is True + assert provider._sandbox is None + + def test_stop_without_start_is_noop(self, provider): + provider.stop_container() # should not raise + + +# --------------------------------------------------------------------------- +# Tests: wait_for_ready +# --------------------------------------------------------------------------- +class TestWaitForReady: + def test_ready_when_health_200(self, provider): + provider.start_container("echo-env:latest") + with patch("requests.get", return_value=MagicMock(status_code=200)): + provider.wait_for_ready("https://x.modal.host", timeout_s=5) + + def test_dead_process_raises(self, provider): + import requests + + provider.start_container("echo-env:latest") + provider._sandbox._exec_fn = lambda cmd: ( + "DEAD" if "kill -0" in cmd else "boom traceback" + ) + with patch("requests.get", side_effect=requests.ConnectionError("refused")): + with pytest.raises(RuntimeError, match="Server process died"): + provider.wait_for_ready("https://x.modal.host", timeout_s=5) + + def test_timeout_raises(self, provider): + import requests + + provider.start_container("echo-env:latest") + # Keep the process "RUNNING" so the death check doesn't short-circuit. + provider._sandbox._exec_fn = lambda cmd: "RUNNING" if "kill -0" in cmd else "" + with ( + patch("requests.get", side_effect=requests.ConnectionError("refused")), + patch( + "openenv.core.containers.runtime.modal_provider.time.time" + ) as mock_time, + ): + mock_time.side_effect = [0, 1, 100] # start, first loop, past deadline + with pytest.raises(TimeoutError, match="did not become ready"): + provider.wait_for_ready("https://x.modal.host", timeout_s=5) From 937ff754786ac94852aef040c2955eb921a400bd Mon Sep 17 00:00:00 2001 From: Joy Liu Date: Thu, 25 Jun 2026 13:36:37 -0700 Subject: [PATCH 2/3] :new: Add Modal Provider --- envs/echo_env/pyproject.toml | 4 +- envs/echo_env/uv.lock | 12 +- examples/modal_echo_env.py | 52 ++ .../core/containers/runtime/modal_provider.py | 413 +++++++++--- tests/test_core/test_modal_provider.py | 625 +++++++++++------- 5 files changed, 773 insertions(+), 333 deletions(-) create mode 100644 examples/modal_echo_env.py diff --git a/envs/echo_env/pyproject.toml b/envs/echo_env/pyproject.toml index cbbdaf4fe..84831f6c6 100644 --- a/envs/echo_env/pyproject.toml +++ b/envs/echo_env/pyproject.toml @@ -14,8 +14,8 @@ version = "0.1.0" description = "Echo Environment for OpenEnv - simple test environment that echoes back messages" requires-python = ">=3.10" dependencies = [ - # Core OpenEnv dependencies (required for server functionality) - "openenv[core]>=0.2.2", + # Core OpenEnv dependencies (required for server functionality). + "openenv[core]>=0.3.1", "fastapi>=0.115.0", "pydantic>=2.0.0", "uvicorn>=0.24.0", diff --git a/envs/echo_env/uv.lock b/envs/echo_env/uv.lock index 9d8225220..f05cfbfcc 100644 --- a/envs/echo_env/uv.lock +++ b/envs/echo_env/uv.lock @@ -1550,8 +1550,8 @@ wheels = [ ] [[package]] -name = "openenv-core" -version = "0.2.2" +name = "openenv" +version = "0.3.1" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "fastapi" }, @@ -1570,9 +1570,9 @@ dependencies = [ { name = "uvicorn" }, { name = "websockets" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/54/b9/f134f9de0fcb4a44c1376872fb19fe86013a69d226e320dc77217ca2ec78/openenv_core-0.2.2.tar.gz", hash = "sha256:b891eeb38845cd0c72e94f72615b0fe44c893e53822fd0843c1fafc53fc31bad", size = 146412, upload-time = "2026-03-20T17:52:36.651Z" } +sdist = { url = "https://files.pythonhosted.org/packages/7a/16/12825046f4180173d8f845ad95ff7b7aac32486391d822df38d4e52b82df/openenv-0.3.1.tar.gz", hash = "sha256:9e99a08872c781c7c16d5ba214e532caf5cf29f2278eb9cc478544dc538f3918", size = 165346, upload-time = "2026-06-02T08:47:23.918Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/2f/fd/9ab2b271ab763ccb6bf83d7495c45cdef4e38877d96ecf9314e1c4a95fae/openenv_core-0.2.2-py3-none-any.whl", hash = "sha256:1b99233448aa824c7974ad7c53d46d2edb9302cdc5a3ab0e2ade3a4943f17a63", size = 174125, upload-time = "2026-03-20T17:52:35.605Z" }, + { url = "https://files.pythonhosted.org/packages/cc/28/20af86f825054561c10eb31236e19835f79036fd4a9f600f529adef42b7b/openenv-0.3.1-py3-none-any.whl", hash = "sha256:9cad4b66c08a04fd9d9fe48d7b13cd31f8322b348ff24891646e4440e0b7468c", size = 194867, upload-time = "2026-06-02T08:47:22.469Z" }, ] [package.optional-dependencies] @@ -1590,7 +1590,7 @@ version = "0.1.0" source = { editable = "." } dependencies = [ { name = "fastapi" }, - { name = "openenv-core", extra = ["core"] }, + { name = "openenv", extra = ["core"] }, { name = "pydantic" }, { name = "requests" }, { name = "uvicorn" }, @@ -1605,7 +1605,7 @@ dev = [ [package.metadata] requires-dist = [ { name = "fastapi", specifier = ">=0.115.0" }, - { name = "openenv-core", extras = ["core"], specifier = ">=0.2.2" }, + { name = "openenv", extras = ["core"], specifier = ">=0.3.1" }, { name = "pydantic", specifier = ">=2.0.0" }, { name = "pytest", marker = "extra == 'dev'", specifier = ">=9.0.3" }, { name = "pytest-cov", marker = "extra == 'dev'", specifier = ">=4.0.0" }, diff --git a/examples/modal_echo_env.py b/examples/modal_echo_env.py new file mode 100644 index 000000000..6761e2dd6 --- /dev/null +++ b/examples/modal_echo_env.py @@ -0,0 +1,52 @@ +#!/usr/bin/env python3 +"""Hello-world example running the Echo environment on Modal. + +Boots the echo-env server inside a Modal sandbox via ``ModalProvider``, then +talks to it through ``EchoEnv`` over the encrypted Modal tunnel (https/wss). + +Usage: + pip install "modal>=1.4" + modal setup (one-time auth) + PYTHONPATH=src:envs uv run python examples/modal_echo_env.py + +Requires: + A configured Modal account/token (see https://modal.com/docs/guide). +""" + +import asyncio + +from echo_env import EchoEnv +from openenv.core.containers.runtime.modal_provider import ModalProvider + + +async def _interact(base_url: str) -> None: + """Run the async WebSocket interaction against the running sandbox.""" + async with EchoEnv(base_url=base_url) as env: + await env.reset() + + tools = await env.list_tools() + print("Available tools:", [t.name for t in tools]) + + echoed = await env.call_tool("echo_message", message="Hello, World!") + print("echo_message ->", echoed) + + +def main() -> int: + image = ModalProvider.image_from_dockerfile("envs/echo_env/server/Dockerfile") + + # Provision the sandbox synchronously. The Modal SDK's blocking API warns + # when driven from inside a running event loop, so the provider lifecycle is + # kept out of asyncio; only the WebSocket client runs under asyncio.run(). + provider = ModalProvider(app_name="openenv-echo") + base_url = provider.start_container(image) + provider.wait_for_ready(base_url, timeout_s=180) + try: + asyncio.run(_interact(base_url)) + finally: + provider.stop_container() + + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/src/openenv/core/containers/runtime/modal_provider.py b/src/openenv/core/containers/runtime/modal_provider.py index ca7285bd1..9acee87ef 100644 --- a/src/openenv/core/containers/runtime/modal_provider.py +++ b/src/openenv/core/containers/runtime/modal_provider.py @@ -9,10 +9,14 @@ Requires the ``modal`` SDK: ``pip install modal>=1.4`` +The provider boots an OpenEnv server inside a Modal sandbox, exposes it on an +encrypted tunnel, and returns an ``https://`` URL that ``EnvClient`` connects to +over ``wss://``. + Supports both the stable Sandbox API (``modal.Sandbox.create``) and the beta Sandbox v2 API (``modal.Sandbox._experimental_create``). Sandbox v2 is opt-in -via ``use_sandbox_v2=True`` because it is an experimental feature; see -https://modal.com/docs/guide/sandbox-v2 . +via ``use_sandbox_v2=True`` because it is an experimental, private SDK feature; +see https://modal.com/docs/guide/sandbox-v2 . """ from __future__ import annotations @@ -29,20 +33,148 @@ logger = logging.getLogger(__name__) +_DEFAULT_MODAL_PORT = 8000 + +# Modal's default per-container resource requests. We pass configured cpu/memory +# as the *limit* half of Modal's ``(request, limit)`` tuple so they cap usage +# rather than reserving (and billing for) the full amount. See +# https://modal.com/docs/guide/resources . +_DEFAULT_CPU_REQUEST = 0.125 +_DEFAULT_MEMORY_REQUEST = 128 + + +def _require_secure_url(url: str) -> str: + """Enforce https/wss transport (RFC 002 security invariant S1). + + ``EnvClient`` derives its WebSocket URL from this base URL, so a plaintext + URL would become a cleartext ``ws://`` connection. The offending URL is + deliberately omitted from the error because a Modal tunnel URL is a bearer + capability that must not leak into logs. + """ + if not isinstance(url, str) or not url.lower().startswith("https://"): + raise RuntimeError( + "Modal sandbox returned a non-HTTPS tunnel URL. OpenEnv requires an " + "https/wss base_url so EnvClient traffic is encrypted. Refusing to " + "connect over plaintext." + ) + return url + + +class _DefaultModalAdapter: + """Thin adapter over the ``modal`` SDK. + + The provider talks to this private adapter instead of spreading SDK details + through its own logic; tests inject a duck-typed fake in its place. Keeping + the SDK surface here also localizes the feature check for the private + Sandbox v2 (``_experimental_create``) API. + """ + + def __init__(self, *, app_name: str, use_sandbox_v2: bool): + import modal + + self._modal = modal + self._app_name = app_name + self._use_sandbox_v2 = use_sandbox_v2 + + # Sandbox v2 rides on a private SDK entry point that is not present in + # every modal release. Fail fast at construction with clear guidance + # rather than at start_container time with an AttributeError. + if use_sandbox_v2 and not hasattr(modal.Sandbox, "_experimental_create"): + raise RuntimeError( + "use_sandbox_v2=True requires modal.Sandbox._experimental_create, " + "which is not available in the installed modal SDK. Upgrade modal, " + "or use the stable Sandbox API (use_sandbox_v2=False). Sandbox v2 " + "is an experimental feature; contact support@modal.com for access." + ) + + def image_from_registry(self, tag: str) -> Any: + return self._modal.Image.from_registry(tag) + + def image_from_dockerfile(self, dockerfile_path: str, context_dir: str) -> Any: + return self._modal.Image.from_dockerfile( + dockerfile_path, context_dir=context_dir + ) + + def create_sandbox( + self, + *, + image: Any, + encrypted_ports: list[int], + timeout: int, + env: Optional[dict[str, str]], + extra: dict[str, Any], + ) -> Any: + app = self._modal.App.lookup(self._app_name, create_if_missing=True) + kwargs: Dict[str, Any] = { + "app": app, + "image": image, + "encrypted_ports": encrypted_ports, + "timeout": timeout, + **extra, + } + if env: + kwargs["env"] = dict(env) + + # The sandbox is started with a keep-alive entrypoint; the server + # command is launched via exec afterwards (see ModalProvider). + if self._use_sandbox_v2: + return self._modal.Sandbox._experimental_create( + "sleep", "infinity", **kwargs + ) + return self._modal.Sandbox.create("sleep", "infinity", **kwargs) + + def exec(self, sandbox: Any, command: str, *, timeout: int = 10) -> str: + """Run *command* through a shell inside *sandbox* and return its stdout.""" + proc = sandbox.exec("bash", "-c", command, timeout=timeout) + try: + out = proc.stdout.read() + except Exception: + out = "" + proc.wait() + return out or "" + + def tunnel_url(self, sandbox: Any, port: int) -> str: + return sandbox.tunnels()[port].url + + def terminate(self, sandbox: Any) -> None: + sandbox.terminate() + class ModalProvider(ContainerProvider): """ Container provider that runs environments in Modal sandboxes. + ``start_container``'s ``image`` is either a registry tag + (``"echo-env:latest"``) or a ``"dockerfile:"`` reference returned by + :meth:`image_from_dockerfile`. The server is exposed on an encrypted Modal + tunnel and the returned ``https://`` URL is what ``EnvClient`` connects to + over ``wss://``. + + The environment runs untrusted code, so the provider is secure by default: + it enforces https/wss transport, treats the tunnel URL as a bearer secret + (never interpolated into errors), and never surfaces raw sandbox output + unless ``surface_server_logs=True``. + + Only one sandbox is active per provider: calling ``start_container`` again + before ``stop_container()``/``close()`` raises ``RuntimeError`` rather than + orphaning the running sandbox. ``close()`` (and context-manager exit) stops + the active sandbox. + Example: - >>> provider = ModalProvider(app_name="openenv") - >>> image = ModalProvider.image_from_dockerfile("envs/echo_env/server/Dockerfile") - >>> base_url = provider.start_container(image) - >>> provider.wait_for_ready(base_url) - >>> provider.stop_container() + ```python + with ModalProvider(app_name="openenv", cpu=2.0, memory=4096) as provider: + image = ModalProvider.image_from_dockerfile( + "envs/echo_env/server/Dockerfile" + ) + base_url = provider.start_container(image) + provider.wait_for_ready(base_url) + # sandbox terminated on exit + ``` Sandbox v2 (beta) is opt-in: - >>> provider = ModalProvider(app_name="openenv", use_sandbox_v2=True) + ```python + provider = ModalProvider(app_name="openenv", use_sandbox_v2=True) + ``` """ _dockerfile_registry: Dict[str, Dict[str, Any]] = {} @@ -52,10 +184,12 @@ def __init__( *, app_name: str = "openenv", use_sandbox_v2: bool = False, - cpu: Optional[float] = None, - memory: Optional[int] = None, timeout: int = 300, - cmd: Optional[str] = None, + cmd: str | None = None, + cpu: float | None = None, + memory: int | None = None, + surface_server_logs: bool = False, + _adapter: Any = None, ): """ Args: @@ -64,50 +198,60 @@ def __init__( created if missing) via ``modal.App.lookup``. use_sandbox_v2 (`bool`, *optional*, defaults to `False`): When `True`, sandboxes are created via the beta - ``modal.Sandbox._experimental_create`` API (Sandbox v2). This - is an experimental feature and is therefore off by default; see - https://modal.com/docs/guide/sandbox-v2 . - cpu (`float`, *optional*): - Number of CPU cores to request for the sandbox. - memory (`int`, *optional*): - Memory in MiB to request. Ignored when `use_sandbox_v2` is - `True` (Sandbox v2 does not accept a memory request). + ``modal.Sandbox._experimental_create`` API (Sandbox v2). This is + an experimental, private SDK feature and is therefore off by + default; a feature check fails fast at construction if the + installed SDK lacks it. See https://modal.com/docs/guide/sandbox-v2 . timeout (`int`, *optional*, defaults to `300`): Maximum sandbox lifetime in seconds. cmd (`str`, *optional*): Shell command to start the server inside the sandbox. When omitted, the command is auto-discovered from ``openenv.yaml`` (falling back to the Dockerfile ``CMD``). + cpu (`float`, *optional*): + Hard CPU-core limit for the sandbox. Passed as the limit half of + Modal's ``(request, limit)`` tuple (request stays at Modal's + default), so it caps usage rather than reserving cores. When + `None`, Modal's default applies. + memory (`int`, *optional*): + Hard memory limit in MiB for the sandbox (containers exceeding it + are OOM-killed). Passed as the limit half of Modal's + ``(request, limit)`` tuple. When `None`, Modal's default applies. + surface_server_logs (`bool`, *optional*, defaults to `False`): + When `False` (default), captured sandbox output is withheld from + raised errors so secrets the workload printed cannot leak into + orchestrator/CI logs. When `True`, a best-effort redacted, + length-bounded excerpt is included in startup-crash errors. """ - # Import eagerly so configuration errors surface at construction time. - import modal - - self._modal = modal self._app_name = app_name self._use_sandbox_v2 = use_sandbox_v2 - self._cpu = cpu - self._memory = memory self._timeout = timeout self._cmd = cmd + self._cpu = cpu + self._memory = memory + self._surface_server_logs = surface_server_logs self._sandbox: Any = None - self._base_url: Optional[str] = None + self._base_url: str | None = None + # Injected env-var values, used to scrub captured server output before + # it is ever surfaced in an error. + self._redact_values: set[str] = set() + + if _adapter is None: + # Import eagerly (inside the adapter) so SDK/configuration errors — + # including the Sandbox v2 feature check — surface at construction. + self._adapter: Any = _DefaultModalAdapter( + app_name=app_name, use_sandbox_v2=use_sandbox_v2 + ) + else: + self._adapter = _adapter if use_sandbox_v2: logger.info( - "Using Modal Sandbox v2 (experimental). This feature must be enabled." + "Using Modal Sandbox v2 (experimental). This feature must be " + "explicitly enabled. Contact support@modal.com to get access." ) - def _exec_capture(self, cmd: str, timeout: int = 10) -> str: - """Run *cmd* inside the sandbox and return its captured stdout.""" - proc = self._sandbox.exec("bash", "-c", cmd, timeout=timeout) - try: - out = proc.stdout.read() - except Exception: - out = "" - proc.wait() - return out or "" - - def _discover_server_cmd(self, port: int = 8000) -> str: + def _discover_server_cmd(self, port: int = _DEFAULT_MODAL_PORT) -> str: """Discover the server command from ``openenv.yaml`` inside the sandbox. Finds the file, reads the ``app`` field, and constructs a command @@ -123,7 +267,7 @@ def _discover_server_cmd(self, port: int = 8000) -> str: "Pass an explicit cmd= to ModalProvider or start_container()." ) - content = self._exec_capture(f"cat {shlex.quote(yaml_path)}") + content = self._adapter.exec(self._sandbox, f"cat {shlex.quote(yaml_path)}") app = self._parse_app_field(content) if app is None: raise ValueError( @@ -138,21 +282,24 @@ def _discover_server_cmd(self, port: int = 8000) -> str: f"python -m uvicorn {shlex.quote(app)} --host 0.0.0.0 --port {port}" ) - def _find_openenv_yaml(self) -> Optional[str]: + def _find_openenv_yaml(self) -> str | None: """Locate ``openenv.yaml`` inside the sandbox. Tries the modern layout path ``/app/env/openenv.yaml`` first, then falls back to a ``find`` command for the old layout. """ # Fast path: modern Dockerfile layout - out = self._exec_capture("test -f /app/env/openenv.yaml && echo found") + out = self._adapter.exec( + self._sandbox, "test -f /app/env/openenv.yaml && echo found" + ) if "found" in (out or ""): return "/app/env/openenv.yaml" # Fallback: search for it (redirect stderr so error messages # like "No such file or directory" don't get mistaken for paths). - path = self._exec_capture( - "find /app -maxdepth 4 -name openenv.yaml -print -quit 2>/dev/null" + path = self._adapter.exec( + self._sandbox, + "find /app -maxdepth 4 -name openenv.yaml -print -quit 2>/dev/null", ).strip() if path and path.startswith("/"): return path @@ -160,7 +307,7 @@ def _find_openenv_yaml(self) -> Optional[str]: return None @staticmethod - def _parse_app_field(yaml_content: str) -> Optional[str]: + def _parse_app_field(yaml_content: str) -> str | None: """Extract the ``app`` value from raw openenv.yaml content. Uses PyYAML to handle comments, quotes, and nested keys correctly. @@ -180,7 +327,7 @@ def _parse_app_field(yaml_content: str) -> Optional[str]: return None @staticmethod - def _parse_dockerfile_cmd(dockerfile_content: str) -> Optional[str]: + def _parse_dockerfile_cmd(dockerfile_content: str) -> str | None: """Extract the server command from the last ``CMD`` in a Dockerfile. Handles exec form (``CMD ["prog", "arg"]``) and shell form @@ -194,10 +341,9 @@ def _parse_dockerfile_cmd(dockerfile_content: str) -> Optional[str]: """ import re - last_cmd: Optional[str] = None + last_cmd: str | None = None for line in dockerfile_content.splitlines(): stripped = line.strip() - # Skip comments if stripped.startswith("#"): continue match = re.match(r"CMD\s+(.+)", stripped, flags=re.IGNORECASE) @@ -304,8 +450,6 @@ def image_from_dockerfile( def _build_image(self, image: str) -> Any: """Build the ``modal.Image`` for *image* (registry tag or dockerfile:).""" - modal = self._modal - if image.startswith("dockerfile:"): dockerfile_path = image[len("dockerfile:") :] meta = self._dockerfile_registry.get(dockerfile_path) @@ -314,18 +458,18 @@ def _build_image(self, image: str) -> Any: f"No registered Dockerfile metadata for {dockerfile_path}. " "Call ModalProvider.image_from_dockerfile() first." ) - return modal.Image.from_dockerfile( - meta["dockerfile_path"], context_dir=meta["context_dir"] + return self._adapter.image_from_dockerfile( + meta["dockerfile_path"], meta["context_dir"] ) # Plain registry tag (e.g. "echo-env:latest"). - return modal.Image.from_registry(image) + return self._adapter.image_from_registry(image) def start_container( self, image: str, - port: Optional[int] = None, - env_vars: Optional[Dict[str, str]] = None, + port: int | None = None, + env_vars: dict[str, str] | None = None, **kwargs: Any, ) -> str: """ @@ -353,15 +497,24 @@ def start_container( env_vars (`dict`, *optional*): Environment variables forwarded to the sandbox. **kwargs: - ``cmd`` (`str`) to override the server command. + ``cmd`` (`str`) to override the server command; any remaining + keyword arguments are forwarded to ``modal.Sandbox.create``. Returns: `str`: HTTPS tunnel URL for the sandbox (base_url). """ - if port is not None and port != 8000: + if self._sandbox is not None: + raise RuntimeError( + "ModalProvider already has an active sandbox. Call " + "stop_container() (or close()) before starting another — a " + "second start would orphan the running sandbox." + ) + + if port is not None and port != _DEFAULT_MODAL_PORT: raise ValueError( - f"ModalProvider only supports port 8000 (got {port}). " - "The Modal tunnel routes to port 8000 inside the sandbox." + f"ModalProvider only supports port {_DEFAULT_MODAL_PORT} " + f"(got {port}). The Modal tunnel routes to port " + f"{_DEFAULT_MODAL_PORT} inside the sandbox." ) # Resolve the server command (may be None; discovery happens after @@ -369,37 +522,38 @@ def start_container( cmd = kwargs.pop("cmd", None) or self._cmd # CMD parsed from Dockerfile (populated for "dockerfile:" images). - parsed_cmd: Optional[str] = None + parsed_cmd: str | None = None if image.startswith("dockerfile:"): meta = self._dockerfile_registry.get(image[len("dockerfile:") :]) if meta is not None: parsed_cmd = meta.get("server_cmd") - modal = self._modal - app = modal.App.lookup(self._app_name, create_if_missing=True) modal_image = self._build_image(image) - # Common creation kwargs shared by both APIs. - create_kwargs: Dict[str, Any] = { - "app": app, - "image": modal_image, - "timeout": self._timeout, - "encrypted_ports": [8000], - } - if env_vars: - create_kwargs["env"] = dict(env_vars) + extra: Dict[str, Any] = dict(kwargs) if self._cpu is not None: - create_kwargs["cpu"] = self._cpu + extra["cpu"] = (_DEFAULT_CPU_REQUEST, self._cpu) + if self._memory is not None: + extra["memory"] = (_DEFAULT_MEMORY_REQUEST, self._memory) - if self._use_sandbox_v2: - # Sandbox v2 (beta) does not accept a memory request. - self._sandbox = modal.Sandbox._experimental_create( - "sleep", "infinity", **create_kwargs + # Record injected secret values so captured server output can be + # scrubbed before it is ever surfaced in an error. + self._redact_values = {value for value in (env_vars or {}).values() if value} + + # A create failure created nothing, so just drop the recorded secrets + # and re-raise (the double-start guard above guarantees there is no + # pre-existing sandbox to delete). + try: + self._sandbox = self._adapter.create_sandbox( + image=modal_image, + encrypted_ports=[_DEFAULT_MODAL_PORT], + timeout=self._timeout, + env=env_vars, + extra=extra, ) - else: - if self._memory is not None: - create_kwargs["memory"] = self._memory - self._sandbox = modal.Sandbox.create("sleep", "infinity", **create_kwargs) + except Exception: + self._redact_values = set() + raise try: # Discover server command from openenv.yaml if not explicitly set. @@ -416,16 +570,23 @@ def start_container( # Launch the server in the background. Write the PID so we can # check whether the process crashed in wait_for_ready(). escaped_cmd = shlex.quote(cmd) - self._exec_capture( + self._adapter.exec( + self._sandbox, f"nohup bash -c {escaped_cmd} > /tmp/openenv-server.log 2>&1 &" - " echo $! > /tmp/openenv-server.pid" + " echo $! > /tmp/openenv-server.pid", ) # Resolve the public tunnel URL for port 8000. - tunnels = self._sandbox.tunnels() - self._base_url = tunnels[8000].url + self._base_url = _require_secure_url( + self._adapter.tunnel_url(self._sandbox, _DEFAULT_MODAL_PORT) + ) except Exception: - self.stop_container() + # A cleanup failure here must not mask the original error: swallow + # any exception from stop_container() so the root cause propagates. + try: + self.stop_container() + except Exception: + pass raise return self._base_url @@ -433,13 +594,78 @@ def start_container( def stop_container(self) -> None: """Terminate the Modal sandbox.""" if self._sandbox is None: + # Still drop any injected secret values recorded by a failed start. + self._redact_values = set() return try: - self._sandbox.terminate() + self._adapter.terminate(self._sandbox) finally: self._sandbox = None self._base_url = None + self._redact_values = set() + + def close(self) -> None: + """Stop the active sandbox. + + Overrides the base no-op so a caller holding a bare ``ContainerProvider`` + reference can release the sandbox polymorphically (also invoked on + context-manager exit). ``ModalProvider`` holds no separate SDK client, so + this is equivalent to ``stop_container()``. + """ + self.stop_container() + + @property + def base_url(self) -> str: + """URL returned by the last ``start_container``.""" + if self._base_url is None: + raise RuntimeError( + "ModalProvider has no active base_url. Start the provider " + "before reading base_url." + ) + return self._base_url + + def _redact(self, text: str, *, max_chars: int = 2000) -> str: + """Scrub injected secret values and bound length before surfacing output. + + Replaces any injected env-var value with `***` and keeps only the tail. + This is best-effort (exact-match only), which is why server output is + withheld entirely unless ``surface_server_logs=True``. + """ + redacted = text or "" + for value in self._redact_values: + redacted = redacted.replace(value, "***") + if len(redacted) > max_chars: + redacted = "...(truncated)...\n" + redacted[-max_chars:] + return redacted + + def _server_died_message(self) -> str: + """Build the startup-crash error, secure by default. + + Untrusted code can print secrets then force a crash to exfiltrate them + through the exception (which lands in orchestrator/CI logs), so sandbox + output is excluded unless ``surface_server_logs=True``, in which case a + best-effort redacted, bounded excerpt is included. + """ + base = ( + "Modal sandbox server process died during startup. Server output is " + "not surfaced to avoid leaking secrets injected into the sandbox; " + "retrieve /tmp/openenv-server.log from the sandbox out of band, or " + "construct the provider with surface_server_logs=True to include a " + "redacted excerpt." + ) + if not self._surface_server_logs or self._sandbox is None: + return base + + log = self._redact( + self._adapter.exec(self._sandbox, "cat /tmp/openenv-server.log 2>/dev/null") + ) + return ( + "Modal sandbox server process died during startup. The excerpt below " + "is the sandbox server output with injected secret values redacted " + "(best-effort); it may still contain secrets the workload printed " + f"by other means.\nLog (redacted):\n{log}" + ) def wait_for_ready(self, base_url: str, timeout_s: float = 120.0) -> None: """ @@ -474,16 +700,19 @@ def wait_for_ready(self, base_url: str, timeout_s: float = 120.0) -> None: # Early exit: if the server process died, raise immediately # instead of waiting for the full health-check timeout. if self._sandbox is not None: - out = self._exec_capture( + out = self._adapter.exec( + self._sandbox, "kill -0 $(cat /tmp/openenv-server.pid) 2>/dev/null" - " && echo RUNNING || echo DEAD" + " && echo RUNNING || echo DEAD", ) if "DEAD" in (out or ""): - log = self._exec_capture("cat /tmp/openenv-server.log 2>/dev/null") - raise RuntimeError(f"Server process died.\nLog:\n{log}") + raise RuntimeError(self._server_died_message()) time.sleep(1.0) - raise TimeoutError( - f"Modal sandbox at {base_url} did not become ready within {timeout_s}s" - ) + # The tunnel URL is a bearer capability, so it is deliberately omitted + # from the timeout error. + raise TimeoutError(f"Modal sandbox did not become ready within {timeout_s}s.") + + +__all__ = ["ModalProvider"] diff --git a/tests/test_core/test_modal_provider.py b/tests/test_core/test_modal_provider.py index 1102de5d8..bef6c0620 100644 --- a/tests/test_core/test_modal_provider.py +++ b/tests/test_core/test_modal_provider.py @@ -4,7 +4,13 @@ # This source code is licensed under the BSD-style license found in the # LICENSE file in the root directory of this source tree. -"""Unit tests for ModalProvider. All tests mock the modal SDK.""" +"""Unit tests for ModalProvider. + +Provider-behavior tests inject a duck-typed fake adapter (``_adapter=``), so +they run without ``pip install modal``. A separate test installs a minimal fake +``modal`` module to pin the real ``_DefaultModalAdapter`` SDK shape (including +the Sandbox v2 feature check). +""" from __future__ import annotations @@ -13,120 +19,90 @@ from unittest.mock import MagicMock, patch import pytest +from openenv.core.containers.runtime.modal_provider import ( + _DefaultModalAdapter, + ModalProvider, +) +from openenv.core.containers.runtime.providers import ContainerProvider # --------------------------------------------------------------------------- -# Fake modal SDK module (so tests run without ``pip install modal``) +# Fake adapter (duck-typed to _DefaultModalAdapter) # --------------------------------------------------------------------------- -def _install_fake_modal(): - """Install a minimal fake ``modal`` package into sys.modules.""" - modal_mod = types.ModuleType("modal") - - class _FakeProcess: - """Mimics modal ContainerProcess: .stdout.read() + .wait().""" - - def __init__(self, output: str = ""): - self.stdout = MagicMock() - self.stdout.read = MagicMock(return_value=output) - - def wait(self): - return 0 - - class _FakeTunnel: - def __init__(self, url: str): - self.url = url +class _FakeImage: + def __init__(self, kind: str, ref: str): + self.kind = kind + self.ref = ref + + +class _FakeAdapter: + def __init__(self): + self.created: list[dict] = [] + self.exec_commands: list[str] = [] + self.terminated = 0 + self.tunnel = "https://sb-abc-8000.modal.host" + self.fail_create = False + self.fail_terminate = False + self.fail_tunnel = False + self.dead_process = False + self.log = "server crashed" + self.has_yaml = True + + def image_from_registry(self, tag): + return _FakeImage("registry", tag) + + def image_from_dockerfile(self, dockerfile_path, context_dir): + return _FakeImage("dockerfile", dockerfile_path) + + def create_sandbox(self, *, image, encrypted_ports, timeout, env, extra): + if self.fail_create: + raise RuntimeError("create failed") + sandbox = object() + self.created.append( + { + "image": image, + "encrypted_ports": encrypted_ports, + "timeout": timeout, + "env": env, + "extra": extra, + "sandbox": sandbox, + } + ) + return sandbox - def _default_exec_output(cmd: str) -> str: - if "test -f /app/env/openenv.yaml" in cmd: - return "found" - if cmd.startswith("cat /app/env/openenv.yaml"): + def exec(self, sandbox, command, *, timeout=10): + self.exec_commands.append(command) + if "test -f /app/env/openenv.yaml" in command: + return "found" if self.has_yaml else "" + if command.startswith("cat /app/env/openenv.yaml"): return "spec_version: 1\nname: test\napp: server.app:app\nport: 8000\n" - if "kill -0" in cmd: - return "RUNNING" + if "find /app" in command: + return "" + if "kill -0" in command: + return "DEAD" if self.dead_process else "RUNNING" + if "cat /tmp/openenv-server.log" in command: + return self.log return "" - class _FakeSandbox: - # Records the (args, kwargs) of the most recent create call. - last_create: tuple = () - last_create_v2: tuple = () - - def __init__(self): - self._exec_fn = _default_exec_output - self.terminated = False - self.exec_calls: list[str] = [] - - def exec(self, *args, **kwargs): - # Provider always invokes ("bash", "-c", ). - cmd = args[2] if len(args) >= 3 else " ".join(args) - self.exec_calls.append(cmd) - return _FakeProcess(self._exec_fn(cmd)) + def tunnel_url(self, sandbox, port): + if self.fail_tunnel: + raise RuntimeError("tunnel failed") + return self.tunnel - def tunnels(self, timeout: int = 50): - return {8000: _FakeTunnel("https://sb-abc-8000.modal.host")} + def terminate(self, sandbox): + self.terminated += 1 + if self.fail_terminate: + raise RuntimeError("terminate failed") - def terminate(self, *, wait: bool = False): - self.terminated = True - return 0 - - class _FakeSandboxCls: - @staticmethod - def create(*args, **kwargs): - sb = _FakeSandbox() - _FakeSandboxCls.last_create = (args, kwargs) - return sb - @staticmethod - def _experimental_create(*args, **kwargs): - sb = _FakeSandbox() - _FakeSandboxCls.last_create_v2 = (args, kwargs) - return sb - - class _FakeApp: - def __init__(self, name): - self.name = name - - class _FakeAppCls: - @staticmethod - def lookup(name, *, create_if_missing=False, **kwargs): - return _FakeApp(name) - - class _FakeImage: - def __init__(self, kind, ref, **kwargs): - self.kind = kind - self.ref = ref - self.kwargs = kwargs - - @staticmethod - def from_dockerfile(path, **kwargs): - return _FakeImage("dockerfile", str(path), **kwargs) - - @staticmethod - def from_registry(tag, *args, **kwargs): - return _FakeImage("registry", tag, **kwargs) - - modal_mod.Sandbox = _FakeSandboxCls - modal_mod.App = _FakeAppCls - modal_mod.Image = _FakeImage - # Exposed for tests that need to introspect fakes. - modal_mod._FakeSandbox = _FakeSandbox - - sys.modules["modal"] = modal_mod - return modal_mod - - -_fake_modal = _install_fake_modal() - -# Now safe to import the provider -from openenv.core.containers.runtime.modal_provider import ModalProvider +@pytest.fixture() +def adapter(): + return _FakeAdapter() -# --------------------------------------------------------------------------- -# Fixtures -# --------------------------------------------------------------------------- @pytest.fixture() -def provider(): - """Return a ModalProvider with default settings.""" - return ModalProvider(app_name="test-app") +def provider(adapter): + return ModalProvider(app_name="test-app", _adapter=adapter) @pytest.fixture(autouse=True) @@ -152,157 +128,112 @@ def _write_dockerfile(tmp_path, body): return df -# --------------------------------------------------------------------------- -# Tests: use_sandbox_v2 flag -# --------------------------------------------------------------------------- -class TestSandboxV2Flag: - def test_default_is_off(self): - p = ModalProvider(app_name="a") - assert p._use_sandbox_v2 is False - - def test_v2_emits_log(self, caplog): - import logging - - with caplog.at_level(logging.INFO): - ModalProvider(app_name="a", use_sandbox_v2=True) - assert any( - "Sandbox v2" in r.message and "must be enabled" in r.message - for r in caplog.records - ) - - def test_off_emits_no_log(self, caplog): - import logging - - with caplog.at_level(logging.INFO): - ModalProvider(app_name="a") - assert not any("Sandbox v2" in r.message for r in caplog.records) - - def test_v2_uses_experimental_create(self): - p = ModalProvider(app_name="a", use_sandbox_v2=True) - p.start_container("echo-env:latest") - # The v2 path must have been taken. - assert _fake_modal.Sandbox.last_create_v2 - args, kwargs = _fake_modal.Sandbox.last_create_v2 - assert args[:2] == ("sleep", "infinity") - assert kwargs["encrypted_ports"] == [8000] - - def test_v1_uses_create(self): - p = ModalProvider(app_name="a", use_sandbox_v2=False) - p.start_container("echo-env:latest") - args, kwargs = _fake_modal.Sandbox.last_create - assert args[:2] == ("sleep", "infinity") - assert kwargs["encrypted_ports"] == [8000] - - def test_v2_drops_memory(self): - """Sandbox v2 does not accept a memory request.""" - p = ModalProvider(app_name="a", use_sandbox_v2=True, memory=2048) - p.start_container("echo-env:latest") - _, kwargs = _fake_modal.Sandbox.last_create_v2 - assert "memory" not in kwargs - - def test_v1_forwards_memory(self): - p = ModalProvider(app_name="a", memory=2048) - p.start_container("echo-env:latest") - _, kwargs = _fake_modal.Sandbox.last_create - assert kwargs["memory"] == 2048 - - # --------------------------------------------------------------------------- # Tests: start_container # --------------------------------------------------------------------------- class TestStartContainer: def test_returns_tunnel_url(self, provider): - url = provider.start_container("echo-env:latest") - assert url == "https://sb-abc-8000.modal.host" + assert provider.start_container("echo-env:latest") == ( + "https://sb-abc-8000.modal.host" + ) def test_rejects_non_8000_port(self, provider): with pytest.raises(ValueError, match="only supports port 8000"): provider.start_container("echo-env:latest", port=9000) def test_port_8000_allowed(self, provider): - url = provider.start_container("echo-env:latest", port=8000) - assert url.startswith("https://") + assert provider.start_container("echo-env:latest", port=8000).startswith( + "https://" + ) - def test_registry_image_used(self, provider): + def test_registry_image_used(self, provider, adapter): provider.start_container("echo-env:latest") - _, kwargs = _fake_modal.Sandbox.last_create - assert kwargs["image"].kind == "registry" - assert kwargs["image"].ref == "echo-env:latest" + image = adapter.created[0]["image"] + assert image.kind == "registry" + assert image.ref == "echo-env:latest" + assert adapter.created[0]["encrypted_ports"] == [8000] - def test_env_vars_forwarded(self, provider): + def test_env_vars_forwarded(self, provider, adapter): provider.start_container("echo-env:latest", env_vars={"FOO": "bar"}) - _, kwargs = _fake_modal.Sandbox.last_create - assert kwargs["env"] == {"FOO": "bar"} - - def test_cpu_forwarded(self): - p = ModalProvider(app_name="a", cpu=4.0) - p.start_container("echo-env:latest") - _, kwargs = _fake_modal.Sandbox.last_create - assert kwargs["cpu"] == 4.0 + assert adapter.created[0]["env"] == {"FOO": "bar"} - def test_server_started_in_background(self, provider): + def test_server_started_in_background(self, provider, adapter): provider.start_container("echo-env:latest") assert any( "nohup" in c and "uvicorn server.app:app" in c - for c in provider._sandbox.exec_calls + for c in adapter.exec_commands ) + def test_rejects_non_https_tunnel(self, adapter): + adapter.tunnel = "http://insecure.modal.host" + provider = ModalProvider(_adapter=adapter) + with pytest.raises(RuntimeError, match="non-HTTPS"): + provider.start_container("echo-env:latest") + # Failed start cleans up the sandbox it created. + assert adapter.terminated == 1 + + +# --------------------------------------------------------------------------- +# Tests: cpu / memory resources +# --------------------------------------------------------------------------- +class TestResources: + def test_cpu_memory_forwarded_as_limits(self, adapter): + provider = ModalProvider(_adapter=adapter, cpu=2.0, memory=4096) + provider.start_container("echo-env:latest") + extra = adapter.created[0]["extra"] + # Passed as the limit half of Modal's (request, limit) tuple. + assert extra["cpu"] == (0.125, 2.0) + assert extra["memory"] == (128, 4096) + + def test_no_resources_means_no_extra_keys(self, provider, adapter): + provider.start_container("echo-env:latest") + extra = adapter.created[0]["extra"] + assert "cpu" not in extra + assert "memory" not in extra + + def test_extra_kwargs_forwarded(self, provider, adapter): + provider.start_container("echo-env:latest", gpu="A100") + assert adapter.created[0]["extra"]["gpu"] == "A100" + # --------------------------------------------------------------------------- # Tests: server command resolution # --------------------------------------------------------------------------- class TestServerCmd: - def test_explicit_cmd_constructor(self): - p = ModalProvider(app_name="a", cmd="python -m myserver") - p.start_container("echo-env:latest") - assert any("python -m myserver" in c for c in p._sandbox.exec_calls) + def test_explicit_cmd_constructor(self, adapter): + provider = ModalProvider(_adapter=adapter, cmd="python -m myserver") + provider.start_container("echo-env:latest") + assert any("python -m myserver" in c for c in adapter.exec_commands) - def test_kwarg_cmd_overrides(self, provider): + def test_kwarg_cmd_overrides(self, provider, adapter): provider.start_container("echo-env:latest", cmd="custom-cmd") - assert any("custom-cmd" in c for c in provider._sandbox.exec_calls) + assert any("custom-cmd" in c for c in adapter.exec_commands) - def test_discovers_from_yaml(self, provider): + def test_discovers_from_yaml(self, provider, adapter): provider.start_container("echo-env:latest") assert any( "cd /app/env && python -m uvicorn server.app:app" in c - for c in provider._sandbox.exec_calls + for c in adapter.exec_commands ) - def test_no_yaml_raises(self, provider): - def _no_yaml(cmd): - return "" - - original = _fake_modal.Sandbox.create + def test_no_yaml_raises(self, adapter): + adapter.has_yaml = False + provider = ModalProvider(_adapter=adapter) + with pytest.raises(ValueError, match="Could not find openenv.yaml"): + provider.start_container("no-yaml:latest") + # The created sandbox is cleaned up after the discovery failure. + assert adapter.terminated == 1 - def patched(*args, **kwargs): - sb = original(*args, **kwargs) - sb._exec_fn = _no_yaml - return sb - - with patch.object(_fake_modal.Sandbox, "create", staticmethod(patched)): - with pytest.raises(ValueError, match="Could not find openenv.yaml"): - provider.start_container("no-yaml:latest") - - def test_falls_back_to_dockerfile_cmd(self, provider, tmp_path): + def test_falls_back_to_dockerfile_cmd(self, adapter, tmp_path): + adapter.has_yaml = False df = _write_dockerfile( tmp_path, 'FROM python:3.11\nCOPY . /app\nCMD ["uvicorn", "fallback:app"]\n', ) image = ModalProvider.image_from_dockerfile(str(df)) - - def _no_yaml(cmd): - return "" - - original = _fake_modal.Sandbox.create - - def patched(*args, **kwargs): - sb = original(*args, **kwargs) - sb._exec_fn = _no_yaml - return sb - - with patch.object(_fake_modal.Sandbox, "create", staticmethod(patched)): - provider.start_container(image) - assert any("uvicorn fallback:app" in c for c in provider._sandbox.exec_calls) + provider = ModalProvider(_adapter=adapter) + provider.start_container(image) + assert any("uvicorn fallback:app" in c for c in adapter.exec_commands) # --------------------------------------------------------------------------- @@ -334,14 +265,13 @@ def test_parses_cmd_into_registry(self, tmp_path): ModalProvider._dockerfile_registry[key]["server_cmd"] == "uvicorn app:app" ) - def test_dockerfile_image_built_from_dockerfile(self, provider, tmp_path): + def test_dockerfile_image_built_from_dockerfile(self, provider, adapter, tmp_path): df = _write_dockerfile( tmp_path, "FROM python:3.11\nCOPY . /app\nCMD uvicorn app:app\n" ) image = ModalProvider.image_from_dockerfile(str(df)) provider.start_container(image) - _, kwargs = _fake_modal.Sandbox.last_create - assert kwargs["image"].kind == "dockerfile" + assert adapter.created[0]["image"].kind == "dockerfile" def test_unregistered_dockerfile_raises(self, provider): with pytest.raises(ValueError, match="No registered Dockerfile metadata"): @@ -363,22 +293,87 @@ def test_invalid_yaml(self): # --------------------------------------------------------------------------- -# Tests: stop_container +# Tests: lifecycle (double-start, stop, close, context manager) # --------------------------------------------------------------------------- -class TestStopContainer: - def test_terminates_sandbox(self, provider): +class TestLifecycle: + def test_double_start_raises_and_preserves_existing_sandbox( + self, provider, adapter + ): + provider.start_container("echo-env:latest") + first = provider._sandbox + + with pytest.raises(RuntimeError, match="already has an active sandbox"): + provider.start_container("other:latest") + + assert adapter.terminated == 0 # existing sandbox untouched + assert len(adapter.created) == 1 # guard fired before a second create + assert provider._sandbox is first + + def test_start_after_stop_is_allowed(self, provider, adapter): + provider.start_container("echo-env:latest") + provider.stop_container() + provider.start_container("other:latest") + assert len(adapter.created) == 2 + + def test_stop_terminates_sandbox(self, provider, adapter): provider.start_container("echo-env:latest") - sb = provider._sandbox provider.stop_container() - assert sb.terminated is True + assert adapter.terminated == 1 assert provider._sandbox is None - def test_stop_without_start_is_noop(self, provider): - provider.stop_container() # should not raise + def test_stop_without_start_is_noop(self, provider, adapter): + provider.stop_container() + assert adapter.terminated == 0 + + def test_close_terminates_sandbox(self, provider, adapter): + provider.start_container("echo-env:latest") + provider.close() + assert adapter.terminated == 1 + assert provider._sandbox is None + + def test_context_manager_terminates_on_exit(self, adapter): + with ModalProvider(_adapter=adapter) as p: + p.start_container("echo-env:latest") + assert adapter.terminated == 1 + + def test_context_manager_terminates_on_exception(self, adapter): + with pytest.raises(RuntimeError, match="boom"): + with ModalProvider(_adapter=adapter) as p: + p.start_container("echo-env:latest") + raise RuntimeError("boom") + assert adapter.terminated == 1 + + def test_is_container_provider(self, provider): + assert isinstance(provider, ContainerProvider) + + +# --------------------------------------------------------------------------- +# Tests: cleanup does not mask the original error +# --------------------------------------------------------------------------- +class TestCleanup: + def test_cleanup_failure_does_not_mask_original_error(self, adapter): + adapter.fail_tunnel = True # original failure (after sandbox exists) + adapter.fail_terminate = True # secondary failure during cleanup + provider = ModalProvider(_adapter=adapter) + + with pytest.raises(RuntimeError, match="tunnel failed"): + provider.start_container("echo-env:latest") + + assert adapter.terminated == 1 # cleanup was still attempted + + def test_create_failure_clears_secrets_without_terminating(self, adapter): + adapter.fail_create = True + provider = ModalProvider(_adapter=adapter) + + with pytest.raises(RuntimeError, match="create failed"): + provider.start_container("echo-env:latest", env_vars={"TOKEN": "secret"}) + + assert provider._redact_values == set() + assert adapter.terminated == 0 # created nothing -> deleted nothing # --------------------------------------------------------------------------- -# Tests: wait_for_ready +# Tests: wait_for_ready (health, secrets, URL non-leak) # --------------------------------------------------------------------------- class TestWaitForReady: def test_ready_when_health_200(self, provider): @@ -386,29 +381,193 @@ def test_ready_when_health_200(self, provider): with patch("requests.get", return_value=MagicMock(status_code=200)): provider.wait_for_ready("https://x.modal.host", timeout_s=5) - def test_dead_process_raises(self, provider): + def test_dead_process_raises_without_log_by_default(self, provider, adapter): import requests - provider.start_container("echo-env:latest") - provider._sandbox._exec_fn = lambda cmd: ( - "DEAD" if "kill -0" in cmd else "boom traceback" + adapter.dead_process = True + adapter.log = "TOKEN=topsecret traceback" + provider.start_container("echo-env:latest", env_vars={"TOKEN": "topsecret"}) + with patch("requests.get", side_effect=requests.ConnectionError("refused")): + with pytest.raises(RuntimeError, match="not surfaced") as exc_info: + provider.wait_for_ready("https://x.modal.host", timeout_s=5) + assert "topsecret" not in str(exc_info.value) + assert "traceback" not in str(exc_info.value) + + def test_dead_process_surfaces_redacted_log_when_opted_in(self, adapter): + import requests + + adapter.dead_process = True + adapter.log = "Traceback: TOKEN=supersecret123 failed" + provider = ModalProvider(_adapter=adapter, surface_server_logs=True) + provider.start_container( + "echo-env:latest", env_vars={"TOKEN": "supersecret123"} ) + with patch("requests.get", side_effect=requests.ConnectionError("refused")): - with pytest.raises(RuntimeError, match="Server process died"): + with pytest.raises(RuntimeError) as exc_info: provider.wait_for_ready("https://x.modal.host", timeout_s=5) - def test_timeout_raises(self, provider): + msg = str(exc_info.value) + assert "supersecret123" not in msg + assert "***" in msg + + def test_timeout_does_not_leak_url(self, provider): import requests provider.start_container("echo-env:latest") # Keep the process "RUNNING" so the death check doesn't short-circuit. - provider._sandbox._exec_fn = lambda cmd: "RUNNING" if "kill -0" in cmd else "" + provider._sandbox = None # skip the in-loop death check entirely + url = "https://secret-bearer.modal.host/tok123" with ( patch("requests.get", side_effect=requests.ConnectionError("refused")), patch( "openenv.core.containers.runtime.modal_provider.time.time" ) as mock_time, ): - mock_time.side_effect = [0, 1, 100] # start, first loop, past deadline - with pytest.raises(TimeoutError, match="did not become ready"): - provider.wait_for_ready("https://x.modal.host", timeout_s=5) + mock_time.side_effect = [0, 1, 100] + with pytest.raises(TimeoutError) as exc_info: + provider.wait_for_ready(url, timeout_s=5) + assert url not in str(exc_info.value) + assert "secret-bearer.modal.host" not in str(exc_info.value) + + +# --------------------------------------------------------------------------- +# Tests: _DefaultModalAdapter SDK shape + Sandbox v2 feature check +# --------------------------------------------------------------------------- +def _install_fake_modal(monkeypatch, *, has_experimental: bool = True): + """Install a minimal fake ``modal`` module pinned to the SDK shape used by + ``_DefaultModalAdapter`` and return it for introspection.""" + modal_mod = types.ModuleType("modal") + calls: dict = {"create": None, "create_v2": None} + + class _FakeProc: + def __init__(self, output=""): + self.stdout = MagicMock() + self.stdout.read = MagicMock(return_value=output) + + def wait(self): + return 0 + + class _FakeTunnel: + url = "https://sb-xyz-8000.modal.host" + + class _FakeSandboxInstance: + def __init__(self): + self.exec_calls = [] + self.terminated = False + + def exec(self, *args, **kwargs): + self.exec_calls.append(args) + return _FakeProc("ok") + + def tunnels(self, *args, **kwargs): + return {8000: _FakeTunnel()} + + def terminate(self, *args, **kwargs): + self.terminated = True + + class _FakeSandbox: + @staticmethod + def create(*args, **kwargs): + calls["create"] = (args, kwargs) + return _FakeSandboxInstance() + + if has_experimental: + + def _experimental_create(*args, **kwargs): + calls["create_v2"] = (args, kwargs) + return _FakeSandboxInstance() + + _FakeSandbox._experimental_create = staticmethod(_experimental_create) + + class _FakeApp: + @staticmethod + def lookup(name, *, create_if_missing=False, **kwargs): + calls["app"] = name + return object() + + class _FakeImage: + @staticmethod + def from_dockerfile(path, **kwargs): + return ("dockerfile", str(path), kwargs) + + @staticmethod + def from_registry(tag, *args, **kwargs): + return ("registry", tag) + + modal_mod.Sandbox = _FakeSandbox + modal_mod.App = _FakeApp + modal_mod.Image = _FakeImage + modal_mod._calls = calls + monkeypatch.setitem(sys.modules, "modal", modal_mod) + return modal_mod + + +class TestDefaultAdapter: + def test_matches_sdk_shape_v1(self, monkeypatch): + modal_mod = _install_fake_modal(monkeypatch) + adapter = _DefaultModalAdapter(app_name="openenv", use_sandbox_v2=False) + + image = adapter.image_from_registry("echo-env:latest") + assert image == ("registry", "echo-env:latest") + + sandbox = adapter.create_sandbox( + image=image, + encrypted_ports=[8000], + timeout=300, + env={"A": "B"}, + extra={"cpu": 2.0}, + ) + args, kwargs = modal_mod._calls["create"] + assert args[:2] == ("sleep", "infinity") + assert kwargs["encrypted_ports"] == [8000] + assert kwargs["timeout"] == 300 + assert kwargs["env"] == {"A": "B"} + assert kwargs["cpu"] == 2.0 + + assert adapter.exec(sandbox, "echo hi") == "ok" + assert adapter.tunnel_url(sandbox, 8000) == "https://sb-xyz-8000.modal.host" + adapter.terminate(sandbox) + assert sandbox.terminated is True + + def test_v2_uses_experimental_create(self, monkeypatch): + modal_mod = _install_fake_modal(monkeypatch) + adapter = _DefaultModalAdapter(app_name="openenv", use_sandbox_v2=True) + adapter.create_sandbox( + image=("registry", "x"), + encrypted_ports=[8000], + timeout=300, + env=None, + extra={}, + ) + assert modal_mod._calls["create_v2"] is not None + assert modal_mod._calls["create"] is None + + def test_v2_feature_check_raises_when_missing(self, monkeypatch): + _install_fake_modal(monkeypatch, has_experimental=False) + with pytest.raises(RuntimeError, match="_experimental_create"): + _DefaultModalAdapter(app_name="openenv", use_sandbox_v2=True) + + def test_v1_works_without_experimental_create(self, monkeypatch): + _install_fake_modal(monkeypatch, has_experimental=False) + # No feature check for the stable API. + _DefaultModalAdapter(app_name="openenv", use_sandbox_v2=False) + + +class TestSandboxV2Flag: + def test_default_is_off(self, adapter): + assert ModalProvider(_adapter=adapter)._use_sandbox_v2 is False + + def test_v2_emits_log(self, adapter, caplog): + import logging + + with caplog.at_level(logging.INFO): + ModalProvider(_adapter=adapter, use_sandbox_v2=True) + assert any("Sandbox v2" in r.message for r in caplog.records) + + def test_off_emits_no_log(self, adapter, caplog): + import logging + + with caplog.at_level(logging.INFO): + ModalProvider(_adapter=adapter) + assert not any("Sandbox v2" in r.message for r in caplog.records) From 1c5f46e8fd3889e185813c7b8529f7c669ed819f Mon Sep 17 00:00:00 2001 From: Joy Liu Date: Thu, 25 Jun 2026 13:37:18 -0700 Subject: [PATCH 3/3] :new: Add comments --- examples/modal_echo_env.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/examples/modal_echo_env.py b/examples/modal_echo_env.py index 6761e2dd6..95c2d845e 100644 --- a/examples/modal_echo_env.py +++ b/examples/modal_echo_env.py @@ -37,12 +37,18 @@ def main() -> int: # Provision the sandbox synchronously. The Modal SDK's blocking API warns # when driven from inside a running event loop, so the provider lifecycle is # kept out of asyncio; only the WebSocket client runs under asyncio.run(). + # (The first run builds the image and cold-starts the sandbox, which can + # take a minute with no output - the prints below show progress.) provider = ModalProvider(app_name="openenv-echo") + print("Starting Modal sandbox (building image on first run)...", flush=True) base_url = provider.start_container(image) + print(f"Sandbox up at {base_url} - waiting for server...", flush=True) provider.wait_for_ready(base_url, timeout_s=180) + print("Server ready.", flush=True) try: asyncio.run(_interact(base_url)) finally: + print("Stopping sandbox...", flush=True) provider.stop_container() return 0