fix: return the created template from appliance install

POST /appliances/{id}/install replied 204 with an empty body, so the MCP
appliance_install tool crashed with 'Expecting value: line 1 column 1'
while the template had actually been created. The route now returns the
created template (201, response_model=schemas.Template), _create_template
propagates it, and the MCP handler parses the body defensively so an
empty reply degrades to a plain success message.
This commit is contained in:
YueGuobin 2026-08-25 23:39:38 +08:00
parent 73e5e27c7b
commit 888afdccbd
No known key found for this signature in database
6 changed files with 44 additions and 12 deletions

View File

@ -1376,7 +1376,7 @@ async def appliance_install(
appliance_id: Annotated[str, Field(description="UUID of the appliance to install")],
version: Annotated[str | None, Field(description="Version to install (e.g. '2.7.0.356'). Required if the appliance has multiple versions. Use appliance_get to see available versions.")] = None,
) -> list[dict[str, Any]]:
"""Create a template from a GNS3 appliance definition.
"""Create a template from a GNS3 appliance definition and return the created template.
NOTE: This does NOT download images. Images must be placed in the
GNS3 images directory (e.g. ~/GNS3/images/) beforehand.

View File

@ -85,5 +85,13 @@ def install_appliance_handler(params: dict[str, Any], gns3_ctx: dict[str, Any])
version = params.get("version")
if version:
request_params["version"] = version
result = conn.http_call("post", url, params=request_params).json()
return {"message": f"Appliance {appliance_id} installation requested", "result": result}
response = conn.http_call("post", url, params=request_params)
result = {"message": f"Appliance {appliance_id} installed"}
if response.content:
# the install endpoint returns the created template (201); tolerate an
# empty body in case an older server still replies with 204
template = response.json()
result["template"] = {
k: template[k] for k in ("template_id", "name", "version", "template_type") if k in template
}
return result

View File

@ -122,7 +122,8 @@ def add_appliance_version(appliance_id: UUID, appliance_version: Union[schemas.A
@router.post(
"/{appliance_id}/install",
status_code=status.HTTP_204_NO_CONTENT,
response_model=schemas.Template,
status_code=status.HTTP_201_CREATED,
dependencies=[Depends(has_privilege("Appliance.Allocate"))]
)
async def install_appliance(
@ -132,15 +133,15 @@ async def install_appliance(
templates_repo: TemplatesRepository = Depends(get_repository(TemplatesRepository)),
current_user: schemas.User = Depends(get_current_active_user),
rbac_repo: RbacRepository = Depends(get_repository(RbacRepository))
) -> None:
) -> schemas.Template:
"""
Install an appliance.
Install an appliance and return the created template.
Required privilege: Appliance.Allocate
"""
controller = Controller.instance()
await controller.appliance_manager.install_appliance(
return await controller.appliance_manager.install_appliance(
appliance_id,
version,
images_repo,

View File

@ -204,9 +204,9 @@ class ApplianceManager:
else:
raise ControllerError(f"Could not find '{appliance_file}'")
async def _create_template(self, template_data, templates_repo, rbac_repo, current_user):
async def _create_template(self, template_data, templates_repo, rbac_repo, current_user) -> dict:
"""
Create a new template
Create a new template and return it as a dict.
"""
try:
@ -217,6 +217,7 @@ class ApplianceManager:
#template_id = template.get("template_id")
#await rbac_repo.add_permission_to_user_with_path(current_user.user_id, f"/templates/{template_id}/*")
log.info(f"Template '{template.get('name')}' has been created")
return template
async def _appliance_to_template(self, appliance: Appliance, version: str = None) -> dict:
"""

View File

@ -3,6 +3,8 @@ MCP handler unit tests with mocked Gns3Connector.
Tests that handlers correctly transform tool parameters into HTTP calls.
"""
import json
import pytest
from unittest.mock import MagicMock, patch
@ -12,6 +14,7 @@ def _mock_conn(json_result=None):
conn = MagicMock()
conn.base_url = "http://192.168.1.3:3080/v3"
conn.http_call.return_value.json.return_value = json_result or {"status": "ok"}
conn.http_call.return_value.content = b"{}" # non-empty body by default
return conn
@ -449,7 +452,7 @@ class TestAppliance:
def test_install_with_version(self, ctx):
from gns3server.agent.mcp.appliances import install_appliance_handler
with patch(f"{BASE}.{self.mod}._get_connector") as m:
conn = _mock_conn({"status": "installed"})
conn = _mock_conn({"template_id": "t1", "name": "FRR", "version": "8.2.2", "template_type": "docker"})
m.return_value = conn
result = install_appliance_handler({
"appliance_id": "a1", "version": "2.7.0.356",
@ -458,6 +461,22 @@ class TestAppliance:
"post", "http://192.168.1.3:3080/v3/appliances/a1/install",
params={"version": "2.7.0.356"},
)
assert result["template"] == {
"template_id": "t1", "name": "FRR", "version": "8.2.2", "template_type": "docker",
}
def test_install_empty_body(self, ctx):
# a 204-style empty response must not blow up with a JSON decode error
# (the template is still created server-side)
from gns3server.agent.mcp.appliances import install_appliance_handler
with patch(f"{BASE}.{self.mod}._get_connector") as m:
conn = _mock_conn()
conn.http_call.return_value.content = b""
conn.http_call.return_value.json.side_effect = json.JSONDecodeError("Expecting value", "", 0)
m.return_value = conn
result = install_appliance_handler({"appliance_id": "a1"}, ctx)
assert "template" not in result
assert result["message"] == "Appliance a1 installed"
def test_install_missing_id(self, ctx):
from gns3server.agent.mcp.appliances import install_appliance_handler

View File

@ -53,7 +53,8 @@ class TestApplianceRoutes:
appliance_id = "fc520ae2-a4e5-48c3-9a13-516bb2e94668" # Alpine Linux appliance
response = await client.post(app.url_path_for("install_appliance", appliance_id=appliance_id))
assert response.status_code == status.HTTP_204_NO_CONTENT
assert response.status_code == status.HTTP_201_CREATED
assert response.json()["name"] == "Alpine Linux"
async def test_docker_appliance_install_with_version(self, app: FastAPI, client: AsyncClient) -> None:
@ -68,7 +69,9 @@ class TestApplianceRoutes:
appliance_id = "1cfdf900-7c30-4cb7-8f03-3f61d2581633" # Empty VM appliance
params = {"version": "8G"}
response = await client.post(app.url_path_for("install_appliance", appliance_id=appliance_id), params=params)
assert response.status_code == status.HTTP_204_NO_CONTENT
assert response.status_code == status.HTTP_201_CREATED
assert response.json()["name"] == "Empty VM"
assert response.json()["version"] == "8G"
async def test_qemu_appliance_install_without_version(self, app: FastAPI, client: AsyncClient, images_dir: str) -> None: