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.
This commit is contained in:
YueGuobin 2026-08-19 22:44:09 +08:00
parent c3145a9f65
commit 210103058f
No known key found for this signature in database
3 changed files with 68 additions and 8 deletions

View File

@ -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):

View File

@ -14,7 +14,7 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
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

View File

@ -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: