Controller side unique node name allocation. Ref #541.

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

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 <http://www.gnu.org/licenses/>.
 
-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):
+    def name(self, new_name):
-        self._name = val
+        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]:

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

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 <http://www.gnu.org/licenses/>.
 
+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)
 

tests/controller/test_link.py

@@ -41,8 +41,8 @@ def compute():
 
 @pytest.fixture
 def link(async_run, project, compute):
-    node1 = Node(project, compute)
+    node1 = Node(project, compute, "node1")
-    node2 = Node(project, compute)
+    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)
+    node1 = Node(project, compute, "node1")
-    node2 = Node(project, compute)
+    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@")
+    node1 = Node(project, compute, "Hello@")
-    node2 = Node(project, compute, name="w0.rld")
+    node2 = Node(project, compute, "w0.rld")
 
     link = Link(project)
     async_run(link.add_node(node1, 0, 4))

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,
+    node = Node(project, compute, "demo",
-                name="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,
+    node = Node(project, compute, "demo",
-                name="demo",
                 node_id=str(uuid.uuid4()),
                 node_type="qemu",
                 properties={"hda_disk_image": "linux.img"})

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

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"}))
+    vm1 = async_run(project.add_node(compute, "test1", None, 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"}))
+    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))

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")
+    node1 = Node(project, compute1, "node1", node_type="vpcs")
-    node2 = Node(project, compute2, 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")
+    node1 = Node(project, compute1, "node1", node_type="vpcs")
-    node2 = Node(project, compute2, 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_vpcs = Node(project, compute1, "node1", node_type="vpcs")
-    node_iou = Node(project, compute2, node_type="iou")
+    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_vpcs = Node(project, compute1, "node1", node_type="vpcs")
-    node_vpcs2 = Node(project, compute1, 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_iou = Node(project, compute1, "node1", node_type="iou")
-    node_iou2 = Node(project, compute2, 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_vpcs = Node(project, compute1, "V1", node_type="vpcs")
-    node_iou = Node(project, compute1, node_type="iou", name="I1")
+    node_iou = Node(project, compute1, "I1", node_type="iou")
 
     link = UDPLink(project)
     async_run(link.add_node(node_vpcs, 0, 4))

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))
+    node1 = async_run(project.add_node(compute, "node1", None))
-    node2 = async_run(project.add_node(compute, 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))
+    node1 = async_run(project.add_node(compute, "node1", None))
-    node2 = async_run(project.add_node(compute, 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), {

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