Refactor aux port allocation

This move the allocation of aux port to the base vm.
Also now the free of console port during the close is in the
base VM.

An aux port is allocated to the docker container but not used
for the moment.

Ref https://github.com/GNS3/gns3-gui/issues/1039

gns3server/modules/base_vm.py

@@ -43,9 +43,11 @@ class BaseVM:
     :param project: Project instance
     :param manager: parent VM Manager
     :param console: TCP console port
+    :param aux: TCP aux console port
+    :param allocate_aux: Boolean if true will allocate an aux console port
     """
 
-    def __init__(self, name, vm_id, project, manager, console=None, console_type="telnet"):
+    def __init__(self, name, vm_id, project, manager, console=None, console_type="telnet", aux=None, allocate_aux=False):
 
         self._name = name
         self._usage = ""
@@ -53,10 +55,12 @@ class BaseVM:
         self._project = project
         self._manager = manager
         self._console = console
+        self._aux = aux
         self._console_type = console_type
         self._temporary_directory = None
         self._hw_virtualization = False
         self._ubridge_hypervisor = None
+        self._closed = False
         self._vm_status = "stopped"
         self._command_line = ""
 
@@ -65,13 +69,21 @@ class BaseVM:
                 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:
+
+        # We need to allocate aux before giving a random console port
+        if self._aux is not None:
+            self._aux = self._manager.port_manager.reserve_tcp_port(self._aux, self._project)
+
+        if self._console is None:
             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, port_range_start=5900, port_range_end=6000)
             else:
                 self._console = self._manager.port_manager.get_free_tcp_port(self._project)
 
+        if self._aux is None and allocate_aux:
+            self._aux = self._manager.port_manager.get_free_tcp_port(self._project)
+
         log.debug("{module}: {name} [{id}] initialized. Console port {console}".format(module=self.manager.module_name,
                                                                                        name=self.name,
                                                                                        id=self.id,
@@ -233,12 +245,61 @@ class BaseVM:
 
         raise NotImplementedError
 
+    @asyncio.coroutine
     def close(self):
         """
         Close the VM process.
         """
 
-        raise NotImplementedError
+        if self._closed:
+            return False
+
+        log.info("{module}: '{name}' [{id}]: is closing".format(
+            module=self.manager.module_name,
+            name=self.name,
+            id=self.id))
+
+        if self._console:
+            self._manager.port_manager.release_tcp_port(self._console, self._project)
+            self._console = None
+
+        if self._aux:
+            self._manager.port_manager.release_tcp_port(self._aux, self._project)
+            self._aux = None
+
+        self._closed = True
+        return True
+
+    @property
+    def aux(self):
+        """
+        Returns the aux console port of this VM.
+
+        :returns: aux console port
+        """
+
+        return self._aux
+
+    @aux.setter
+    def aux(self, aux):
+        """
+        Changes the aux port
+
+        :params aux: Console port (integer) or None to free the port
+        """
+
+        if aux == self._aux:
+            return
+
+        if self._aux:
+            self._manager.port_manager.release_tcp_port(self._aux, self._project)
+            self._aux = None
+        if aux is not None:
+            self._aux = self._manager.port_manager.reserve_tcp_port(aux, self._project)
+            log.info("{module}: '{name}' [{id}]: aux port set to {port}".format(module=self.manager.module_name,
+                                                                                name=self.name,
+                                                                                id=self.id,
+                                                                                port=aux))
 
     @property
     def console(self):
@@ -255,22 +316,24 @@ class BaseVM:
         """
         Changes the console port
 
