From e5c76750b1474dab458d7defca60f0e487de0da7 Mon Sep 17 00:00:00 2001 From: grossmj Date: Wed, 31 Jan 2018 15:51:29 +0700 Subject: [PATCH] Fix issue when running multiple project containing IOU nodes on the same server. Ref #1239. --- gns3server/compute/iou/__init__.py | 42 +++------------- gns3server/compute/iou/iou_vm.py | 8 ++- .../compute/iou/utils/application_id.py | 5 +- gns3server/controller/project.py | 49 ++++++++----------- .../handlers/api/compute/iou_handler.py | 4 ++ 5 files changed, 38 insertions(+), 70 deletions(-) diff --git a/gns3server/compute/iou/__init__.py b/gns3server/compute/iou/__init__.py index acc994df..4cba97d7 100644 --- a/gns3server/compute/iou/__init__.py +++ b/gns3server/compute/iou/__init__.py @@ -25,6 +25,7 @@ import asyncio from ..base_manager import BaseManager from .iou_error import IOUError from .iou_vm import IOUVM +from .utils.application_id import get_next_application_id import logging log = logging.getLogger(__name__) @@ -38,8 +39,7 @@ class IOU(BaseManager): def __init__(self): super().__init__() - self._free_application_ids = list(range(1, 512)) - self._used_application_ids = {} + self._iou_id_lock = asyncio.Lock() @asyncio.coroutine def create_node(self, *args, **kwargs): @@ -49,40 +49,14 @@ class IOU(BaseManager): :returns: IOUVM instance """ - node = yield from super().create_node(*args, **kwargs) - try: - self._used_application_ids[node.id] = self._free_application_ids.pop(0) - except IndexError: - raise IOUError("Cannot create a new IOU VM (limit of 512 VMs reached on this host)") + with (yield from self._iou_id_lock): + # wait for a node to be completely created before adding a new one + # this is important otherwise we allocate the same application ID + # when creating multiple IOU node at the same time + application_id = get_next_application_id(self.nodes) + node = yield from super().create_node(*args, application_id=application_id, **kwargs) return node - @asyncio.coroutine - def close_node(self, node_id, *args, **kwargs): - """ - Closes an IOU VM. - - :returns: IOUVM instance - """ - - node = self.get_node(node_id) - if node_id in self._used_application_ids: - i = self._used_application_ids[node_id] - self._free_application_ids.insert(0, i) - del self._used_application_ids[node_id] - yield from super().close_node(node_id, *args, **kwargs) - return node - - def get_application_id(self, node_id): - """ - Get an unique application identifier for IOU. - - :param node_id: Node identifier - - :returns: IOU application identifier - """ - - return self._used_application_ids.get(node_id, 1) - @staticmethod def get_legacy_vm_workdir(legacy_vm_id, name): """ diff --git a/gns3server/compute/iou/iou_vm.py b/gns3server/compute/iou/iou_vm.py index 24747179..68664501 100644 --- a/gns3server/compute/iou/iou_vm.py +++ b/gns3server/compute/iou/iou_vm.py @@ -65,7 +65,7 @@ class IOUVM(BaseNode): :param console: TCP console port """ - def __init__(self, name, node_id, project, manager, path=None, console=None): + def __init__(self, name, node_id, project, manager, application_id=None, path=None, console=None): super().__init__(name, node_id, project, manager, console=console) @@ -86,7 +86,7 @@ class IOUVM(BaseNode): self._startup_config = "" self._private_config = "" self._ram = 256 # Megabytes - self._application_id = None + self._application_id = application_id self._l1_keepalives = False # used to overcome the always-up Ethernet interfaces (not supported by all IOSes). def _config(self): @@ -1141,9 +1141,7 @@ class IOUVM(BaseNode): :returns: integer between 1 and 512 """ - if self._application_id is None: - #FIXME: is this necessary? application ID is allocated by controller and should not be None - return self._manager.get_application_id(self.id) + return self._application_id @application_id.setter diff --git a/gns3server/compute/iou/utils/application_id.py b/gns3server/compute/iou/utils/application_id.py index d3de5fe4..5bd2c125 100644 --- a/gns3server/compute/iou/utils/application_id.py +++ b/gns3server/compute/iou/utils/application_id.py @@ -24,13 +24,14 @@ log = logging.getLogger(__name__) def get_next_application_id(nodes): """ Calculates free application_id from given nodes + :param nodes: :raises IOUError when exceeds number :return: integer first free id """ - used = set([n.properties.get('application_id') for n in nodes if n.node_type == 'iou']) + used = set([n.application_id for n in nodes]) pool = set(range(1, 512)) try: return (pool - used).pop() except KeyError: - raise IOUError("Cannot create a new IOU VM (limit of 512 VMs reached)") + raise IOUError("Cannot create a new IOU VM (limit of 512 VMs on one host reached)") diff --git a/gns3server/controller/project.py b/gns3server/controller/project.py index 4716db81..a0bd8209 100644 --- a/gns3server/controller/project.py +++ b/gns3server/controller/project.py @@ -39,7 +39,6 @@ from ..utils.asyncio.pool import Pool from ..utils.asyncio import locked_coroutine from .export_project import export_project from .import_project import import_project -from ..compute.iou.utils.application_id import get_next_application_id import logging log = logging.getLogger(__name__) @@ -86,7 +85,6 @@ class Project: self._show_grid = show_grid self._show_interface_labels = show_interface_labels self._loading = False - self._add_node_lock = asyncio.Lock() # Disallow overwrite of existing project if project_id is None and path is not None: @@ -439,34 +437,27 @@ class Project: if node_id in self._nodes: return self._nodes[node_id] - with (yield from self._add_node_lock): - # wait for a node to be completely created before adding a new one - # this is important otherwise we allocate the same application ID - # when creating multiple IOU node at the same time - if node_type == "iou" and 'application_id' not in kwargs.keys(): - kwargs['application_id'] = get_next_application_id(self._nodes.values()) + node = Node(self, compute, name, node_id=node_id, node_type=node_type, **kwargs) + if compute not in self._project_created_on_compute: + # For a local server we send the project path + if compute.id == "local": + yield from compute.post("/projects", data={ + "name": self._name, + "project_id": self._id, + "path": self._path + }) + else: + yield from compute.post("/projects", data={ + "name": self._name, + "project_id": self._id, + }) - node = Node(self, compute, name, node_id=node_id, node_type=node_type, **kwargs) - if compute not in self._project_created_on_compute: - # For a local server we send the project path - if compute.id == "local": - yield from compute.post("/projects", data={ - "name": self._name, - "project_id": self._id, - "path": self._path - }) - else: - yield from compute.post("/projects", data={ - "name": self._name, - "project_id": self._id, - }) - - self._project_created_on_compute.add(compute) - yield from node.create() - self._nodes[node.id] = node - self.controller.notification.emit("node.created", node.__json__()) - if dump: - self.dump() + self._project_created_on_compute.add(compute) + yield from node.create() + self._nodes[node.id] = node + self.controller.notification.emit("node.created", node.__json__()) + if dump: + self.dump() return node @locked_coroutine diff --git a/gns3server/handlers/api/compute/iou_handler.py b/gns3server/handlers/api/compute/iou_handler.py index 70f8f35a..ae661c36 100644 --- a/gns3server/handlers/api/compute/iou_handler.py +++ b/gns3server/handlers/api/compute/iou_handler.py @@ -65,6 +65,8 @@ class IOUHandler: for name, value in request.json.items(): if hasattr(vm, name) and getattr(vm, name) != value: + if name == "application_id": + continue # we must ignore this to avoid overwriting the application_id allocated by the IOU manager if name == "startup_config_content" and (vm.startup_config_content and len(vm.startup_config_content) > 0): continue if name == "private_config_content" and (vm.private_config_content and len(vm.private_config_content) > 0): @@ -116,6 +118,8 @@ class IOUHandler: for name, value in request.json.items(): if hasattr(vm, name) and getattr(vm, name) != value: + if name == "application_id": + continue # we must ignore this to avoid overwriting the application_id allocated by the IOU manager setattr(vm, name, value) if vm.use_default_iou_values: