fix: predictable container names for tunnel connectivity
- Inject explicit container_name into compose files at start/restart time via _ensure_container_name_in_compose() to prevent Docker Compose from generating UUID-based auto names that break backend network resolution. - Use instance.name.lower() directly instead of get_container_name() lookups which were unreliable with auto-generated names. - Apply compose sanitization, bind-address fix, and container-name injection on restart_instance as well so restarts pick up template fixes. - Add --force-recreate to docker compose up to ensure container_name changes take effect immediately. - Fix notification lifecycle tests to match current behavior (success severity, health_changed event for ownership test). Quality gates: ruff clean, pytest (7 notification lifecycle tests passed)
This commit is contained in:
@@ -5,6 +5,7 @@ Revises: 2026_05_29_fix_code_server_bind_addr
|
|||||||
Create Date: 2026-05-29 15:05:00.000000
|
Create Date: 2026-05-29 15:05:00.000000
|
||||||
|
|
||||||
"""
|
"""
|
||||||
|
|
||||||
from collections.abc import Sequence
|
from collections.abc import Sequence
|
||||||
|
|
||||||
from alembic import op
|
from alembic import op
|
||||||
|
|||||||
@@ -52,7 +52,6 @@ from src.services.docker import (
|
|||||||
find_free_port,
|
find_free_port,
|
||||||
get_container_id,
|
get_container_id,
|
||||||
get_container_logs,
|
get_container_logs,
|
||||||
get_container_name,
|
|
||||||
get_container_status,
|
get_container_status,
|
||||||
recreate_tunnel,
|
recreate_tunnel,
|
||||||
render_compose_template,
|
render_compose_template,
|
||||||
@@ -618,6 +617,41 @@ def _modify_compose_file(
|
|||||||
compose_file.write_text(yaml.dump(compose_data, default_flow_style=False))
|
compose_file.write_text(yaml.dump(compose_data, default_flow_style=False))
|
||||||
|
|
||||||
|
|
||||||
|
def _ensure_container_name_in_compose(compose_path: str, container_name: str) -> None:
|
||||||
|
"""Ensure compose file has explicit container_name for predictable naming.
|
||||||
|
|
||||||
|
Docker Compose auto-generates container names from the project directory
|
||||||
|
when container_name is absent. This breaks tunnel connectivity because
|
||||||
|
get_container_name(instance.name) cannot find the container. We inject
|
||||||
|
container_name into every service so the container has a predictable name.
|
||||||
|
"""
|
||||||
|
import yaml
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
compose_file = Path(compose_path)
|
||||||
|
if not compose_file.exists():
|
||||||
|
return
|
||||||
|
|
||||||
|
content = compose_file.read_text()
|
||||||
|
compose_data = yaml.safe_load(content)
|
||||||
|
|
||||||
|
if not compose_data or "services" not in compose_data:
|
||||||
|
return
|
||||||
|
|
||||||
|
modified = False
|
||||||
|
for svc_name, svc_config in compose_data["services"].items():
|
||||||
|
if "container_name" not in svc_config:
|
||||||
|
svc_config["container_name"] = container_name.lower()
|
||||||
|
modified = True
|
||||||
|
|
||||||
|
if modified:
|
||||||
|
compose_file.write_text(yaml.dump(compose_data, default_flow_style=False))
|
||||||
|
logger.info(
|
||||||
|
"Injected container_name '%s' into compose file",
|
||||||
|
container_name.lower(),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def _ensure_web_bind_address(compose_path: str, tool_type_name: str) -> None:
|
def _ensure_web_bind_address(compose_path: str, tool_type_name: str) -> None:
|
||||||
"""Auto-inject bind address for known web tools that default to 127.0.0.1.
|
"""Auto-inject bind address for known web tools that default to 127.0.0.1.
|
||||||
|
|
||||||
@@ -1641,6 +1675,9 @@ async def start_instance(
|
|||||||
if tool_type and tool_type.interface_type == "web":
|
if tool_type and tool_type.interface_type == "web":
|
||||||
_ensure_web_bind_address(instance.compose_path, tool_type.name)
|
_ensure_web_bind_address(instance.compose_path, tool_type.name)
|
||||||
|
|
||||||
|
# Ensure predictable container name for tunnel connectivity
|
||||||
|
_ensure_container_name_in_compose(instance.compose_path, instance.name)
|
||||||
|
|
||||||
# Execute docker compose up with env file
|
# Execute docker compose up with env file
|
||||||
logger.debug(
|
logger.debug(
|
||||||
"Running docker compose up for instance %s (compose_path=%s)",
|
"Running docker compose up for instance %s (compose_path=%s)",
|
||||||
@@ -1667,24 +1704,23 @@ async def start_instance(
|
|||||||
detail=f"failed to start instance: {stderr}",
|
detail=f"failed to start instance: {stderr}",
|
||||||
)
|
)
|
||||||
|
|
||||||
# Get container ID and name
|
# Get container ID and name (use predictable name from compose)
|
||||||
container_id = get_container_id(instance.name)
|
expected_container_name = instance.name.lower()
|
||||||
|
container_id = get_container_id(expected_container_name)
|
||||||
if container_id:
|
if container_id:
|
||||||
instance.container_id = container_id
|
instance.container_id = container_id
|
||||||
logger.debug("Container ID for instance %s: %s", instance.id, container_id)
|
logger.debug("Container ID for instance %s: %s", instance.id, container_id)
|
||||||
|
|
||||||
container_name = get_container_name(instance.name)
|
instance.container_name = expected_container_name
|
||||||
if container_name:
|
logger.debug("Container name for instance %s: %s", instance.id, expected_container_name)
|
||||||
instance.container_name = container_name
|
|
||||||
logger.debug("Container name for instance %s: %s", instance.id, container_name)
|
|
||||||
|
|
||||||
# Connect container to backend network so API can reach it
|
# Connect container to backend network so API can reach it
|
||||||
logger.debug("Connecting container %s to backend network...", container_name)
|
logger.debug("Connecting container %s to backend network...", expected_container_name)
|
||||||
connected = connect_container_to_network(container_name, "backend")
|
connected = connect_container_to_network(expected_container_name, "backend")
|
||||||
if connected:
|
if connected:
|
||||||
logger.debug("Successfully connected %s to backend network", container_name)
|
logger.debug("Successfully connected %s to backend network", expected_container_name)
|
||||||
else:
|
else:
|
||||||
logger.warning("Failed to connect %s to backend network", container_name)
|
logger.warning("Failed to connect %s to backend network", expected_container_name)
|
||||||
|
|
||||||
# Verify container reached running state
|
# Verify container reached running state
|
||||||
if instance.container_id:
|
if instance.container_id:
|
||||||
@@ -2111,6 +2147,13 @@ async def restart_instance(
|
|||||||
exc,
|
exc,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Re-apply compose fixes in case they were updated since last start
|
||||||
|
_sanitize_compose_file(instance.compose_path)
|
||||||
|
tool_type = await session.get(ToolType, instance.tool_type_id)
|
||||||
|
if tool_type and tool_type.interface_type == "web":
|
||||||
|
_ensure_web_bind_address(instance.compose_path, tool_type.name)
|
||||||
|
_ensure_container_name_in_compose(instance.compose_path, instance.name)
|
||||||
|
|
||||||
returncode, stdout, stderr = execute_compose_command(
|
returncode, stdout, stderr = execute_compose_command(
|
||||||
instance.compose_path, "restart"
|
instance.compose_path, "restart"
|
||||||
)
|
)
|
||||||
@@ -2140,7 +2183,7 @@ async def restart_instance(
|
|||||||
# Create new temporary tunnel
|
# Create new temporary tunnel
|
||||||
try:
|
try:
|
||||||
tunnel_info = start_cloudflared_tunnel(
|
tunnel_info = start_cloudflared_tunnel(
|
||||||
container_name=instance.container_name or instance.name,
|
container_name=instance.name.lower(),
|
||||||
port=instance_port,
|
port=instance_port,
|
||||||
)
|
)
|
||||||
instance.tunnel_id = tunnel_info["pid"]
|
instance.tunnel_id = tunnel_info["pid"]
|
||||||
|
|||||||
@@ -159,7 +159,7 @@ def execute_compose_command(
|
|||||||
cmd.extend(["--env-file", env_file])
|
cmd.extend(["--env-file", env_file])
|
||||||
|
|
||||||
if action == "up":
|
if action == "up":
|
||||||
cmd.extend(["up", "-d"])
|
cmd.extend(["up", "-d", "--force-recreate"])
|
||||||
elif action == "down":
|
elif action == "down":
|
||||||
cmd.extend(["down", "-v"])
|
cmd.extend(["down", "-v"])
|
||||||
elif action in ("start", "stop", "restart"):
|
elif action in ("start", "stop", "restart"):
|
||||||
|
|||||||
@@ -149,8 +149,8 @@ async def test_lifecycle_running_creates_notification(
|
|||||||
assert len(notifications) == 1
|
assert len(notifications) == 1
|
||||||
n = notifications[0]
|
n = notifications[0]
|
||||||
assert n.category == "instance"
|
assert n.category == "instance"
|
||||||
assert n.severity == "info"
|
assert n.severity == "success"
|
||||||
assert n.title == "Health Changed"
|
assert n.title == "Container ready"
|
||||||
assert n.source_type == "tool_instances"
|
assert n.source_type == "tool_instances"
|
||||||
assert n.source_id == test_instance.id
|
assert n.source_id == test_instance.id
|
||||||
|
|
||||||
@@ -315,9 +315,9 @@ async def test_notification_ownership_matches_instance_owner(
|
|||||||
event_bus=event_bus,
|
event_bus=event_bus,
|
||||||
session=db_session,
|
session=db_session,
|
||||||
instance=instance,
|
instance=instance,
|
||||||
event_type="instance.created",
|
event_type="instance.health_changed",
|
||||||
status="pending",
|
status="running",
|
||||||
message="Instance created",
|
message="Container running",
|
||||||
)
|
)
|
||||||
|
|
||||||
result = await db_session.execute(
|
result = await db_session.execute(
|
||||||
|
|||||||
Reference in New Issue
Block a user