github/OpenHands
1
1
Fork 0
mirror of https://github.com/All-Hands-AI/OpenHands.git synced 2026-01-09 14:57:59 -05:00
Code Issues Packages Projects Releases Wiki Activity

Feat: Stop runtimes rather than delete them (#6403)

Browse Source 
Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
tofarr
2025-01-23 07:43:02 -07:00
committed by GitHub
parent 5ba9a6d321
commit 21d4ba0bbd
7 changed files with 121 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
openhands/core/config/sandbox_config.py
View File

@@ -39,7 +39,7 @@ class SandboxConfig(BaseModel):
remote_runtime_api_url: str = Field(default='http://localhost:8000')
local_runtime_url: str = Field(default='http://localhost')
keep_runtime_alive: bool = Field(default=True)
keep_runtime_alive: bool = Field(default=False)
rm_all_containers: bool = Field(default=False)
api_key: str | None = Field(default=None)
base_container_image: str = Field(

4
openhands/runtime/base.py
View File

@@ -136,6 +136,10 @@ class Runtime(FileEditRuntimeMixin):
def close(self) -> None:
pass
@classmethod
async def delete(cls, conversation_id: str) -> None:
pass
def log(self, level: str, message: str) -> None:
message = f'[runtime {self.sid}] {message}'
getattr(logger, level)(message, stacklevel=2)

7
openhands/runtime/impl/docker/containers.py
View File

@@ -1,18 +1,19 @@
import docker
def remove_all_containers(prefix: str):
def stop_all_containers(prefix: str):
docker_client = docker.from_env()
try:
containers = docker_client.containers.list(all=True)
for container in containers:
try:
if container.name.startswith(prefix):
container.remove(force=True)
container.stop()
except docker.errors.APIError:
pass
except docker.errors.NotFound:
pass
except docker.errors.NotFound: # yes, this can happen!
pass
finally:
docker_client.close()

60
openhands/runtime/impl/docker/docker_runtime.py
View File

@@ -5,6 +5,7 @@ from typing import Callable
import docker
import requests
import tenacity
from docker.models.containers import Container
from openhands.core.config import AppConfig
from openhands.core.exceptions import (
@@ -18,7 +19,7 @@ from openhands.runtime.builder import DockerRuntimeBuilder
from openhands.runtime.impl.action_execution.action_execution_client import (
ActionExecutionClient,
)
from openhands.runtime.impl.docker.containers import remove_all_containers
from openhands.runtime.impl.docker.containers import stop_all_containers
from openhands.runtime.plugins import PluginRequirement
from openhands.runtime.utils import find_available_tcp_port
from openhands.runtime.utils.command import get_action_execution_server_startup_command
@@ -35,8 +36,8 @@ APP_PORT_RANGE_1 = (50000, 54999)
APP_PORT_RANGE_2 = (55000, 59999)
def remove_all_runtime_containers():
remove_all_containers(CONTAINER_NAME_PREFIX)
def stop_all_runtime_containers():
stop_all_containers(CONTAINER_NAME_PREFIX)
_atexit_registered = False
@@ -66,9 +67,9 @@ class DockerRuntime(ActionExecutionClient):
headless_mode: bool = True,
):
global _atexit_registered
if not _atexit_registered and not config.sandbox.keep_runtime_alive:
if not _atexit_registered:
_atexit_registered = True
atexit.register(remove_all_runtime_containers)
atexit.register(stop_all_runtime_containers)
self.config = config
self._runtime_initialized: bool = False
@@ -85,7 +86,7 @@ class DockerRuntime(ActionExecutionClient):
self.base_container_image = self.config.sandbox.base_container_image
self.runtime_container_image = self.config.sandbox.runtime_container_image
self.container_name = CONTAINER_NAME_PREFIX + sid
self.container = None
self.container: Container | None = None
self.runtime_builder = DockerRuntimeBuilder(self.docker_client)
@@ -187,7 +188,6 @@ class DockerRuntime(ActionExecutionClient):
def _init_container(self):
self.log('debug', 'Preparing to start container...')
self.send_status_message('STATUS$PREPARING_CONTAINER')
self._host_port = self._find_available_port(EXECUTION_SERVER_PORT_RANGE)
self._container_port = self._host_port
self._vscode_port = self._find_available_port(VSCODE_PORT_RANGE)
@@ -287,7 +287,7 @@ class DockerRuntime(ActionExecutionClient):
'warning',
f'Container {self.container_name} already exists. Removing...',
)
remove_all_containers(self.container_name)
stop_all_containers(self.container_name)
return self._init_container()
else:
@@ -308,20 +308,20 @@ class DockerRuntime(ActionExecutionClient):
def _attach_to_container(self):
self.container = self.docker_client.containers.get(self.container_name)
for port in self.container.attrs['NetworkSettings']['Ports']: # type: ignore
port = int(port.split('/')[0])
if (
port >= EXECUTION_SERVER_PORT_RANGE[0]
and port <= EXECUTION_SERVER_PORT_RANGE[1]
):
self._container_port = port
if port >= VSCODE_PORT_RANGE[0] and port <= VSCODE_PORT_RANGE[1]:
self._vscode_port = port
elif port >= APP_PORT_RANGE_1[0] and port <= APP_PORT_RANGE_1[1]:
self._app_ports.append(port)
elif port >= APP_PORT_RANGE_2[0] and port <= APP_PORT_RANGE_2[1]:
self._app_ports.append(port)
self._host_port = self._container_port
if self.container.status == 'exited':
self.container.start()
config = self.container.attrs['Config']
for env_var in config['Env']:
if env_var.startswith('port='):
self._host_port = int(env_var.split('port=')[1])
self._container_port = self._host_port
elif env_var.startswith('VSCODE_PORT='):
self._vscode_port = int(env_var.split('VSCODE_PORT=')[1])
self._app_ports = []
for exposed_port in config['ExposedPorts'].keys():
exposed_port = int(exposed_port.split('/tcp')[0])
if exposed_port != self._host_port and exposed_port != self._vscode_port:
self._app_ports.append(exposed_port)
self.api_url = f'{self.config.sandbox.local_runtime_url}:{self._container_port}'
self.log(
'debug',
@@ -368,7 +368,7 @@ class DockerRuntime(ActionExecutionClient):
close_prefix = (
CONTAINER_NAME_PREFIX if rm_all_containers else self.container_name
)
remove_all_containers(close_prefix)
stop_all_containers(close_prefix)
def _is_port_in_use_docker(self, port):
containers = self.docker_client.containers.list()
@@ -404,3 +404,17 @@ class DockerRuntime(ActionExecutionClient):
hosts[f'http://localhost:{port}'] = port
return hosts
@classmethod
async def delete(cls, conversation_id: str):
docker_client = cls._init_docker_client()
try:
container_name = CONTAINER_NAME_PREFIX + conversation_id
container = docker_client.containers.get(container_name)
container.remove(force=True)
except docker.errors.APIError:
pass
except docker.errors.NotFound:
pass
finally:
docker_client.close()

3
openhands/server/routes/manage_conversations.py
View File

@@ -8,6 +8,7 @@ from pydantic import BaseModel
from openhands.core.logger import openhands_logger as logger
from openhands.events.stream import EventStreamSubscriber
from openhands.runtime import get_runtime_cls
from openhands.server.auth import get_user_id
from openhands.server.routes.settings import ConversationStoreImpl, SettingsStoreImpl
from openhands.server.session.conversation_init_data import ConversationInitData
@@ -227,6 +228,8 @@ async def delete_conversation(
is_running = await session_manager.is_agent_loop_running(conversation_id)
if is_running:
await session_manager.close_session(conversation_id)
runtime_cls = get_runtime_cls(config.runtime)
await runtime_cls.delete(conversation_id)
await conversation_store.delete_metadata(conversation_id)
return True

5
openhands/server/session/manager.py
View File

@@ -456,7 +456,10 @@ class SessionManager:
response_ids = await self.get_running_agent_loops(user_id)
if len(response_ids) >= MAX_RUNNING_CONVERSATIONS:
logger.info('too_many_sessions_for:{user_id}')
await self.close_session(next(iter(response_ids)))
# Order is not guaranteed, but response_ids tend to be in descending chronological order
# By reversing, we are likely to pick the oldest (or at least an older) conversation
session_id = next(iter(reversed(list(response_ids))))
await self.close_session(session_id)
session = Session(
sid=sid,

68
tests/unit/test_docker_runtime.py Normal file
View File

@@ -0,0 +1,68 @@
from unittest.mock import MagicMock, patch
import pytest
from openhands.core.config import AppConfig
from openhands.events import EventStream
from openhands.runtime.impl.docker.docker_runtime import DockerRuntime
@pytest.fixture
def mock_docker_client():
with patch('docker.from_env') as mock_client:
container_mock = MagicMock()
container_mock.status = 'running'
container_mock.attrs = {
'Config': {
'Env': ['port=12345', 'VSCODE_PORT=54321'],
'ExposedPorts': {'12345/tcp': {}, '54321/tcp': {}},
}
}
mock_client.return_value.containers.get.return_value = container_mock
mock_client.return_value.containers.run.return_value = container_mock
# Mock version info for BuildKit check
mock_client.return_value.version.return_value = {'Version': '20.10.0'}
yield mock_client.return_value
@pytest.fixture
def config():
config = AppConfig()
config.sandbox.keep_runtime_alive = False
return config
@pytest.fixture
def event_stream():
return MagicMock(spec=EventStream)
@patch('openhands.runtime.impl.docker.docker_runtime.stop_all_containers')
def test_container_stopped_when_keep_runtime_alive_false(
mock_stop_containers, mock_docker_client, config, event_stream
):
# Arrange
runtime = DockerRuntime(config, event_stream, sid='test-sid')
runtime.container = mock_docker_client.containers.get.return_value
# Act
runtime.close()
# Assert
mock_stop_containers.assert_called_once_with('openhands-runtime-test-sid')
@patch('openhands.runtime.impl.docker.docker_runtime.stop_all_containers')
def test_container_not_stopped_when_keep_runtime_alive_true(
mock_stop_containers, mock_docker_client, config, event_stream
):
# Arrange
config.sandbox.keep_runtime_alive = True
runtime = DockerRuntime(config, event_stream, sid='test-sid')
runtime.container = mock_docker_client.containers.get.return_value
# Act
runtime.close()
# Assert
mock_stop_containers.assert_not_called()