Replace by another TCP port if port is already used

Another version of #370

This time we replace a free TCP port if port is used and raise
a warning to the user.

gns3server/modules/base_vm.py

@@ -60,11 +60,14 @@ class BaseVM:
         self._vm_status = "stopped"
 
         if self._console is not None:
-            self._console = self._manager.port_manager.reserve_tcp_port(self._console, self._project)
+            if console_type == "vnc":
+                self._console = self._manager.port_manager.reserve_tcp_port(self._console, self._project, port_range_start=5900, port_range_end=6000)
+            else:
+                self._console = self._manager.port_manager.reserve_tcp_port(self._console, self._project)
         else:
             if console_type == "vnc":
                 # VNC is a special case and the range must be 5900-6000
-                self._console = self._manager.port_manager.get_free_tcp_port(self._project, 5900, 6000)
+                self._console = self._manager.port_manager.get_free_tcp_port(self._project, port_range_start=5900, port_range_end=6000)
             else:
                 self._console = self._manager.port_manager.get_free_tcp_port(self._project)
 

gns3server/modules/port_manager.py

@@ -142,22 +142,16 @@ class PortManager:
         if end_port < start_port:
             raise HTTPConflict(text="Invalid port range {}-{}".format(start_port, end_port))
 
-        if socket_type == "UDP":
-            socket_type = socket.SOCK_DGRAM
-        else:
-            socket_type = socket.SOCK_STREAM
 
         last_exception = None
         for port in range(start_port, end_port + 1):
             if port in ignore_ports:
                 continue
+
+            last_exception
             try:
-                for res in socket.getaddrinfo(host, port, socket.AF_UNSPEC, socket_type, 0, socket.AI_PASSIVE):
+                 PortManager._check_port(host, port, socket_type)
-                    af, socktype, proto, _, sa = res
+                 return port
-                    with socket.socket(af, socktype, proto) as s:
-                        s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
-                        s.bind(sa)  # the port is available if bind is a success
-                    return port
             except OSError as e:
                 last_exception = e
                 if port + 1 == end_port:
@@ -169,6 +163,25 @@ class PortManager:
                                                                                                                      end_port,
                                                                                                                      host,
                                                                                                                      last_exception))
+    @staticmethod
+    def _check_port(host, port, socket_type):
+        """
+        Check if an a port is available and raise an OSError if port is not available
+
+        :returns: boolean
+        """
+        if socket_type == "UDP":
+            socket_type = socket.SOCK_DGRAM
+        else:
+            socket_type = socket.SOCK_STREAM
+
+        for res in socket.getaddrinfo(host, port, socket.AF_UNSPEC, socket_type, 0, socket.AI_PASSIVE):
+            af, socktype, proto, _, sa = res
+            with socket.socket(af, socktype, proto) as s:
+                s.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
+                s.bind(sa)  # the port is available if bind is a success
+            return True
+
 
     def get_free_tcp_port(self, project, port_range_start=None, port_range_end=None):
         """
@@ -193,18 +206,47 @@ class PortManager:
         log.debug("TCP port {} has been allocated".format(port))
         return port
 
-    def reserve_tcp_port(self, port, project):
+    def reserve_tcp_port(self, port, project, port_range_start=None, port_range_end=None):
         """
-        Reserve a specific TCP port number
+        Reserve a specific TCP port number. If not available replace it
+        by another.
 
         :param port: TCP port number
         :param project: Project instance
