Feat: Stop runtimes rather than delete them

openhands/core/config/sandbox_config.py

@@ -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(

openhands/runtime/base.py

@@ -136,6 +136,10 @@ def setup_initial_env(self) -> None:
     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)

openhands/runtime/impl/docker/containers.py

@@ -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()

openhands/runtime/impl/docker/docker_runtime.py

@@ -5,6 +5,7 @@
 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.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_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 @@ def __init__(
         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 @@ def __init__(
         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 @@ def _init_docker_client() -> docker.DockerClient:
     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 @@ def _init_container(self):
                     '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 @@ def _init_container(self):
 
     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 @@ def close(self, rm_all_containers: bool | None = None):
         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 @@ def web_hosts(self):
             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()

openhands/server/routes/manage_conversations.py

@@ -8,6 +8,7 @@
 
 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