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