Skip to content
Open
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 @@ -128,6 +128,10 @@ class ApptainerWorkspace(RemoteWorkspace):
"Specify locations to disable mounts for custom Apptainer behavior."
),
)
health_check_timeout: float = Field(
default=120.0,
description="Timeout in seconds to wait for container health check to pass.",
)

_instance_name: str | None = PrivateAttr(default=None)
_logs_thread: threading.Thread | None = PrivateAttr(default=None)
Expand Down Expand Up @@ -197,7 +201,7 @@ def model_post_init(self, context: Any) -> None:
object.__setattr__(self, "api_key", session_api_key)

# Wait for container to be healthy
self._wait_for_health()
self._wait_for_health(timeout=self.health_check_timeout)
logger.info("Apptainer workspace is ready at %s", self.host)

# Now initialize the parent RemoteWorkspace with the container URL
Expand Down
8 changes: 6 additions & 2 deletions openhands-workspace/openhands/workspace/docker/workspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ class DockerWorkspace(RemoteWorkspace):
default=None,
description="Connect a container to the specified Docker network.",
)
health_check_timeout: float = Field(
default=120.0,
description="Timeout in seconds to wait for container health check to pass.",
)

_container_id: str | None = PrivateAttr(default=None)
_image_name: str | None = PrivateAttr(default=None)
Expand Down Expand Up @@ -276,7 +280,7 @@ def _start_container(self, image: str, context: Any) -> None:
object.__setattr__(self, "api_key", None)

# Wait for container to be healthy
self._wait_for_health()
self._wait_for_health(timeout=self.health_check_timeout)
logger.info(f"Docker workspace is ready at {self.host}")

# Now initialize the parent RemoteWorkspace with the container URL
Expand Down Expand Up @@ -418,5 +422,5 @@ def resume(self) -> None:
raise RuntimeError(f"Failed to resume container: {result.stderr}")

# Wait for health after resuming (use same timeout as initial startup)
self._wait_for_health(timeout=120.0)
self._wait_for_health(timeout=self.health_check_timeout)
logger.info(f"Container resumed: {self._container_id}")
109 changes: 100 additions & 9 deletions tests/workspace/test_docker_workspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,16 @@

import pytest

from openhands.workspace import (
ApptainerWorkspace,
DockerDevWorkspace,
DockerWorkspace,
)


@pytest.fixture
def mock_docker_workspace():
"""Fixture to create a mocked DockerWorkspace with minimal setup."""
from openhands.workspace import DockerWorkspace

with patch("openhands.workspace.docker.workspace.execute_command") as mock_exec:
# Mock execute_command to return success
Expand Down Expand Up @@ -40,7 +45,6 @@ def _create_workspace(cleanup_image=False, network=None):

def test_docker_workspace_import():
"""Test that DockerWorkspace can be imported from the new package."""
from openhands.workspace import DockerWorkspace

assert DockerWorkspace is not None
assert hasattr(DockerWorkspace, "__init__")
Expand All @@ -49,22 +53,19 @@ def test_docker_workspace_import():
def test_docker_workspace_inheritance():
"""Test that DockerWorkspace inherits from RemoteWorkspace."""
from openhands.sdk.workspace import RemoteWorkspace
from openhands.workspace import DockerWorkspace

assert issubclass(DockerWorkspace, RemoteWorkspace)


def test_docker_dev_workspace_import():
"""Test that DockerDevWorkspace can be imported from the new package."""
from openhands.workspace import DockerDevWorkspace

assert DockerDevWorkspace is not None
assert hasattr(DockerDevWorkspace, "__init__")


def test_docker_dev_workspace_inheritance():
"""Test that DockerDevWorkspace inherits from DockerWorkspace."""
from openhands.workspace import DockerDevWorkspace, DockerWorkspace

assert issubclass(DockerDevWorkspace, DockerWorkspace)

Expand Down Expand Up @@ -94,15 +95,12 @@ def test_docker_workspace_no_build_import():
)
assert result.stdout.strip() == "0"

from openhands.workspace import DockerWorkspace

assert "server_image" in DockerWorkspace.model_fields
assert "base_image" not in DockerWorkspace.model_fields