-        :params console: Console port (integer)
+        :params console: Console port (integer) or None to free the port
         """
 
         if console == self._console:
             return
 
-        if self._console_type == "vnc" and console < 5900:
+        if self._console_type == "vnc" and console is not None and console < 5900:
             raise VMError("VNC console require a port superior or equal to 5900")
 
         if self._console:
             self._manager.port_manager.release_tcp_port(self._console, self._project)
-        self._console = self._manager.port_manager.reserve_tcp_port(console, self._project)
-        log.info("{module}: '{name}' [{id}]: console port set to {port}".format(module=self.manager.module_name,
-                                                                                name=self.name,
-                                                                                id=self.id,
-                                                                                port=console))
+            self._console = None
+        if console is not None:
+            self._console = self._manager.port_manager.reserve_tcp_port(console, self._project)
+            log.info("{module}: '{name}' [{id}]: console port set to {port}".format(module=self.manager.module_name,
+                                                                                    name=self.name,
+                                                                                    id=self.id,
+                                                                                    port=console))
 
     @property
     def console_type(self):

gns3server/modules/docker/docker_vm.py

@@ -49,10 +49,12 @@ class DockerVM(BaseVM):
     :param project: Project instance
     :param manager: Manager instance
     :param image: Docker image
+    :param console: TCP console port
+    :param aux: TCP aux console port
     """
 
-    def __init__(self, name, vm_id, project, manager, image, console=None, start_command=None, adapters=None, environment=None):
-        super().__init__(name, vm_id, project, manager, console=console)
+    def __init__(self, name, vm_id, project, manager, image, console=None, aux=None, start_command=None, adapters=None, environment=None):
+        super().__init__(name, vm_id, project, manager, console=console, aux=aux, allocate_aux=True)
 
         self._image = image
         self._start_command = start_command
@@ -62,7 +64,6 @@ class DockerVM(BaseVM):
         self._ubridge_hypervisor = None
         self._temporary_directory = None
         self._telnet_server = None
-        self._closed = False
 
         if adapters is None:
             self.adapters = 1
@@ -84,6 +85,7 @@ class DockerVM(BaseVM):
             "image": self._image,
             "adapters": self.adapters,
             "console": self.console,
+            "aux": self.aux,
             "start_command": self.start_command,
             "environment": self.environment,
             "vm_directory": self.working_dir
@@ -377,11 +379,9 @@ class DockerVM(BaseVM):
     def close(self):
         """Closes this Docker container."""
 
-        if self._closed:
-            return
+        if not (yield from super().close()):
+            return False
 
-        log.debug("Docker container '{name}' [{id}] is closing".format(
-            name=self.name, id=self._cid))
         for adapter in self._ethernet_adapters:
             if adapter is not None:
                 for nio in adapter.ports.values():
@@ -391,10 +391,6 @@ class DockerVM(BaseVM):
 
         yield from self.remove()
 
