Fix container hash-prefix + status-aware redeploy
Bug 1: Container name hash-prefix (e.g. f93cb8b504f7_jenkins) - _strip_hash_prefix(): strips 12-char hex prefix and leading slash - _resolve_container_name(): looks up actual DSM name from container list - Applied in list_containers (display), container_stats (matching), get_container_status/get_container_logs/exec_in_container (lookup) Bug 2: redeploy_project DSM 2101/1202 on wrong project state - Fetch project status before acting - RUNNING → stop then start - STOPPED → start directly (nothing to stop) - BUILD_FAILED → suppress stop error, then start - Other → return error with workaround hint 36 tests all passing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -51,6 +51,18 @@ SAMPLE_CONTAINERS_DATA = {
|
||||
]
|
||||
}
|
||||
|
||||
# Container data where DSM returns hash-prefixed names
|
||||
HASH_PREFIXED_CONTAINERS_DATA = {
|
||||
"containers": [
|
||||
{
|
||||
"name": "f93cb8b504f7_jenkins",
|
||||
"status": "running",
|
||||
"image": "jenkins/jenkins:lts",
|
||||
"project_name": "frostiq",
|
||||
},
|
||||
]
|
||||
}
|
||||
|
||||
SAMPLE_LOGS_DATA = {
|
||||
"logs": [
|
||||
{
|
||||
@@ -315,6 +327,161 @@ async def test_container_stats_api_error():
|
||||
assert "Error" in result
|
||||
|
||||
|
||||
# ──────────────────────────────────────────────────────────────────────────────
|
||||
# Bug 1: hash-prefix stripping
|
||||
# ──────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def test_strip_hash_prefix_strips_prefix():
|
||||
from mcp_synology_container.modules.containers import _strip_hash_prefix
|
||||
|
||||
assert _strip_hash_prefix("f93cb8b504f7_jenkins") == "jenkins"
|
||||
|
||||
|
||||
def test_strip_hash_prefix_no_prefix():
|
||||
from mcp_synology_container.modules.containers import _strip_hash_prefix
|
||||
|
||||
assert _strip_hash_prefix("jenkins") == "jenkins"
|
||||
|
||||
|
||||
def test_strip_hash_prefix_leading_slash():
|
||||
from mcp_synology_container.modules.containers import _strip_hash_prefix
|
||||
|
||||
assert _strip_hash_prefix("/jenkins") == "jenkins"
|
||||
|
||||
|
||||
def test_strip_hash_prefix_slash_with_hash():
|
||||
from mcp_synology_container.modules.containers import _strip_hash_prefix
|
||||
|
||||
assert _strip_hash_prefix("/f93cb8b504f7_jenkins") == "jenkins"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_list_containers_strips_hash_prefix():
|
||||
"""list_containers must display the clean name without the hash prefix."""
|
||||
from mcp_synology_container.modules.containers import register_containers
|
||||
|
||||
client = AsyncMock()
|
||||
client.request.return_value = HASH_PREFIXED_CONTAINERS_DATA
|
||||
|
||||
mcp, tools = make_mock_mcp()
|
||||
register_containers(mcp, make_config(), client)
|
||||
|
||||
result = await tools["list_containers"]()
|
||||
assert "jenkins" in result
|
||||
assert "f93cb8b504f7_jenkins" not in result
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_container_stats_strips_hash_prefix():
|
||||
"""container_stats must match even when DSM returns a hash-prefixed name."""
|
||||
from mcp_synology_container.modules.containers import register_containers
|
||||
|
||||
stats_with_hash = {
|
||||
"abc123": {
|
||||
"name": "/f93cb8b504f7_jenkins",
|
||||
"cpu_stats": {
|
||||
"cpu_usage": {"total_usage": 500_000, "percpu_usage": [500_000]},
|
||||
"system_cpu_usage": 100_000_000_000,
|
||||
"online_cpus": 1,
|
||||
},
|
||||
"precpu_stats": {
|
||||
"cpu_usage": {"total_usage": 0},
|
||||
"system_cpu_usage": 0,
|
||||
},
|
||||
"memory_stats": {"usage": 1024, "limit": 2048},
|
||||
"networks": {},
|
||||
"blkio_stats": {"io_service_bytes_recursive": []},
|
||||
}
|
||||
}
|
||||
|
||||
client = AsyncMock()
|
||||
client.request.return_value = stats_with_hash
|
||||
|
||||
mcp, tools = make_mock_mcp()
|
||||
register_containers(mcp, make_config(), client)
|
||||
|
||||
# User passes clean name; must still match the hash-prefixed entry
|
||||
result = await tools["container_stats"]("jenkins")
|
||||
assert "not found" not in result.lower()
|
||||
assert "CPU" in result
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_container_status_resolves_hash_prefix():
|
||||
"""get_container_status resolves 'jenkins' to 'f93cb8b504f7_jenkins' for DSM call."""
|
||||
from mcp_synology_container.modules.containers import register_containers
|
||||
|
||||
detail_data = {"State": {"Status": "running", "Running": True}, "Config": {"Image": "jenkins/jenkins:lts"}}
|
||||
|
||||
async def mock_request(api, method, **kwargs):
|
||||
if api == "SYNO.Docker.Container" and method == "list":
|
||||
return HASH_PREFIXED_CONTAINERS_DATA
|
||||
if api == "SYNO.Docker.Container" and method == "get":
|
||||
# Must be called with the hash-prefixed name
|
||||
assert kwargs["params"]["name"] == "f93cb8b504f7_jenkins"
|
||||
return detail_data
|
||||
return {}
|
||||
|
||||
client = AsyncMock()
|
||||
client.request.side_effect = mock_request
|
||||
|
||||
mcp, tools = make_mock_mcp()
|
||||
register_containers(mcp, make_config(), client)
|
||||
|
||||
result = await tools["get_container_status"]("jenkins")
|
||||
assert "Error" not in result
|
||||
# Display name should be clean (no hash prefix)
|
||||
assert "f93cb8b504f7" not in result
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_container_logs_resolves_hash_prefix():
|
||||
"""get_container_logs resolves 'jenkins' to 'f93cb8b504f7_jenkins' for DSM call."""
|
||||
from mcp_synology_container.modules.containers import register_containers
|
||||
|
||||
async def mock_request(api, method, **kwargs):
|
||||
if api == "SYNO.Docker.Container" and method == "list":
|
||||
return HASH_PREFIXED_CONTAINERS_DATA
|
||||
if api == "SYNO.Docker.Container.Log" and method == "get":
|
||||
assert kwargs["params"]["name"] == "f93cb8b504f7_jenkins"
|
||||
return {"logs": [{"created": "2025-01-01", "stream": "stdout", "text": "started"}], "total": 1}
|
||||
return {}
|
||||
|
||||
client = AsyncMock()
|
||||
client.request.side_effect = mock_request
|
||||
|
||||
mcp, tools = make_mock_mcp()
|
||||
register_containers(mcp, make_config(), client)
|
||||
|
||||
result = await tools["get_container_logs"]("jenkins")
|
||||
assert "started" in result
|
||||
assert "f93cb8b504f7" not in result
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_exec_in_container_resolves_hash_prefix():
|
||||
"""exec_in_container resolves 'jenkins' to 'f93cb8b504f7_jenkins' for DSM call."""
|
||||
from mcp_synology_container.modules.containers import register_containers
|
||||
|
||||
async def mock_request(api, method, **kwargs):
|
||||
if api == "SYNO.Docker.Container" and method == "list":
|
||||
return HASH_PREFIXED_CONTAINERS_DATA
|
||||
if api == "SYNO.Docker.Container" and method == "exec":
|
||||
assert kwargs["params"]["name"] == "f93cb8b504f7_jenkins"
|
||||
return {"output": "ok", "exit_code": 0}
|
||||
return {}
|
||||
|
||||
client = AsyncMock()
|
||||
client.request.side_effect = mock_request
|
||||
|
||||
mcp, tools = make_mock_mcp()
|
||||
register_containers(mcp, make_config(), client)
|
||||
|
||||
result = await tools["exec_in_container"]("jenkins", "echo ok", confirmed=True)
|
||||
assert "ok" in result
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_container_stats_no_precpu_graceful():
|
||||
"""When precpu_stats has no system_cpu_usage (first poll), CPU% = 0."""
|
||||
|
||||
@@ -161,3 +161,150 @@ async def test_redeploy_project_requires_confirmation():
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=False)
|
||||
assert "confirmed=True" in result
|
||||
client.request.assert_not_called()
|
||||
|
||||
|
||||
# ──────────────────────────────────────────────────────────────────────────────
|
||||
# Bug 2: status-aware redeploy
|
||||
# ──────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def make_projects_tools(client):
|
||||
from mcp_synology_container.modules.projects import register_projects
|
||||
from mcp_synology_container.config import AppConfig, ConnectionConfig
|
||||
|
||||
config = AppConfig(
|
||||
schema_version=1,
|
||||
connection=ConnectionConfig(host="nas.local", port=443, https=True, verify_ssl=True),
|
||||
)
|
||||
tools: dict = {}
|
||||
|
||||
class MockMCP:
|
||||
def tool(self):
|
||||
def decorator(fn):
|
||||
tools[fn.__name__] = fn
|
||||
return fn
|
||||
return decorator
|
||||
|
||||
register_projects(MockMCP(), config, client)
|
||||
return tools
|
||||
|
||||
|
||||
def project_list(status: str) -> dict:
|
||||
return {
|
||||
"uuid-1": {
|
||||
"id": "uuid-1",
|
||||
"name": "myapp",
|
||||
"status": status,
|
||||
"path": "/volume1/docker/myapp",
|
||||
"containerIds": ["abc123"],
|
||||
"services": [],
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_redeploy_running_project():
|
||||
"""RUNNING project: stop then start (2 steps)."""
|
||||
client = AsyncMock()
|
||||
calls = []
|
||||
|
||||
async def mock_request(api, method, **kwargs):
|
||||
calls.append((api, method))
|
||||
return project_list("RUNNING") if method == "list" else {}
|
||||
|
||||
client.request.side_effect = mock_request
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
|
||||
assert "redeployed successfully" in result
|
||||
methods = [m for _, m in calls]
|
||||
assert "stop" in methods
|
||||
assert "start" in methods
|
||||
# stop must come before start
|
||||
assert methods.index("stop") < methods.index("start")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_redeploy_stopped_project_starts_directly():
|
||||
"""STOPPED project: skip stop, just start."""
|
||||
client = AsyncMock()
|
||||
calls = []
|
||||
|
||||
async def mock_request(api, method, **kwargs):
|
||||
calls.append((api, method))
|
||||
return project_list("STOPPED") if method == "list" else {}
|
||||
|
||||
client.request.side_effect = mock_request
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
|
||||
assert "redeployed successfully" in result
|
||||
methods = [m for _, m in calls]
|
||||
assert "stop" not in methods
|
||||
assert "start" in methods
|
||||
assert "STOPPED" in result or "starting directly" in result.lower()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_redeploy_build_failed_project():
|
||||
"""BUILD_FAILED project: force-stop (non-fatal), then start."""
|
||||
client = AsyncMock()
|
||||
calls = []
|
||||
|
||||
async def mock_request(api, method, **kwargs):
|
||||
calls.append((api, method))
|
||||
return project_list("BUILD_FAILED") if method == "list" else {}
|
||||
|
||||
client.request.side_effect = mock_request
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
|
||||
assert "redeployed successfully" in result
|
||||
methods = [m for _, m in calls]
|
||||
assert "stop" in methods
|
||||
assert "start" in methods
|
||||
assert methods.index("stop") < methods.index("start")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_redeploy_build_failed_stop_error_nonfatal():
|
||||
"""BUILD_FAILED: stop failure must not abort the redeploy."""
|
||||
from mcp_synology_container.dsm_client import SynologyError
|
||||
|
||||
client = AsyncMock()
|
||||
|
||||
async def mock_request(api, method, **kwargs):
|
||||
if method == "list":
|
||||
return project_list("BUILD_FAILED")
|
||||
if method == "stop":
|
||||
raise SynologyError("already stopped", code=2101)
|
||||
return {} # start succeeds
|
||||
|
||||
client.request.side_effect = mock_request
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
|
||||
assert "redeployed successfully" in result
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_redeploy_unknown_status_returns_error():
|
||||
"""Unknown status must return a clear error with a workaround hint."""
|
||||
client = AsyncMock()
|
||||
|
||||
async def mock_request(api, method, **kwargs):
|
||||
if method == "list":
|
||||
return project_list("UPDATING")
|
||||
return {}
|
||||
|
||||
client.request.side_effect = mock_request
|
||||
tools = make_projects_tools(client)
|
||||
|
||||
result = await tools["redeploy_project"]("myapp", confirmed=True)
|
||||
|
||||
assert "UPDATING" in result
|
||||
assert "Workaround" in result or "stop_project" in result
|
||||
|
||||
Reference in New Issue
Block a user