From 2bde02d4593306fc763ee7a82fc4e0139ca4eb17 Mon Sep 17 00:00:00 2001 From: grossmj Date: Sat, 11 Jun 2016 17:31:30 -0600 Subject: [PATCH] Controller side unique node name allocation. Ref #541. --- gns3server/controller/compute.py | 4 +- gns3server/controller/node.py | 15 ++-- gns3server/controller/project.py | 82 ++++++++++++++++++- .../handlers/api/controller/node_handler.py | 4 +- tests/controller/test_link.py | 14 ++-- tests/controller/test_node.py | 8 +- tests/controller/test_notification.py | 2 +- tests/controller/test_project.py | 12 +-- tests/controller/test_udp_link.py | 24 +++--- tests/handlers/api/controller/test_link.py | 8 +- tests/handlers/api/controller/test_node.py | 2 +- 11 files changed, 128 insertions(+), 47 deletions(-) diff --git a/gns3server/controller/compute.py b/gns3server/controller/compute.py index 7cc0aacd..c6db4cc0 100644 --- a/gns3server/controller/compute.py +++ b/gns3server/controller/compute.py @@ -332,7 +332,7 @@ class Compute: if body: try: msg = json.loads(body)["message"] - except (KeyError, json.decoder.JSONDecodeError): + except (KeyError, ValueError): msg = body else: msg = "" @@ -349,7 +349,7 @@ class Compute: try: raise ComputeConflict(json.loads(body)) # If the 409 doesn't come from a GNS3 server - except json.decoder.JSONDecodeError: + except ValueError: raise aiohttp.web.HTTPConflict(text=msg) elif response.status == 500: raise aiohttp.web.HTTPInternalServerError(text="Internal server error {}".format(url)) diff --git a/gns3server/controller/node.py b/gns3server/controller/node.py index 15a996b3..cf5b5ddb 100644 --- a/gns3server/controller/node.py +++ b/gns3server/controller/node.py @@ -15,7 +15,6 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . -import aiohttp import asyncio import copy import uuid @@ -30,10 +29,11 @@ class Node: # This properties are used only on controller and are not forwarded to the compute CONTROLLER_ONLY_PROPERTIES = ["x", "y", "z", "symbol", "label", "console_host"] - def __init__(self, project, compute, node_id=None, node_type=None, **kwargs): + def __init__(self, project, compute, name, node_id=None, node_type=None, **kwargs): """ :param project: Project of the node :param compute: Compute server where the server will run + :param name: Node name :param node_id: UUID of the node (integer) :param node_type: Type of emulator :param kwargs: Node properties @@ -48,7 +48,7 @@ class Node: self._compute = compute self._node_type = node_type - self._name = None + self._name = name self._console = None self._console_type = None self._properties = {} @@ -83,10 +83,10 @@ class Node: return self._name @name.setter - def name(self, val): - self._name = val + def name(self, new_name): + self._name = new_name # The text in label need to be always the node name - self._label["text"] = val + self._label["text"] = new_name @property def node_type(self): @@ -205,6 +205,9 @@ class Node: # When updating properties used only on controller we don't need to call the compute update_compute = False + # update the node name if present + self._project.update_node_name(self, kwargs.get("name")) + # Update node properties with additional elements for prop in kwargs: if getattr(self, prop) != kwargs[prop]: diff --git a/gns3server/controller/project.py b/gns3server/controller/project.py index ee1e3d65..725c94ab 100644 --- a/gns3server/controller/project.py +++ b/gns3server/controller/project.py @@ -54,6 +54,7 @@ class Project: self.path = path self._computes = set() + self._allocated_node_names = set() self._nodes = {} self._links = {} @@ -105,15 +106,89 @@ class Project: def add_compute(self, compute): self._computes.add(compute) + def allocate_node_name(self, base_name): + """ + Allocates a new unique name for a node in this project. + + :param base_name: base name for the node which will be completed with a unique number. + + :returns: allocated name or None if one could not be found + """ + + if '{0}' in base_name or '{id}' in base_name: + # base name is a template, replace {0} or {id} by an unique identifier + for number in range(1, 1000000): + name = base_name.format(number, id=number) + if name not in self._allocated_node_names: + self._allocated_node_names.add(name) + return name + else: + if base_name not in self._allocated_node_names: + return base_name + # base name is not unique, let's find a unique name by appending a number + for number in range(1, 1000000): + name = base_name + str(number) + if name not in self._allocated_node_names: + self._allocated_node_names.add(name) + return name + return None + + def remove_allocated_node_name(self, name): + """ + Removes an allocated node name + + :param name: allocated node name + """ + + if name in self._allocated_node_names: + self._allocated_node_names.remove(name) + + def update_allocated_node_name(self, name): + """ + Updates a node name + + :param name: new node name + """ + + self.remove_allocated_node_name(name) + self._allocated_node_names.add(name) + + def has_allocated_node_name(self, name): + """ + Returns either a node name is already allocated or not. + + :param name: node name + + :returns: boolean + """ + + if name in self._allocated_node_names: + return True + return False + + def update_node_name(self, node, new_name): + + if new_name and node.name != new_name: + if self.has_allocated_node_name(new_name): + raise aiohttp.web.HTTPConflict(text="{} node name is already allocated in this project".format(new_name)) + self.update_allocated_node_name(new_name) + return True + return False + @asyncio.coroutine - def add_node(self, compute, node_id, **kwargs): + def add_node(self, compute, name, node_id, **kwargs): """ Create a node or return an existing node :param kwargs: See the documentation of node """ if node_id not in self._nodes: - node = Node(self, compute, node_id=node_id, **kwargs) + + name = self.allocate_node_name(name) + if not name: + raise aiohttp.web.HTTPConflict(text="A node name could not be allocated (node limit reached?)") + + node = Node(self, compute, name, node_id=node_id, **kwargs) if compute not in self._project_created_on_compute: # For a local server we send the project path if compute.id == "local": @@ -137,7 +212,9 @@ class Project: @asyncio.coroutine def delete_node(self, node_id): + node = self.get_node(node_id) + self.remove_allocated_node_name(node.name) del self._nodes[node.id] yield from node.destroy() self.controller.notification.emit("node.deleted", node.__json__()) @@ -194,6 +271,7 @@ class Project: def close(self): for compute in self._project_created_on_compute: yield from compute.post("/projects/{}/close".format(self._id)) + self._allocated_node_names.clear() @asyncio.coroutine def commit(self): diff --git a/gns3server/handlers/api/controller/node_handler.py b/gns3server/handlers/api/controller/node_handler.py index c578d43c..079b6a99 100644 --- a/gns3server/handlers/api/controller/node_handler.py +++ b/gns3server/handlers/api/controller/node_handler.py @@ -15,6 +15,8 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . +import aiohttp + from gns3server.web.route import Route from gns3server.controller import Controller from gns3server.utils.asyncio.pool import Pool @@ -47,7 +49,7 @@ class NodeHandler: controller = Controller.instance() compute = controller.get_compute(request.json.pop("compute_id")) project = controller.get_project(request.match_info["project_id"]) - node = yield from project.add_node(compute, request.json.pop("node_id", None), **request.json) + node = yield from project.add_node(compute, request.json.pop("name"), request.json.pop("node_id", None), **request.json) response.set_status(201) response.json(node) diff --git a/tests/controller/test_link.py b/tests/controller/test_link.py index 46b5ef97..676ecdf2 100644 --- a/tests/controller/test_link.py +++ b/tests/controller/test_link.py @@ -41,8 +41,8 @@ def compute(): @pytest.fixture def link(async_run, project, compute): - node1 = Node(project, compute) - node2 = Node(project, compute) + node1 = Node(project, compute, "node1") + node2 = Node(project, compute, "node2") link = Link(project) async_run(link.add_node(node1, 0, 4)) @@ -51,7 +51,7 @@ def link(async_run, project, compute): def test_addNode(async_run, project, compute): - node1 = Node(project, compute) + node1 = Node(project, compute, "node1") link = Link(project) async_run(link.add_node(node1, 0, 4)) @@ -65,8 +65,8 @@ def test_addNode(async_run, project, compute): def test_json(async_run, project, compute): - node1 = Node(project, compute) - node2 = Node(project, compute) + node1 = Node(project, compute, "node1") + node2 = Node(project, compute, "node2") link = Link(project) async_run(link.add_node(node1, 0, 4)) @@ -110,8 +110,8 @@ def test_start_streaming_pcap(link, async_run, tmpdir, project): def test_default_capture_file_name(project, compute, async_run): - node1 = Node(project, compute, name="Hello@") - node2 = Node(project, compute, name="w0.rld") + node1 = Node(project, compute, "Hello@") + node2 = Node(project, compute, "w0.rld") link = Link(project) async_run(link.add_node(node1, 0, 4)) diff --git a/tests/controller/test_node.py b/tests/controller/test_node.py index 39786ae4..31761466 100644 --- a/tests/controller/test_node.py +++ b/tests/controller/test_node.py @@ -38,8 +38,7 @@ def compute(): @pytest.fixture def node(compute, controller): project = Project(str(uuid.uuid4()), controller=controller) - node = Node(project, compute, - name="demo", + node = Node(project, compute, "demo", node_id=str(uuid.uuid4()), node_type="vpcs", console_type="vnc", @@ -70,7 +69,7 @@ def test_json(node, compute): def test_init_without_uuid(project, compute): - node = Node(project, compute, + node = Node(project, compute, "demo", node_type="vpcs", console_type="vnc") assert node.id is not None @@ -258,8 +257,7 @@ def test_dynamips_idlepc_proposals(node, async_run, compute): def test_upload_missing_image(compute, controller, async_run, images_dir): project = Project(str(uuid.uuid4()), controller=controller) - node = Node(project, compute, - name="demo", + node = Node(project, compute, "demo", node_id=str(uuid.uuid4()), node_type="qemu", properties={"hda_disk_image": "linux.img"}) diff --git a/tests/controller/test_notification.py b/tests/controller/test_notification.py index dbb2aedf..2e201075 100644 --- a/tests/controller/test_notification.py +++ b/tests/controller/test_notification.py @@ -35,7 +35,7 @@ def node(project, async_run): response.json = {"console": 2048} compute.post = AsyncioMagicMock(return_value=response) - return async_run(project.add_node(compute, None, name="test", node_type="vpcs", properties={"startup_config": "test.cfg"})) + return async_run(project.add_node(compute, "test", None, node_type="vpcs", properties={"startup_config": "test.cfg"})) def test_emit_to_all(async_run, controller, project): diff --git a/tests/controller/test_project.py b/tests/controller/test_project.py index cca26ae8..2adcd29e 100644 --- a/tests/controller/test_project.py +++ b/tests/controller/test_project.py @@ -94,7 +94,7 @@ def test_add_node_local(async_run, controller): response.json = {"console": 2048} compute.post = AsyncioMagicMock(return_value=response) - node = async_run(project.add_node(compute, None, name="test", node_type="vpcs", properties={"startup_config": "test.cfg"})) + node = async_run(project.add_node(compute, "test", None, node_type="vpcs", properties={"startup_config": "test.cfg"})) assert node.id in project._nodes compute.post.assert_any_call('/projects', data={ @@ -123,7 +123,7 @@ def test_add_node_non_local(async_run, controller): response.json = {"console": 2048} compute.post = AsyncioMagicMock(return_value=response) - node = async_run(project.add_node(compute, None, name="test", node_type="vpcs", properties={"startup_config": "test.cfg"})) + node = async_run(project.add_node(compute, "test", None, node_type="vpcs", properties={"startup_config": "test.cfg"})) compute.post.assert_any_call('/projects', data={ "name": project._name, @@ -149,7 +149,7 @@ def test_delete_node(async_run, controller): response.json = {"console": 2048} compute.post = AsyncioMagicMock(return_value=response) - node = async_run(project.add_node(compute, None, name="test", node_type="vpcs", properties={"startup_config": "test.cfg"})) + node = async_run(project.add_node(compute, "test", None, node_type="vpcs", properties={"startup_config": "test.cfg"})) assert node.id in project._nodes async_run(project.delete_node(node.id)) assert node.id not in project._nodes @@ -166,7 +166,7 @@ def test_getVM(async_run, controller): response.json = {"console": 2048} compute.post = AsyncioMagicMock(return_value=response) - vm = async_run(project.add_node(compute, None, name="test", node_type="vpcs", properties={"startup_config": "test.cfg"})) + vm = async_run(project.add_node(compute, "test", None, node_type="vpcs", properties={"startup_config": "test.cfg"})) assert project.get_node(vm.id) == vm with pytest.raises(aiohttp.web_exceptions.HTTPNotFound): @@ -180,8 +180,8 @@ def test_addLink(async_run, project, controller): response.json = {"console": 2048} compute.post = AsyncioMagicMock(return_value=response) - vm1 = async_run(project.add_node(compute, None, name="test1", node_type="vpcs", properties={"startup_config": "test.cfg"})) - vm2 = async_run(project.add_node(compute, None, name="test2", node_type="vpcs", properties={"startup_config": "test.cfg"})) + vm1 = async_run(project.add_node(compute, "test1", None, node_type="vpcs", properties={"startup_config": "test.cfg"})) + vm2 = async_run(project.add_node(compute, "test2", None, node_type="vpcs", properties={"startup_config": "test.cfg"})) controller._notification = MagicMock() link = async_run(project.add_link()) async_run(link.add_node(vm1, 3, 1)) diff --git a/tests/controller/test_udp_link.py b/tests/controller/test_udp_link.py index f00c4674..d83c3fb7 100644 --- a/tests/controller/test_udp_link.py +++ b/tests/controller/test_udp_link.py @@ -35,8 +35,8 @@ def test_create(async_run, project): compute1 = MagicMock() compute2 = MagicMock() - node1 = Node(project, compute1, node_type="vpcs") - node2 = Node(project, compute2, node_type="vpcs") + node1 = Node(project, compute1, "node1", node_type="vpcs") + node2 = Node(project, compute2, "node2", node_type="vpcs") link = UDPLink(project) async_run(link.add_node(node1, 0, 4)) @@ -86,8 +86,8 @@ def test_delete(async_run, project): compute1 = MagicMock() compute2 = MagicMock() - node1 = Node(project, compute1, node_type="vpcs") - node2 = Node(project, compute2, node_type="vpcs") + node1 = Node(project, compute1, "node1", node_type="vpcs") + node2 = Node(project, compute2, "node2", node_type="vpcs") link = UDPLink(project) async_run(link.add_node(node1, 0, 4)) @@ -107,8 +107,8 @@ def test_choose_capture_side(async_run, project): compute2 = MagicMock() compute2.id = "local" - node_vpcs = Node(project, compute1, node_type="vpcs") - node_iou = Node(project, compute2, node_type="iou") + node_vpcs = Node(project, compute1, "node1", node_type="vpcs") + node_iou = Node(project, compute2, "node2", node_type="iou") link = UDPLink(project) async_run(link.add_node(node_vpcs, 0, 4)) @@ -116,8 +116,8 @@ def test_choose_capture_side(async_run, project): assert link._choose_capture_side()["node"] == node_iou - node_vpcs = Node(project, compute1, node_type="vpcs") - node_vpcs2 = Node(project, compute1, node_type="vpcs") + node_vpcs = Node(project, compute1, "node1", node_type="vpcs") + node_vpcs2 = Node(project, compute1, "node2", node_type="vpcs") link = UDPLink(project) async_run(link.add_node(node_vpcs, 0, 4)) @@ -128,8 +128,8 @@ def test_choose_capture_side(async_run, project): link._choose_capture_side()["node"] # Capture should run on the local node - node_iou = Node(project, compute1, node_type="iou") - node_iou2 = Node(project, compute2, node_type="iou") + node_iou = Node(project, compute1, "node1", node_type="iou") + node_iou2 = Node(project, compute2, "node2", node_type="iou") link = UDPLink(project) async_run(link.add_node(node_iou, 0, 4)) @@ -141,8 +141,8 @@ def test_choose_capture_side(async_run, project): def test_capture(async_run, project): compute1 = MagicMock() - node_vpcs = Node(project, compute1, node_type="vpcs", name="V1") - node_iou = Node(project, compute1, node_type="iou", name="I1") + node_vpcs = Node(project, compute1, "V1", node_type="vpcs") + node_iou = Node(project, compute1, "I1", node_type="iou") link = UDPLink(project) async_run(link.add_node(node_vpcs, 0, 4)) diff --git a/tests/handlers/api/controller/test_link.py b/tests/handlers/api/controller/test_link.py index 7f39fcbd..eeb4bce3 100644 --- a/tests/handlers/api/controller/test_link.py +++ b/tests/handlers/api/controller/test_link.py @@ -53,8 +53,8 @@ def test_create_link(http_controller, tmpdir, project, compute, async_run): response.json = {"console": 2048} compute.post = AsyncioMagicMock(return_value=response) - node1 = async_run(project.add_node(compute, None)) - node2 = async_run(project.add_node(compute, None)) + node1 = async_run(project.add_node(compute, "node1", None)) + node2 = async_run(project.add_node(compute, "node2", None)) with asyncio_patch("gns3server.controller.udp_link.UDPLink.create") as mock: response = http_controller.post("/projects/{}/links".format(project.id), { @@ -82,8 +82,8 @@ def test_list_link(http_controller, tmpdir, project, compute, async_run): response.json = {"console": 2048} compute.post = AsyncioMagicMock(return_value=response) - node1 = async_run(project.add_node(compute, None)) - node2 = async_run(project.add_node(compute, None)) + node1 = async_run(project.add_node(compute, "node1", None)) + node2 = async_run(project.add_node(compute, "node2", None)) with asyncio_patch("gns3server.controller.udp_link.UDPLink.create") as mock: response = http_controller.post("/projects/{}/links".format(project.id), { diff --git a/tests/handlers/api/controller/test_node.py b/tests/handlers/api/controller/test_node.py index b9cd6b1b..d860dcfc 100644 --- a/tests/handlers/api/controller/test_node.py +++ b/tests/handlers/api/controller/test_node.py @@ -50,7 +50,7 @@ def project(http_controller, async_run): @pytest.fixture def node(project, compute, async_run): - node = Node(project, compute, name="test", node_type="vpcs") + node = Node(project, compute, "test", node_type="vpcs") project._nodes[node.id] = node return node