From 2b60912d5667350eae7ecbc67d4dba3658518d10 Mon Sep 17 00:00:00 2001 From: Lucio Seki Date: Wed, 18 Apr 2018 17:35:56 -0300 Subject: [PATCH] NetApp ONTAP iSCSI: Force exception on online extend The Netapp ONTAP iSCSI driver does not support online volume extend. It may work if the requested size does not exceed the LUN max geometry, otherwise it will require the LUN to be detached. In such case, the backend currently detaches and leaves the volume in an inconsistent state. This patch forces the ONTAP iSCSI driver to raise an exception whenever an online extend is requested and it detects it would exceed the LUN max geometry. Change-Id: Ie3dddbc05c6cd32e27168d68f4cb819364b0438c Closes-Bug: #1712651 --- .../volume/drivers/netapp/dataontap/fakes.py | 3 + .../netapp/dataontap/test_block_base.py | 103 +++++++++++++++--- .../drivers/netapp/dataontap/block_base.py | 10 ++ .../notes/bug-1712651-7bc90264eb5001ea.yaml | 6 + 4 files changed, 108 insertions(+), 14 deletions(-) create mode 100644 releasenotes/notes/bug-1712651-7bc90264eb5001ea.yaml diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py index 14749304a61..d1f0e43b64e 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py @@ -45,6 +45,8 @@ NFS_SHARE_PATH = '/export' NFS_EXPORT_1 = '%s:%s' % (NFS_HOST, NFS_SHARE_PATH) NFS_EXPORT_2 = 'nfs-host2:/export' MOUNT_POINT = '/mnt/nfs' +ATTACHED = 'attached' +DETACHED = 'detached' LUN_METADATA = { 'OsType': None, 'SpaceReserved': 'true', @@ -57,6 +59,7 @@ VOLUME = { 'size': SIZE, 'id': VOLUME_ID, 'host': HOST_STRING, + 'attach_status': DETACHED, } NFS_VOLUME = { 'name': NFS_FILE_PATH, 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 e89835d7222..7d964b8f7b3 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py @@ -165,12 +165,12 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase): self.assertEqual(1, block_base.LOG.exception.call_count) def test_create_volume_no_pool_provided_by_scheduler(self): - fake_volume = copy.deepcopy(fake.VOLUME) + volume_copy = copy.deepcopy(fake.VOLUME) # Set up fake volume whose 'host' field is missing pool information. - fake_volume['host'] = '%s@%s' % (fake.HOST_NAME, fake.BACKEND_NAME) + volume_copy['host'] = '%s@%s' % (fake.HOST_NAME, fake.BACKEND_NAME) self.assertRaises(exception.InvalidHost, self.library.create_volume, - fake_volume) + volume_copy) @mock.patch.object(block_base.NetAppBlockStorageLibrary, '_get_lun_attr') @@ -1114,8 +1114,6 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase): new_size_bytes = new_size * units.Gi max_size = fake.LUN_SIZE * 10 max_size_bytes = max_size * units.Gi - fake_volume = copy.copy(fake.VOLUME) - fake_volume['size'] = new_size fake_lun = block_base.NetAppLun(fake.LUN_HANDLE, fake.LUN_ID, @@ -1144,6 +1142,45 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase): self.assertEqual(six.text_type(new_size_bytes), self.library.lun_table[fake.VOLUME['name']].size) + def test__extend_attached_volume_direct(self): + + current_size = fake.LUN_SIZE + current_size_bytes = current_size * units.Gi + new_size = fake.LUN_SIZE * 2 + new_size_bytes = new_size * units.Gi + max_size = fake.LUN_SIZE * 10 + max_size_bytes = max_size * units.Gi + volume_copy = copy.copy(fake.VOLUME) + volume_copy['size'] = new_size + volume_copy['attach_status'] = fake.ATTACHED + + fake_lun = block_base.NetAppLun(fake.LUN_HANDLE, + fake.LUN_ID, + current_size_bytes, + fake.LUN_METADATA) + mock_get_lun_from_table = self.mock_object( + self.library, '_get_lun_from_table', return_value=fake_lun) + fake_lun_geometry = {'max_resize': six.text_type(max_size_bytes)} + mock_get_lun_geometry = self.mock_object( + self.library.zapi_client, 'get_lun_geometry', + return_value=fake_lun_geometry) + mock_do_direct_resize = self.mock_object(self.library.zapi_client, + 'do_direct_resize') + mock_do_sub_clone_resize = self.mock_object(self.library, + '_do_sub_clone_resize') + self.library.lun_table = {volume_copy['name']: fake_lun} + + self.library._extend_volume(volume_copy, new_size, 'fake_qos_policy') + + mock_get_lun_from_table.assert_called_once_with(volume_copy['name']) + mock_get_lun_geometry.assert_called_once_with( + fake.LUN_METADATA['Path']) + mock_do_direct_resize.assert_called_once_with( + fake.LUN_METADATA['Path'], six.text_type(new_size_bytes)) + self.assertFalse(mock_do_sub_clone_resize.called) + self.assertEqual(six.text_type(new_size_bytes), + self.library.lun_table[volume_copy['name']].size) + def test__extend_volume_clone(self): current_size = fake.LUN_SIZE @@ -1152,8 +1189,6 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase): new_size_bytes = new_size * units.Gi max_size = fake.LUN_SIZE * 10 max_size_bytes = max_size * units.Gi - fake_volume = copy.copy(fake.VOLUME) - fake_volume['size'] = new_size fake_lun = block_base.NetAppLun(fake.LUN_HANDLE, fake.LUN_ID, @@ -1183,15 +1218,15 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase): self.assertEqual(six.text_type(new_size_bytes), self.library.lun_table[fake.VOLUME['name']].size) - def test__extend_volume_no_change(self): + def test__extend_attached_volume_clone_error(self): current_size = fake.LUN_SIZE current_size_bytes = current_size * units.Gi - new_size = fake.LUN_SIZE + new_size = fake.LUN_SIZE * 20 max_size = fake.LUN_SIZE * 10 max_size_bytes = max_size * units.Gi - fake_volume = copy.copy(fake.VOLUME) - fake_volume['size'] = new_size + volume_copy = copy.copy(fake.VOLUME) + volume_copy['attach_status'] = fake.ATTACHED fake_lun = block_base.NetAppLun(fake.LUN_HANDLE, fake.LUN_ID, @@ -1207,11 +1242,51 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase): 'do_direct_resize') mock_do_sub_clone_resize = self.mock_object(self.library, '_do_sub_clone_resize') - self.library.lun_table = {fake_volume['name']: fake_lun} + self.library.lun_table = {volume_copy['name']: fake_lun} - self.library._extend_volume(fake_volume, new_size, 'fake_qos_policy') + self.assertRaises(exception.VolumeBackendAPIException, + self.library._extend_volume, + volume_copy, + new_size, + fake.QOS_POLICY_GROUP_NAME) - mock_get_lun_from_table.assert_called_once_with(fake_volume['name']) + mock_get_lun_from_table.assert_called_once_with(volume_copy['name']) + mock_get_lun_geometry.assert_called_once_with( + fake.LUN_METADATA['Path']) + self.assertFalse(mock_do_direct_resize.called) + self.assertFalse(mock_do_sub_clone_resize.called) + self.assertEqual(current_size_bytes, + self.library.lun_table[volume_copy['name']].size) + + def test__extend_volume_no_change(self): + + current_size = fake.LUN_SIZE + current_size_bytes = current_size * units.Gi + new_size = fake.LUN_SIZE + max_size = fake.LUN_SIZE * 10 + max_size_bytes = max_size * units.Gi + volume_copy = copy.copy(fake.VOLUME) + volume_copy['size'] = new_size + + fake_lun = block_base.NetAppLun(fake.LUN_HANDLE, + fake.LUN_ID, + current_size_bytes, + fake.LUN_METADATA) + mock_get_lun_from_table = self.mock_object( + self.library, '_get_lun_from_table', return_value=fake_lun) + fake_lun_geometry = {'max_resize': six.text_type(max_size_bytes)} + mock_get_lun_geometry = self.mock_object( + self.library.zapi_client, 'get_lun_geometry', + return_value=fake_lun_geometry) + mock_do_direct_resize = self.mock_object(self.library.zapi_client, + 'do_direct_resize') + mock_do_sub_clone_resize = self.mock_object(self.library, + '_do_sub_clone_resize') + self.library.lun_table = {volume_copy['name']: fake_lun} + + self.library._extend_volume(volume_copy, new_size, 'fake_qos_policy') + + mock_get_lun_from_table.assert_called_once_with(volume_copy['name']) self.assertFalse(mock_get_lun_geometry.called) self.assertFalse(mock_do_direct_resize.called) self.assertFalse(mock_do_sub_clone_resize.called) diff --git a/cinder/volume/drivers/netapp/dataontap/block_base.py b/cinder/volume/drivers/netapp/dataontap/block_base.py index 819d2949ca6..411f2c3cb46 100644 --- a/cinder/volume/drivers/netapp/dataontap/block_base.py +++ b/cinder/volume/drivers/netapp/dataontap/block_base.py @@ -589,6 +589,16 @@ class NetAppBlockStorageLibrary(object): int(new_size_bytes)): self.zapi_client.do_direct_resize(path, new_size_bytes) else: + if volume['attach_status'] != 'detached': + msg = _('Volume %(vol_id)s cannot be resized from ' + '%(old_size)s to %(new_size)s, because would ' + 'exceed its max geometry %(max_geo)s while not ' + 'being detached.') + raise exception.VolumeBackendAPIException(data=msg % { + 'vol_id': name, + 'old_size': curr_size_bytes, + 'new_size': new_size_bytes, + 'max_geo': lun_geometry.get("max_resize")}) self._do_sub_clone_resize( path, new_size_bytes, qos_policy_group_name=qos_policy_group_name) diff --git a/releasenotes/notes/bug-1712651-7bc90264eb5001ea.yaml b/releasenotes/notes/bug-1712651-7bc90264eb5001ea.yaml new file mode 100644 index 00000000000..495e09a1805 --- /dev/null +++ b/releasenotes/notes/bug-1712651-7bc90264eb5001ea.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + NetApp ONTAP iSCSI (bug 1712651): Fix ONTAP NetApp iSCSI driver not + raising a proper exception when trying to extend an attached volume + beyond its max geometry.