NetApp: volume resize using clone fails with QoS

Data ONTAP cannot resize a LUN past the geometry established
when the LUN was created. So to resize past that limit, the
DOT drivers create a new LUN of the needed size and clone
the original LUN into it. This process doesn't always work
if a QoS policy is in place, and the fix is to not pass the
QoS policy to the clone operation.

The fix is trivial, and I improved the surrounding code a
little, but there was no unit test coverage for the method
in question, so this commit also adds full coverage for the
LUN clone method.

Change-Id: I04dcf7ba329a9b94a39580b1115a9dec41231b85
Closes-Bug: #1550478

diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py
index 9d282cb56a65104c8d5187f80596069e733c9ff0..02eec8ac59265c4618db7d797ec2209cc1010d5e 100644 (file)
--- a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py
+++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py
@@ -1028,6 +1028,214 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase):
         self.assertFalse(mock_do_direct_resize.called)
         self.assertFalse(mock_do_sub_clone_resize.called)
 
+    def test_do_sub_clone_resize(self):
+
+        fake_lun = block_base.NetAppLun(fake.LUN_HANDLE,
+                                        fake.LUN_ID,
+                                        fake.LUN_SIZE,
+                                        fake.LUN_METADATA)
+        new_lun_size = fake.LUN_SIZE * 10
+        new_lun_name = 'new-%s' % fake.LUN_NAME
+        block_count = fake.LUN_SIZE * units.Gi / 512
+
+        mock_get_lun_from_table = self.mock_object(
+            self.library, '_get_lun_from_table',
+            mock.Mock(return_value=fake_lun))
+        mock_get_vol_option = self.mock_object(
+            self.library, '_get_vol_option', mock.Mock(return_value='off'))
+        mock_get_lun_block_count = self.mock_object(
+            self.library, '_get_lun_block_count',
+            mock.Mock(return_value=block_count))
+        mock_create_lun = self.mock_object(
+            self.library.zapi_client, 'create_lun')
+        mock_clone_lun = self.mock_object(self.library, '_clone_lun')
+        mock_post_sub_clone_resize = self.mock_object(
+            self.library, '_post_sub_clone_resize')
+        mock_destroy_lun = self.mock_object(
+            self.library.zapi_client, 'destroy_lun')
+
+        self.library._do_sub_clone_resize(fake.LUN_PATH,
+                                          new_lun_size,
+                                          fake.QOS_POLICY_GROUP_NAME)
+
+        mock_get_lun_from_table.assert_called_once_with(fake.LUN_NAME)
+        mock_get_vol_option.assert_called_once_with('vol0', 'compression')
+        mock_get_lun_block_count.assert_called_once_with(fake.LUN_PATH)
+        mock_create_lun.assert_called_once_with(
+            'vol0', new_lun_name, new_lun_size, fake.LUN_METADATA,
+            qos_policy_group_name=fake.QOS_POLICY_GROUP_NAME)
+        mock_clone_lun.assert_called_once_with(
+            fake.LUN_NAME, new_lun_name, block_count=block_count)
+        mock_post_sub_clone_resize.assert_called_once_with(fake.LUN_PATH)
+        self.assertFalse(mock_destroy_lun.called)
+
+    def test_do_sub_clone_resize_compression_on(self):
+
+        fake_lun = block_base.NetAppLun(fake.LUN_HANDLE,
+                                        fake.LUN_ID,
+                                        fake.LUN_SIZE,
+                                        fake.LUN_METADATA)
+        new_lun_size = fake.LUN_SIZE * 10
+        block_count = fake.LUN_SIZE * units.Gi / 512
+
+        mock_get_lun_from_table = self.mock_object(
+            self.library, '_get_lun_from_table',
+            mock.Mock(return_value=fake_lun))
+        mock_get_vol_option = self.mock_object(
+            self.library, '_get_vol_option', mock.Mock(return_value='on'))
+        mock_get_lun_block_count = self.mock_object(
+            self.library, '_get_lun_block_count',
+            mock.Mock(return_value=block_count))
+        mock_create_lun = self.mock_object(
+            self.library.zapi_client, 'create_lun')
+        mock_clone_lun = self.mock_object(self.library, '_clone_lun')
+        mock_post_sub_clone_resize = self.mock_object(
+            self.library, '_post_sub_clone_resize')
+        mock_destroy_lun = self.mock_object(
+            self.library.zapi_client, 'destroy_lun')
+
+        self.assertRaises(exception.VolumeBackendAPIException,
+                          self.library._do_sub_clone_resize,
+                          fake.LUN_PATH,
+                          new_lun_size,
+                          fake.QOS_POLICY_GROUP_NAME)
+
+        mock_get_lun_from_table.assert_called_once_with(fake.LUN_NAME)
+        mock_get_vol_option.assert_called_once_with('vol0', 'compression')
+        self.assertFalse(mock_get_lun_block_count.called)
+        self.assertFalse(mock_create_lun.called)
+        self.assertFalse(mock_clone_lun.called)
+        self.assertFalse(mock_post_sub_clone_resize.called)
+        self.assertFalse(mock_destroy_lun.called)
+
+    def test_do_sub_clone_resize_no_blocks(self):
+
+        fake_lun = block_base.NetAppLun(fake.LUN_HANDLE,
+                                        fake.LUN_ID,
+                                        fake.LUN_SIZE,
+                                        fake.LUN_METADATA)
+        new_lun_size = fake.LUN_SIZE * 10
+        block_count = 0
+
+        mock_get_lun_from_table = self.mock_object(
+            self.library, '_get_lun_from_table',
+            mock.Mock(return_value=fake_lun))
+        mock_get_vol_option = self.mock_object(
+            self.library, '_get_vol_option', mock.Mock(return_value='off'))
+        mock_get_lun_block_count = self.mock_object(
+            self.library, '_get_lun_block_count',
+            mock.Mock(return_value=block_count))
+        mock_create_lun = self.mock_object(
+            self.library.zapi_client, 'create_lun')
+        mock_clone_lun = self.mock_object(self.library, '_clone_lun')
+        mock_post_sub_clone_resize = self.mock_object(
+            self.library, '_post_sub_clone_resize')
+        mock_destroy_lun = self.mock_object(
+            self.library.zapi_client, 'destroy_lun')
+
+        self.assertRaises(exception.VolumeBackendAPIException,
+                          self.library._do_sub_clone_resize,
+                          fake.LUN_PATH,
+                          new_lun_size,
+                          fake.QOS_POLICY_GROUP_NAME)
+
+        mock_get_lun_from_table.assert_called_once_with(fake.LUN_NAME)
+        mock_get_vol_option.assert_called_once_with('vol0', 'compression')
+        mock_get_lun_block_count.assert_called_once_with(fake.LUN_PATH)
+        self.assertFalse(mock_create_lun.called)
+        self.assertFalse(mock_clone_lun.called)
+        self.assertFalse(mock_post_sub_clone_resize.called)
+        self.assertFalse(mock_destroy_lun.called)
+
+    def test_do_sub_clone_resize_create_error(self):
+
+        fake_lun = block_base.NetAppLun(fake.LUN_HANDLE,
+                                        fake.LUN_ID,
+                                        fake.LUN_SIZE,
+                                        fake.LUN_METADATA)
+        new_lun_size = fake.LUN_SIZE * 10
+        new_lun_name = 'new-%s' % fake.LUN_NAME
+        block_count = fake.LUN_SIZE * units.Gi / 512
+
+        mock_get_lun_from_table = self.mock_object(
+            self.library, '_get_lun_from_table',
+            mock.Mock(return_value=fake_lun))
+        mock_get_vol_option = self.mock_object(
+            self.library, '_get_vol_option', mock.Mock(return_value='off'))
+        mock_get_lun_block_count = self.mock_object(
+            self.library, '_get_lun_block_count',
+            mock.Mock(return_value=block_count))
+        mock_create_lun = self.mock_object(
+            self.library.zapi_client, 'create_lun',
+            mock.Mock(side_effect=netapp_api.NaApiError))
+        mock_clone_lun = self.mock_object(self.library, '_clone_lun')
+        mock_post_sub_clone_resize = self.mock_object(
+            self.library, '_post_sub_clone_resize')
+        mock_destroy_lun = self.mock_object(
+            self.library.zapi_client, 'destroy_lun')
+
+        self.assertRaises(netapp_api.NaApiError,
+                          self.library._do_sub_clone_resize,
+                          fake.LUN_PATH,
+                          new_lun_size,
+                          fake.QOS_POLICY_GROUP_NAME)
+
+        mock_get_lun_from_table.assert_called_once_with(fake.LUN_NAME)
+        mock_get_vol_option.assert_called_once_with('vol0', 'compression')
+        mock_get_lun_block_count.assert_called_once_with(fake.LUN_PATH)
+        mock_create_lun.assert_called_once_with(
+            'vol0', new_lun_name, new_lun_size, fake.LUN_METADATA,
+            qos_policy_group_name=fake.QOS_POLICY_GROUP_NAME)
+        self.assertFalse(mock_clone_lun.called)
+        self.assertFalse(mock_post_sub_clone_resize.called)
+        self.assertFalse(mock_destroy_lun.called)
+
+    def test_do_sub_clone_resize_clone_error(self):
+
+        fake_lun = block_base.NetAppLun(fake.LUN_HANDLE,
+                                        fake.LUN_ID,
+                                        fake.LUN_SIZE,
+                                        fake.LUN_METADATA)
+        new_lun_size = fake.LUN_SIZE * 10
+        new_lun_name = 'new-%s' % fake.LUN_NAME
+        new_lun_path = '/vol/vol0/%s' % new_lun_name
+        block_count = fake.LUN_SIZE * units.Gi / 512
+
+        mock_get_lun_from_table = self.mock_object(
+            self.library, '_get_lun_from_table',
+            mock.Mock(return_value=fake_lun))
+        mock_get_vol_option = self.mock_object(
+            self.library, '_get_vol_option', mock.Mock(return_value='off'))
+        mock_get_lun_block_count = self.mock_object(
+            self.library, '_get_lun_block_count',
+            mock.Mock(return_value=block_count))
+        mock_create_lun = self.mock_object(
+            self.library.zapi_client, 'create_lun')
+        mock_clone_lun = self.mock_object(
+            self.library, '_clone_lun',
+            mock.Mock(side_effect=netapp_api.NaApiError))
+        mock_post_sub_clone_resize = self.mock_object(
+            self.library, '_post_sub_clone_resize')
+        mock_destroy_lun = self.mock_object(
+            self.library.zapi_client, 'destroy_lun')
+
+        self.assertRaises(netapp_api.NaApiError,
+                          self.library._do_sub_clone_resize,
+                          fake.LUN_PATH,
+                          new_lun_size,
+                          fake.QOS_POLICY_GROUP_NAME)
+
+        mock_get_lun_from_table.assert_called_once_with(fake.LUN_NAME)
+        mock_get_vol_option.assert_called_once_with('vol0', 'compression')
+        mock_get_lun_block_count.assert_called_once_with(fake.LUN_PATH)
+        mock_create_lun.assert_called_once_with(
+            'vol0', new_lun_name, new_lun_size, fake.LUN_METADATA,
+            qos_policy_group_name=fake.QOS_POLICY_GROUP_NAME)
+        mock_clone_lun.assert_called_once_with(
+            fake.LUN_NAME, new_lun_name, block_count=block_count)
+        self.assertFalse(mock_post_sub_clone_resize.called)
+        mock_destroy_lun.assert_called_once_with(new_lun_path)
+
     def test_configure_chap_generate_username_and_password(self):
         """Ensure that a CHAP username and password are generated."""
         initiator_name = fake.ISCSI_CONNECTOR['initiator']

