From abb51f79d2d6c9960f3ce48e6d895219f2bef3d0 Mon Sep 17 00:00:00 2001 From: Galit Date: Tue, 26 Jun 2018 14:20:52 +0300 Subject: [PATCH 1/2] add friendly msg,when file to copy not exist fixed: copy-from-vm copy-to-vm --- lago/plugins/vm.py | 99 ++++++++++++++++++++++++---------------------- 1 file changed, 52 insertions(+), 47 deletions(-) diff --git a/lago/plugins/vm.py b/lago/plugins/vm.py index 995c16ca..2aaf6449 100644 --- a/lago/plugins/vm.py +++ b/lago/plugins/vm.py @@ -35,7 +35,6 @@ import os import warnings from abc import (ABCMeta, abstractmethod) - from scp import SCPClient, SCPException from .. import ( @@ -67,6 +66,22 @@ def __init__(self, vm_name): super().__init__('VM {} is not running'.format(vm_name)) +class LagoCopyFilesToVMError(utils.LagoUserException): + def __init__(self, local_files, err): + super().__init__( + 'Failed to copy files/directory {}\n{}'.format(local_files, err) + ) + + +class LagoCopyFilesFromVMError(utils.LagoUserException): + def __init__(self, remote_files, local_files, err=''): + super().__init__( + 'Failed to copy files/directory from {} to {}\n{}'.format( + remote_files, local_files, err + ) + ) + + class LagoVMDoesNotExistError(utils.LagoException): pass @@ -91,7 +106,6 @@ class VMProviderPlugin(plugins.Plugin): you have to inherit from this class, and then define the 'default_vm_provider' in your config to be your plugin, or explicitly specify it on each domain definition in the initfile with 'vm-provider' key - You will have to override at least all the abstractmethods in order to write a provider plugin, even if they are just runnig `pass`. """ @@ -103,7 +117,6 @@ def __init__(self, vm): def start(self, *args, **kwargs): """ Start a domain - Returns: None """ @@ -113,7 +126,6 @@ def start(self, *args, **kwargs): def stop(self, *args, **kwargs): """ Stop a domain - Returns: None """ @@ -123,7 +135,6 @@ def stop(self, *args, **kwargs): def shutdown(self, *args, **kwargs): """ Shutdown a domain - Returns: None """ @@ -133,7 +144,6 @@ def shutdown(self, *args, **kwargs): def reboot(self, *args, **kwargs): """ Reboot a domain - Returns: None """ @@ -144,7 +154,6 @@ def bootstrap(self, *args, **kwargs): """ Does any actions needed to get the domain ready to be used, ran on prefix init. - Return: None """ @@ -154,7 +163,6 @@ def bootstrap(self, *args, **kwargs): def state(self, *args, **kwargs): """ Return the current state of the domain - Returns: str: Small description of the current domain state """ @@ -172,11 +180,9 @@ def running(self, *args, **kwargs): def create_snapshot(self, name, *args, **kwargs): """ Take any actions needed to create a snapshot - Args: name(str): Name for the snapshot, will be used as key to retrieve it later - Returns: None """ @@ -186,10 +192,8 @@ def create_snapshot(self, name, *args, **kwargs): def revert_snapshot(self, name, *args, **kwargs): """ Take any actions needed to revert/restore a snapshot - Args: name(str): Name for the snapshot, same that was set on creation - Returns: None """ @@ -198,7 +202,6 @@ def revert_snapshot(self, name, *args, **kwargs): def export_disks(self, standalone, dst_dir, compress, *args, **kwargs): """ Export 'disks' as a standalone image or a layered image. - Args: disks(list): The names of the disks to export (None means all the disks) @@ -213,7 +216,6 @@ def export_disks(self, standalone, dst_dir, compress, *args, **kwargs): def interactive_console(self): """ Run an interactive console - Returns: lago.utils.CommandStatus: resulf of the interactive execution """ @@ -232,14 +234,11 @@ def name(self): def extract_paths(self, paths, ignore_nopath): """ Extract the given paths from the domain - Args: paths(list of str): paths to extract ignore_nopath(boolean): if True will ignore none existing paths. - Returns: None - Raises: :exc:`~lago.plugins.vm.ExtractPathNoPathError`: if a none existing path was found on the VM, and ``ignore_nopath`` is True. @@ -268,15 +267,9 @@ def _extract_paths_scp(self, paths, ignore_nopath): remote_path=host_path, propagate_fail=False ) - except SCPException as err: - err_substr = ': No such file or directory' - if len(err.args) > 0 and isinstance( - err.args[0], basestring - ) and err_substr in err.args[0]: - if ignore_nopath: - LOGGER.debug('%s: ignoring', err.args[0]) - else: - raise ExtractPathNoPathError(err.args[0]) + except ExtractPathNoPathError as err: + if ignore_nopath: + LOGGER.debug('%s: ignoring', err.args[0]) else: raise @@ -288,9 +281,7 @@ class VMPlugin(plugins.Plugin): the initfile lingo). From starting/stopping it to loading and calling the provider if needed. If you want to change only the way the VM is provisioned you can take a look to the `class:VMProviderPlugin` instead. - This base class includes also some basic methods implemented with ssh. - VM properties: * name * cpus @@ -411,22 +402,43 @@ def copy_to(self, local_path, remote_path, recursive=True): with LogTask( 'Copy %s to %s:%s' % (local_path, self.name(), remote_path), ): - with self._scp() as scp: - scp.put( - files=local_path, - remote_path=remote_path, - recursive=recursive, - ) + try: + with self._scp() as scp: + scp.put( + files=local_path, + remote_path=remote_path, + recursive=recursive, + ) + except (OSError, SCPException) as err: + raise LagoCopyFilesToVMError(local_path, str(err)) def copy_from( self, remote_path, local_path, recursive=True, propagate_fail=True ): - with self._scp(propagate_fail=propagate_fail) as scp: - scp.get( - recursive=recursive, - remote_path=remote_path, - local_path=local_path, - ) + with LogTask( + 'Copy from %s:%s to %s' % (self.name(), remote_path, local_path), + ): + try: + with self._scp(propagate_fail=propagate_fail) as scp: + scp.get( + recursive=recursive, + remote_path=remote_path, + local_path=local_path, + ) + except SCPException as err: + err_substr = ': No such file or directory' + if all( + ( + len(err.args) > 0, + isinstance(err.args[0], basestring), + err_substr in err.args[0], + ) + ): + raise ExtractPathNoPathError(err.args[0]) + else: + raise LagoCopyFilesFromVMError( + remote_path, local_path, err.args[0] + ) @property def metadata(self): @@ -553,13 +565,11 @@ def ssh_script(self, path, show_output=True): def ssh_reachable(self, tries=None, propagate_fail=True): """ Check if the VM is reachable with ssh - Args: tries(int): Number of tries to try connecting to the host propagate_fail(bool): If set to true, this event will appear in the log and fail the outter stage. Otherwise, it will be discarded. - Returns: bool: True if the VM is reachable. """ @@ -736,7 +746,6 @@ def _get_service_provider(self): """ **NOTE**: Can be reduced to just one get call once we remove support for the service_class spec entry - Returns: class: class for the loaded provider for that vm_spec None: if no provider was specified in the vm_spec @@ -764,18 +773,14 @@ def _resolve_service_class(class_name, service_providers): """ **NOTE**: This must be remved once the service_class spec entry is fully deprecated - Retrieves a service plugin class from the class name instead of the provider name - Args: class_name(str): Class name of the service plugin to retrieve service_providers(dict): provider_name->provider_class of the loaded service providers - Returns: class: Class of the plugin that matches that name - Raises: lago.plugins.NoSuchPluginError: if there was no service plugin that matched the search From 3210f40e294a4f753bf2d9d22433862ffc951a4c Mon Sep 17 00:00:00 2001 From: gbenhaim Date: Thu, 6 Dec 2018 18:19:19 +0200 Subject: [PATCH 2/2] Improve lago collect error handling 1. Don't display the entire stack trace on error (but still write it to the log). This was achieved by making VMError a subclass of LagoException. 2. When ignoring missing files, explain why. 3. Make the error message of ExtractPathNoPathError more informative. 4. Don't propagate errors to the outer log task when ignoring missing files, thus avoid false positives on stdout. Signed-off-by: gbenhaim --- lago/plugins/vm.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/lago/plugins/vm.py b/lago/plugins/vm.py index 2aaf6449..9e4eea3d 100644 --- a/lago/plugins/vm.py +++ b/lago/plugins/vm.py @@ -49,7 +49,7 @@ LogTask = functools.partial(log_utils.LogTask, logger=LOGGER) -class VMError(Exception): +class VMError(utils.LagoException): pass @@ -58,7 +58,8 @@ class ExtractPathError(VMError): class ExtractPathNoPathError(VMError): - pass + def __init__(self, err): + super().__init__('Failed to extract files: {}'.format(err)) class LagoVMNotRunningError(utils.LagoUserException): @@ -265,11 +266,14 @@ def _extract_paths_scp(self, paths, ignore_nopath): self.vm.copy_from( local_path=guest_path, remote_path=host_path, - propagate_fail=False + propagate_fail=not ignore_nopath ) except ExtractPathNoPathError as err: if ignore_nopath: - LOGGER.debug('%s: ignoring', err.args[0]) + LOGGER.debug( + '%s: ignoring since ignore_nopath was set to True', + err.args[0] + ) else: raise @@ -417,6 +421,7 @@ def copy_from( ): with LogTask( 'Copy from %s:%s to %s' % (self.name(), remote_path, local_path), + propagate_fail=propagate_fail ): try: with self._scp(propagate_fail=propagate_fail) as scp: