From 1668fef9cabea6a23023aab6b97617b4368b14d6 Mon Sep 17 00:00:00 2001
From: Vipin Balachandran <vbala@vmware.com>
Date: Thu, 23 Apr 2015 02:58:41 -0700
Subject: [PATCH] Raise VimFaultException for unknown faults

Currently VMwareDriverException is raised for unknown
VIM faults. Sometimes clients may need to handle such
faults. Therefore it is better if we throw
VimFaultException with the fault_list set to the relevant
VIM fault class name.

Change-Id: I34f84f3ea3f5f0d6878f1d4438cf25bf22f293fd
---
 oslo_vmware/api.py                   | 10 ++++++++--
 oslo_vmware/exceptions.py            |  1 -
 oslo_vmware/tests/test_api.py        | 19 ++++++++++++++++---
 oslo_vmware/tests/test_exceptions.py |  3 +--
 tests/test_api.py                    |  6 +++---
 5 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/oslo_vmware/api.py b/oslo_vmware/api.py
index df68bc9..dabc461 100644
--- a/oslo_vmware/api.py
+++ b/oslo_vmware/api.py
@@ -321,7 +321,8 @@ class VMwareAPISession(object):
                         LOG.debug("Fault list: %s", excep.fault_list)
                         fault = excep.fault_list[0]
                         clazz = exceptions.get_fault_class(fault)
-                        raise clazz(six.text_type(excep), excep.details)
+                        if clazz:
+                            raise clazz(six.text_type(excep), excep.details)
                     raise
 
             except exceptions.VimConnectionException:
@@ -413,7 +414,12 @@ class VMwareAPISession(object):
                 error_msg = six.text_type(task_info.error.localizedMessage)
                 error = task_info.error
                 name = error.fault.__class__.__name__
-                task_ex = exceptions.get_fault_class(name)(error_msg)
+                fault_class = exceptions.get_fault_class(name)
+                if fault_class:
+                    task_ex = fault_class(error_msg)
+                else:
+                    task_ex = exceptions.VimFaultException([name],
+                                                           error_msg)
                 raise task_ex
 
     def wait_for_lease_ready(self, lease):
diff --git a/oslo_vmware/exceptions.py b/oslo_vmware/exceptions.py
index 039be05..cd53319 100644
--- a/oslo_vmware/exceptions.py
+++ b/oslo_vmware/exceptions.py
@@ -254,7 +254,6 @@ def get_fault_class(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/tests/test_api.py b/oslo_vmware/tests/test_api.py
index d0459b6..31120bf 100644
--- a/oslo_vmware/tests/test_api.py
+++ b/oslo_vmware/tests/test_api.py
@@ -373,6 +373,19 @@ class VMwareAPISessionTest(base.TestCase):
             userName=api_session._session_username)
         api_session._create_session.assert_called_once_with()
 
+    def test_invoke_api_with_unknown_fault(self):
+        api_session = self._create_api_session(True)
+        fault_list = ['NotAFile']
+
+        module = mock.Mock()
+        module.api.side_effect = exceptions.VimFaultException(fault_list,
+                                                              'Not a file.')
+        ex = self.assertRaises(exceptions.VimFaultException,
+                               api_session.invoke_api,
+                               module,
+                               'api')
+        self.assertEqual(fault_list, ex.fault_list)
+
     def test_wait_for_task(self):
         api_session = self._create_api_session(True)
         task_info_list = [('queued', 0), ('running', 40), ('success', 100)]
@@ -413,7 +426,7 @@ class VMwareAPISessionTest(base.TestCase):
         api_session.invoke_api = mock.Mock(side_effect=invoke_api_side_effect)
         task = mock.Mock()
         with mock.patch.object(greenthread, 'sleep'):
-            self.assertRaises(exceptions.VMwareDriverException,
+            self.assertRaises(exceptions.VimFaultException,
                               api_session.wait_for_task,
                               task)
         api_session.invoke_api.assert_called_with(vim_util,
@@ -531,8 +544,8 @@ class VMwareAPISessionTest(base.TestCase):
 
     def test_poll_task_unknown_exception(self):
         _unknown_exceptions = {
-            'NotAFile': exceptions.VMwareDriverException,
-            'RuntimeFault': exceptions.VMwareDriverException
+            'NotAFile': exceptions.VimFaultException,
+            'RuntimeFault': exceptions.VimFaultException
         }
 
         for k, v in six.iteritems(_unknown_exceptions):
diff --git a/oslo_vmware/tests/test_exceptions.py b/oslo_vmware/tests/test_exceptions.py
index 2cf2462..46c3660 100644
--- a/oslo_vmware/tests/test_exceptions.py
+++ b/oslo_vmware/tests/test_exceptions.py
@@ -107,5 +107,4 @@ class ExceptionsTest(base.TestCase):
         self.assertEqual(exceptions.NoDiskSpaceException,
                          exceptions.get_fault_class("NoDiskSpace"))
         # Test unknown fault.
-        self.assertEqual(exceptions.VMwareDriverException,
-                         exceptions.get_fault_class("NotAFile"))
+        self.assertIsNone(exceptions.get_fault_class("NotAFile"))
diff --git a/tests/test_api.py b/tests/test_api.py
index 8d18f92..ebf83c0 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -412,7 +412,7 @@ class VMwareAPISessionTest(base.TestCase):
         api_session.invoke_api = mock.Mock(side_effect=invoke_api_side_effect)
         task = mock.Mock()
         with mock.patch.object(greenthread, 'sleep'):
-            self.assertRaises(exceptions.VMwareDriverException,
+            self.assertRaises(exceptions.VimFaultException,
                               api_session.wait_for_task,
                               task)
         api_session.invoke_api.assert_called_with(new_vim_util,
@@ -526,8 +526,8 @@ class VMwareAPISessionTest(base.TestCase):
 
     def test_poll_task_unknown_exception(self):
         _unknown_exceptions = {
-            'NoDiskSpace': exceptions.VMwareDriverException,
-            'RuntimeFault': exceptions.VMwareDriverException
+            'NotAFile': exceptions.VimFaultException,
+            'RuntimeFault': exceptions.VimFaultException
         }
 
         for k, v in six.iteritems(_unknown_exceptions):