From 290c542afe1663980df775a1b01e996b01b79408 Mon Sep 17 00:00:00 2001 From: YueGuobin Date: Fri, 16 Oct 2026 16:49:06 +0800 Subject: [PATCH] fix: roll back a node update the compute refused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A compute 409 (for instance "must be stopped to change the console type to vnc") left every value that had already been applied locally on the controller node, because the update loop sets them before the PUT. Nothing was dumped at that point, but project.dump() serializes the whole topology: the next successful change anywhere in the project wrote the refused values to disk, and the next project open recreated the node with them — this time accepted by a stopped node. A refused change silently landed later. Restore the applied properties, the node properties, custom adapters, missing images and the derived ports before re-raising, the way the degraded-image create() path already cleans up after its own failure. --- gns3server/controller/node.py | 14 ++++++++++++++ tests/controller/test_node.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/gns3server/controller/node.py b/gns3server/controller/node.py index 6022ff982..fb36ee8ef 100644 --- a/gns3server/controller/node.py +++ b/gns3server/controller/node.py @@ -711,6 +711,9 @@ class Node: old_properties = copy.deepcopy(self._properties) old_custom_adapters = copy.deepcopy(self._custom_adapters) old_missing_images = copy.deepcopy(self._missing_images) + # Values applied locally below, kept to roll the node back if the + # compute refuses the update (see the ComputeConflictError handler). + old_prop_values = {prop: getattr(self, prop) for prop in kwargs if prop != "properties"} compute_properties = None # Update node properties with additional elements @@ -766,9 +769,20 @@ class Node: try: response = await self.put(None, data=data) except ComputeConflictError: + # The compute refused the update: roll back everything applied + # locally. Keeping the requested values would leave the + # controller describing a node the compute never accepted, and + # the next project dump persists them — a refused change would + # then silently land the next time the node is created. + for prop, old_value in old_prop_values.items(): + setattr(self, prop, old_value) if old_name != self.name: # special case when the new name is already updated on controller but refused by the compute self.name = old_name + self._properties = old_properties + self._custom_adapters = old_custom_adapters + self._missing_images = old_missing_images + self._list_ports() raise await self.parse_node_response(response.json) elif old_json != self.asdict(): diff --git a/tests/controller/test_node.py b/tests/controller/test_node.py index f17eaea3d..4d6a561db 100644 --- a/tests/controller/test_node.py +++ b/tests/controller/test_node.py @@ -811,6 +811,36 @@ async def test_update_properties(node, compute, controller): # controller._notification.emit.assert_called_with("node.updated", node_notif) +@pytest.mark.asyncio +async def test_update_rolls_back_when_compute_refuses(node, compute, project, controller): + """ + Values applied locally before the PUT must be rolled back when the compute + refuses the update: kept in memory they describe a node the compute never + accepted, and the next project dump persists them — the refused change + would then silently land the next time the node is created. + """ + + async def resp(*args, **kwargs): + raise ComputeConflictError( + f"/projects/{project.id}/vpcs/nodes/{node.id}", + {"message": '"demo" must be stopped to change the console type to telnet'}, + ) + + compute.put = AsyncioMagicMock(side_effect=resp) + controller._notification = AsyncioMagicMock() + project.dump = MagicMock() + old_json = node.asdict() + + with pytest.raises(ComputeConflictError): + await node.update(x=42, console_type="telnet", name="renamed", properties={"startup_script": "changed"}) + + assert node.name == "demo" + assert node.console_type == "vnc" + assert node.properties == {"startup_script": "echo test"} + assert node.asdict() == old_json + assert not project.dump.called + + @pytest.mark.asyncio async def test_update_only_controller(node, compute): """