From 13d5a757db5add0881c3a266bb71226c6f0fa045 Mon Sep 17 00:00:00 2001 From: Michael McAleer Date: Thu, 3 Sep 2020 11:44:59 +0100 Subject: [PATCH] PowerMax Driver - REST Iterator Expiration Fix This change addresses an issue whereby the Unisphere REST data iterator expires before all data can be read from it. Change-Id: Ifa953f72ca670843c25c5be04a004835c348476c Closes-Bug: #1894086 --- .../dell_emc/powermax/test_powermax_rest.py | 47 ++++++++++++++++++- .../volume/drivers/dell_emc/powermax/rest.py | 35 ++++++++++++++ ...-iterator-expiration-674a28d8b9e13b34.yaml | 8 ++++ 3 files changed, 89 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/powermax-bug-1894086-iterator-expiration-674a28d8b9e13b34.yaml diff --git a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py index fa8684728da..6df340b808b 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py @@ -1710,7 +1710,42 @@ class PowerMaxRestTest(test.TestCase): volume = self.rest.get_private_volume_list(array_id) self.assertEqual(response, volume) - def test_get_iterator_list(self): + @mock.patch.object(rest.PowerMaxRest, 'get_resource') + def test_get_private_volume_list_params_dict_input(self, mck_get): + array_id = self.data.array + input_param = {'unit-test': True} + ref = {'unit-test': True, + 'expiration_time_mins': rest.ITERATOR_EXPIRATION} + + self.rest.get_private_volume_list(array_id, input_param) + mck_get.assert_called_once_with( + self.data.array, rest.SLOPROVISIONING, 'volume', params=ref, + private='/private') + + @mock.patch.object(rest.PowerMaxRest, 'get_resource') + def test_get_private_volume_list_params_str_input(self, mck_get): + array_id = self.data.array + input_param = '&unit-test=True' + ref = '&unit-test=True&expiration_time_mins=%(expire)s' % { + 'expire': rest.ITERATOR_EXPIRATION} + + self.rest.get_private_volume_list(array_id, input_param) + mck_get.assert_called_once_with( + self.data.array, rest.SLOPROVISIONING, 'volume', params=ref, + private='/private') + + @mock.patch.object(rest.PowerMaxRest, 'get_resource') + def test_get_private_volume_list_params_no_input(self, mck_get): + array_id = self.data.array + ref = {'expiration_time_mins': rest.ITERATOR_EXPIRATION} + + self.rest.get_private_volume_list(array_id) + mck_get.assert_called_once_with( + self.data.array, rest.SLOPROVISIONING, 'volume', params=ref, + private='/private') + + @mock.patch.object(rest.PowerMaxRest, '_delete_iterator') + def test_get_iterator_list(self, mck_del): with mock.patch.object( self.rest, 'get_request', side_effect=[ self.data.rest_iterator_resonse_one, @@ -1731,8 +1766,18 @@ class PowerMaxRestTest(test.TestCase): actual_response = self.rest.get_iterator_page_list( iterator_id, result_count, start_position, end_position, max_page_size) + mck_del.assert_called_once_with(iterator_id) self.assertEqual(expected_response, actual_response) + @mock.patch.object(rest.PowerMaxRest, 'request', + return_value=(204, 'Deleted Iterator')) + def test_delete_iterator(self, mck_del): + iterator_id = 'test_iterator_id' + self.rest._delete_iterator(iterator_id) + mck_del.assert_called_once_with( + '/common/Iterator/%(iter)s' % {'iter': iterator_id}, + rest.DELETE) + def test_set_rest_credentials(self): array_info = { 'RestServerIp': '10.10.10.10', diff --git a/cinder/volume/drivers/dell_emc/powermax/rest.py b/cinder/volume/drivers/dell_emc/powermax/rest.py index 986df42dbb4..c14a5598a3b 100644 --- a/cinder/volume/drivers/dell_emc/powermax/rest.py +++ b/cinder/volume/drivers/dell_emc/powermax/rest.py @@ -51,6 +51,7 @@ STATUS_201 = 201 STATUS_202 = 202 STATUS_204 = 204 SERVER_ERROR_STATUS_CODES = [408, 501, 502, 503, 504] +ITERATOR_EXPIRATION = 180 # Job constants INCOMPLETE_LIST = ['created', 'unscheduled', 'scheduled', 'running', 'validating', 'validated'] @@ -1463,6 +1464,14 @@ class PowerMaxRest(object): :param params: filter parameters :returns: list -- dicts with volume information """ + if isinstance(params, dict): + params['expiration_time_mins'] = ITERATOR_EXPIRATION + elif isinstance(params, str): + params += '&expiration_time_mins=%(expire)s' % { + 'expire': ITERATOR_EXPIRATION} + else: + params = {'expiration_time_mins': ITERATOR_EXPIRATION} + return self.get_resource( array, SLOPROVISIONING, 'volume', params=params, private='/private') @@ -3255,6 +3264,9 @@ class PowerMaxRest(object): :param max_page_size: the max page size :returns: list -- merged results from multiple pages """ + LOG.debug('Iterator %(it)s contains %(cnt)s results.', { + 'it': iterator_id, 'cnt': result_count}) + iterator_result = [] has_more_entries = True @@ -3264,6 +3276,9 @@ class PowerMaxRest(object): has_more_entries = False params = {'to': end_position, 'from': start_position} + LOG.debug('Retrieving iterator %(it)s page %(st)s to %(fn)s', { + 'it': iterator_id, 'st': start_position, 'fn': end_position}) + target_uri = ('/common/Iterator/%(iterator_id)s/page' % { 'iterator_id': iterator_id}) iterator_response = self.get_request(target_uri, 'iterator', @@ -3275,8 +3290,28 @@ class PowerMaxRest(object): except (KeyError, TypeError): pass + LOG.info('All results extracted, deleting iterator %(it)s', { + 'it': iterator_id}) + self._delete_iterator(iterator_id) + return iterator_result + def _delete_iterator(self, iterator_id): + """Delete an iterator containing full request result list. + + Note: This should only be called once all required results have been + extracted from the iterator. + + :param iterator_id: the iterator ID -- str + """ + target_uri = self.build_uri( + category='common', resource_level='Iterator', + resource_level_id=iterator_id, no_version=True) + status_code, message = self.request(target_uri, DELETE) + operation = 'delete iterator' + self.check_status_code_success(operation, status_code, message) + LOG.info('Successfully deleted iterator %(it)s', {'it': iterator_id}) + def validate_unisphere_version(self): """Validate that the running Unisphere version meets min requirement diff --git a/releasenotes/notes/powermax-bug-1894086-iterator-expiration-674a28d8b9e13b34.yaml b/releasenotes/notes/powermax-bug-1894086-iterator-expiration-674a28d8b9e13b34.yaml new file mode 100644 index 00000000000..3d9b05d20a5 --- /dev/null +++ b/releasenotes/notes/powermax-bug-1894086-iterator-expiration-674a28d8b9e13b34.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + `Bug #1894086 `_: + PowerMax Cinder driver addresses an issue whereby Unisphere REST iterators + expire before all data can be read from them. The iterator expiration is now + set to 180mins and deleted once all data has been read so no artifacts are + left behind.