From: Abhishek Shrivastava Date: Mon, 13 Jul 2015 11:00:59 +0000 (+0530) Subject: Adding delete-wait-loop for CloudByte Volumes X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=dddc995b0981327c6527bdfbc042582cc998f54b;p=openstack-build%2Fcinder-build.git Adding delete-wait-loop for CloudByte Volumes Deleting volumes in CloudByte Storage is an asynchronous process. So, the driver needs to wait till the job/process is completed. The driver uses the jobid in a wait loop for a configured interval of time, till the volume deletion is completed. Added two new config options for this operation: "cb_confirm_volume_delete_retry_interval" "cb_confirm_volume_delete_retries" DocImpact Change-Id: I8139ecfb1b49c0b69ed956e3a53b5bb895ffc1fa --- diff --git a/cinder/tests/unit/test_cloudbyte.py b/cinder/tests/unit/test_cloudbyte.py index 662908dc7..e51aecf39 100644 --- a/cinder/tests/unit/test_cloudbyte.py +++ b/cinder/tests/unit/test_cloudbyte.py @@ -351,11 +351,8 @@ FAKE_LIST_ISCSI_INITIATOR_RESPONSE = """{ "listInitiatorsResponse" : { }}""" # A fake delete file system response of cloudbyte's elasticenter -FAKE_DELETE_FILE_SYSTEM_RESPONSE = """{ "deleteResponse" : { - "response" : [{ - "code": "0", - "description": "success" - }] +FAKE_DELETE_FILE_SYSTEM_RESPONSE = """{ "deleteFileSystemResponse" : { + "jobid": "e1fe861a-17e3-41b5-ae7c-937caac62cdf" }}""" # A fake create storage snapshot response of cloudbyte's elasticenter @@ -666,6 +663,8 @@ class CloudByteISCSIDriverTestCase(testtools.TestCase): self.driver.configuration.cb_tsm_name = 'openstack' self.driver.configuration.cb_account_name = 'CustomerA' self.driver.configuration.cb_auth_group = 'fakeauthgroup' + self.driver.configuration.cb_apikey = 'G4ZUB39WH7lbiZhPhL3nbd' + self.driver.configuration.san_ip = '172.16.51.30' def _side_effect_api_req(self, cmd, params, version='1.0'): """This is a side effect function. @@ -683,6 +682,22 @@ class CloudByteISCSIDriverTestCase(testtools.TestCase): return MAP_COMMAND_TO_FAKE_RESPONSE[cmd] + def _side_effect_api_req_to_delete_file_system( + self, cmd, params, version='1.0'): + """This is a side effect function.""" + if cmd == 'deleteFileSystem': + return {} + + return MAP_COMMAND_TO_FAKE_RESPONSE[cmd] + + def _side_effect_api_req_to_query_asyncjob_response( + self, cmd, params, version='1.0'): + """This is a side effect function.""" + if cmd == 'queryAsyncJobResult': + return {} + + return MAP_COMMAND_TO_FAKE_RESPONSE[cmd] + def _side_effect_api_req_to_query_asyncjob( self, cmd, params, version='1.0'): """This is a side effect function.""" @@ -864,8 +879,8 @@ class CloudByteISCSIDriverTestCase(testtools.TestCase): # run the test self.driver.delete_volume(volume) - # assert that 2 api calls were invoked - self.assertEqual(2, mock_api_req.call_count) + # assert that 3 api calls were invoked + self.assertEqual(3, mock_api_req.call_count) # Test-II @@ -880,6 +895,43 @@ class CloudByteISCSIDriverTestCase(testtools.TestCase): # assert that no api calls were invoked self.assertEqual(0, mock_api_req.call_count) + # Test-III + + # re-configure the dependencies + volume['provider_id'] = fake_volume_id + + # reset & re-configure mock + mock_api_req.reset_mock() + + # configure or re-configure the mocks + mock_api_req.side_effect = ( + self._side_effect_api_req_to_delete_file_system) + + # Now run the test & assert the exception + self.assertRaises(exception.VolumeBackendAPIException, + self.driver.delete_volume, + volume) + + # assert that 2 api calls were invoked + self.assertEqual(2, mock_api_req.call_count) + + # Test - IV + + # reset the mocks + mock_api_req.reset_mock() + + # configure or re-configure the mocks + mock_api_req.side_effect = ( + self._side_effect_api_req_to_query_asyncjob_response) + + # Now run the test & assert the exception + self.assertRaises(exception.VolumeBackendAPIException, + self.driver.delete_volume, + volume) + + # assert that 3 api calls were invoked + self.assertEqual(3, mock_api_req.call_count) + @mock.patch.object(cloudbyte.CloudByteISCSIDriver, '_api_request_for_cloudbyte') def test_delete_snapshot(self, mock_api_req): @@ -1052,13 +1104,10 @@ class CloudByteISCSIDriverTestCase(testtools.TestCase): mock_api_req.reset_mock() mock_api_req.side_effect = self._side_effect_api_req - # now run the test & assert the exception - with testtools.ExpectedException( - exception.VolumeBackendAPIException, - "Bad or unexpected response from the storage volume " - "backend API: Volume \[NotExists\] not found in " - "CloudByte storage."): - self.driver.create_volume(volume) + # Now run the test & assert the exception + self.assertRaises(exception.VolumeBackendAPIException, + self.driver.create_volume, + volume) # Test - IV @@ -1071,13 +1120,10 @@ class CloudByteISCSIDriverTestCase(testtools.TestCase): # configure or re-configure the mocks mock_api_req.side_effect = self._side_effect_api_req_to_create_vol - # now run the test & assert the exception - with testtools.ExpectedException( - exception.VolumeBackendAPIException, - 'Bad or unexpected response from the storage volume ' - 'backend API: Null response received while ' - 'creating volume'): - self.driver.create_volume(volume) + # Now run the test & assert the exception + self.assertRaises(exception.VolumeBackendAPIException, + self.driver.create_volume, + volume) # Test - V @@ -1088,13 +1134,10 @@ class CloudByteISCSIDriverTestCase(testtools.TestCase): # configure or re-configure the mocks mock_api_req.side_effect = self._side_effect_api_req_to_list_filesystem - # now run the test - with testtools.ExpectedException( - exception.VolumeBackendAPIException, - "Bad or unexpected response from the storage volume " - "backend API: Null response received from CloudByte's " - "list filesystem."): - self.driver.create_volume(volume) + # Now run the test & assert the exception + self.assertRaises(exception.VolumeBackendAPIException, + self.driver.create_volume, + volume) # Test - VI @@ -1107,13 +1150,10 @@ class CloudByteISCSIDriverTestCase(testtools.TestCase): mock_api_req.side_effect = ( self._side_effect_api_req_to_list_vol_iscsi_service) - # now run the test - with testtools.ExpectedException( - exception.VolumeBackendAPIException, - "Bad or unexpected response from the storage volume " - "backend API: Null response received from CloudByte's " - "list volume iscsi service."): - self.driver.create_volume(volume) + # Now run the test & assert the exception + self.assertRaises(exception.VolumeBackendAPIException, + self.driver.create_volume, + volume) # Test - VII @@ -1125,13 +1165,10 @@ class CloudByteISCSIDriverTestCase(testtools.TestCase): mock_api_req.side_effect = ( self._side_effect_api_req_to_list_iscsi_initiator) - # now run the test - with testtools.ExpectedException( - exception.VolumeBackendAPIException, - "Bad or unexpected response from the storage volume " - "backend API: Null response received from CloudByte's " - "list iscsi initiators."): - self.driver.create_volume(volume) + # Now run the test & assert the exception + self.assertRaises(exception.VolumeBackendAPIException, + self.driver.create_volume, + volume) # Test - VIII @@ -1146,13 +1183,42 @@ class CloudByteISCSIDriverTestCase(testtools.TestCase): mock_api_req.side_effect = ( self._none_response_to_list_tsm) - # now run the test - with testtools.ExpectedException( - exception.VolumeBackendAPIException, - "Bad or unexpected response from the storage volume " - "backend API: TSM \[openstack\] was not found in CloudByte " - "storage for account \[CustomerA\]."): - self.driver.create_volume(volume) + # Now run the test & assert the exception + self.assertRaises(exception.VolumeBackendAPIException, + self.driver.create_volume, + volume) + + # Test - IX + + volume['id'] = fake_volume_id + volume['size'] = 22 + + # reconfigure the dependencies + # reset the mocks + mock_api_req.reset_mock() + + # configure or re-configure the mocks + mock_api_req.side_effect = ( + self._side_effect_api_req_to_create_vol) + + # Now run the test & assert the exception + self.assertRaises(exception.VolumeBackendAPIException, + self.driver.create_volume, + volume) + + # Test - X + + # reset the mocks + mock_api_req.reset_mock() + + # configure or re-configure the mocks + mock_api_req.side_effect = ( + self._side_effect_api_req_to_query_asyncjob_response) + + # Now run the test & assert the exception + self.assertRaises(exception.VolumeBackendAPIException, + self.driver.create_volume, + volume) @mock.patch.object(cloudbyte.CloudByteISCSIDriver, '_api_request_for_cloudbyte') diff --git a/cinder/volume/drivers/cloudbyte/cloudbyte.py b/cinder/volume/drivers/cloudbyte/cloudbyte.py index c6c8e6bc6..7ac680cf8 100644 --- a/cinder/volume/drivers/cloudbyte/cloudbyte.py +++ b/cinder/volume/drivers/cloudbyte/cloudbyte.py @@ -37,9 +37,10 @@ class CloudByteISCSIDriver(san.SanISCSIDriver): Version history: 1.0.0 - Initial driver 1.1.0 - Add chap support and minor bug fixes + 1.1.1 - Add wait logic for delete volumes """ - VERSION = '1.1.0' + VERSION = '1.1.1' volume_stats = {} def __init__(self, *args, **kwargs): @@ -256,6 +257,53 @@ class CloudByteISCSIDriver(san.SanISCSIDriver): return tsmdetails + def _retry_volume_operation(self, operation, retries, + max_retries, jobid, + cb_volume): + """CloudByte async calls via the FixedIntervalLoopingCall.""" + + # Query the CloudByte storage with this jobid + volume_response = self._queryAsyncJobResult_request(jobid) + count = retries['count'] + + result_res = None + if volume_response is not None: + result_res = volume_response.get('queryasyncjobresultresponse') + + if result_res is None: + msg = (_( + "Null response received while querying " + "for [%(operation)s] based job [%(job)s] " + "at CloudByte storage.") % + {'operation': operation, 'job': jobid}) + raise exception.VolumeBackendAPIException(data=msg) + + status = result_res.get('jobstatus') + + if status == 1: + LOG.info(_LI("CloudByte operation [%(operation)s] succeeded for " + "volume [%(cb_volume)s]."), + {'operation': operation, 'cb_volume': cb_volume}) + raise loopingcall.LoopingCallDone() + elif count == max_retries: + # All attempts exhausted + LOG.error(_LE("CloudByte operation [%(operation)s] failed" + " for volume [%(vol)s]. Exhausted all" + " [%(max)s] attempts."), + {'operation': operation, + 'vol': cb_volume, + 'max': max_retries}) + raise loopingcall.LoopingCallDone(retvalue=False) + else: + count += 1 + retries['count'] = count + LOG.debug("CloudByte operation [%(operation)s] for" + " volume [%(vol)s]: retry [%(retry)s] of [%(max)s].", + {'operation': operation, + 'vol': cb_volume, + 'retry': count, + 'max': max_retries}) + def _wait_for_volume_creation(self, volume_response, cb_volume_name): """Given the job wait for it to complete.""" @@ -269,69 +317,57 @@ class CloudByteISCSIDriver(san.SanISCSIDriver): jobid = vol_res.get('jobid') if jobid is None: - msg = _("Jobid not found in CloudByte's " + msg = _("Job id not found in CloudByte's " "create volume [%s] response.") % cb_volume_name raise exception.VolumeBackendAPIException(data=msg) - def _retry_check_for_volume_creation(): - """Called at an interval till the volume is created.""" - - retries = kwargs['retries'] - max_retries = kwargs['max_retries'] - jobid = kwargs['jobid'] - cb_vol = kwargs['cb_vol'] + retry_interval = ( + self.configuration.cb_confirm_volume_create_retry_interval) - # Query the CloudByte storage with this jobid - volume_response = self._queryAsyncJobResult_request(jobid) + max_retries = ( + self.configuration.cb_confirm_volume_create_retries) + retries = {'count': 0} - result_res = None - if volume_response is not None: - result_res = volume_response.get('queryasyncjobresultresponse') + timer = loopingcall.FixedIntervalLoopingCall( + self._retry_volume_operation, + 'Create Volume', + retries, + max_retries, + jobid, + cb_volume_name) + timer.start(interval=retry_interval).wait() - if volume_response is None or result_res is None: - msg = _( - "Null response received while querying " - "for create volume job [%s] " - "at CloudByte storage.") % jobid - raise exception.VolumeBackendAPIException(data=msg) + def _wait_for_volume_deletion(self, volume_response, cb_volume_id): + """Given the job wait for it to complete.""" - status = result_res.get('jobstatus') + vol_res = volume_response.get('deleteFileSystemResponse') - if status == 1: - LOG.info(_LI("Volume [%s] created successfully in " - "CloudByte storage."), cb_vol) - raise loopingcall.LoopingCallDone() + if vol_res is None: + msg = _("Null response received while deleting volume [%s] " + "at CloudByte storage.") % cb_volume_id + raise exception.VolumeBackendAPIException(data=msg) - elif retries == max_retries: - # All attempts exhausted - LOG.error(_LE("Error in creating volume [%(vol)s] in " - "CloudByte storage. " - "Exhausted all [%(max)s] attempts."), - {'vol': cb_vol, 'max': retries}) - raise loopingcall.LoopingCallDone(retvalue=False) + jobid = vol_res.get('jobid') - else: - retries += 1 - kwargs['retries'] = retries - LOG.debug("Wait for volume [%(vol)s] creation, " - "retry [%(retry)s] of [%(max)s].", - {'vol': cb_vol, - 'retry': retries, - 'max': max_retries}) + if jobid is None: + msg = _("Job id not found in CloudByte's " + "delete volume [%s] response.") % cb_volume_id + raise exception.VolumeBackendAPIException(data=msg) retry_interval = ( - self.configuration.cb_confirm_volume_create_retry_interval) + self.configuration.cb_confirm_volume_delete_retry_interval) max_retries = ( - self.configuration.cb_confirm_volume_create_retries) - - kwargs = {'retries': 0, - 'max_retries': max_retries, - 'jobid': jobid, - 'cb_vol': cb_volume_name} + self.configuration.cb_confirm_volume_delete_retries) + retries = {'count': 0} timer = loopingcall.FixedIntervalLoopingCall( - _retry_check_for_volume_creation) + self._retry_volume_operation, + 'Delete Volume', + retries, + max_retries, + jobid, + cb_volume_id) timer.start(interval=retry_interval).wait() def _get_volume_id_from_response(self, cb_volumes, volume_name): @@ -794,7 +830,10 @@ class CloudByteISCSIDriver(san.SanISCSIDriver): if cb_volume_id is not None: params = {"id": cb_volume_id} - self._api_request_for_cloudbyte('deleteFileSystem', params) + del_res = self._api_request_for_cloudbyte('deleteFileSystem', + params) + + self._wait_for_volume_deletion(del_res, cb_volume_id) LOG.info( _LI("Successfully deleted volume [%(cb_vol)s] " diff --git a/cinder/volume/drivers/cloudbyte/options.py b/cinder/volume/drivers/cloudbyte/options.py index 581e3963f..c40cf35a5 100644 --- a/cinder/volume/drivers/cloudbyte/options.py +++ b/cinder/volume/drivers/cloudbyte/options.py @@ -17,15 +17,15 @@ from oslo_config import cfg cloudbyte_connection_opts = [ cfg.StrOpt("cb_apikey", - default="None", + default=None, help="Driver will use this API key to authenticate " "against the CloudByte storage's management interface."), cfg.StrOpt("cb_account_name", - default="None", + default=None, help="CloudByte storage specific account name. " "This maps to a project name in OpenStack."), cfg.StrOpt("cb_tsm_name", - default="None", + default=None, help="This corresponds to the name of " "Tenant Storage Machine (TSM) in CloudByte storage. " "A volume will be created in this TSM."), @@ -39,6 +39,16 @@ cloudbyte_connection_opts = [ help="Will confirm a successful volume " "creation in CloudByte storage by making " "this many number of attempts."), + cfg.IntOpt("cb_confirm_volume_delete_retry_interval", + default=5, + help="A retry value in seconds. Will be used by the driver " + "to check if volume deletion was successful in " + "CloudByte storage."), + cfg.IntOpt("cb_confirm_volume_delete_retries", + default=3, + help="Will confirm a successful volume " + "deletion in CloudByte storage by making " + "this many number of attempts."), cfg.StrOpt("cb_auth_group", default="None", help="This corresponds to the discovery authentication "