+        :param port_range_start: Port range to use
+        :param port_range_end: Port range to use
+        :returns: The TCP port
         """
 
+        # use the default range is not specific one is given
+        if port_range_start is None and port_range_end is None:
+            port_range_start = self._console_port_range[0]
+            port_range_end = self._console_port_range[1]
+
         if port in self._used_tcp_ports:
-            raise HTTPConflict(text="TCP port {} already in use on host".format(port, self._console_host))
+            old_port = port
+            port = self.get_free_tcp_port(project, port_range_start=port_range_start, port_range_end=port_range_end)
+            msg = "TCP port {} already in use on host {}. Port has been replaced by {}".format(old_port, self._console_host, port)
+            log.warning(msg)
+            project.emit("log.warning", {"message": msg})
+            return port
         if port < self._console_port_range[0] or port > self._console_port_range[1]:
-            raise HTTPConflict(text="TCP port {} is outside the range {}-{}".format(port, self._console_port_range[0], self._console_port_range[1]))
+            old_port = port
+            port = self.get_free_tcp_port(project, port_range_start=port_range_start, port_range_end=port_range_end)
+            msg = "TCP port {} is outside the range {}-{} on host {}. Port has been replaced by {}".format(old_port, port_range_start, port_range_end, self._console_host, port)
+            log.warning(msg)
+            project.emit("log.warning", {"message": msg})
+            return port
+        try:
+            PortManager._check_port(self._console_host, port, "TCP")
+        except OSError:
+            old_port = port
+            port = self.get_free_tcp_port(project, port_range_start=port_range_start, port_range_end=port_range_end)
+            msg = "TCP port {} already in use on host {}. Port has been replaced by {}".format(old_port, self._console_host, port)
+            log.warning(msg)
+            project.emit("log.warning", {"message": msg})
+            return port
+
         self._used_tcp_ports.add(port)
         project.record_tcp_port(port)
         log.debug("TCP port {} has been reserved".format(port))

tests/modules/test_port_manager.py

@@ -27,15 +27,66 @@ def test_reserve_tcp_port():
     pm = PortManager()
     project = Project()
     pm.reserve_tcp_port(2001, project)
-    with pytest.raises(aiohttp.web.HTTPConflict):
+    with patch("gns3server.modules.project.Project.emit") as mock_emit:
-        pm.reserve_tcp_port(2001, project)
+        port = pm.reserve_tcp_port(2001, project)
+        assert port != 2001
+        assert mock_emit.call_args[0][0] == "log.warning"
 
 
 def test_reserve_tcp_port_outside_range():
     pm = PortManager()
     project = Project()
-    with pytest.raises(aiohttp.web.HTTPConflict):
+    with patch("gns3server.modules.project.Project.emit") as mock_emit:
-        pm.reserve_tcp_port(80, project)
+        port = pm.reserve_tcp_port(80, project)
+        assert port != 80
+        assert mock_emit.call_args[0][0] == "log.warning"
+
+
+def test_reserve_tcp_port_already_used():
+    """
+    This test simulate a scenario where the port is already taken
+    by another programm on the server
+    """
+
+    pm = PortManager()
+    project = Project()
+    with patch("gns3server.modules.port_manager.PortManager._check_port") as mock_check:
+
+        def execute_mock(host, port, *args):
+            if port == 2001:
+                raise OSError("Port is already used")
+            else:
+                return True
+
+        mock_check.side_effect = execute_mock
+
+        with patch("gns3server.modules.project.Project.emit") as mock_emit:
+            port = pm.reserve_tcp_port(2001, project)
+            assert port != 2001
+            assert mock_emit.call_args[0][0] == "log.warning"
+
+def test_reserve_tcp_port_already_used():
+    """
+    This test simulate a scenario where the port is already taken
+    by another programm on the server
+    """
+
+    pm = PortManager()
+    project = Project()
+    with patch("gns3server.modules.port_manager.PortManager._check_port") as mock_check:
+
+        def execute_mock(host, port, *args):
+            if port == 2001:
+                raise OSError("Port is already used")
+            else:
+                return True
+
+        mock_check.side_effect = execute_mock
+
+        with patch("gns3server.modules.project.Project.emit") as mock_emit:
+            port = pm.reserve_tcp_port(2001, project)
+            assert port != 2001
+            assert mock_emit.call_args[0][0] == "log.warning"
 
 
 def test_reserve_udp_port():