From 4eadacdb75426219d74a5ebe51e633c8523a1593 Mon Sep 17 00:00:00 2001 From: Gary Kotton Date: Thu, 20 Feb 2014 06:52:59 -0800 Subject: [PATCH] VMware: raise more specific exceptions In certain cases the exception handling for backend errors would be handled too broadly. This patch elevates the backend exception to the application so that it can treat specific errors. In the application we can now handle the following specific exceptions: - FileAlreadyExists - for example when moving a directory to a directory that already exists - InvalidProperty - for example when using neutron and opaque networks are not supported - AlreadyExists - for example a port group already exists - NotAuthenticated - for example the operation is denied as because a session is not established - CannotDeleteFile - the file cannot be deleted - FileFault - a file access exception - FileLocked - an attempt is made to lock a file that is already in use - FileNotFound - the specific file does not exist Change-Id: I789165f3879ec5df73f72c2737377e0fcfc99656 --- oslo/vmware/api.py | 11 ++++- oslo/vmware/exceptions.py | 101 +++++++++++++++++++++++++++++++++++++- oslo/vmware/vim.py | 9 ++-- tests/test_api.py | 39 ++++++++++++++- tests/test_vim.py | 2 +- 5 files changed, 152 insertions(+), 10 deletions(-) diff --git a/oslo/vmware/api.py b/oslo/vmware/api.py index 7d7397a..8ade849 100644 --- a/oslo/vmware/api.py +++ b/oslo/vmware/api.py @@ -251,7 +251,7 @@ class VMwareAPISession(object): except exceptions.VimFaultException as excep: # If this is due to an inactive session, we should re-create # the session and retry. - if exceptions.NOT_AUTHENTICATED_FAULT in excep.fault_list: + if exceptions.NOT_AUTHENTICATED in excep.fault_list: # The NotAuthenticated fault is set by the fault checker # due to an empty response. An empty response could be a # valid response; for e.g., response for the query to @@ -281,6 +281,10 @@ class VMwareAPISession(object): else: # no need to retry for other VIM faults like # InvalidArgument + # Raise specific exceptions here if possible + if excep.fault_list: + LOG.debug(_("Fault list: %s"), excep.fault_list) + raise exceptions.get_fault_class(excep.fault_list[0]) raise except exceptions.VimConnectionException: @@ -369,7 +373,10 @@ class VMwareAPISession(object): "%(error)s.") % {'task': task, 'error': error_msg} LOG.error(excep_msg) - raise exceptions.VimException(excep_msg) + error = task_info.error + name = error.fault.__class__.__name__ + task_ex = exceptions.get_fault_class(name)(error_msg) + raise task_ex def wait_for_lease_ready(self, lease): """Waits for the given lease to be ready. diff --git a/oslo/vmware/exceptions.py b/oslo/vmware/exceptions.py index e72dac4..14eb056 100644 --- a/oslo/vmware/exceptions.py +++ b/oslo/vmware/exceptions.py @@ -17,7 +17,21 @@ Exception definitions. """ -NOT_AUTHENTICATED_FAULT = "NotAuthenticated" +import six + +from oslo.vmware.openstack.common.gettextutils import _ +from oslo.vmware.openstack.common import log as logging + +LOG = logging.getLogger(__name__) + +ALREADY_EXISTS = 'AlreadyExists' +CANNOT_DELETE_FILE = 'CannotDeleteFile' +FILE_ALREADY_EXISTS = 'FileAlreadyExists' +FILE_FAULT = 'FileFault' +FILE_LOCKED = 'FileLocked' +FILE_NOT_FOUND = 'FileNotFound' +INVALID_PROPERTY = 'InvalidProperty' +NOT_AUTHENTICATED = 'NotAuthenticated' class VimException(Exception): @@ -66,3 +80,88 @@ class VimFaultException(VimException): class ImageTransferException(VimException): """Thrown when there is an error during image transfer.""" pass + + +class VMwareDriverException(Exception): + """Base VMware Driver Exception + + To correctly use this class, inherit from it and define + a 'msg_fmt' property. That msg_fmt will get printf'd + with the keyword arguments provided to the constructor. + + """ + msg_fmt = _("An unknown exception occurred.") + + def __init__(self, message=None, **kwargs): + self.kwargs = kwargs + + if not message: + try: + message = self.msg_fmt % kwargs + + except Exception: + # kwargs doesn't match a variable in the message + # log the issue and the kwargs + LOG.exception(_('Exception in string format operation')) + for name, value in six.iteritems(kwargs): + LOG.error("%s: %s" % (name, value)) + # at least get the core message out if something happened + message = self.msg_fmt + + super(VMwareDriverException, self).__init__(message) + + +class AlreadyExistsException(VMwareDriverException): + msg_fmt = _("Resource already exists.") + + +class CannotDeleteFileException(VMwareDriverException): + msg_fmt = _("Cannot delete file.") + + +class FileAlreadyExistsException(VMwareDriverException): + msg_fmt = _("File already exists.") + + +class FileFaultException(VMwareDriverException): + msg_fmt = _("File fault.") + + +class FileLockedException(VMwareDriverException): + msg_fmt = _("File locked.") + + +class FileNotFoundException(VMwareDriverException): + msg_fmt = _("File not found.") + + +class InvalidPropertyException(VMwareDriverException): + msg_fmt = _("Invalid property.") + + +class NotAuthenticatedException(VMwareDriverException): + msg_fmt = _("Not Authenticated.") + + +# Populate the fault registry with the exceptions that have +# special treatment. +_fault_classes_registry = { + ALREADY_EXISTS: AlreadyExistsException, + CANNOT_DELETE_FILE: CannotDeleteFileException, + FILE_ALREADY_EXISTS: FileAlreadyExistsException, + FILE_FAULT: FileFaultException, + FILE_LOCKED: FileLockedException, + FILE_NOT_FOUND: FileNotFoundException, + INVALID_PROPERTY: InvalidPropertyException, + NOT_AUTHENTICATED: NotAuthenticatedException, +} + + +def get_fault_class(name): + """Get a named subclass of NovaException.""" + name = str(name) + fault_class = _fault_classes_registry.get(name) + if not fault_class: + LOG.debug(_('Fault %s not matched.'), name) + fault_class = VMwareDriverException + return fault_class diff --git a/oslo/vmware/vim.py b/oslo/vmware/vim.py index 6f2f2c5..2ed5597 100644 --- a/oslo/vmware/vim.py +++ b/oslo/vmware/vim.py @@ -135,8 +135,8 @@ class Vim(object): # fault. LOG.debug(_("RetrievePropertiesEx API response is empty; setting " "fault to %s."), - exceptions.NOT_AUTHENTICATED_FAULT) - fault_list = [exceptions.NOT_AUTHENTICATED_FAULT] + exceptions.NOT_AUTHENTICATED) + fault_list = [exceptions.NOT_AUTHENTICATED] else: for obj_cont in response.objects: if hasattr(obj_cont, 'missingSet'): @@ -195,8 +195,9 @@ class Vim(object): doc = excep.document detail = doc.childAtPath('/Envelope/Body/Fault/detail') fault_list = [] - for child in detail.getChildren(): - fault_list.append(child.get('type')) + if detail: + for child in detail.getChildren(): + fault_list.append(child.get('type')) raise exceptions.VimFaultException( fault_list, _("Web fault in %s.") % attr_name, excep) diff --git a/tests/test_api.py b/tests/test_api.py index 0f89f42..4182b3f 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -200,7 +200,7 @@ class VMwareAPISessionTest(base.TestCase): def api(*args, **kwargs): raise exceptions.VimFaultException( - [exceptions.NOT_AUTHENTICATED_FAULT], None) + [exceptions.NOT_AUTHENTICATED], None) module = mock.Mock() module.api = api @@ -249,7 +249,7 @@ class VMwareAPISessionTest(base.TestCase): api_session.invoke_api = mock.Mock(side_effect=invoke_api_side_effect) task = mock.Mock() - self.assertRaises(exceptions.VimException, + self.assertRaises(exceptions.VMwareDriverException, lambda: api_session.wait_for_task(task)) api_session.invoke_api.assert_called_with(vim_util, 'get_object_property', @@ -329,3 +329,38 @@ class VMwareAPISessionTest(base.TestCase): api_session.invoke_api.assert_called_once_with( vim_util, 'get_object_property', api_session.vim, lease, 'state') + + def _poll_task_well_known_exceptions(self, fault, + expected_exception): + api_session = self._create_api_session(False) + + def fake_invoke_api(self, module, method, *args, **kwargs): + task_info = mock.Mock() + task_info.progress = -1 + task_info.state = 'error' + error = mock.Mock() + error.localizedMessage = "Error message" + error_fault = mock.Mock() + error_fault.__class__.__name__ = fault + error.fault = error_fault + task_info.error = error + return task_info + + with ( + mock.patch.object(api_session, 'invoke_api', fake_invoke_api) + ): + self.assertRaises(expected_exception, + api_session._poll_task, 'fake-task') + + def test_poll_task_well_known_exceptions(self): + for k, v in exceptions._fault_classes_registry.iteritems(): + self._poll_task_well_known_exceptions(k, v) + + def test_poll_task_unknown_exception(self): + _unknown_exceptions = { + 'NoDiskSpace': exceptions.VMwareDriverException, + 'RuntimeFault': exceptions.VMwareDriverException + } + + for k, v in _unknown_exceptions.iteritems(): + self._poll_task_well_known_exceptions(k, v) diff --git a/tests/test_vim.py b/tests/test_vim.py index f13eb66..6dbca70 100644 --- a/tests/test_vim.py +++ b/tests/test_vim.py @@ -71,7 +71,7 @@ class VimTest(base.TestCase): vim.Vim._retrieve_properties_ex_fault_checker(None) assert False except exceptions.VimFaultException as ex: - self.assertEqual([exceptions.NOT_AUTHENTICATED_FAULT], + self.assertEqual([exceptions.NOT_AUTHENTICATED], ex.fault_list) def test_retrieve_properties_ex_fault_checker(self):