From: Vipin Balachandran Date: Wed, 16 Apr 2014 08:36:31 +0000 (+0530) Subject: vmware: Fix problems with VIM API retry logic X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=bc115eaa477d30826ac17986a6e45844b456e206;p=openstack-build%2Fcinder-build.git 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 09aa53615..ccea4ae71 100644 --- 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 68e6844fc..df3348db4 100644 --- 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 fdfc36210..fc69d4326 100644 --- 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 d7f85ada8..2135a5672 100644 --- 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