-        log.info("Docker container '{name}' [{id}] closed".format(
-            name=self.name, id=self._cid))
-        self._closed = True
-
     @asyncio.coroutine
     def _add_ubridge_connection(self, nio, adapter_number, namespace):
         """

gns3server/modules/dynamips/nodes/router.py

@@ -61,12 +61,12 @@ class Router(BaseVM):
 
     def __init__(self, name, vm_id, project, manager, dynamips_id=None, console=None, aux=None, platform="c7200", hypervisor=None, ghost_flag=False):
 
-        super().__init__(name, vm_id, project, manager, console=console)
+        allocate_aux = manager.config.get_section_config("Dynamips").getboolean("allocate_aux_console_ports", False)
+
+        super().__init__(name, vm_id, project, manager, console=console, aux=aux, allocate_aux=aux)
 
         self._hypervisor = hypervisor
         self._dynamips_id = dynamips_id
-        self._closed = False
-        self._name = name
         self._platform = platform
         self._image = ""
         self._startup_config = ""
@@ -88,7 +88,6 @@ class Router(BaseVM):
         self._disk0 = 0  # Megabytes
         self._disk1 = 0  # Megabytes
         self._auto_delete_disks = False
-        self._aux = aux
         self._mac_addr = ""
         self._system_id = "FTX0945W0MY"  # processor board ID in IOS
         self._slots = []
@@ -100,19 +99,12 @@ class Router(BaseVM):
             else:
                 self._dynamips_id = dynamips_id
                 manager.take_dynamips_id(project.id, dynamips_id)
-
-            if self._aux is not None:
-                self._aux = self._manager.port_manager.reserve_tcp_port(self._aux, self._project)
-            else:
-                allocate_aux = self.manager.config.get_section_config("Dynamips").getboolean("allocate_aux_console_ports", False)
-                if allocate_aux:
-                    self._aux = self._manager.port_manager.get_free_tcp_port(self._project)
         else:
             log.info("Creating a new ghost IOS instance")
             if self._console:
                 # Ghost VMs do not need a console port.
-                self._manager.port_manager.release_tcp_port(self._console, self._project)
-                self._console = None
+                self.console = None
+
             self._dynamips_id = 0
             self._name = "Ghost"
 
@@ -140,8 +132,8 @@ class Router(BaseVM):
                        "disk0": self._disk0,
                        "disk1": self._disk1,
                        "auto_delete_disks": self._auto_delete_disks,
-                       "console": self._console,
-                       "aux": self._aux,
+                       "console": self.console,
+                       "aux": self.aux,
                        "mac_addr": self._mac_addr,
                        "system_id": self._system_id}
 
@@ -195,8 +187,8 @@ class Router(BaseVM):
 
             yield from self._hypervisor.send('vm set_con_tcp_port "{name}" {console}'.format(name=self._name, console=self._console))
 
-            if self._aux is not None:
-                yield from self._hypervisor.send('vm set_aux_tcp_port "{name}" {aux}'.format(name=self._name, aux=self._aux))
+            if self.aux is not None:
+                yield from self._hypervisor.send('vm set_aux_tcp_port "{name}" {aux}'.format(name=self._name, aux=self.aux))
 
             # get the default base MAC address
             mac_addr = yield from self._hypervisor.send('{platform} get_mac_addr "{name}"'.format(platform=self._platform,
@@ -328,19 +320,8 @@ class Router(BaseVM):
     @asyncio.coroutine
     def close(self):
 
-        if self._closed:
-            # router is already closed
-            return
-
-        log.debug('Router "{name}" [{id}] is closing'.format(name=self._name, id=self._id))
-
-        if self._console:
-            self._manager.port_manager.release_tcp_port(self._console, self._project)
-            self._console = None
-
-        if self._aux:
-            self._manager.port_manager.release_tcp_port(self._aux, self._project)
-            self._aux = None
+        if not (yield from super().close()):
+            return False
 
         for adapter in self._slots:
             if adapter is not None:
@@ -375,7 +356,6 @@ class Router(BaseVM):
                 except OSError as e:
                     log.warn("Could not delete file {}: {}".format(file, e))
                     continue
-        self._closed = True
 
     @property
     def platform(self):
@@ -913,26 +893,9 @@ class Router(BaseVM):
         :param console: console port (integer)
         """
 
+        self.console = console
         yield from self._hypervisor.send('vm set_con_tcp_port "{name}" {console}'.format(name=self._name, console=console))
 
-        log.info('Router "{name}" [{id}]: console port updated from {old_console} to {new_console}'.format(name=self._name,
-                                                                                                           id=self._id,
-                                                                                                           old_console=self._console,
-                                                                                                           new_console=console))
-
-        self._manager.port_manager.release_tcp_port(self._console, self._project)
-        self._console = self._manager.port_manager.reserve_tcp_port(console, self._project)
-
-    @property
-    def aux(self):
-        """
-        Returns the TCP auxiliary port.
-
-        :returns: console auxiliary port (integer)
-        """
-
-        return self._aux
-
     @asyncio.coroutine
     def set_aux(self, aux):
         """
@@ -941,16 +904,9 @@ class Router(BaseVM):
         :param aux: console auxiliary port (integer)
         """
 
+        self.aux = aux
         yield from self._hypervisor.send('vm set_aux_tcp_port "{name}" {aux}'.format(name=self._name, aux=aux))
 
-        log.info('Router "{name}" [{id}]: aux port updated from {old_aux} to {new_aux}'.format(name=self._name,
-                                                                                               id=self._id,
-                                                                                               old_aux=self._aux,
-                                                                                               new_aux=aux))
-
-        self._manager.port_manager.release_tcp_port(self._aux, self._project)
-        self._aux = self._manager.port_manager.reserve_tcp_port(aux, self._project)
-
     @asyncio.coroutine
     def get_cpu_usage(self, cpu_id=0):
         """

gns3server/modules/iou/iou_vm.py

@@ -97,11 +97,8 @@ class IOUVM(BaseVM):
         Closes this IOU VM.
         """
 
-        log.debug('IOU "{name}" [{id}] is closing'.format(name=self._name, id=self._id))
-
-        if self._console:
-            self._manager.port_manager.release_tcp_port(self._console, self._project)
-            self._console = None
+        if not (yield from super().close()):
+            return False
 
         adapters = self._ethernet_adapters + self._serial_adapters
         for adapter in adapters:

gns3server/modules/qemu/qemu_vm.py

@@ -988,22 +988,18 @@ class QemuVM(BaseVM):
         Closes this QEMU VM.
         """
 
-        log.debug('QEMU VM "{name}" [{id}] is closing'.format(name=self._name, id=self._id))
+        if not (yield from super().close()):
+            return False
+
         self.acpi_shutdown = False
         yield from self.stop()
 
-        if self._console:
-            self._manager.port_manager.release_tcp_port(self._console, self._project)
-            self._console = None
-
         for adapter in self._ethernet_adapters:
             if adapter is not None:
                 for nio in adapter.ports.values():
                     if nio and isinstance(nio, NIOUDP):
                         self.manager.port_manager.release_udp_port(nio.lport, self._project)
 
-        yield from self.stop()
-
     @asyncio.coroutine
     def _get_vm_status(self):
         """

gns3server/modules/virtualbox/virtualbox_vm.py

@@ -60,7 +60,6 @@ class VirtualBoxVM(BaseVM):
         self._system_properties = {}
         self._telnet_server_thread = None
         self._serial_pipe = None
-        self._closed = False
 
         # VirtualBox settings
         self._adapters = adapters
@@ -344,14 +343,8 @@ class VirtualBoxVM(BaseVM):
         Closes this VirtualBox VM.
         """
 
-        if self._closed:
-            # VM is already closed
-            return
-
-        log.debug("VirtualBox VM '{name}' [{id}] is closing".format(name=self.name, id=self.id))
-        if self._console:
-            self._manager.port_manager.release_tcp_port(self._console, self._project)
-            self._console = None
+        if not (yield from super().close()):
+            return False
 
         for adapter in self._ethernet_adapters.values():
             if adapter is not None:
@@ -404,9 +397,6 @@ class VirtualBoxVM(BaseVM):
                                                                                                               id=self.id,
                                                                                                               error=e.strerror))
 
-        log.info("VirtualBox VM '{name}' [{id}] closed".format(name=self.name, id=self.id))
-        self._closed = True
-
     @property
     def headless(self):
         """

gns3server/modules/vmware/vmware_vm.py

@@ -542,14 +542,8 @@ class VMwareVM(BaseVM):
         Closes this VMware VM.
         """
 
-        if self._closed:
-            # VM is already closed
-            return
-
-        log.debug("VMware VM '{name}' [{id}] is closing".format(name=self.name, id=self.id))
-        if self._console:
-            self._manager.port_manager.release_tcp_port(self._console, self._project)
-            self._console = None
+        if not (yield from super().close()):
+            return False
 
         for adapter in self._ethernet_adapters.values():
             if adapter is not None:
@@ -567,9 +561,6 @@ class VMwareVM(BaseVM):
         if self._linked_clone:
             yield from self.manager.remove_from_vmware_inventory(self._vmx_path)
 
-        log.info("VirtualBox VM '{name}' [{id}] closed".format(name=self.name, id=self.id))
-        self._closed = True
-
     @property
     def headless(self):
         """