diff --git a/cinder/volume/drivers/netapp/dataontap/block_base.py b/cinder/volume/drivers/netapp/dataontap/block_base.py
index d28705c00f9f3b2927e2ffa21b3dc62c6aec4179..50abc00bc7dcfc394e0dd5b608a8f7dc21ef377e 100644 (file)
--- a/cinder/volume/drivers/netapp/dataontap/block_base.py
+++ b/cinder/volume/drivers/netapp/dataontap/block_base.py
@@ -553,45 +553,42 @@ class NetAppBlockStorageLibrary(object):
                 break
         return value
 
-    def _do_sub_clone_resize(self, path, new_size_bytes,
+    def _do_sub_clone_resize(self, lun_path, new_size_bytes,
                              qos_policy_group_name=None):
-        """Does sub LUN clone after verification.
+        """Resize a LUN beyond its original geometry using sub-LUN cloning.
 
-            Clones the block ranges and swaps
-            the LUNs also deletes older LUN
-            after a successful clone.
+        Clones the block ranges, swaps the LUNs, and deletes the source LUN.
         """
-        seg = path.split("/")
-        LOG.info(_LI("Resizing LUN %s to new size using clone operation."),
-                 seg[-1])
-        name = seg[-1]
+        seg = lun_path.split("/")
+        LOG.info(_LI("Resizing LUN %s using clone operation."), seg[-1])
+        lun_name = seg[-1]
         vol_name = seg[2]
-        lun = self._get_lun_from_table(name)
+        lun = self._get_lun_from_table(lun_name)
         metadata = lun.metadata
+
         compression = self._get_vol_option(vol_name, 'compression')
         if compression == "on":
             msg = _('%s cannot be resized using clone operation'
                     ' as it is hosted on compressed volume')
-            raise exception.VolumeBackendAPIException(data=msg % name)
-        else:
-            block_count = self._get_lun_block_count(path)
-            if block_count == 0:
-                msg = _('%s cannot be resized using clone operation'
-                        ' as it contains no blocks.')
-                raise exception.VolumeBackendAPIException(data=msg % name)
-            new_lun = 'new-%s' % name
-            self.zapi_client.create_lun(
-                vol_name, new_lun, new_size_bytes, metadata,
-                qos_policy_group_name=qos_policy_group_name)
-            try:
-                self._clone_lun(name, new_lun, block_count=block_count,
-                                qos_policy_group_name=qos_policy_group_name)
-
-                self._post_sub_clone_resize(path)
-            except Exception:
-                with excutils.save_and_reraise_exception():
-                    new_path = '/vol/%s/%s' % (vol_name, new_lun)
-                    self.zapi_client.destroy_lun(new_path)
+            raise exception.VolumeBackendAPIException(data=msg % lun_name)
+
+        block_count = self._get_lun_block_count(lun_path)
+        if block_count == 0:
+            msg = _('%s cannot be resized using clone operation'
+                    ' as it contains no blocks.')
+            raise exception.VolumeBackendAPIException(data=msg % lun_name)
+
+        new_lun_name = 'new-%s' % lun_name
+        self.zapi_client.create_lun(
+            vol_name, new_lun_name, new_size_bytes, metadata,
+            qos_policy_group_name=qos_policy_group_name)
+        try:
+            self._clone_lun(lun_name, new_lun_name, block_count=block_count)
+            self._post_sub_clone_resize(lun_path)
+        except Exception:
+            with excutils.save_and_reraise_exception():
+                new_lun_path = '/vol/%s/%s' % (vol_name, new_lun_name)
+                self.zapi_client.destroy_lun(new_lun_path)
 
     def _post_sub_clone_resize(self, path):
         """Try post sub clone resize in a transactional manner."""