From 210103058f10a461b810c2a316bf49797c9f5697 Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Wed, 19 Aug 2026 22:44:09 +0800 Subject: [PATCH] docker: don't recreate containers on empty-string property PUTs Web clients serialize empty form fields as "" while unset values are stored as None on the node. The bare != diff in the update handler then sees a phantom change on every full PUT and recreates the container for nothing -- even when the user only changed a controller-only field such as netmiko_device_type. Normalize at the schema boundary ("" -> None for start_command, environment and extra_hosts; "" -> "/" for console_http_path), make the setters apply the same canonicalization, and create nodes through the setters instead of bypassing them in __init__ so both paths store identical values. --- gns3server/compute/docker/docker_vm.py | 20 ++++++---- gns3server/schemas/compute/docker_nodes.py | 17 +++++++- tests/api/routes/compute/test_docker_nodes.py | 39 +++++++++++++++++++ 3 files changed, 68 insertions(+), 8 deletions(-) diff --git a/gns3server/compute/docker/docker_vm.py b/gns3server/compute/docker/docker_vm.py index fa45c43e6..1fade0fd1 100644 --- a/gns3server/compute/docker/docker_vm.py +++ b/gns3server/compute/docker/docker_vm.py @@ -129,8 +129,10 @@ class DockerVM(BaseNode): if ":" not in image: image = f"{image}:latest" self._image = image - self._start_command = start_command - self._environment = environment + # assign through the property setters so creation and updates apply + # the same value normalization (e.g. "" -> None) + self.start_command = start_command + self.environment = environment self._cid = None self._ethernet_adapters = [] self._temporary_directory = None @@ -138,10 +140,10 @@ class DockerVM(BaseNode): self._vnc_process = None self._vncconfig_process = None self._console_resolution = console_resolution - self._console_http_path = console_http_path + self.console_http_path = console_http_path self._console_http_port = console_http_port self._console_websocket = None - self._extra_hosts = extra_hosts + self.extra_hosts = extra_hosts self._extra_volumes = extra_volumes or [] self._extra_configs = extra_configs or [] self._memory = memory @@ -288,7 +290,9 @@ class DockerVM(BaseNode): @console_http_path.setter def console_http_path(self, path): - self._console_http_path = path + # the canonical "no path" value is "/" so that "", None and "/" + # all compare equal in the update diff + self._console_http_path = path or "/" @property def console_http_port(self): @@ -304,7 +308,8 @@ class DockerVM(BaseNode): @environment.setter def environment(self, command): - self._environment = command + # "" and None are the same "no environment variables" value + self._environment = command or None @property def extra_hosts(self): @@ -312,7 +317,8 @@ class DockerVM(BaseNode): @extra_hosts.setter def extra_hosts(self, extra_hosts): - self._extra_hosts = extra_hosts + # "" and None are the same "no extra hosts" value + self._extra_hosts = extra_hosts or None @property def extra_volumes(self): diff --git a/gns3server/schemas/compute/docker_nodes.py b/gns3server/schemas/compute/docker_nodes.py index 1461c18c1..7b39921f6 100644 --- a/gns3server/schemas/compute/docker_nodes.py +++ b/gns3server/schemas/compute/docker_nodes.py @@ -14,7 +14,7 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, field_validator from typing import Optional, List from uuid import UUID @@ -26,6 +26,21 @@ class DockerBase(BaseModel): Common Docker node properties. """ + @field_validator("start_command", "environment", "extra_hosts", mode="before") + @classmethod + def _empty_string_to_none(cls, value): + # Web clients serialize empty form fields as "" while unset values are + # stored as None on the node: normalize before the update diff runs, + # otherwise every full PUT would see a phantom change and recreate + # the container for nothing. + return value or None + + @field_validator("console_http_path", mode="before") + @classmethod + def _empty_string_to_root_path(cls, value): + # the canonical "no path" value is "/" (the creation default) + return value or "/" + name: str image: str = Field(..., description="Docker image name") node_id: Optional[UUID] = None diff --git a/tests/api/routes/compute/test_docker_nodes.py b/tests/api/routes/compute/test_docker_nodes.py index 13e4609b7..278596dba 100644 --- a/tests/api/routes/compute/test_docker_nodes.py +++ b/tests/api/routes/compute/test_docker_nodes.py @@ -306,6 +306,45 @@ class TestDockerNodesRoutes: assert response.json()["environment"] == "GNS3=1\nGNS4=0" assert response.json()["extra_hosts"] == "test:127.0.0.1" + async def test_docker_update_empty_strings_do_not_recreate_container( + self, + app: FastAPI, + compute_client: AsyncClient, + compute_project: Project + ) -> None: + """ + Web clients serialize empty form fields as "" while unset values are + stored as None on the node: a full PUT must not see a phantom change + and recreate the container for nothing. + """ + + params = {"name": "DOCKER-EMPTY", "image": "nginx", "environment": ""} + with asyncio_patch("gns3server.compute.docker.Docker.list_images", return_value=[{"image": "nginx"}]): + with asyncio_patch("gns3server.compute.docker.Docker.query", return_value={"Id": "8bd8153ea8f5"}): + response = await compute_client.post( + app.url_path_for("compute:create_docker_node", project_id=compute_project.id), json=params + ) + assert response.status_code == status.HTTP_201_CREATED + assert response.json()["environment"] is None # "" normalized at creation + assert response.json()["console_http_path"] == "/" + node_id = response.json()["node_id"] + + with asyncio_patch("gns3server.compute.docker.docker_vm.DockerVM.update") as mock: + response = await compute_client.put( + app.url_path_for("compute:update_docker_node", project_id=compute_project.id, node_id=node_id), + json={ + "name": "DOCKER-EMPTY", + "start_command": "", + "environment": "", + "extra_hosts": "", + "console_http_path": "", + }, + ) + assert response.status_code == 200 + assert not mock.called # no real change: the container must not be recreated + assert response.json()["start_command"] is None + assert response.json()["console_http_path"] == "/" + async def test_docker_start_capture(self, app: FastAPI, compute_client: AsyncClient, vm: dict) -> None: