vmware: Fix problems with VIM API retry logic

Currently the VIM APIs including session creation API are retried for
cases which are unrelated to connection or session overload problems.
This change fix the retry logic as follows:
* Add an exception to indicate connection problem and raise it
  appropriately from the VIM API client
* Modify the expected exceptions in the retry decorator to include
  only connection and session overload related exceptions

This change also fixes the base class of VimFaultException
(VolumeBackendAPIException -> VimException). This change is required
so that we can handle the exceptions thrown by the VIM API client in
a consistent manner. Currently if we need to handle VIM API related
exceptions, we have to catch both VimException and VimFaultException.
For example, the API for checking active session raises
VimFaultException in some cases and we are handling only VimException.
Due to this, when session expires, the session re-establishment and API
retry fails (intermittently) since the check for active session throws
VimFaultException in some cases which is not handled.

Change-Id: I72cfc7777c3ce693b8598633f822c12c2cee2235
Closes-Bug: #1294598
Closes-Bug: #1302514

cinder/tests/test_vmware_api.py

@@ -165,7 +165,7 @@ class VMwareAPISessionTest(test.TestCase):
     def test_invoke_api_with_expected_exception(self):
         api_session = self._create_api_session(True)
         ret = mock.Mock()
-        responses = [error_util.VimException(None), ret]
+        responses = [error_util.VimConnectionException(None), ret]
 
         def api(*args, **kwargs):
             response = responses.pop(0)
@@ -231,3 +231,29 @@ class VMwareAPISessionTest(test.TestCase):
             sessionID=api_session._session_id,
             userName=api_session._session_username)
         api_session.create_session.assert_called_once_with()
+
+    def test_invoke_api_with_session_is_active_error(self):
+        api_session = self._create_api_session(True)
+        api_session.create_session = mock.Mock()
+        vim_obj = api_session.vim
+        vim_obj.SessionIsActive.side_effect = error_util.VimFaultException(
+            None, None)
+        result = mock.Mock()
+        responses = [error_util.VimFaultException(
+            [error_util.NOT_AUTHENTICATED], "error"), result]
+
+        def api(*args, **kwargs):
+            response = responses.pop(0)
+            if isinstance(response, Exception):
+                raise response
+            return response
+
+        module = mock.Mock()
+        module.api = api
+        ret = api_session.invoke_api(module, 'api')
+        self.assertEqual(result, ret)
+        vim_obj.SessionIsActive.assert_called_once_with(
+            vim_obj.service_content.sessionManager,
+            sessionID=api_session._session_id,
+            userName=api_session._session_username)
+        api_session.create_session.assert_called_once_with()

cinder/volume/drivers/vmware/api.py

@@ -94,7 +94,6 @@ class Retry(object):
 class VMwareAPISession(object):
     """Sets up a session with the server and handles all calls made to it."""
 
-    @Retry(exceptions=(Exception))
     def __init__(self, server_ip, server_username, server_password,
                  api_retry_count, task_poll_interval, scheme='https',
                  create_session=True, wsdl_loc=None, pbm_wsdl=None):
@@ -145,6 +144,7 @@ class VMwareAPISession(object):
                                       host=self._server_ip)
         return self._pbm
 
+    @Retry(exceptions=(error_util.VimConnectionException,))
     def create_session(self):
         """Establish session with the server."""
         # Login and setup the session with the server for making
@@ -213,7 +213,8 @@ class VMwareAPISession(object):
         """
 
         @Retry(max_retry_count=self._api_retry_count,
-               exceptions=(error_util.VimException))
+               exceptions=(error_util.SessionOverLoadException,
+                           error_util.VimConnectionException))
         def _invoke_api(module, method, *args, **kwargs):
             while True:
                 try:

cinder/volume/drivers/vmware/error_util.py

@@ -39,11 +39,16 @@ class VimAttributeException(VimException):
     pass
 
 
-class VimFaultException(exception.VolumeBackendAPIException):
-    """The VIM Fault exception class."""
+class VimConnectionException(VimException):
+    """Thrown when there is a connection problem."""
+    pass
+
+
+class VimFaultException(VimException):
+    """Exception thrown when there are faults during VIM API calls."""
 
     def __init__(self, fault_list, msg):
-        exception.VolumeBackendAPIException.__init__(self, msg)
+        super(VimFaultException, self).__init__(msg)
         self.fault_list = fault_list
 
 

cinder/volume/drivers/vmware/vim.py

@@ -18,6 +18,7 @@ Classes for making VMware VI SOAP calls.
 """
 
 import httplib
+import urllib2
 
 import suds
 
@@ -193,6 +194,12 @@ class Vim(object):
                                                           {'attr': attr_name,
                                                            'excep': excep})
 
+            except (urllib2.URLError, urllib2.HTTPError) as excep:
+                raise error_util.VimConnectionException(
+                    _("urllib2 error in %(attr)s: %(excep)s.") %
+                    {'attr': attr_name,
+                     'excep': excep})
+
             except Exception as excep:
                 # Socket errors which need special handling for they
                 # might be caused by server API call overload