From: Michael Price Date: Fri, 2 Oct 2015 20:21:23 +0000 (-0500) Subject: NetApp: Fix issue with E-Series volume expand X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=f410bfb76a60e83e16ced20be6d4b3351de191d7;p=openstack-build%2Fcinder-build.git NetApp: Fix issue with E-Series volume expand The volume extend operation is using the wrong backend API to expand the volume capacity. A volume copy is being used to achieve the expansion for performance reasons, but this results in snapshots defined on the original volume being lost. This patch updates the volume extend API to use the correct method for expanding the volume capacity. For a thin-provisioned volume or a volume defined on a DDP, this operation is very fast. However, for a standard volume-group, this method will block until the expansion has completed, because the expansion capacity will not be available until completion. Closes-Bug: 1506212 Change-Id: I1cb91b96ede732a76204287043b63a2a67f805f4 --- diff --git a/cinder/tests/unit/volume/drivers/netapp/eseries/fakes.py b/cinder/tests/unit/volume/drivers/netapp/eseries/fakes.py index 68b81fd8a..1971150ca 100644 --- a/cinder/tests/unit/volume/drivers/netapp/eseries/fakes.py +++ b/cinder/tests/unit/volume/drivers/netapp/eseries/fakes.py @@ -471,6 +471,7 @@ VOLUME = VOLUMES[0] STORAGE_POOL = { 'label': 'DDP', + 'id': 'fakevolgroupref', 'volumeGroupRef': 'fakevolgroupref', 'raidLevel': 'raidDiskPool', 'usedSpace': '16413217521664', @@ -789,6 +790,22 @@ HARDWARE_INVENTORY = { ] } +FAKE_POOL_ACTION_PROGRESS = [ + { + "volumeRef": "0200000060080E50002998A00000945355C37C19", + "progressPercentage": 55, + "estimatedTimeToCompletion": 1, + "currentAction": "initializing" + }, + { + "volumeRef": "0200000060080E50002998A00000945355C37C18", + "progressPercentage": 0, + "estimatedTimeToCompletion": 0, + "currentAction": "progressDve" + }, +] + + FAKE_RESOURCE_URL = '/devmgr/v2/devmgr/utils/about' FAKE_APP_VERSION = '2015.2|2015.2.dev59|vendor|Linux-3.13.0-24-generic' FAKE_BACKEND = 'eseriesiSCSI' diff --git a/cinder/tests/unit/volume/drivers/netapp/eseries/test_client.py b/cinder/tests/unit/volume/drivers/netapp/eseries/test_client.py index dcd9fca5e..33a53d4d0 100644 --- a/cinder/tests/unit/volume/drivers/netapp/eseries/test_client.py +++ b/cinder/tests/unit/volume/drivers/netapp/eseries/test_client.py @@ -629,6 +629,19 @@ class NetAppEseriesClientDriverTestCase(test.TestCase): ) self.assertDictMatch(expected_volume, updated_volume) + def test_get_pool_operation_progress(self): + fake_pool = copy.deepcopy(eseries_fake.STORAGE_POOL) + fake_response = copy.deepcopy(eseries_fake.FAKE_POOL_ACTION_PROGRESS) + self.my_client._invoke = mock.Mock(return_value=fake_response) + + response = self.my_client.get_pool_operation_progress(fake_pool['id']) + + url = self.my_client.RESOURCE_PATHS.get('pool_operation_progress') + self.my_client._invoke.assert_called_once_with('GET', url, + **{'object-id': + fake_pool['id']}) + self.assertEqual(fake_response, response) + def test_extend_volume(self): new_capacity = 10 fake_volume = copy.deepcopy(eseries_fake.VOLUME) @@ -638,27 +651,33 @@ class NetAppEseriesClientDriverTestCase(test.TestCase): self.my_client._invoke = mock.Mock(return_value=fake_volume) expanded_volume = self.my_client.expand_volume(fake_volume['id'], - new_capacity) + new_capacity, False) - url = self.my_client.RESOURCE_PATHS.get('ssc_volume') - body = {'newSize': new_capacity, 'sizeUnit': 'gb'} + url = self.my_client.RESOURCE_PATHS.get('volume_expand') + body = {'expansionSize': new_capacity, 'sizeUnit': 'gb'} self.my_client._invoke.assert_called_once_with('POST', url, body, **{'object-id': fake_volume['id']}) self.assertEqual(fake_volume, expanded_volume) - def test_extend_volume_unsupported(self): + def test_extend_volume_thin(self): new_capacity = 10 - min_version = 1 fake_volume = copy.deepcopy(eseries_fake.VOLUME) self.my_client.features = mock.Mock() self.my_client.features.SSC_API_V2 = na_utils.FeatureState( - supported=False, minimum_version=min_version) + supported=True) self.my_client._invoke = mock.Mock(return_value=fake_volume) - self.assertRaises(exception.NetAppDriverException, - self.my_client.expand_volume, fake_volume['id'], - new_capacity) + expanded_volume = self.my_client.expand_volume(fake_volume['id'], + new_capacity, True) + + url = self.my_client.RESOURCE_PATHS.get('thin_volume_expand') + body = {'newVirtualSize': new_capacity, 'sizeUnit': 'gb', + 'newRepositorySize': new_capacity} + self.my_client._invoke.assert_called_once_with('POST', url, body, + **{'object-id': + fake_volume['id']}) + self.assertEqual(fake_volume, expanded_volume) @ddt.data(True, False) def test_delete_volume(self, ssc_api_enabled): diff --git a/cinder/tests/unit/volume/drivers/netapp/eseries/test_library.py b/cinder/tests/unit/volume/drivers/netapp/eseries/test_library.py index de69391b6..8e435accb 100644 --- a/cinder/tests/unit/volume/drivers/netapp/eseries/test_library.py +++ b/cinder/tests/unit/volume/drivers/netapp/eseries/test_library.py @@ -1088,66 +1088,107 @@ class NetAppEseriesLibraryMultiAttachTestCase(test.TestCase): # Ensure the volume we created is not cleaned up self.assertEqual(0, self.library._client.delete_volume.call_count) - def test_extend_volume(self): + @ddt.data(False, True) + def test_get_pool_operation_progress(self, expect_complete): + """Validate the operation progress is interpreted correctly""" + + pool = copy.deepcopy(eseries_fake.STORAGE_POOL) + if expect_complete: + pool_progress = [] + else: + pool_progress = copy.deepcopy( + eseries_fake.FAKE_POOL_ACTION_PROGRESS) + + expected_actions = set(action['currentAction'] for action in + pool_progress) + expected_eta = reduce(lambda x, y: x + y['estimatedTimeToCompletion'], + pool_progress, 0) + + self.library._client.get_pool_operation_progress = mock.Mock( + return_value=pool_progress) + + complete, actions, eta = self.library._get_pool_operation_progress( + pool['id']) + self.assertEqual(expect_complete, complete) + self.assertEqual(expected_actions, actions) + self.assertEqual(expected_eta, eta) + + @ddt.data(False, True) + def test_get_pool_operation_progress_with_action(self, expect_complete): + """Validate the operation progress is interpreted correctly""" + + expected_action = 'fakeAction' + pool = copy.deepcopy(eseries_fake.STORAGE_POOL) + if expect_complete: + pool_progress = copy.deepcopy( + eseries_fake.FAKE_POOL_ACTION_PROGRESS) + for progress in pool_progress: + progress['currentAction'] = 'none' + else: + pool_progress = copy.deepcopy( + eseries_fake.FAKE_POOL_ACTION_PROGRESS) + pool_progress[0]['currentAction'] = expected_action + + expected_actions = set(action['currentAction'] for action in + pool_progress) + expected_eta = reduce(lambda x, y: x + y['estimatedTimeToCompletion'], + pool_progress, 0) + + self.library._client.get_pool_operation_progress = mock.Mock( + return_value=pool_progress) + + complete, actions, eta = self.library._get_pool_operation_progress( + pool['id'], expected_action) + self.assertEqual(expect_complete, complete) + self.assertEqual(expected_actions, actions) + self.assertEqual(expected_eta, eta) + + @mock.patch('eventlet.greenthread.sleep') + def test_extend_volume(self, _mock_sleep): + """Test volume extend with a thick-provisioned volume""" + + def get_copy_progress(): + for eta in xrange(5, -1, -1): + action_status = 'none' if eta == 0 else 'remappingDve' + complete = action_status == 'none' + yield complete, action_status, eta + fake_volume = copy.deepcopy(get_fake_volume()) volume = copy.deepcopy(eseries_fake.VOLUME) new_capacity = 10 volume['objectType'] = 'volume' - self.library.create_cloned_volume = mock.Mock() + self.library._client.expand_volume = mock.Mock() + self.library._get_pool_operation_progress = mock.Mock( + side_effect=get_copy_progress()) self.library._get_volume = mock.Mock(return_value=volume) - self.library._client.update_volume = mock.Mock() self.library.extend_volume(fake_volume, new_capacity) - self.library.create_cloned_volume.assert_called_with(mock.ANY, - fake_volume) + # Ensure that the extend method waits until the expansion is completed + self.assertEqual(6, + self.library._get_pool_operation_progress.call_count + ) + self.library._client.expand_volume.assert_called_with(volume['id'], + new_capacity, + False) def test_extend_volume_thin(self): + """Test volume extend with a thin-provisioned volume""" + fake_volume = copy.deepcopy(get_fake_volume()) volume = copy.deepcopy(eseries_fake.VOLUME) new_capacity = 10 volume['objectType'] = 'thinVolume' self.library._client.expand_volume = mock.Mock(return_value=volume) + self.library._get_volume_operation_progress = mock.Mock() self.library._get_volume = mock.Mock(return_value=volume) self.library.extend_volume(fake_volume, new_capacity) + self.assertFalse(self.library._get_volume_operation_progress.called) self.library._client.expand_volume.assert_called_with(volume['id'], - new_capacity) - - def test_extend_volume_stage_2_failure(self): - fake_volume = copy.deepcopy(get_fake_volume()) - volume = copy.deepcopy(eseries_fake.VOLUME) - new_capacity = 10 - volume['objectType'] = 'volume' - self.library.create_cloned_volume = mock.Mock() - self.library._client.delete_volume = mock.Mock() - # Create results for multiple calls to _get_volume and _update_volume - get_volume_results = [volume, {'id': 'newId', 'label': 'newVolume'}] - self.library._get_volume = mock.Mock(side_effect=get_volume_results) - update_volume_results = [volume, exception.NetAppDriverException, - volume] - self.library._client.update_volume = mock.Mock( - side_effect=update_volume_results) - - self.assertRaises(exception.NetAppDriverException, - self.library.extend_volume, fake_volume, - new_capacity) - self.assertTrue(self.library._client.delete_volume.called) - - def test_extend_volume_stage_1_failure(self): - fake_volume = copy.deepcopy(get_fake_volume()) - volume = copy.deepcopy(eseries_fake.VOLUME) - new_capacity = 10 - volume['objectType'] = 'volume' - self.library.create_cloned_volume = mock.Mock() - self.library._get_volume = mock.Mock(return_value=volume) - self.library._client.update_volume = mock.Mock( - side_effect=exception.NetAppDriverException) - - self.assertRaises(exception.NetAppDriverException, - self.library.extend_volume, fake_volume, - new_capacity) + new_capacity, + True) def test_delete_non_existing_volume(self): volume2 = get_fake_volume() diff --git a/cinder/volume/drivers/netapp/eseries/client.py b/cinder/volume/drivers/netapp/eseries/client.py index a97ced89d..bedd742d4 100644 --- a/cinder/volume/drivers/netapp/eseries/client.py +++ b/cinder/volume/drivers/netapp/eseries/client.py @@ -105,6 +105,13 @@ class RestClient(WebserviceClient): RESOURCE_PATHS = { 'volumes': '/storage-systems/{system-id}/volumes', 'volume': '/storage-systems/{system-id}/volumes/{object-id}', + 'pool_operation_progress': + '/storage-systems/{system-id}/storage-pools/{object-id}' + '/action-progress', + 'volume_expand': + '/storage-systems/{system-id}/volumes/{object-id}/expand', + 'thin_volume_expand': + '/storage-systems/{system-id}/thin-volumes/{object-id}/expand', 'ssc_volumes': '/storage-systems/{system-id}/ssc/volumes', 'ssc_volume': '/storage-systems/{system-id}/ssc/volumes/{object-id}' } @@ -396,20 +403,38 @@ class RestClient(WebserviceClient): data = {'name': label} return self._invoke('POST', path, data, **{'object-id': object_id}) - def expand_volume(self, object_id, new_capacity, capacity_unit='gb'): - """Increase the capacity of a volume""" - if not self.features.SSC_API_V2: - msg = _("E-series proxy API version %(current_version)s does " - "not support this expansion API. The proxy" - " version must be at at least %(min_version)s") - min_version = self.features.SSC_API_V2.minimum_version - msg_args = {'current_version': self.api_version, - 'min_version': min_version} - raise exception.NetAppDriverException(msg % msg_args) + def get_pool_operation_progress(self, object_id): + """Retrieve the progress long-running operations on a storage pool + + Example: + [ + { + "volumeRef": "3232....", # Volume being referenced + "progressPercentage": 0, # Approxmate percent complete + "estimatedTimeToCompletion": 0, # ETA in minutes + "currentAction": "none" # Current volume action + } + ... + ] + + :param object_id: A pool id + :returns: A dict representing the action progress + """ + path = self.RESOURCE_PATHS.get('pool_operation_progress') + return self._invoke('GET', path, **{'object-id': object_id}) - path = self.RESOURCE_PATHS.get('ssc_volume') - data = {'newSize': new_capacity, 'sizeUnit': capacity_unit} - return self._invoke('POST', path, data, **{'object-id': object_id}) + def expand_volume(self, object_id, new_capacity, thin_provisioned, + capacity_unit='gb'): + """Increase the capacity of a volume""" + if thin_provisioned: + path = self.RESOURCE_PATHS.get('thin_volume_expand') + data = {'newVirtualSize': new_capacity, 'sizeUnit': capacity_unit, + 'newRepositorySize': new_capacity} + return self._invoke('POST', path, data, **{'object-id': object_id}) + else: + path = self.RESOURCE_PATHS.get('volume_expand') + data = {'expansionSize': new_capacity, 'sizeUnit': capacity_unit} + return self._invoke('POST', path, data, **{'object-id': object_id}) def get_volume_mappings(self): """Creates volume mapping on array.""" diff --git a/cinder/volume/drivers/netapp/eseries/library.py b/cinder/volume/drivers/netapp/eseries/library.py index 2c90a5bda..81f4425a7 100644 --- a/cinder/volume/drivers/netapp/eseries/library.py +++ b/cinder/volume/drivers/netapp/eseries/library.py @@ -1170,32 +1170,67 @@ class NetAppESeriesLibrary(object): return volume.get('objectType') == 'thinVolume' or volume.get( 'thinProvisioned', False) + def _get_pool_operation_progress(self, pool_id, action=None): + """Retrieve the progress of a long running operation on a pool + + The return will be a tuple containing: a bool representing whether + or not the operation is complete, a set of actions that are + currently running on the storage pool, and the estimated time + remaining in minutes. + + An action type may be passed in such that once no actions of that type + remain active on the pool, the operation will be considered + completed. If no action str is passed in, it is assumed that + multiple actions compose the operation, and none are terminal, + so the operation will not be considered completed until there are no + actions remaining to be completed on any volume on the pool. + + :param pool_id: The id of a storage pool + :param action: The anticipated action + :return: A tuple (bool, set(str), int) + """ + actions = set() + eta = 0 + for progress in self._client.get_pool_operation_progress(pool_id): + actions.add(progress.get('currentAction')) + eta += progress.get('estimatedTimeToCompletion', 0) + if action is not None: + complete = action not in actions + else: + complete = not actions + return complete, actions, eta + def extend_volume(self, volume, new_size): """Extend an existing volume to the new size.""" src_vol = self._get_volume(volume['name_id']) - if self._is_thin_provisioned(src_vol): - self._client.expand_volume(src_vol['id'], new_size) - else: - stage_1, stage_2 = 0, 0 - src_label = src_vol['label'] - stage_label = 'tmp-%s' % utils.convert_uuid_to_es_fmt(uuid.uuid4()) - extend_vol = {'id': uuid.uuid4(), 'size': new_size} - self.create_cloned_volume(extend_vol, volume) - new_vol = self._get_volume(extend_vol['id']) - try: - stage_1 = self._client.update_volume(src_vol['id'], - stage_label) - stage_2 = self._client.update_volume(new_vol['id'], src_label) - new_vol = stage_2 - LOG.info(_LI('Extended volume with label %s.'), src_label) - except exception.NetAppDriverException: - if stage_1 == 0: - with excutils.save_and_reraise_exception(): - self._client.delete_volume(new_vol['id']) - elif stage_2 == 0: - with excutils.save_and_reraise_exception(): - self._client.update_volume(src_vol['id'], src_label) - self._client.delete_volume(new_vol['id']) + thin_provisioned = self._is_thin_provisioned(src_vol) + self._client.expand_volume(src_vol['id'], new_size, thin_provisioned) + + # If the volume is thin or defined on a disk pool, there is no need + # to block. + if not (thin_provisioned or src_vol.get('diskPool')): + # Wait for the expansion to start + + def check_progress(): + complete, actions, eta = ( + self._get_pool_operation_progress(src_vol[ + 'volumeGroupRef'], + 'remappingDve')) + if complete: + raise loopingcall.LoopingCallDone() + else: + msg = _LI("Waiting for volume expansion of %(vol)s to " + "complete, current remaining actions are " + "%(action)s. ETA: %(eta)s mins.") + LOG.info(msg, {'vol': volume['name_id'], + 'action': ', '.join(actions), 'eta': eta}) + + checker = loopingcall.FixedIntervalLoopingCall( + check_progress) + + checker.start(interval=self.SLEEP_SECS, + initial_delay=self.SLEEP_SECS, + stop_on_exception=True).wait() def _garbage_collect_tmp_vols(self): """Removes tmp vols with no snapshots."""