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

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)

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.

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"

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']
+        if self._is_thin_provisioned(src_vol):
-        stage_label = 'tmp-%s' % utils.convert_uuid_to_es_fmt(uuid.uuid4())
+            self._client.expand_volume(src_vol['id'], new_size)
-        extend_vol = {'id': uuid.uuid4(), 'size': new_size}
+        else:
-        self.create_cloned_volume(extend_vol, volume)
+            stage_1, stage_2 = 0, 0
-        new_vol = self._get_volume(extend_vol['id'])
+            src_label = src_vol['label']
-        try:
+            stage_label = 'tmp-%s' % utils.convert_uuid_to_es_fmt(uuid.uuid4())
-            stage_1 = self._client.update_volume(src_vol['id'], stage_label)
+            extend_vol = {'id': uuid.uuid4(), 'size': new_size}
-            stage_2 = self._client.update_volume(new_vol['id'], src_label)
+            self.create_cloned_volume(extend_vol, volume)
-            new_vol = stage_2
+            new_vol = self._get_volume(extend_vol['id'])
-            LOG.info(_LI('Extended volume with label %s.'), src_label)
+            try:
-        except exception.NetAppDriverException:
+                stage_1 = self._client.update_volume(src_vol['id'],
-            if stage_1 == 0:
+                                                     stage_label)
-                with excutils.save_and_reraise_exception():
+                stage_2 = self._client.update_volume(new_vol['id'], src_label)
-                    self._client.delete_volume(new_vol['id'])
+                new_vol = stage_2
-            if stage_2 == 0:
+                LOG.info(_LI('Extended volume with label %s.'), src_label)
-                with excutils.save_and_reraise_exception():
+            except exception.NetAppDriverException:
-                    self._client.update_volume(src_vol['id'], src_label)
+                if stage_1 == 0:
-                    self._client.delete_volume(new_vol['id'])
+                    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."""