gns3server/modules/vpcs/vpcs_vm.py

@@ -75,10 +75,8 @@ class VPCSVM(BaseVM):
         Closes this VPCS VM.
         """
 
-        log.debug('VPCS "{name}" [{id}] is closing'.format(name=self._name, id=self._id))
-        if self._console:
-            self._manager.port_manager.release_tcp_port(self._console, self._project)
-            self._console = None
+        if not (yield from super().close()):
+            return False
 
         nio = self._ethernet_adapter.get_nio(0)
         if isinstance(nio, NIOUDP):
@@ -87,6 +85,8 @@ class VPCSVM(BaseVM):
         if self.is_running():
             self._terminate_process()
 
+        return True
+
     @asyncio.coroutine
     def _check_requirements(self):
         """

gns3server/schemas/docker.py

@@ -39,6 +39,12 @@ DOCKER_CREATE_SCHEMA = {
             "maximum": 65535,
             "type": ["integer", "null"]
         },
+        "aux": {
+            "description": "auxilary TCP port",
+            "minimum": 1,
+            "maximum": 65535,
+            "type": ["integer", "null"]
+        },
         "start_command": {
             "description": "Docker CMD entry",
             "type": ["string", "null"],
@@ -82,6 +88,12 @@ DOCKER_UPDATE_SCHEMA = {
             "maximum": 65535,
             "type": ["integer", "null"]
         },
+        "aux": {
+            "description": "auxilary TCP port",
+            "minimum": 1,
+            "maximum": 65535,
+            "type": ["integer", "null"]
+        },
         "start_command": {
             "description": "Docker CMD entry",
             "type": ["string", "null"],
@@ -119,6 +131,12 @@ DOCKER_OBJECT_SCHEMA = {
             "maxLength": 36,
             "pattern": "^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}$"
         },
+        "aux": {
+            "description": "auxilary TCP port",
+            "minimum": 1,
+            "maximum": 65535,
+            "type": ["integer", "null"]
+        },
         "console": {
             "description": "console TCP port",
             "minimum": 1,
@@ -166,7 +184,7 @@ DOCKER_OBJECT_SCHEMA = {
         }
     },
     "additionalProperties": False,
-    "required": ["vm_id", "project_id", "image", "container_id", "adapters", "console", "start_command", "environment", "vm_directory"]
+    "required": ["vm_id", "project_id", "image", "container_id", "adapters", "aux", "console", "start_command", "environment", "vm_directory"]
 }
 
 

tests/modules/docker/test_docker_vm.py

@@ -54,6 +54,7 @@ def test_json(vm, project):
         'vm_id': vm.id,
         'adapters': 1,
         'console': vm.console,
+        'aux': vm.aux,
         'start_command': vm.start_command,
         'environment': vm.environment,
         'vm_directory': vm.working_dir

tests/modules/test_base_vm.py

@@ -24,6 +24,7 @@ from tests.utils import asyncio_patch
 
 from unittest.mock import patch, MagicMock
 from gns3server.modules.vpcs.vpcs_vm import VPCSVM
+from gns3server.modules.docker.docker_vm import DockerVM
 from gns3server.modules.vpcs.vpcs_error import VPCSError
 from gns3server.modules.vm_error import VMError
 from gns3server.modules.vpcs import VPCS
@@ -50,6 +51,19 @@ def test_console(project, manager):
     vm = VPCSVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", project, manager)
     vm.console = 2111
     assert vm.console == 2111
+    vm.console = None
+    assert vm.console is None
+
+
+def test_change_console_port(vm, port_manager):
+    port1 = port_manager.get_free_tcp_port(vm.project)
+    port2 = port_manager.get_free_tcp_port(vm.project)
+    port_manager.release_tcp_port(port1, vm.project)
+    port_manager.release_tcp_port(port2, vm.project)
+    vm.console = port1
+    vm.console = port2
+    assert vm.console == port2
+    port_manager.reserve_tcp_port(port1, vm.project)
 
 
 def test_console_vnc_invalid(project, manager):
@@ -57,3 +71,53 @@ def test_console_vnc_invalid(project, manager):
     vm.console_type = "vnc"
     with pytest.raises(VMError):
         vm.console = 2012
+
+
+def test_close(vm, loop, port_manager):
+    assert vm.console is not None
+
+    aux = port_manager.get_free_tcp_port(vm.project)
+    port_manager.release_tcp_port(aux, vm.project)
+
+    vm.aux = aux
+    port = vm.console
+    assert loop.run_until_complete(asyncio.async(vm.close()))
+    # Raise an exception if the port is not free
+    port_manager.reserve_tcp_port(port, vm.project)
+    # Raise an exception if the port is not free
+    port_manager.reserve_tcp_port(aux, vm.project)
+    assert vm.console is None
+    assert vm.aux is None
+
+    # Called twice closed should return False
+    assert loop.run_until_complete(asyncio.async(vm.close())) is False
+
+
+def test_aux(project, manager, port_manager):
+    aux = port_manager.get_free_tcp_port(project)
+    port_manager.release_tcp_port(aux, project)
+
+    vm = DockerVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", project, manager, "ubuntu", aux=aux)
+    assert vm.aux == aux
+    vm.aux = None
+    assert vm.aux is None
+
+
+def test_allocate_aux(project, manager):
+    vm = VPCSVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", project, manager)
+    assert vm.aux is None
+
+    # Docker has an aux port by default
+    vm = DockerVM("test", "00010203-0405-0607-0809-0a0b0c0d0e0f", project, manager, "ubuntu")
+    assert vm.aux is not None
+
+
+def test_change_aux_port(vm, port_manager):
+    port1 = port_manager.get_free_tcp_port(vm.project)
+    port2 = port_manager.get_free_tcp_port(vm.project)
+    port_manager.release_tcp_port(port1, vm.project)
+    port_manager.release_tcp_port(port2, vm.project)
+    vm.aux = port1
+    vm.aux = port2
+    assert vm.aux == port2
+    port_manager.reserve_tcp_port(port1, vm.project)

tests/modules/vpcs/test_vpcs_vm.py

@@ -273,17 +273,6 @@ def test_get_startup_script_using_default_script(vm):
     assert vm.script_file == filepath
 
 
-def test_change_console_port(vm, port_manager):
-    port1 = port_manager.get_free_tcp_port(vm.project)
-    port2 = port_manager.get_free_tcp_port(vm.project)
-    port_manager.release_tcp_port(port1, vm.project)
-    port_manager.release_tcp_port(port2, vm.project)
-    vm.console = port1
-    vm.console = port2
-    assert vm.console == port2
-    port_manager.reserve_tcp_port(port1, vm.project)
-
-
 def test_change_name(vm, tmpdir):
     path = os.path.join(vm.working_dir, 'startup.vpc')
     vm.name = "world"
@@ -299,8 +288,5 @@ def test_close(vm, port_manager, loop):
     with asyncio_patch("gns3server.modules.vpcs.vpcs_vm.VPCSVM._check_requirements", return_value=True):
         with asyncio_patch("asyncio.create_subprocess_exec", return_value=MagicMock()):
             vm.start()
-            port = vm.console
             loop.run_until_complete(asyncio.async(vm.close()))
-            # Raise an exception if the port is not free
-            port_manager.reserve_tcp_port(port, vm.project)
             assert vm.is_running() is False