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

diff --git a/cinder/tests/test_vmware_api.py b/cinder/tests/test_vmware_api.py
index 09aa53615702c24040065d2c0178947bb3d33e38..ccea4ae71ce37a02cbad1a30f702632d608d1bce 100644 (file)
--- a/cinder/tests/test_vmware_api.py
+++ b/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()

diff --git a/cinder/volume/drivers/vmware/api.py b/cinder/volume/drivers/vmware/api.py
index 68e6844fc97c7b1cd6de383b1e6b4522efd1cd4e..df3348db44a327ce72e0f0fe4bf1d1276cf7a51e 100644 (file)
--- a/cinder/volume/drivers/vmware/api.py
+++ b/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:

diff --git a/cinder/volume/drivers/vmware/error_util.py b/cinder/volume/drivers/vmware/error_util.py
index fdfc36210d3715a51d6f98f80db2fb9bfce45ab4..fc69d432668fca085d794985925cac61050fcd24 100644 (file)
--- a/cinder/volume/drivers/vmware/error_util.py
+++ b/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
 
 

diff --git a/cinder/volume/drivers/vmware/vim.py b/cinder/volume/drivers/vmware/vim.py
index d7f85ada81dede8cddfcf22e1ed1f5e45cce550f..2135a5672556241e894ede4e3f59dfb26ea031da 100644 (file)
--- a/cinder/volume/drivers/vmware/vim.py
+++ b/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