From 6cea6c9162c5e33cb98d0bca7e693ba0d69ae51f Mon Sep 17 00:00:00 2001 From: grossmj Date: Fri, 13 May 2016 18:48:10 -0600 Subject: [PATCH] Some more (spring) cleaning. --- gns3server/compute/notification_manager.py | 7 +-- gns3server/compute/port_manager.py | 9 ++- gns3server/compute/project.py | 14 ++--- gns3server/compute/qemu/qcow2.py | 2 +- gns3server/controller/__init__.py | 71 +++++++++++----------- gns3server/controller/compute.py | 2 +- gns3server/controller/link.py | 21 +++---- gns3server/controller/project.py | 6 +- gns3server/controller/udp_link.py | 3 +- tests/controller/test_controller.py | 4 +- 10 files changed, 67 insertions(+), 72 deletions(-) diff --git a/gns3server/compute/notification_manager.py b/gns3server/compute/notification_manager.py index 35336623..381e2a87 100644 --- a/gns3server/compute/notification_manager.py +++ b/gns3server/compute/notification_manager.py @@ -15,16 +15,15 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . + from contextlib import contextmanager - - from ..notification_queue import NotificationQueue class NotificationManager: """ Manage the notification queue where the controller - will connect to get notifications from computes + will connect to get notifications from compute servers """ def __init__(self): @@ -48,7 +47,7 @@ class NotificationManager: :param action: Action name :param event: Event to send - :param kwargs: Add this meta to the notif (project_id for example) + :param kwargs: Add this meta to the notification (project_id for example) """ for listener in self._listeners: listener.put_nowait((action, event, kwargs)) diff --git a/gns3server/compute/port_manager.py b/gns3server/compute/port_manager.py index f3846bd8..5283c0f4 100644 --- a/gns3server/compute/port_manager.py +++ b/gns3server/compute/port_manager.py @@ -24,8 +24,11 @@ import logging log = logging.getLogger(__name__) -# This ports are disallowed by Chrome and Firefox to avoid trouble with skip them -BANNED_PORTS = set((1, 7, 9, 11, 13, 15, 17, 19, 20, 21, 22, 23, 25, 37, 42, 43, 53, 77, 79, 87, 95, 101, 102, 103, 104, 109, 110, 111, 113, 115, 117, 119, 123, 135, 139, 143, 179, 389, 465, 512, 513, 514, 515, 526, 530, 531, 532, 540, 556, 563, 587, 601, 636, 993, 995, 2049, 3659, 4045, 6000, 6665, 6666, 6667, 6668, 6669)) +# This ports are disallowed by Chrome and Firefox to avoid issues, we skip them as well +BANNED_PORTS = set((1, 7, 9, 11, 13, 15, 17, 19, 20, 21, 22, 23, 25, 37, 42, 43, 53, 77, 79, 87, 95, 101, 102, 103, + 104, 109, 110, 111, 113, 115, 117, 119, 123, 135, 139, 143, 179, 389, 465, 512, 513, 514, 515, 526, + 530, 531, 532, 540, 556, 563, 587, 601, 636, 993, 995, 2049, 3659, 4045, 6000, 6665, 6666, 6667, + 6668, 6669)) class PortManager: @@ -106,7 +109,7 @@ class PortManager: return self._udp_host @udp_host.setter - def host(self, new_host): + def udp_host(self, new_host): self._udp_host = new_host diff --git a/gns3server/compute/project.py b/gns3server/compute/project.py index 1fe495f4..1c1d2d61 100644 --- a/gns3server/compute/project.py +++ b/gns3server/compute/project.py @@ -201,8 +201,7 @@ class Project: def _update_temporary_file(self): """ - Update the .gns3_temporary file in order to reflect current - project status. + Update the .gns3_temporary file in order to reflect the current project status. """ if not hasattr(self, "_path"): @@ -224,7 +223,7 @@ class Project: def module_working_directory(self, module_name): """ Returns a working directory for the module - If the directory doesn't exist, the directory is created. + The directory is created if the directory doesn't exist. :param module_name: name for the module :returns: working directory @@ -335,7 +334,7 @@ class Project: """ Closes the project, and cleanup the disk if cleanup is True - :param cleanup: If True drop the project directory + :param cleanup: Whether to delete the project directory """ tasks = [] @@ -431,7 +430,7 @@ class Project: @asyncio.coroutine def list_files(self): """ - :returns: Array of files in project without temporary files. The files are dictionnary {"path": "test.bin", "md5sum": "aaaaa"} + :returns: Array of files in project without temporary files. The files are dictionary {"path": "test.bin", "md5sum": "aaaaa"} """ files = [] @@ -479,8 +478,7 @@ class Project: """ z = zipstream.ZipFile() - # topdown allo to modify the list of directory in order to ignore - # directory + # topdown allows to modify the list of directory in order to ignore the directory for root, dirs, files in os.walk(self._path, topdown=True): # Remove snapshots and capture if os.path.split(root)[-1:][0] == "project-files": @@ -638,4 +636,4 @@ class Project: shutil.move(path, dst) # Cleanup the project - shutil.rmtree(root) + shutil.rmtree(root, ignore_errors=True) diff --git a/gns3server/compute/qemu/qcow2.py b/gns3server/compute/qemu/qcow2.py index d9bc438a..563d5120 100644 --- a/gns3server/compute/qemu/qcow2.py +++ b/gns3server/compute/qemu/qcow2.py @@ -26,7 +26,7 @@ class Qcow2Error(Exception): class Qcow2: """ - Allow to parse a Qcow2 file + Allows to parse a Qcow2 file """ def __init__(self, path): diff --git a/gns3server/controller/__init__.py b/gns3server/controller/__init__.py index 9dafcc0f..e50887dd 100644 --- a/gns3server/controller/__init__.py +++ b/gns3server/controller/__init__.py @@ -31,7 +31,7 @@ log = logging.getLogger(__name__) class Controller: - """The controller manage multiple gns3 compute servers""" + """The controller is responsible to manage one or more compute servers""" def __init__(self): self._computes = {} @@ -47,17 +47,14 @@ class Controller: """ Save the controller configuration on disk """ - data = { - "computes": [{ - "host": c.host, - "port": c.port, - "protocol": c.protocol, - "user": c.user, - "password": c.password, - "compute_id": c.id - } for c in self._computes.values()], - "version": __version__ - } + data = {"computes": [{"host": c.host, + "port": c.port, + "protocol": c.protocol, + "user": c.user, + "password": c.password, + "compute_id": c.id + } for c in self._computes.values()], + "version": __version__} os.makedirs(os.path.dirname(self._config_file), exist_ok=True) with open(self._config_file, 'w+') as f: json.dump(data, f, indent=4) @@ -73,15 +70,15 @@ class Controller: with open(self._config_file) as f: data = json.load(f) except OSError as e: - log.critical("Can not load %s: %s", self._config_file, str(e)) + log.critical("Cannot load %s: %s", self._config_file, str(e)) return for c in data["computes"]: compute_id = c.pop("compute_id") yield from self.add_compute(compute_id, **c) - def isEnabled(self): + def is_enabled(self): """ - :returns: True if current instance is the controller + :returns: whether the current instance is the controller of our GNS3 infrastructure. """ return Config.instance().get_section_config("Server").getboolean("controller") @@ -89,9 +86,9 @@ class Controller: @asyncio.coroutine def add_compute(self, compute_id, **kwargs): """ - Add a server to the dictionnary of computes controlled by GNS3 + Add a server to the dictionary of compute servers controlled by this controller - :param compute_id: Id of the compute node + :param compute_id: Compute server identifier :param kwargs: See the documentation of Compute """ @@ -100,36 +97,35 @@ class Controller: return self._create_local_compute() if compute_id not in self._computes: - compute = Compute(compute_id=compute_id, controller=self, **kwargs) - self._computes[compute_id] = compute + compute_server = Compute(compute_id=compute_id, controller=self, **kwargs) + self._computes[compute_id] = compute_server self.save() return self._computes[compute_id] def _create_local_compute(self): """ - Create the local compute node. It's the controller itself + Create the local compute node. It is the controller itself. """ server_config = Config.instance().get_section_config("Server") - self._computes["local"] = Compute( - compute_id="local", - controller=self, - protocol=server_config.get("protocol", "http"), - host=server_config.get("host", "localhost"), - port=server_config.getint("port", 3080), - user=server_config.get("user", ""), - password=server_config.get("password", "")) + self._computes["local"] = Compute(compute_id="local", + controller=self, + protocol=server_config.get("protocol", "http"), + host=server_config.get("host", "localhost"), + port=server_config.getint("port", 3080), + user=server_config.get("user", ""), + password=server_config.get("password", "")) return self._computes["local"] @property def computes(self): """ - :returns: The dictionnary of computes managed by GNS3 + :returns: The dictionary of compute server managed by this controller """ return self._computes def get_compute(self, compute_id): """ - Return an compute or raise a 404 + Returns a compute server or raise a 404 error. """ try: return self._computes[compute_id] @@ -141,21 +137,21 @@ class Controller: @asyncio.coroutine def add_project(self, project_id=None, **kwargs): """ - Create a project or return an existing project + Creates a project or returns an existing project :param kwargs: See the documentation of Project """ if project_id not in self._projects: project = Project(project_id=project_id, **kwargs) self._projects[project.id] = project - for compute in self._computes.values(): - yield from project.add_compute(compute) + for compute_server in self._computes.values(): + yield from project.add_compute(compute_server) return self._projects[project.id] return self._projects[project_id] def get_project(self, project_id): """ - Return a project or raise a 404 + Returns a compute server or raise a 404 error. """ try: return self._projects[project_id] @@ -168,7 +164,7 @@ class Controller: @property def projects(self): """ - :returns: The dictionnary of computes managed by GNS3 + :returns: The dictionary of computes managed by GNS3 """ return self._projects @@ -176,6 +172,7 @@ class Controller: def instance(): """ Singleton to return only on instance of Controller. + :returns: instance of Controller """ @@ -195,5 +192,5 @@ class Controller: except KeyError: pass else: - for project in self._projects.values(): - project.emit(action, event, **kwargs) + for project_instance in self._projects.values(): + project_instance.emit(action, event, **kwargs) diff --git a/gns3server/controller/compute.py b/gns3server/controller/compute.py index fa492116..170f21e7 100644 --- a/gns3server/controller/compute.py +++ b/gns3server/controller/compute.py @@ -18,8 +18,8 @@ import aiohttp import asyncio import json -from pkg_resources import parse_version +from ..utils import parse_version from ..controller.controller_error import ControllerError from ..config import Config from ..version import __version__ diff --git a/gns3server/controller/link.py b/gns3server/controller/link.py index 303aefb4..321a7dcf 100644 --- a/gns3server/controller/link.py +++ b/gns3server/controller/link.py @@ -33,6 +33,7 @@ class Link: self._project = project self._capturing = False self._capture_file_name = None + self._streaming_pcap = None @asyncio.coroutine def add_node(self, node, adapter_number, port_number): @@ -73,13 +74,12 @@ class Link: @asyncio.coroutine def _start_streaming_pcap(self): """ - Dump the pcap file on disk + Dump a pcap file on disk """ stream = yield from self.read_pcap_from_source() with open(self.capture_file_path, "wb+") as f: while self._capturing: - # We read 1 bytes by 1 otherwise if the traffic stop the remaining data is not read - # this is slow + # We read 1 bytes by 1 otherwise the remaining data is not read if the traffic stops data = yield from stream.read(1) if data: f.write(data) @@ -98,7 +98,7 @@ class Link: @asyncio.coroutine def read_pcap_from_source(self): """ - Return a FileStream of the Pcap from the compute node + Return a FileStream of the Pcap from the compute server """ raise NotImplementedError @@ -106,13 +106,12 @@ class Link: """ :returns: File name for a capture on this link """ - capture_file_name = "{}_{}-{}_to_{}_{}-{}".format( - self._nodes[0]["node"].name, - self._nodes[0]["adapter_number"], - self._nodes[0]["port_number"], - self._nodes[1]["node"].name, - self._nodes[1]["adapter_number"], - self._nodes[1]["port_number"]) + capture_file_name = "{}_{}-{}_to_{}_{}-{}".format(self._nodes[0]["node"].name, + self._nodes[0]["adapter_number"], + self._nodes[0]["port_number"], + self._nodes[1]["node"].name, + self._nodes[1]["adapter_number"], + self._nodes[1]["port_number"]) return re.sub("[^0-9A-Za-z_-]", "", capture_file_name) + ".pcap" @property diff --git a/gns3server/controller/project.py b/gns3server/controller/project.py index 28e39d01..d7a52af1 100644 --- a/gns3server/controller/project.py +++ b/gns3server/controller/project.py @@ -32,7 +32,7 @@ from ..utils.path import check_path_allowed, get_default_project_directory class Project: """ - A project inside controller + A project inside a controller :param project_id: force project identifier (None by default auto generate an UUID) :param path: path of the project. (None use the standard directory) @@ -89,7 +89,7 @@ class Project: raise aiohttp.web.HTTPInternalServerError(text="Could not create project directory: {}".format(e)) if '"' in path: - raise aiohttp.web.HTTPForbidden(text="You are not allowed to use \" in the project directory path. It's not supported by Dynamips.") + raise aiohttp.web.HTTPForbidden(text="You are not allowed to use \" in the project directory path. Not supported by Dynamips.") self._path = path @@ -201,7 +201,7 @@ class Project: :param action: Action name :param event: Event to send - :param kwargs: Add this meta to the notif (project_id for example) + :param kwargs: Add this meta to the notification (project_id for example) """ for listener in self._listeners: listener.put_nowait((action, event, kwargs)) diff --git a/gns3server/controller/udp_link.py b/gns3server/controller/udp_link.py index 2276bb0f..4543cfd1 100644 --- a/gns3server/controller/udp_link.py +++ b/gns3server/controller/udp_link.py @@ -108,8 +108,7 @@ class UDPLink(Link): """ Run capture on the best candidate. - The ideal candidate is a node who support capture on controller - server + The ideal candidate is a node who support capture on controller server :returns: Node where the capture should run """ diff --git a/tests/controller/test_controller.py b/tests/controller/test_controller.py index 5afecb38..0ab557cc 100644 --- a/tests/controller/test_controller.py +++ b/tests/controller/test_controller.py @@ -69,9 +69,9 @@ def test_load(controller, controller_config_path, async_run): def test_isEnabled(controller): Config.instance().set("Server", "controller", False) - assert not controller.isEnabled() + assert not controller.is_enabled() Config.instance().set("Server", "controller", True) - assert controller.isEnabled() + assert controller.is_enabled() def test_addCompute(controller, controller_config_path, async_run):