VMWare: bug fix for Vim exception handling

The exception handling in the Vim module was too broad and did not treat
the suds.MethodNotFound exception. This would result in a exception
that "__deepcopy__" is not supported in the VIM module.

In the case of an exception the VMwareAPISession class may be deleted.
The patch also ensures that the deletion is able to logout if necessary.

Closes-bug: #1228847

(cherry picked from commit be5d39eacc84c11f96357a624ffa1cdeda8a568d)

Conflicts:

	nova/tests/virt/vmwareapi/test_vmwareapi.py

Change-Id: Ic85be9b3407c444db4b3a074108c01d9141c61be

4 files changed, 59 insertions, 14 deletions

diff --git a/nova/tests/virt/vmwareapi/test_vmwareapi.py b/nova/tests/virt/vmwareapi/test_vmwareapi.py
index 7058dd9170..00d3812341 100755
--- a/nova/tests/virt/vmwareapi/test_vmwareapi.py
+++ b/nova/tests/virt/vmwareapi/test_vmwareapi.py
@@ -22,9 +22,12 @@ Test suite for VMwareAPI.
 """
 
 import contextlib
+import copy
+
 import mock
 import mox
 from oslo.config import cfg
+import suds
 
 from nova import block_device
 from nova.compute import api as compute_api
@@ -138,6 +141,46 @@ class VMwareSessionTestCase(test.NoDBTestCase):
                               *args, **kwargs)
 
 
+class fake_vm_ref(object):
+    def __init__(self):
+        self.value = 4
+        self._type = 'VirtualMachine'
+
+
+class fake_service_content(object):
+    def __init__(self):
+        self.ServiceContent = vmwareapi_fake.DataObject()
+        self.ServiceContent.fake = 'fake'
+
+
+class VMwareSudsTest(test.NoDBTestCase):
+
+    def setUp(self):
+        super(VMwareSudsTest, self).setUp()
+
+        def new_client_init(self, url, **kwargs):
+            return
+
+        mock.patch.object(suds.client.Client,
+                          '__init__', new=new_client_init).start()
+        self.vim = self._vim_create()
+        self.addCleanup(mock.patch.stopall)
+
+    def _vim_create(self):
+
+        def fake_retrieve_service_content(fake):
+            return fake_service_content()
+
+        self.stubs.Set(vim.Vim, 'retrieve_service_content',
+                fake_retrieve_service_content)
+        return vim.Vim()
+
+    def test_exception_with_deepcopy(self):
+        self.assertIsNotNone(self.vim)
+        self.assertRaises(error_util.VimException,
+                          copy.deepcopy, self.vim)
+
+
 class VMwareAPIConfTestCase(test.NoDBTestCase):
     """Unit tests for VMWare API configurations."""
     def setUp(self):
diff --git a/nova/tests/virt/vmwareapi/test_vmwareapi_volumeops.py b/nova/tests/virt/vmwareapi/test_vmwareapi_volumeops.py
index a4eefd11c8..3d5d831380 100644
--- a/nova/tests/virt/vmwareapi/test_vmwareapi_volumeops.py
+++ b/nova/tests/virt/vmwareapi/test_vmwareapi_volumeops.py
@@ -27,11 +27,17 @@ from nova.virt.vmwareapi import volumeops
 class VMwareVolumeOpsTestCase(test.NoDBTestCase):
 
     def setUp(self):
+
+        def fake_del():
+            return
+
         super(VMwareVolumeOpsTestCase, self).setUp()
         vmwareapi_fake.reset()
         stubs.set_stubs(self.stubs)
-
         self._session = driver.VMwareAPISession()
+        self.stubs.Set(self._session, '__del__',
+                       fake_del)
+
         self._volumeops = volumeops.VMwareVolumeOps(self._session)
         self.instance = {'name': 'fake_name', 'uuid': 'fake_uuid'}
 
diff --git a/nova/virt/vmwareapi/driver.py b/nova/virt/vmwareapi/driver.py
index 38e546772d..251be90f7f 100644
--- a/nova/virt/vmwareapi/driver.py
+++ b/nova/virt/vmwareapi/driver.py
@@ -808,16 +808,8 @@ class VMwareAPISession(object):
 
     def __del__(self):
         """Logs-out the session."""
-        # Logout to avoid un-necessary increase in session count at the
-        # ESX host
-        try:
-            # May not have been able to connect to VC, so vim is still None
-            if self.vim:
-                self.vim.Logout(self.vim.get_service_content().sessionManager)
-        except Exception as excep:
-            # It is just cautionary on our part to do a logout in del just
-            # to ensure that the session is not left active.
-            LOG.debug(excep)
+        if hasattr(self, 'vim') and self.vim:
+            self.vim.Logout(self.vim.get_service_content().sessionManager)
 
     def _is_vim_object(self, module):
         """Check if the module is a VIM Object instance."""
diff --git a/nova/virt/vmwareapi/vim.py b/nova/virt/vmwareapi/vim.py
index 5928eb45f9..4b64e66f21 100644
--- a/nova/virt/vmwareapi/vim.py
+++ b/nova/virt/vmwareapi/vim.py
@@ -93,8 +93,10 @@ class Vim:
         self.url = Vim.get_soap_url(protocol, host)
         self.client = suds.client.Client(self.wsdl_url, location=self.url,
                                          plugins=[VIMMessagePlugin()])
-        self._service_content = self.RetrieveServiceContent(
-                                        "ServiceInstance")
+        self._service_content = self.retrieve_service_content()
+
+    def retrieve_service_content(self):
+        return self.RetrieveServiceContent("ServiceInstance")
 
     @staticmethod
     def get_wsdl_url(protocol, host_name):
@@ -162,7 +164,9 @@ class Vim:
                 return response
             # Catch the VimFaultException that is raised by the fault
             # check of the SOAP response
-            except error_util.VimFaultException as excep:
+            except error_util.VimFaultException:
+                raise
+            except suds.MethodNotFound:
                 raise
             except suds.WebFault as excep:
                 doc = excep.document