Merge branch 'fix/container-name-case-sensitivity'
This commit is contained in:
@@ -49,3 +49,5 @@ apps/web/dist/
|
|||||||
.DS_Store
|
.DS_Store
|
||||||
Thumbs.db
|
Thumbs.db
|
||||||
/.stoneforge/.worktrees/
|
/.stoneforge/.worktrees/
|
||||||
|
# Local Pi runtime state
|
||||||
|
.atl/
|
||||||
|
|||||||
@@ -35,6 +35,7 @@ def ensure_instance_directory(instance_id: str, base_path: str | None = None) ->
|
|||||||
"""
|
"""
|
||||||
if base_path is None:
|
if base_path is None:
|
||||||
from src.config import Settings
|
from src.config import Settings
|
||||||
|
|
||||||
base_path = Settings().instance_base_path
|
base_path = Settings().instance_base_path
|
||||||
instance_dir = Path(base_path) / instance_id
|
instance_dir = Path(base_path) / instance_id
|
||||||
instance_dir.mkdir(parents=True, exist_ok=True)
|
instance_dir.mkdir(parents=True, exist_ok=True)
|
||||||
@@ -87,7 +88,7 @@ def write_config_files(instance_dir: str, files: dict[str, str]) -> None:
|
|||||||
full_path.resolve().relative_to(instance_path.resolve())
|
full_path.resolve().relative_to(instance_path.resolve())
|
||||||
except ValueError:
|
except ValueError:
|
||||||
raise ValueError(f"File path '{file_path}' escapes instance directory")
|
raise ValueError(f"File path '{file_path}' escapes instance directory")
|
||||||
|
|
||||||
full_path.parent.mkdir(parents=True, exist_ok=True)
|
full_path.parent.mkdir(parents=True, exist_ok=True)
|
||||||
full_path.write_text(content)
|
full_path.write_text(content)
|
||||||
|
|
||||||
@@ -109,7 +110,7 @@ def execute_compose_command(
|
|||||||
instance_dir = Path(compose_path).parent
|
instance_dir = Path(compose_path).parent
|
||||||
|
|
||||||
cmd = ["docker", "compose", "-f", compose_path]
|
cmd = ["docker", "compose", "-f", compose_path]
|
||||||
|
|
||||||
if env_file:
|
if env_file:
|
||||||
cmd.extend(["--env-file", env_file])
|
cmd.extend(["--env-file", env_file])
|
||||||
|
|
||||||
@@ -142,8 +143,9 @@ def get_container_id(instance_name: str) -> str | None:
|
|||||||
Returns:
|
Returns:
|
||||||
Container ID or None if not found
|
Container ID or None if not found
|
||||||
"""
|
"""
|
||||||
|
# Docker container names are lowercase internally; normalize to ensure match
|
||||||
result = subprocess.run(
|
result = subprocess.run(
|
||||||
["docker", "ps", "-q", "--filter", f"name={instance_name}"],
|
["docker", "ps", "-q", "--filter", f"name={instance_name.lower()}"],
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
text=True,
|
text=True,
|
||||||
)
|
)
|
||||||
@@ -162,8 +164,16 @@ def get_container_name(instance_name: str) -> str | None:
|
|||||||
Returns:
|
Returns:
|
||||||
Container name or None if not found
|
Container name or None if not found
|
||||||
"""
|
"""
|
||||||
|
# Docker container names are lowercase internally; normalize to ensure match
|
||||||
result = subprocess.run(
|
result = subprocess.run(
|
||||||
["docker", "ps", "--format", "{{.Names}}", "--filter", f"name={instance_name}"],
|
[
|
||||||
|
"docker",
|
||||||
|
"ps",
|
||||||
|
"--format",
|
||||||
|
"{{.Names}}",
|
||||||
|
"--filter",
|
||||||
|
f"name={instance_name.lower()}",
|
||||||
|
],
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
text=True,
|
text=True,
|
||||||
)
|
)
|
||||||
@@ -173,7 +183,9 @@ def get_container_name(instance_name: str) -> str | None:
|
|||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
||||||
def connect_container_to_network(container_name: str, network_name: str = "backend") -> bool:
|
def connect_container_to_network(
|
||||||
|
container_name: str, network_name: str = "backend"
|
||||||
|
) -> bool:
|
||||||
"""Connect a Docker container to an existing network.
|
"""Connect a Docker container to an existing network.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
@@ -198,12 +210,14 @@ def get_container_status(container_id: str) -> dict[str, Any]:
|
|||||||
container_id: Docker container ID
|
container_id: Docker container ID
|
||||||
|
|
||||||
Returns:
|
Returns:
|
||||||
Dict with 'status' (running, exited, restarting, not_found),
|
Dict with 'status' (running, exited, restarting, not_found),
|
||||||
'exit_code' (int or None), and 'health' (health status or None)
|
'exit_code' (int or None), and 'health' (health status or None)
|
||||||
"""
|
"""
|
||||||
result = subprocess.run(
|
result = subprocess.run(
|
||||||
[
|
[
|
||||||
"docker", "inspect", "-f",
|
"docker",
|
||||||
|
"inspect",
|
||||||
|
"-f",
|
||||||
"{{.State.Status}}|{{.State.ExitCode}}|{{if .State.Health}}{{.State.Health.Status}}{{else}}none{{end}}",
|
"{{.State.Status}}|{{.State.ExitCode}}|{{if .State.Health}}{{.State.Health.Status}}{{else}}none{{end}}",
|
||||||
container_id,
|
container_id,
|
||||||
],
|
],
|
||||||
@@ -213,12 +227,12 @@ def get_container_status(container_id: str) -> dict[str, Any]:
|
|||||||
|
|
||||||
if result.returncode != 0:
|
if result.returncode != 0:
|
||||||
return {"status": "not_found", "exit_code": None, "health": None}
|
return {"status": "not_found", "exit_code": None, "health": None}
|
||||||
|
|
||||||
parts = result.stdout.strip().split("|")
|
parts = result.stdout.strip().split("|")
|
||||||
status = parts[0] if parts else "unknown"
|
status = parts[0] if parts else "unknown"
|
||||||
exit_code = int(parts[1]) if len(parts) > 1 and parts[1].isdigit() else None
|
exit_code = int(parts[1]) if len(parts) > 1 and parts[1].isdigit() else None
|
||||||
health = parts[2] if len(parts) > 2 and parts[2] != "none" else None
|
health = parts[2] if len(parts) > 2 and parts[2] != "none" else None
|
||||||
|
|
||||||
return {"status": status, "exit_code": exit_code, "health": health}
|
return {"status": status, "exit_code": exit_code, "health": health}
|
||||||
|
|
||||||
|
|
||||||
@@ -241,10 +255,10 @@ def wait_for_container_running(
|
|||||||
import time
|
import time
|
||||||
|
|
||||||
start_time = time.time()
|
start_time = time.time()
|
||||||
|
|
||||||
while time.time() - start_time < timeout:
|
while time.time() - start_time < timeout:
|
||||||
info = get_container_status(container_id)
|
info = get_container_status(container_id)
|
||||||
|
|
||||||
if info["status"] == "running":
|
if info["status"] == "running":
|
||||||
return {
|
return {
|
||||||
"success": True,
|
"success": True,
|
||||||
@@ -252,7 +266,7 @@ def wait_for_container_running(
|
|||||||
"exit_code": None,
|
"exit_code": None,
|
||||||
"waited_seconds": time.time() - start_time,
|
"waited_seconds": time.time() - start_time,
|
||||||
}
|
}
|
||||||
|
|
||||||
if info["status"] == "exited":
|
if info["status"] == "exited":
|
||||||
return {
|
return {
|
||||||
"success": False,
|
"success": False,
|
||||||
@@ -260,7 +274,7 @@ def wait_for_container_running(
|
|||||||
"exit_code": info["exit_code"],
|
"exit_code": info["exit_code"],
|
||||||
"waited_seconds": time.time() - start_time,
|
"waited_seconds": time.time() - start_time,
|
||||||
}
|
}
|
||||||
|
|
||||||
if info["status"] == "not_found":
|
if info["status"] == "not_found":
|
||||||
return {
|
return {
|
||||||
"success": False,
|
"success": False,
|
||||||
@@ -268,9 +282,9 @@ def wait_for_container_running(
|
|||||||
"exit_code": None,
|
"exit_code": None,
|
||||||
"waited_seconds": time.time() - start_time,
|
"waited_seconds": time.time() - start_time,
|
||||||
}
|
}
|
||||||
|
|
||||||
time.sleep(interval)
|
time.sleep(interval)
|
||||||
|
|
||||||
# Timeout reached
|
# Timeout reached
|
||||||
info = get_container_status(container_id)
|
info = get_container_status(container_id)
|
||||||
return {
|
return {
|
||||||
@@ -354,18 +368,29 @@ def start_cloudflared_tunnel(
|
|||||||
logger.info("Checking connectivity to %s:%d...", container_name, port)
|
logger.info("Checking connectivity to %s:%d...", container_name, port)
|
||||||
for attempt in range(10):
|
for attempt in range(10):
|
||||||
check = subprocess.run(
|
check = subprocess.run(
|
||||||
["curl", "-s", "-o", "/dev/null", "-w", "%{http_code}",
|
[
|
||||||
f"http://{container_name}:{port}"],
|
"curl",
|
||||||
|
"-s",
|
||||||
|
"-o",
|
||||||
|
"/dev/null",
|
||||||
|
"-w",
|
||||||
|
"%{http_code}",
|
||||||
|
f"http://{container_name}:{port}",
|
||||||
|
],
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
text=True,
|
text=True,
|
||||||
timeout=5,
|
timeout=5,
|
||||||
)
|
)
|
||||||
logger.info("Connectivity check %d: http_code=%s", attempt + 1, check.stdout.strip())
|
logger.info(
|
||||||
|
"Connectivity check %d: http_code=%s", attempt + 1, check.stdout.strip()
|
||||||
|
)
|
||||||
if check.returncode == 0:
|
if check.returncode == 0:
|
||||||
break
|
break
|
||||||
time.sleep(1)
|
time.sleep(1)
|
||||||
else:
|
else:
|
||||||
logger.warning("Container %s:%d not responding to curl checks", container_name, port)
|
logger.warning(
|
||||||
|
"Container %s:%d not responding to curl checks", container_name, port
|
||||||
|
)
|
||||||
|
|
||||||
# Run cloudflared in background, capture output
|
# Run cloudflared in background, capture output
|
||||||
logger.info("Starting cloudflared tunnel to http://%s:%d", container_name, port)
|
logger.info("Starting cloudflared tunnel to http://%s:%d", container_name, port)
|
||||||
@@ -381,9 +406,15 @@ def start_cloudflared_tunnel(
|
|||||||
start_time = time.time()
|
start_time = time.time()
|
||||||
url = None
|
url = None
|
||||||
|
|
||||||
|
if proc.stdout is None:
|
||||||
|
proc.terminate()
|
||||||
|
proc.wait(timeout=5)
|
||||||
|
raise RuntimeError("Failed to capture cloudflared output")
|
||||||
|
|
||||||
while time.time() - start_time < timeout:
|
while time.time() - start_time < timeout:
|
||||||
# Read available output
|
# Read available output
|
||||||
import select
|
import select
|
||||||
|
|
||||||
readable, _, _ = select.select([proc.stdout], [], [], 1.0)
|
readable, _, _ = select.select([proc.stdout], [], [], 1.0)
|
||||||
if readable:
|
if readable:
|
||||||
line = proc.stdout.readline()
|
line = proc.stdout.readline()
|
||||||
@@ -455,14 +486,23 @@ def check_tunnel_health(url: str, timeout: int = 10) -> dict[str, Any]:
|
|||||||
|
|
||||||
try:
|
try:
|
||||||
result = subprocess.run(
|
result = subprocess.run(
|
||||||
["curl", "-s", "-o", "/dev/null", "-w", "%{http_code}",
|
[
|
||||||
"--max-time", str(timeout), url],
|
"curl",
|
||||||
|
"-s",
|
||||||
|
"-o",
|
||||||
|
"/dev/null",
|
||||||
|
"-w",
|
||||||
|
"%{http_code}",
|
||||||
|
"--max-time",
|
||||||
|
str(timeout),
|
||||||
|
url,
|
||||||
|
],
|
||||||
capture_output=True,
|
capture_output=True,
|
||||||
text=True,
|
text=True,
|
||||||
timeout=timeout + 5,
|
timeout=timeout + 5,
|
||||||
)
|
)
|
||||||
status_code = int(result.stdout.strip())
|
status_code = int(result.stdout.strip())
|
||||||
|
|
||||||
if 200 <= status_code < 400:
|
if 200 <= status_code < 400:
|
||||||
return {
|
return {
|
||||||
"tunnel_status": "healthy",
|
"tunnel_status": "healthy",
|
||||||
@@ -495,7 +535,15 @@ def check_tunnel_health(url: str, timeout: int = 10) -> dict[str, Any]:
|
|||||||
except (ValueError, Exception) as e:
|
except (ValueError, Exception) as e:
|
||||||
error_str = str(e).lower()
|
error_str = str(e).lower()
|
||||||
# Classify connection errors
|
# Classify connection errors
|
||||||
if any(err in error_str for err in ["connection refused", "econnrefused", "could not resolve", "nodename"]):
|
if any(
|
||||||
|
err in error_str
|
||||||
|
for err in [
|
||||||
|
"connection refused",
|
||||||
|
"econnrefused",
|
||||||
|
"could not resolve",
|
||||||
|
"nodename",
|
||||||
|
]
|
||||||
|
):
|
||||||
return {
|
return {
|
||||||
"tunnel_status": "unreachable",
|
"tunnel_status": "unreachable",
|
||||||
"status_code": None,
|
"status_code": None,
|
||||||
|
|||||||
@@ -0,0 +1,52 @@
|
|||||||
|
"""Unit tests for docker service utilities."""
|
||||||
|
|
||||||
|
from unittest.mock import MagicMock, patch
|
||||||
|
|
||||||
|
from src.services.docker import get_container_id, get_container_name
|
||||||
|
|
||||||
|
|
||||||
|
class TestGetContainerId:
|
||||||
|
"""Tests for get_container_id."""
|
||||||
|
|
||||||
|
@patch("subprocess.run")
|
||||||
|
def test_lowercases_name_for_filter(self, mock_run) -> None:
|
||||||
|
"""Docker ps name filter is case-sensitive; we must lowercase."""
|
||||||
|
mock_run.return_value = MagicMock(returncode=0, stdout="abc123\n")
|
||||||
|
|
||||||
|
result = get_container_id("MyContainer-ABC")
|
||||||
|
|
||||||
|
assert result == "abc123"
|
||||||
|
call_args = mock_run.call_args[0][0]
|
||||||
|
# The filter must use lowercase
|
||||||
|
assert "name=mycontainer-abc" in call_args
|
||||||
|
|
||||||
|
@patch("subprocess.run")
|
||||||
|
def test_returns_none_when_not_found(self, mock_run) -> None:
|
||||||
|
mock_run.return_value = MagicMock(returncode=0, stdout="")
|
||||||
|
|
||||||
|
result = get_container_id("missing")
|
||||||
|
|
||||||
|
assert result is None
|
||||||
|
|
||||||
|
|
||||||
|
class TestGetContainerName:
|
||||||
|
"""Tests for get_container_name."""
|
||||||
|
|
||||||
|
@patch("subprocess.run")
|
||||||
|
def test_lowercases_name_for_filter(self, mock_run) -> None:
|
||||||
|
"""Docker ps name filter is case-sensitive; we must lowercase."""
|
||||||
|
mock_run.return_value = MagicMock(returncode=0, stdout="mycontainer-abc\n")
|
||||||
|
|
||||||
|
result = get_container_name("MyContainer-ABC")
|
||||||
|
|
||||||
|
assert result == "mycontainer-abc"
|
||||||
|
call_args = mock_run.call_args[0][0]
|
||||||
|
assert "name=mycontainer-abc" in call_args
|
||||||
|
|
||||||
|
@patch("subprocess.run")
|
||||||
|
def test_returns_none_when_not_found(self, mock_run) -> None:
|
||||||
|
mock_run.return_value = MagicMock(returncode=0, stdout="")
|
||||||
|
|
||||||
|
result = get_container_name("missing")
|
||||||
|
|
||||||
|
assert result is None
|
||||||
Reference in New Issue
Block a user