fix: prevent failed containers from showing as running on dashboard
- Add final get_container_status check in start_tool_instance before writing status=running; mark as error and return logs if container stopped - Treat restarting as error in HealthMonitor when DB status was already running, so crash loops are surfaced instead of preserved - Disable auto-restart (restart: unless-stopped -> restart: no) for tool instances in manifest compiler, legacy dockerfile path, and built-in seeds Quality gates: - pytest tests/unit: 210 passed - ruff: clean on changed files - mypy: clean on changed files
This commit is contained in:
Developer
@@ -2,7 +2,7 @@
|
||||
dir: apps/api/src/services
|
||||
|
||||
## role
|
||||
Provides business logic and service layer abstractions for the API application.
|
||||
Marker package for the services layer in the API application
|
||||
## parent
|
||||
index: apps/api/src/.pi-map.index.md
|
||||
map: apps/api/src/.pi-map.md
|
||||
|
||||
@@ -4,11 +4,11 @@ dir: apps/api/src/services
|
||||
index: apps/api/src/services/.pi-map.index.md
|
||||
|
||||
## role
|
||||
Provides business logic and service layer abstractions for the API application.
|
||||
Marker package for the services layer in the API application
|
||||
## files
|
||||
- __init__.py | Empty file with no functionality
|
||||
## arch
|
||||
Minimal or placeholder package structure with no implemented services yet, following standard Python package conventions.
|
||||
Standard Python package structure using __init__.py for namespace declaration
|
||||
## tags
|
||||
init, empty, functionality
|
||||
## symbols
|
||||
|
||||
@@ -411,7 +411,7 @@ def compile_compose(manifest: dict, variables: dict[str, Any]) -> str:
|
||||
service: dict[str, Any] = {
|
||||
"image": variables["IMAGE_TAG"],
|
||||
"container_name": variables["INSTANCE_NAME"],
|
||||
"restart": "unless-stopped",
|
||||
"restart": "no",
|
||||
}
|
||||
|
||||
# Terminal-specific fields
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
dir: apps/api/src/services/instance
|
||||
|
||||
## role
|
||||
Provides infrastructure for managing tool instance lifecycle events, health monitoring, and asynchronous communication within the API service.
|
||||
Coordinates tool instance lifecycle events, health monitoring, and notifications across the API service.
|
||||
## parent
|
||||
index: apps/api/src/services/.pi-map.index.md
|
||||
map: apps/api/src/services/.pi-map.md
|
||||
|
||||
@@ -4,14 +4,14 @@ dir: apps/api/src/services/instance
|
||||
index: apps/api/src/services/instance/.pi-map.index.md
|
||||
|
||||
## role
|
||||
Provides infrastructure for managing tool instance lifecycle events, health monitoring, and asynchronous communication within the API service.
|
||||
Coordinates tool instance lifecycle events, health monitoring, and notifications across the API service.
|
||||
## files
|
||||
- __init__.py | Exports public API for instance lifecycle services module | dep: src.services.instance.event_bus, src.services.instance.health_monitor, src.services.instance.lifecycle_hooks
|
||||
- event_bus.py | Implements a singleton in-memory typed event bus with publish/subscribe pattern for instance lifecycle and health events, supporting both sync and async callbacks with exception isolation. | exp: class:InstanceEventBus, method:__init__(self) → None, method:__new__(cls) → "InstanceEventBus", call:super().__new__, method:_reset_for_testing(self) → None, call:self._subscribers.clear, method:subscribe(self, event_type: str, callback: EventCallback) → Callable[[], None], call:str, call:uuid.uuid4, call:self._subscribers[event_type].append, call:self.unsubscribe, method:unsubscribe(self, event_type: str, callback_id: str) → None, method:unsubscribe_all(self, event_type: str) → None, call:self._subscribers.pop, method:publish(self, event_type: str, payload: InstanceEventPayload) → None, call:callbacks.extend, call:self._subscribers.get, call:inspect.iscoroutinefunction, call:callback, call:payload.get, call:logger.exception | dep: asyncio, inspect, logging, uuid, collections.abc, typing
|
||||
- health_monitor.py | Background health monitor that periodically polls Docker container and tunnel health for tool instances, publishing state change events and notifications. | exp: class:HealthSnapshot, class:HealthMonitor, method:__init__(self, event_bus: InstanceEventBus) → None, method:start(self) → None, call:self._task.done, call:asyncio.get_running_loop, call:loop.create_task, call:self._poll_loop, method:stop(self) → None, call:self._task.done, call:self._task.cancel, call:self._last_known_state.clear, method:_poll_loop(self) → None, call:asyncio.sleep, call:self._run_check_cycle, call:logger.exception, method:_run_check_cycle(self) → None, call:SessionLocal, call:session.execute, call:select(ToolInstance).where, call:ToolInstance.status.in_, call:result.scalars().all, call:self._check_instance, method:_check_instance(self, session: AsyncSession, instance: ToolInstance) → None, call:logger.debug, call:get_container_status, call:logger.exception, call:str, call:get_correlation_id, call:check_tunnel_health, call:tunnel_result.get, call:HealthSnapshot, call:self._last_known_state.get, call:self._derive_status, call:self._snapshots_equal, call:self._handle_state_change, method:_derive_status(self, snapshot: HealthSnapshot, previous: HealthSnapshot | None, current_status: str | None) → str, method:_snapshots_equal(self, a: HealthSnapshot, b: HealthSnapshot) → bool, method:_handle_state_change(self, session: AsyncSession, instance: ToolInstance, previous: HealthSnapshot | None, snapshot: HealthSnapshot, new_status: str) → None, call:HealthCheck, call:session.add, call:session.commit, call:get_correlation_id, call:str, call:datetime.now(timezone.utc).isoformat, call:self._event_bus.publish, call:notification_service.create_notification, call:logger.exception | dep: asyncio, logging, uuid, dataclasses, datetime, sqlalchemy, sqlalchemy.ext.asyncio, src.database, src.models, src.services.shared.correlation, src.services.docker, src.services.shared.tunnel, src.services.instance.event_bus, src.services.shared.notification_service
|
||||
- health_monitor.py | Background health monitor that polls Docker container and tunnel health for tool instances, publishes state change events, and creates notifications for errors/unhealthy states. | exp: class:HealthSnapshot, class:HealthMonitor, method:__init__(self, event_bus: InstanceEventBus) → None, method:start(self) → None, call:self._task.done, call:asyncio.get_running_loop, call:loop.create_task, call:self._poll_loop, method:stop(self) → None, call:self._task.done, call:self._task.cancel, call:self._last_known_state.clear, method:_poll_loop(self) → None, call:asyncio.sleep, call:self._run_check_cycle, call:logger.exception, method:_run_check_cycle(self) → None, call:SessionLocal, call:session.execute, call:select(ToolInstance).where, call:ToolInstance.status.in_, call:result.scalars().all, call:self._check_instance, method:_check_instance(self, session: AsyncSession, instance: ToolInstance) → None, call:logger.debug, call:get_container_status, call:logger.exception, call:str, call:get_correlation_id, call:check_tunnel_health, call:tunnel_result.get, call:HealthSnapshot, call:self._last_known_state.get, call:self._derive_status, call:self._snapshots_equal, call:self._handle_state_change, method:_derive_status(self, snapshot: HealthSnapshot, previous: HealthSnapshot | None, current_status: str | None) → str, method:_snapshots_equal(self, a: HealthSnapshot, b: HealthSnapshot) → bool, method:_handle_state_change(self, session: AsyncSession, instance: ToolInstance, previous: HealthSnapshot | None, snapshot: HealthSnapshot, new_status: str) → None, call:HealthCheck, call:session.add, call:session.commit, call:get_correlation_id, call:str, call:datetime.now(timezone.utc).isoformat, call:self._event_bus.publish, call:notification_service.create_notification, call:logger.exception | dep: asyncio, logging, uuid, dataclasses, datetime, sqlalchemy, sqlalchemy.ext.asyncio, src.database, src.models, src.services.shared.correlation, src.services.docker, src.services.shared.tunnel, src.services.instance.event_bus, src.services.shared.notification_service
|
||||
- lifecycle_hooks.py | Provides helpers to publish tool instance lifecycle events, persist audit records, and conditionally send user notifications. | exp: func:_derive_title(event_type: str) → str, call:mapping.get, call:event_type.replace("instance.", "").replace("_", " ").title, func:_should_notify(event_type: str, status: str | None) → bool, func:_build_payload(event_type: str, instance: ToolInstance, status, message, metadata) → InstanceEventPayload, call:str, call:datetime.now(timezone.utc).isoformat, call:get_correlation_id, func:_write_audit_row(session: AsyncSession, instance: ToolInstance, event_type: str, created_by, status, message, metadata) → InstanceEvent, call:InstanceEvent, call:event_type.replace, call:session.add, call:session.commit, func:publish_lifecycle_event(event_bus: InstanceEventBus, session: AsyncSession, instance: ToolInstance, event_type: str, created_by, status, message, metadata) → None, call:_build_payload, call:_write_audit_row, call:event_bus.publish, call:_should_notify, call:_derive_title, call:notification_service.create_notification, call:logger.exception, call:payload.get | dep: logging, uuid, datetime, sqlalchemy.ext.asyncio, src.models, src.services.shared.correlation, src.services.instance.event_bus, src.services.shared.notification_service
|
||||
## arch
|
||||
Event-driven architecture using a singleton in-memory pub/sub event bus with typed messages, background polling workers, and lifecycle hooks that bridge domain events to persistence and notifications with exception isolation between sync/async handlers.
|
||||
Observer pattern via typed singleton event bus with async/sync subscribers, background polling loops, and side-effect hooks for persistence and notifications.
|
||||
## tags
|
||||
call:self., instance, src, services, event, health, call:logger.exception, check
|
||||
## symbols
|
||||
|
||||
@@ -203,6 +203,11 @@ class HealthMonitor:
|
||||
# Transient states (created, restarting) — preserve current status
|
||||
# instead of treating them as an error. The next poll will resolve.
|
||||
if snapshot.container_status in ("created", "restarting"):
|
||||
# A container that was already running and is now restarting has
|
||||
# crashed (e.g. entrypoint failure / restart loop). Mark it failed
|
||||
# so the dashboard does not keep showing it as running.
|
||||
if current_status == "running":
|
||||
return "error"
|
||||
return current_status or "starting"
|
||||
|
||||
# Unknown/unexpected state (paused, etc.)
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
dir: apps/api/src/services/tool
|
||||
|
||||
## role
|
||||
Provides containerized execution environment for tools by managing Docker instances, git repositories, and compose orchestration.
|
||||
Provides Docker container lifecycle management for tool instances with git repository mounting, configuration resolution, and SSH tunnel connectivity.
|
||||
## parent
|
||||
index: apps/api/src/services/.pi-map.index.md
|
||||
map: apps/api/src/services/.pi-map.md
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -13,7 +13,14 @@ from fastapi import HTTPException, status
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
|
||||
from src.models import ConfigProfile, GitRepository, Project, SSHKey, ToolInstance, ToolType
|
||||
from src.models import (
|
||||
ConfigProfile,
|
||||
GitRepository,
|
||||
Project,
|
||||
SSHKey,
|
||||
ToolInstance,
|
||||
ToolType,
|
||||
)
|
||||
from src.schemas.tool import CreateInstanceRequest, StartInstanceRequest
|
||||
from src.services.git.clone import check_dirty_state, clone_repository
|
||||
from src.services.config.config_profile_resolver import (
|
||||
@@ -32,6 +39,7 @@ from src.services.docker import (
|
||||
get_container_id,
|
||||
get_container_ip_on_network,
|
||||
get_container_logs,
|
||||
get_container_status,
|
||||
is_container_on_network,
|
||||
render_compose_template,
|
||||
sort_volumes_by_specificity,
|
||||
@@ -774,7 +782,6 @@ def ensure_backend_network_in_compose(compose_path: str) -> None:
|
||||
logger.info("Injected backend network '%s' into compose file", network_name)
|
||||
|
||||
|
||||
|
||||
async def prepare_manifest_instance(
|
||||
session: AsyncSession,
|
||||
instance: ToolInstance,
|
||||
@@ -918,8 +925,6 @@ async def prepare_manifest_instance(
|
||||
return image_tag, compose_content, manifest, home_dir
|
||||
|
||||
|
||||
|
||||
|
||||
async def create_tool_instance(
|
||||
session: AsyncSession,
|
||||
user_id: uuid.UUID,
|
||||
@@ -1047,15 +1052,25 @@ async def create_tool_instance(
|
||||
repo_name = os.path.basename(os.path.normpath(repo_path))
|
||||
workspace_target = f"{home_dir}/{repo_name}"
|
||||
|
||||
compose_content = f"""version: "3.8"\nservices:\n app:\n image: {image_tag}\n container_name: {instance_name.lower()}\n stdin_open: true\n tty: true\n{ports_section} environment:\n - HOME={home_dir}\n volumes:\n - {repo_path}:{workspace_target}\n working_dir: {workspace_target}\n restart: unless-stopped\n"""
|
||||
compose_content = f"""version: "3.8"\nservices:
|
||||
app:
|
||||
image: {image_tag}
|
||||
container_name: {instance_name.lower()}
|
||||
stdin_open: true
|
||||
tty: true
|
||||
{ports_section} environment:
|
||||
- HOME={home_dir}
|
||||
volumes:
|
||||
- {repo_path}:{workspace_target}
|
||||
working_dir: {workspace_target}
|
||||
restart: "no"
|
||||
"""
|
||||
write_compose_file(instance_dir, compose_content)
|
||||
|
||||
elif tool_type.definition_type == "manifest":
|
||||
from src.models import ToolDefinitionManifest
|
||||
|
||||
manifest_def = await session.get(
|
||||
ToolDefinitionManifest, tool_type.manifest_id
|
||||
)
|
||||
manifest_def = await session.get(ToolDefinitionManifest, tool_type.manifest_id)
|
||||
if not manifest_def:
|
||||
raise RuntimeError("Manifest definition not found for this tool type")
|
||||
|
||||
@@ -1065,9 +1080,7 @@ async def create_tool_instance(
|
||||
ToolDefinitionManifest, manifest_def.base_definition_id
|
||||
)
|
||||
if base_def:
|
||||
manifest = resolve_base(
|
||||
deep_merge(dict(base_def.manifest), manifest)
|
||||
)
|
||||
manifest = resolve_base(deep_merge(dict(base_def.manifest), manifest))
|
||||
|
||||
image_tag = compute_image_tag(tool_type.name, manifest)
|
||||
|
||||
@@ -1105,9 +1118,7 @@ async def create_tool_instance(
|
||||
"WORKSPACE_NAME": os.path.basename(os.path.normpath(repo_path)),
|
||||
"HOME_DIRECTORY": tool_type.home_directory or "/home/user",
|
||||
}
|
||||
compose_content = render_compose_template(
|
||||
tool_type.compose_template, variables
|
||||
)
|
||||
compose_content = render_compose_template(tool_type.compose_template, variables)
|
||||
|
||||
write_compose_file(instance_dir, compose_content)
|
||||
|
||||
@@ -1674,6 +1685,41 @@ async def start_tool_instance(
|
||||
|
||||
logger.info("Readiness probe succeeded for instance %s", instance.id)
|
||||
|
||||
# Final stability check: the container must still be running after all
|
||||
# post-start setup. If it has already exited/restarted, mark it failed now
|
||||
# instead of optimistically reporting "running".
|
||||
if instance.container_id:
|
||||
final_check = get_container_status(instance.container_id)
|
||||
if final_check["status"] != "running":
|
||||
error_msg = (
|
||||
f"Container stopped during startup: status={final_check['status']}"
|
||||
)
|
||||
if final_check["exit_code"] is not None:
|
||||
error_msg += f", exit_code={final_check['exit_code']}"
|
||||
logs = get_container_logs(instance.container_id, tail=50)
|
||||
instance.status = "error"
|
||||
await session.commit()
|
||||
await publish_lifecycle_event(
|
||||
event_bus=_event_bus,
|
||||
session=session,
|
||||
instance=instance,
|
||||
event_type="instance.error",
|
||||
created_by=user_id,
|
||||
status="error",
|
||||
message=error_msg,
|
||||
metadata={
|
||||
"exit_code": final_check["exit_code"],
|
||||
"error_type": "container",
|
||||
},
|
||||
)
|
||||
logger.error(
|
||||
"Instance %s container stopped during startup: %s\nLogs:\n%s",
|
||||
instance.id,
|
||||
error_msg,
|
||||
logs,
|
||||
)
|
||||
return {"status": "error", "error": error_msg, "logs": logs}
|
||||
|
||||
instance.status = "running"
|
||||
await session.commit()
|
||||
await publish_lifecycle_event(
|
||||
@@ -2102,7 +2148,9 @@ async def stop_tool_instance(
|
||||
try:
|
||||
stop_tunnel(instance.name)
|
||||
except Exception as exc:
|
||||
logger.warning("Failed to stop tunnel for instance %s: %s", instance.id, exc)
|
||||
logger.warning(
|
||||
"Failed to stop tunnel for instance %s: %s", instance.id, exc
|
||||
)
|
||||
|
||||
if instance.compose_path and os.path.exists(instance.compose_path):
|
||||
execute_compose_command(instance.compose_path, "stop")
|
||||
@@ -2144,4 +2192,3 @@ async def rename_tool_instance(
|
||||
await session.commit()
|
||||
await session.refresh(instance)
|
||||
return instance
|
||||
|
||||
|
||||
Reference in New Issue
Block a user