Merge "PowerMax Driver - REST Iterator Expiration Fix"

cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_rest.py

@@ -1725,7 +1725,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,
@@ -1746,8 +1781,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',

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']
@@ -1472,6 +1473,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')
@@ -3264,6 +3273,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
 
@@ -3273,6 +3285,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',
@@ -3284,8 +3299,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
 

releasenotes/notes/powermax-bug-1894086-iterator-expiration-674a28d8b9e13b34.yaml

@@ -0,0 +1,8 @@
+---
+fixes:
+  - |
+    `Bug #1894086 <https://bugs.launchpad.net/cinder/+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.