From 9d28f4c0c3793a8e9c5fba1637a846184ddea87a Mon Sep 17 00:00:00 2001 From: Julien Duponchelle Date: Mon, 29 Feb 2016 10:38:30 +0100 Subject: [PATCH] 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 | 83 ++++++++++++++++--- gns3server/modules/docker/docker_vm.py | 18 ++-- gns3server/modules/dynamips/nodes/router.py | 70 +++------------- gns3server/modules/iou/iou_vm.py | 7 +- gns3server/modules/qemu/qemu_vm.py | 10 +-- .../modules/virtualbox/virtualbox_vm.py | 14 +--- gns3server/modules/vmware/vmware_vm.py | 13 +-- gns3server/modules/vpcs/vpcs_vm.py | 8 +- gns3server/schemas/docker.py | 20 ++++- tests/modules/docker/test_docker_vm.py | 1 + tests/modules/test_base_vm.py | 64 ++++++++++++++ tests/modules/vpcs/test_vpcs_vm.py | 14 ---- 12 files changed, 190 insertions(+), 132 deletions(-) diff --git a/gns3server/modules/base_vm.py b/gns3server/modules/base_vm.py index 5b5d8561..1540489b 100644 --- a/gns3server/modules/base_vm.py +++ b/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): diff --git a/gns3server/modules/docker/docker_vm.py b/gns3server/modules/docker/docker_vm.py index 6ad6df6b..ff2861d8 100644 --- a/gns3server/modules/docker/docker_vm.py +++ b/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): """ diff --git a/gns3server/modules/dynamips/nodes/router.py b/gns3server/modules/dynamips/nodes/router.py index 2821169c..0b301d0a 100644 --- a/gns3server/modules/dynamips/nodes/router.py +++ b/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): """ diff --git a/gns3server/modules/iou/iou_vm.py b/gns3server/modules/iou/iou_vm.py index 9ebec9d2..aaae74fd 100644 --- a/gns3server/modules/iou/iou_vm.py +++ b/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: diff --git a/gns3server/modules/qemu/qemu_vm.py b/gns3server/modules/qemu/qemu_vm.py index 996451a5..72f0cc7b 100644 --- a/gns3server/modules/qemu/qemu_vm.py +++ b/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): """ diff --git a/gns3server/modules/virtualbox/virtualbox_vm.py b/gns3server/modules/virtualbox/virtualbox_vm.py index a3748414..c4705db9 100644 --- a/gns3server/modules/virtualbox/virtualbox_vm.py +++ b/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): """ diff --git a/gns3server/modules/vmware/vmware_vm.py b/gns3server/modules/vmware/vmware_vm.py index c7a7d44e..6ec909d2 100644 --- a/gns3server/modules/vmware/vmware_vm.py +++ b/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): """ diff --git a/gns3server/modules/vpcs/vpcs_vm.py b/gns3server/modules/vpcs/vpcs_vm.py index d4fa36c4..3e5bd3c0 100644 --- a/gns3server/modules/vpcs/vpcs_vm.py +++ b/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): """ diff --git a/gns3server/schemas/docker.py b/gns3server/schemas/docker.py index c3ad5554..d90b3a8e 100644 --- a/gns3server/schemas/docker.py +++ b/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"] } diff --git a/tests/modules/docker/test_docker_vm.py b/tests/modules/docker/test_docker_vm.py index 47b08d95..fdb4936c 100644 --- a/tests/modules/docker/test_docker_vm.py +++ b/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 diff --git a/tests/modules/test_base_vm.py b/tests/modules/test_base_vm.py index 65c138e5..c38f1bdc 100644 --- a/tests/modules/test_base_vm.py +++ b/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) diff --git a/tests/modules/vpcs/test_vpcs_vm.py b/tests/modules/vpcs/test_vpcs_vm.py index 63305897..41558516 100644 --- a/tests/modules/vpcs/test_vpcs_vm.py +++ b/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