From 6984e0921e629c8b1b6e1f160f093818d233cb2f Mon Sep 17 00:00:00 2001 From: Michael Price Date: Tue, 25 Aug 2015 17:36:44 -0500 Subject: [PATCH] NetApp: Fix volume extend with E-Series Thin provisioned volumes were being converted to thick provisioned volumes by extend_volume. When thin provisioning support was added, the extend_volume method was not updated to use the correct API for performing the capacity expansion operation. This patch changes the volume extension strategy when a volume is thin provisioned to use the correct API. With a thick provisioned volume, a clone will be created with the new capacity, and the old volume removed, but a thin provisioned volume's capacity can simply be expanded directly, because that is merely the capacity that the user sees, as opposed to the backend provisioned capacity. Closes-Bug: 1493874 Change-Id: Iec25f29381d5fba3409d1de83bf6816ae1a9ac22 --- .../drivers/netapp/eseries/test_client.py | 32 ++++++++++ .../drivers/netapp/eseries/test_library.py | 61 +++++++++++++++++++ .../volume/drivers/netapp/eseries/client.py | 15 +++++ .../volume/drivers/netapp/eseries/library.py | 47 ++++++++------ 4 files changed, 136 insertions(+), 19 deletions(-) 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 95a05e9b9..686166c0c 100644 --- a/cinder/tests/unit/volume/drivers/netapp/eseries/test_client.py +++ b/cinder/tests/unit/volume/drivers/netapp/eseries/test_client.py @@ -52,6 +52,7 @@ class NetAppEseriesClientDriverTestCase(test.TestCase): fake_response = mock.Mock() fake_response.status_code = 200 self.my_client.invoke_service = mock.Mock(return_value=fake_response) + self.my_client.api_version = '01.52.9000.1' @ddt.data(200, 201, 203, 204) def test_eval_response_success(self, status_code): @@ -631,6 +632,37 @@ class NetAppEseriesClientDriverTestCase(test.TestCase): ) self.assertDictMatch(expected_volume, updated_volume) + def test_extend_volume(self): + new_capacity = 10 + fake_volume = copy.deepcopy(eseries_fake.VOLUME) + self.my_client.features = mock.Mock() + self.my_client.features.SSC_API_V2 = na_utils.FeatureState( + supported=True) + self.my_client._invoke = mock.Mock(return_value=fake_volume) + + expanded_volume = self.my_client.expand_volume(fake_volume['id'], + new_capacity) + + url = self.my_client.RESOURCE_PATHS.get('ssc_volume') + body = {'newSize': 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): + 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) + self.my_client._invoke = mock.Mock(return_value=fake_volume) + + self.assertRaises(exception.NetAppDriverException, + self.my_client.expand_volume, fake_volume['id'], + new_capacity) + @ddt.data(True, False) def test_delete_volume(self, ssc_api_enabled): fake_volume = copy.deepcopy(eseries_fake.VOLUME) 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 3657af40e..eb0f9a483 100644 --- a/cinder/tests/unit/volume/drivers/netapp/eseries/test_library.py +++ b/cinder/tests/unit/volume/drivers/netapp/eseries/test_library.py @@ -1051,6 +1051,67 @@ 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): + 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() + + self.library.extend_volume(fake_volume, new_capacity) + + self.library.create_cloned_volume.assert_called_with(mock.ANY, + fake_volume) + + def test_extend_volume_thin(self): + 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 = mock.Mock(return_value=volume) + + self.library.extend_volume(fake_volume, new_capacity) + + 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) + def test_delete_non_existing_volume(self): volume2 = get_fake_volume() # Change to a nonexistent id. diff --git a/cinder/volume/drivers/netapp/eseries/client.py b/cinder/volume/drivers/netapp/eseries/client.py index e045d36cf..7b96f02ad 100644 --- a/cinder/volume/drivers/netapp/eseries/client.py +++ b/cinder/volume/drivers/netapp/eseries/client.py @@ -350,6 +350,21 @@ class RestClient(object): 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) + + 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 get_volume_mappings(self): """Creates volume mapping on array.""" path = "/storage-systems/{system-id}/volume-mappings" diff --git a/cinder/volume/drivers/netapp/eseries/library.py b/cinder/volume/drivers/netapp/eseries/library.py index 8ed9765f5..1f6909151 100644 --- a/cinder/volume/drivers/netapp/eseries/library.py +++ b/cinder/volume/drivers/netapp/eseries/library.py @@ -1144,28 +1144,37 @@ class NetAppESeriesLibrary(object): "%s."), size_gb) return avl_pools + def _is_thin_provisioned(self, volume): + """Determine if a volume is thin provisioned""" + return volume.get('objectType') == 'thinVolume' or volume.get( + 'thinProvisioned', False) + def extend_volume(self, volume, new_size): """Extend an existing volume to the new size.""" - stage_1, stage_2 = 0, 0 src_vol = self._get_volume(volume['name_id']) - 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']) - if 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']) + 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']) def _garbage_collect_tmp_vols(self): """Removes tmp vols with no snapshots.""" -- 2.45.2