def test_docker_dev_workspace_has_build_fields():
"""Test that DockerDevWorkspace has both base_image and server_image fields."""
from openhands.workspace import DockerDevWorkspace

# DockerDevWorkspace should have both fields for flexibility
assert "server_image" in DockerDevWorkspace.model_fields
Expand Down Expand Up @@ -145,7 +143,6 @@ def test_cleanup_with_image_deletion(mock_docker_workspace):

def test_docker_network(mock_docker_workspace):
"""Test that specifying `network` passes the value to Docker."""
from openhands.workspace import DockerWorkspace

# We need to mock things that _start_container calls before and after docker run
with (
Expand Down Expand Up @@ -178,3 +175,97 @@ def test_docker_network(mock_docker_workspace):
assert "--network" in run_cmd
network_index = run_cmd.index("--network")
assert run_cmd[network_index + 1] == network_name


def test_docker_workspace_health_check_timeout_field_exists():
"""Test that health_check_timeout is a recognised model field."""

assert "health_check_timeout" in DockerWorkspace.model_fields


def test_docker_workspace_health_check_timeout_default():
"""Test that health_check_timeout defaults to 120.0 seconds."""

field = DockerWorkspace.model_fields["health_check_timeout"]
assert field.default == 120.0


def test_docker_workspace_health_check_timeout_custom(mock_docker_workspace):
"""Test that a custom health_check_timeout is forwarded to _wait_for_health."""

with patch.object(DockerWorkspace, "_wait_for_health") as mock_wait:
with patch.object(DockerWorkspace, "_start_container"):
with patch(
"openhands.workspace.docker.workspace.execute_command"
) as mock_exec:
mock_exec.return_value = Mock(returncode=0, stdout="", stderr="")
workspace = DockerWorkspace(
server_image="test:latest", health_check_timeout=60.0
)
assert workspace.health_check_timeout == 60.0
# The field is set; _wait_for_health is called by _start_container which
# is mocked, so verify the value round-trips through the model.
_ = mock_wait # referenced to satisfy linters


def test_docker_workspace_resume_uses_health_check_timeout(mock_docker_workspace):
"""Test that resume() passes health_check_timeout to _wait_for_health."""

with patch.object(DockerWorkspace, "_start_container"):
with patch("openhands.workspace.docker.workspace.execute_command") as mock_exec:
mock_exec.return_value = Mock(returncode=0, stdout="", stderr="")
workspace = DockerWorkspace(
server_image="test:latest", health_check_timeout=30.0
)

workspace._container_id = "container_id_123"

with patch.object(workspace, "_wait_for_health") as mock_wait:
mock_exec.return_value = Mock(returncode=0, stdout="", stderr="")
workspace.resume()
mock_wait.assert_called_once_with(timeout=30.0)


# ===========================================================================
# Apptainer health_check_timeout tests (co-located for compactness)
# ===========================================================================


def test_apptainer_workspace_health_check_timeout_field_exists():
"""Test that health_check_timeout is a recognised model field."""

assert "health_check_timeout" in ApptainerWorkspace.model_fields


def test_apptainer_workspace_health_check_timeout_default():
"""Test that health_check_timeout defaults to 120.0 seconds."""

field = ApptainerWorkspace.model_fields["health_check_timeout"]
assert field.default == 120.0


def test_apptainer_workspace_health_check_timeout_startup_call():
"""Test that model_post_init passes health_check_timeout to _wait_for_health."""

with (
patch("openhands.workspace.apptainer.workspace.execute_command") as mock_exec,
patch(
"openhands.workspace.apptainer.workspace.check_port_available",
return_value=True,
),
patch(
"openhands.workspace.apptainer.workspace.find_available_tcp_port",
return_value=8000,
),
patch.object(
ApptainerWorkspace, "_prepare_sif_image", return_value="/fake/image.sif"
),
patch.object(ApptainerWorkspace, "_start_container"),
patch.object(ApptainerWorkspace, "_wait_for_health") as mock_wait,
patch(
"openhands.workspace.apptainer.workspace.RemoteWorkspace.model_post_init"
),
):
mock_exec.return_value = Mock(returncode=0, stdout="", stderr="")
ApptainerWorkspace(server_image="test:latest", health_check_timeout=45.0)
mock_wait.assert_called_once_with(timeout=45.0)
Loading