From: Goutham Pacha Ravi Date: Fri, 26 Jun 2015 16:32:06 +0000 (-0400) Subject: Fix use of wrong storage pools for NetApp Drivers X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=04888d1868f0adaf4ed8e9ce9994e56a1dbee97f;p=openstack-build%2Fcinder-build.git Fix use of wrong storage pools for NetApp Drivers When using NetApp drivers, all available flexvols on a datastore are being reported to the Cinder Scheduler as available storage pools. This allows Cinder volumes to be created on flexvols not designated for OpenStack. This commit fixes the problem by delimiting which pools are eligible for provisioning via a configuration parameter 'netapp_pool_name_search_pattern'. This allows the Cinder drivers to operate against a restricted set of storage containers. DocImpact Co-Authored-By: Rushil Chugh Co-Authored-By: Michael Price Closes-Bug: #1493399 Change-Id: I7b6f2205470ecbbcb165889db19e1168117dd0b2 --- diff --git a/cinder/tests/unit/test_netapp.py b/cinder/tests/unit/test_netapp.py index dd7198a37..dbc4bb1e9 100644 --- a/cinder/tests/unit/test_netapp.py +++ b/cinder/tests/unit/test_netapp.py @@ -1,4 +1,5 @@ # Copyright (c) 2012 NetApp, Inc. +# Copyright (c) 2015 Goutham Pacha Ravi. All rights reserved. # All rights reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); you may @@ -24,8 +25,11 @@ from cinder import exception from cinder import test from cinder.tests.unit.volume.drivers.netapp.dataontap.client import ( fake_api as netapp_api) +from cinder.tests.unit.volume.drivers.netapp.dataontap import fakes from cinder.volume import configuration as conf from cinder.volume.drivers.netapp import common +from cinder.volume.drivers.netapp.dataontap import block_7mode +from cinder.volume.drivers.netapp.dataontap import block_cmode from cinder.volume.drivers.netapp.dataontap.client import client_7mode from cinder.volume.drivers.netapp.dataontap.client import client_base from cinder.volume.drivers.netapp.dataontap.client import client_cmode @@ -583,6 +587,9 @@ class NetAppDirectCmodeISCSIDriverTestCase(test.TestCase): self.driver.library.zapi_client = mock.MagicMock() self.driver.library.zapi_client.get_ontapi_version.return_value = \ (1, 20) + self.mock_object(block_cmode.NetAppBlockStorageCmodeLibrary, + '_get_filtered_pools', + mock.Mock(return_value=fakes.FAKE_CMODE_POOLS)) self.driver.check_for_setup_error() def test_do_setup_all_default(self): @@ -1279,6 +1286,9 @@ class NetAppDirect7modeISCSIDriverTestCase_NV(test.TestCase): self.driver.library.zapi_client = mock.MagicMock() self.driver.library.zapi_client.get_ontapi_version.\ return_value = (1, 20) + self.mock_object(block_7mode.NetAppBlockStorage7modeLibrary, + '_get_filtered_pools', + mock.Mock(return_value=fakes.FAKE_7MODE_POOLS)) self.driver.check_for_setup_error() def test_check_for_setup_error_version(self): diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py index b7f61ff97..f097d4943 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py @@ -13,8 +13,12 @@ # License for the specific language governing permissions and limitations # under the License. +from lxml import etree + from cinder.tests.unit.volume.drivers.netapp.dataontap.client import ( fake_api as netapp_api) +from cinder.volume.drivers.netapp.dataontap import ssc_cmode + VOLUME_ID = 'f10d1a84-9b7b-427e-8fec-63c48b509a56' LUN_ID = 'ee6b4cc7-477b-4016-aa0c-7127b4e3af86' @@ -212,6 +216,80 @@ SNAPSHOT = { VOLUME_REF = {'name': 'fake_vref_name', 'size': 42} +FAKE_CMODE_POOLS = [ + { + 'QoS_support': True, + 'free_capacity_gb': 3.72, + 'netapp_compression': u'true', + 'netapp_dedup': u'true', + 'netapp_disk_type': 'SSD', + 'netapp_mirrored': u'true', + 'netapp_nocompression': u'false', + 'netapp_nodedup': u'false', + 'netapp_raid_type': 'raiddp', + 'netapp_thick_provisioned': u'false', + 'netapp_thin_provisioned': u'true', + 'netapp_unmirrored': u'false', + 'pool_name': 'open123', + 'reserved_percentage': 0, + 'total_capacity_gb': 4.65, + 'thin_provisioned_support': True, + 'thick_provisioned_support': False, + 'provisioned_capacity_gb': 0.93, + 'max_over_subscription_ratio': 20.0, + } +] + +FAKE_CMODE_VOLUME = { + 'all': [ssc_cmode.NetAppVolume(name='open123', vserver='vs'), + ssc_cmode.NetAppVolume(name='mixed', vserver='vs'), + ssc_cmode.NetAppVolume(name='open321', vserver='vs')], +} + +FAKE_7MODE_VOLUME = { + 'all': [ + netapp_api.NaElement( + etree.XML(""" + open123 + """)), + netapp_api.NaElement( + etree.XML(""" + mixed3 + """)), + netapp_api.NaElement( + etree.XML(""" + open1234 + """)) + ], +} + + +FAKE_CMODE_VOL1 = ssc_cmode.NetAppVolume(name='open123', vserver='openstack') +FAKE_CMODE_VOL1.state['vserver_root'] = False +FAKE_CMODE_VOL1.state['status'] = 'online' +FAKE_CMODE_VOL1.state['junction_active'] = True +FAKE_CMODE_VOL1.space['size_avl_bytes'] = '4000000000' +FAKE_CMODE_VOL1.space['size_total_bytes'] = '5000000000' +FAKE_CMODE_VOL1.space['space-guarantee-enabled'] = False +FAKE_CMODE_VOL1.space['space-guarantee'] = 'file' +FAKE_CMODE_VOL1.space['thin_provisioned'] = True +FAKE_CMODE_VOL1.mirror['mirrored'] = True +FAKE_CMODE_VOL1.qos['qos_policy_group'] = None +FAKE_CMODE_VOL1.aggr['name'] = 'aggr1' +FAKE_CMODE_VOL1.aggr['junction'] = '/vola' +FAKE_CMODE_VOL1.sis['dedup'] = True +FAKE_CMODE_VOL1.sis['compression'] = True +FAKE_CMODE_VOL1.aggr['raid_type'] = 'raiddp' +FAKE_CMODE_VOL1.aggr['ha_policy'] = 'cfo' +FAKE_CMODE_VOL1.aggr['disk_type'] = 'SSD' +ssc_map = { + 'mirrored': [FAKE_CMODE_VOL1], + 'dedup': [FAKE_CMODE_VOL1], + 'compression': [FAKE_CMODE_VOL1], + 'thin': [FAKE_CMODE_VOL1], + 'all': [FAKE_CMODE_VOL1], +} + FILE_LIST = ['file1', 'file2', 'file3'] FAKE_LUN = netapp_api.NaElement.create_node_with_children( @@ -239,6 +317,31 @@ FAKE_LUN = netapp_api.NaElement.create_node_with_children( 'volume': 'fakeLUN', 'vserver': 'fake_vserver'}) +FAKE_7MODE_VOL1 = [netapp_api.NaElement( + etree.XML(""" + open123 + online + 0 + 0 + 0 + false + false + """))] + +FAKE_7MODE_POOLS = [ + { + 'pool_name': 'open123', + 'QoS_support': False, + 'reserved_percentage': 0, + 'total_capacity_gb': 0.0, + 'free_capacity_gb': 0.0, + 'max_over_subscription_ratio': 20.0, + 'thin_provisioned_support': False, + 'thick_provisioned_support': True, + 'provisioned_capacity_gb': 0.0, + } +] + class test_volume(object): pass diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_7mode.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_7mode.py index c181ed60b..a217506c3 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_7mode.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_7mode.py @@ -1,6 +1,7 @@ # Copyright (c) 2014 Alex Meade. All rights reserved. # Copyright (c) 2014 Clinton Knight. All rights reserved. # Copyright (c) 2015 Tom Barron. All rights reserved. +# Copyright (c) 2015 Goutham Pacha Ravi. All rights reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); you may # not use this file except in compliance with the License. You may obtain @@ -44,7 +45,7 @@ class NetAppBlockStorage7modeLibraryTestCase(test.TestCase): super(NetAppBlockStorage7modeLibraryTestCase, self).setUp() # Inject fake netapp_lib module classes. - netapp_api.mock_netapp_lib([block_7mode]) + netapp_api.mock_netapp_lib([block_7mode, client_base]) kwargs = {'configuration': self.get_config_7mode()} self.library = block_7mode.NetAppBlockStorage7modeLibrary( @@ -53,6 +54,8 @@ class NetAppBlockStorage7modeLibraryTestCase(test.TestCase): self.library.zapi_client = mock.Mock() self.zapi_client = self.library.zapi_client self.library.vfiler = mock.Mock() + # Deprecated option + self.library.configuration.netapp_volume_list = None def tearDown(self): super(NetAppBlockStorage7modeLibraryTestCase, self).tearDown() @@ -106,11 +109,21 @@ class NetAppBlockStorage7modeLibraryTestCase(test.TestCase): block_base.NetAppBlockStorageLibrary, 'check_for_setup_error') def test_check_for_setup_error(self, super_check_for_setup_error): self.zapi_client.get_ontapi_version.return_value = (1, 9) + self.mock_object(self.library, '_refresh_volume_info') + self.library.volume_list = ['open1', 'open2'] self.library.check_for_setup_error() super_check_for_setup_error.assert_called_once_with() + def test_check_for_setup_error_no_filtered_pools(self): + self.zapi_client.get_ontapi_version.return_value = (1, 9) + self.mock_object(self.library, '_refresh_volume_info') + self.library.volume_list = [] + + self.assertRaises(exception.NetAppDriverException, + self.library.check_for_setup_error) + def test_check_for_setup_error_too_old(self): self.zapi_client.get_ontapi_version.return_value = (1, 8) self.assertRaises(exception.VolumeBackendAPIException, @@ -422,7 +435,7 @@ class NetAppBlockStorage7modeLibraryTestCase(test.TestCase): def test_manage_existing_lun_same_name(self): mock_lun = block_base.NetAppLun('handle', 'name', '1', - {'Path': '/vol/vol1/name'}) + {'Path': '/vol/FAKE_CMODE_VOL1/name'}) self.library._get_existing_vol_with_manage_ref = mock.Mock( return_value=mock_lun) self.mock_object(na_utils, 'get_volume_extra_specs') @@ -441,7 +454,7 @@ class NetAppBlockStorage7modeLibraryTestCase(test.TestCase): def test_manage_existing_lun_new_path(self): mock_lun = block_base.NetAppLun( - 'handle', 'name', '1', {'Path': '/vol/vol1/name'}) + 'handle', 'name', '1', {'Path': '/vol/FAKE_CMODE_VOL1/name'}) self.library._get_existing_vol_with_manage_ref = mock.Mock( return_value=mock_lun) self.mock_object(na_utils, 'get_volume_extra_specs') @@ -457,7 +470,7 @@ class NetAppBlockStorage7modeLibraryTestCase(test.TestCase): self.assertEqual(1, self.library._check_volume_type_for_lun.call_count) self.assertEqual(1, self.library._add_lun_to_table.call_count) self.zapi_client.move_lun.assert_called_once_with( - '/vol/vol1/name', '/vol/vol1/volume') + '/vol/FAKE_CMODE_VOL1/name', '/vol/FAKE_CMODE_VOL1/volume') def test_get_pool_stats_no_volumes(self): @@ -499,3 +512,87 @@ class NetAppBlockStorage7modeLibraryTestCase(test.TestCase): }] self.assertEqual(expected, result) + + def test_get_filtered_pools_invalid_conf(self): + """Verify an exception is raised if the regex pattern is invalid.""" + self.library.configuration.netapp_pool_name_search_pattern = '(.+' + + self.assertRaises(exception.InvalidConfigurationValue, + self.library._get_filtered_pools) + + @ddt.data('.*?3$|mix.+', '(.+?[0-9]+) ', '^.+3$', '^[a-z].*?[^4]$') + def test_get_filtered_pools_match_select_pools(self, patterns): + self.library.vols = fake.FAKE_7MODE_VOLUME['all'] + self.library.configuration.netapp_pool_name_search_pattern = patterns + + filtered_pools = self.library._get_filtered_pools() + + self.assertEqual( + fake.FAKE_7MODE_VOLUME['all'][0].get_child_content('name'), + filtered_pools[0] + ) + self.assertEqual( + fake.FAKE_7MODE_VOLUME['all'][1].get_child_content('name'), + filtered_pools[1] + ) + + @ddt.data('', 'mix.+|open.+', '.+', 'open123, mixed3, open1234', '.+') + def test_get_filtered_pools_match_all_pools(self, patterns): + self.library.vols = fake.FAKE_7MODE_VOLUME['all'] + self.library.configuration.netapp_pool_name_search_pattern = patterns + + filtered_pools = self.library._get_filtered_pools() + + self.assertEqual( + fake.FAKE_7MODE_VOLUME['all'][0].get_child_content('name'), + filtered_pools[0] + ) + self.assertEqual( + fake.FAKE_7MODE_VOLUME['all'][1].get_child_content('name'), + filtered_pools[1] + ) + self.assertEqual( + fake.FAKE_7MODE_VOLUME['all'][2].get_child_content('name'), + filtered_pools[2] + ) + + @ddt.data('abc|stackopen|openstack|abc.*', 'abc', + 'stackopen, openstack, open', '^$') + def test_get_filtered_pools_non_matching_patterns(self, patterns): + + self.library.vols = fake.FAKE_7MODE_VOLUME['all'] + self.library.configuration.netapp_pool_name_search_pattern = patterns + + filtered_pools = self.library._get_filtered_pools() + + self.assertListEqual([], filtered_pools) + + def test_get_pool_stats_no_ssc_vols(self): + + self.library.vols = {} + + pools = self.library._get_pool_stats() + + self.assertListEqual([], pools) + + def test_get_pool_stats_with_filtered_pools(self): + + self.library.vols = fake.FAKE_7MODE_VOL1 + self.library.volume_list = [ + fake.FAKE_7MODE_VOL1[0].get_child_content('name') + ] + self.library.root_volume_name = '' + + pools = self.library._get_pool_stats() + + self.assertListEqual(fake.FAKE_7MODE_POOLS, pools) + + def test_get_pool_stats_no_filtered_pools(self): + + self.library.vols = fake.FAKE_7MODE_VOL1 + self.library.volume_list = ['open1', 'open2'] + self.library.root_volume_name = '' + + pools = self.library._get_pool_stats() + + self.assertListEqual([], pools) 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 1008b5f66..2b377bb36 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 @@ -2,6 +2,7 @@ # Copyright (c) 2014 Clinton Knight. All rights reserved. # Copyright (c) 2014 Andrew Kerr. All rights reserved. # Copyright (c) 2015 Tom Barron. All rights reserved. +# Copyright (c) 2015 Goutham Pacha Ravi. All rights reserved. # All Rights Reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); you may @@ -87,10 +88,10 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase): @mock.patch.object(block_base.NetAppBlockStorageLibrary, '_get_lun_attr', - mock.Mock(return_value={'Volume': 'vol1'})) + mock.Mock(return_value={'Volume': 'FAKE_CMODE_VOL1'})) def test_get_pool(self): pool = self.library.get_pool({'name': 'volume-fake-uuid'}) - self.assertEqual('vol1', pool) + self.assertEqual('FAKE_CMODE_VOL1', pool) @mock.patch.object(block_base.NetAppBlockStorageLibrary, '_get_lun_attr', diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_cmode.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_cmode.py index bf8805a19..ed26b7535 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_cmode.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_cmode.py @@ -91,6 +91,9 @@ class NetAppBlockStorageCmodeLibraryTestCase(test.TestCase): ssc_cmode, 'check_ssc_api_permissions') mock_start_periodic_tasks = self.mock_object( self.library, '_start_periodic_tasks') + self.mock_object(ssc_cmode, 'refresh_cluster_ssc') + self.mock_object(self.library, '_get_filtered_pools', + mock.Mock(return_value=fake.FAKE_CMODE_POOLS)) self.library.check_for_setup_error() @@ -99,6 +102,18 @@ class NetAppBlockStorageCmodeLibraryTestCase(test.TestCase): self.library.zapi_client) self.assertEqual(1, mock_start_periodic_tasks.call_count) + def test_check_for_setup_error_no_filtered_pools(self): + self.mock_object(block_base.NetAppBlockStorageLibrary, + 'check_for_setup_error') + self.mock_object(ssc_cmode, 'check_ssc_api_permissions') + self.mock_object(self.library, '_start_periodic_tasks') + self.mock_object(ssc_cmode, 'refresh_cluster_ssc') + self.mock_object(self.library, '_get_filtered_pools', + mock.Mock(return_value=[])) + + self.assertRaises(exception.NetAppDriverException, + self.library.check_for_setup_error) + def test_find_mapped_lun_igroup(self): igroups = [fake.IGROUP1] self.zapi_client.get_igroup_by_initiators.return_value = igroups @@ -472,7 +487,7 @@ class NetAppBlockStorageCmodeLibraryTestCase(test.TestCase): def test_manage_existing_lun_same_name(self): mock_lun = block_base.NetAppLun('handle', 'name', '1', - {'Path': '/vol/vol1/name'}) + {'Path': '/vol/FAKE_CMODE_VOL1/name'}) self.library._get_existing_vol_with_manage_ref = mock.Mock( return_value=mock_lun) self.mock_object(na_utils, 'get_volume_extra_specs') @@ -497,7 +512,7 @@ class NetAppBlockStorageCmodeLibraryTestCase(test.TestCase): def test_manage_existing_lun_new_path(self): mock_lun = block_base.NetAppLun( - 'handle', 'name', '1', {'Path': '/vol/vol1/name'}) + 'handle', 'name', '1', {'Path': '/vol/FAKE_CMODE_VOL1/name'}) self.library._get_existing_vol_with_manage_ref = mock.Mock( return_value=mock_lun) self.mock_object(na_utils, 'get_volume_extra_specs') @@ -513,7 +528,7 @@ class NetAppBlockStorageCmodeLibraryTestCase(test.TestCase): self.assertEqual(1, self.library._check_volume_type_for_lun.call_count) self.assertEqual(1, self.library._add_lun_to_table.call_count) self.zapi_client.move_lun.assert_called_once_with( - '/vol/vol1/name', '/vol/vol1/volume') + '/vol/FAKE_CMODE_VOL1/name', '/vol/FAKE_CMODE_VOL1/volume') def test_start_periodic_tasks(self): @@ -532,3 +547,80 @@ class NetAppBlockStorageCmodeLibraryTestCase(test.TestCase): mock_loopingcall.assert_has_calls([ mock.call(mock_remove_unused_qos_policy_groups)]) self.assertTrue(harvest_qos_periodic_task.start.called) + + @ddt.data('open+|demix+', 'open.+', '.+\d', '^((?!mix+).)*$', + 'open123, open321') + def test_get_filtered_pools_match_selected_pools(self, patterns): + + self.library.ssc_vols = fake.FAKE_CMODE_VOLUME + self.library.configuration.netapp_pool_name_search_pattern = patterns + + filtered_pools = self.library._get_filtered_pools() + + self.assertEqual(fake.FAKE_CMODE_VOLUME['all'][0].id['name'], + filtered_pools[0].id['name']) + self.assertEqual(fake.FAKE_CMODE_VOLUME['all'][2].id['name'], + filtered_pools[1].id['name']) + + @ddt.data('', 'mix.+|open.+', '.+', 'open123, mixed, open321', + '.*?') + def test_get_filtered_pools_match_all_pools(self, patterns): + + self.library.ssc_vols = fake.FAKE_CMODE_VOLUME + self.library.configuration.netapp_pool_name_search_pattern = patterns + + filtered_pools = self.library._get_filtered_pools() + + self.assertEqual(fake.FAKE_CMODE_VOLUME['all'][0].id['name'], + filtered_pools[0].id['name']) + self.assertEqual(fake.FAKE_CMODE_VOLUME['all'][1].id['name'], + filtered_pools[1].id['name']) + self.assertEqual(fake.FAKE_CMODE_VOLUME['all'][2].id['name'], + filtered_pools[2].id['name']) + + def test_get_filtered_pools_invalid_conf(self): + """Verify an exception is raised if the regex pattern is invalid""" + self.library.configuration.netapp_pool_name_search_pattern = '(.+' + + self.assertRaises(exception.InvalidConfigurationValue, + self.library._get_filtered_pools) + + @ddt.data('abc|stackopen|openstack|abc*', 'abc', 'stackopen', 'openstack', + 'abc*', '^$') + def test_get_filtered_pools_non_matching_patterns(self, patterns): + + self.library.ssc_vols = fake.FAKE_CMODE_VOLUME + self.library.configuration.netapp_pool_name_search_pattern = patterns + + filtered_pools = self.library._get_filtered_pools() + + self.assertListEqual([], filtered_pools) + + @ddt.data({}, None) + def test_get_pool_stats_no_ssc_vols(self, vols): + + self.library.ssc_vols = vols + + pools = self.library._get_pool_stats() + + self.assertListEqual([], pools) + + def test_get_pool_stats_with_filtered_pools(self): + + self.library.ssc_vols = fake.ssc_map + self.mock_object(self.library, '_get_filtered_pools', + mock.Mock(return_value=[fake.FAKE_CMODE_VOL1])) + + pools = self.library._get_pool_stats() + + self.assertListEqual(fake.FAKE_CMODE_POOLS, pools) + + def test_get_pool_stats_no_filtered_pools(self): + + self.library.ssc_vols = fake.ssc_map + self.mock_object(self.library, '_get_filtered_pools', + mock.Mock(return_value=[])) + + pools = self.library._get_pool_stats() + + self.assertListEqual([], pools) 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 eb0f9a483..de69391b6 100644 --- a/cinder/tests/unit/volume/drivers/netapp/eseries/test_library.py +++ b/cinder/tests/unit/volume/drivers/netapp/eseries/test_library.py @@ -60,6 +60,8 @@ class NetAppEseriesLibraryTestCase(test.TestCase): eseries_fake.create_configuration_eseries()} self.library = library.NetAppESeriesLibrary('FAKE', **kwargs) + # Deprecated Option + self.library.configuration.netapp_storage_pools = None self.library._client = eseries_fake.FakeEseriesClient() self.library.check_for_setup_error() @@ -73,19 +75,54 @@ class NetAppEseriesLibraryTestCase(test.TestCase): self.assertTrue(mock_check_flags.called) - def test_get_storage_pools(self): - pool_labels = list() - # Retrieve the first pool's label - for pool in eseries_fake.STORAGE_POOLS: - pool_labels.append(pool['label']) - break - self.library.configuration.netapp_storage_pools = ( - ",".join(pool_labels)) + def test_get_storage_pools_empty_result(self): + """Verify an exception is raised if no pools are returned.""" + self.library.configuration.netapp_pool_name_search_pattern = '$' + + self.assertRaises(exception.NetAppDriverException, + self.library.check_for_setup_error) + + def test_get_storage_pools_invalid_conf(self): + """Verify an exception is raised if the regex pattern is invalid.""" + self.library.configuration.netapp_pool_name_search_pattern = '(.*' + + self.assertRaises(exception.InvalidConfigurationValue, + self.library._get_storage_pools) + + def test_get_storage_pools_default(self): + """Verify that all pools are returned if the search option is empty.""" + filtered_pools = self.library._get_storage_pools() + + self.assertEqual(eseries_fake.STORAGE_POOLS, filtered_pools) + + @ddt.data(('[\d]+,a', ['1', '2', 'a', 'b'], ['1', '2', 'a']), + ('1 , 3', ['1', '2', '3'], ['1', '3']), + ('$,3', ['1', '2', '3'], ['3']), + ('[a-zA-Z]+', ['1', 'a', 'B'], ['a', 'B']), + ('', ['1', '2'], ['1', '2']) + ) + @ddt.unpack + def test_get_storage_pools(self, pool_filter, pool_labels, + expected_pool_labels): + """Verify that pool filtering via the search_pattern works correctly + + :param pool_filter: A regular expression to be used for filtering via + pool labels + :param pool_labels: A list of pool labels + :param expected_pool_labels: The labels from 'pool_labels' that + should be matched by 'pool_filter' + """ + self.library.configuration.netapp_pool_name_search_pattern = ( + pool_filter) + pools = [{'label': label} for label in pool_labels] + + self.library._client.list_storage_pools = mock.Mock( + return_value=pools) filtered_pools = self.library._get_storage_pools() filtered_pool_labels = [pool['label'] for pool in filtered_pools] - self.assertListEqual(pool_labels, filtered_pool_labels) + self.assertEqual(expected_pool_labels, filtered_pool_labels) def test_get_volume(self): fake_volume = copy.deepcopy(get_fake_volume()) @@ -120,7 +157,7 @@ class NetAppEseriesLibraryTestCase(test.TestCase): False, minimum_version="1.53.9000.1") self.library._client.SSC_VALID_VERSIONS = [(1, 53, 9000, 1), (1, 53, 9010, 15)] - self.library.configuration.netapp_storage_pools = "test_vg1" + self.library.configuration.netapp_pool_name_search_pattern = "test_vg1" self.library._client.list_storage_pools = mock.Mock(return_value=pools) self.library._client.list_drives = mock.Mock(return_value=drives) diff --git a/cinder/volume/drivers/netapp/dataontap/block_7mode.py b/cinder/volume/drivers/netapp/dataontap/block_7mode.py index d3d6dc0f9..c93fdab17 100644 --- a/cinder/volume/drivers/netapp/dataontap/block_7mode.py +++ b/cinder/volume/drivers/netapp/dataontap/block_7mode.py @@ -6,6 +6,7 @@ # Copyright (c) 2014 Andrew Kerr. All rights reserved. # Copyright (c) 2014 Jeff Applewhite. All rights reserved. # Copyright (c) 2015 Tom Barron. All rights reserved. +# Copyright (c) 2015 Goutham Pacha Ravi. All rights reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); you may # not use this file except in compliance with the License. You may obtain @@ -23,6 +24,7 @@ Volume driver library for NetApp 7-mode block storage systems. """ from oslo_log import log as logging +from oslo_log import versionutils from oslo_utils import timeutils from oslo_utils import units import six @@ -54,10 +56,7 @@ class NetAppBlockStorage7modeLibrary(block_base.NetAppBlockStorageLibrary): def do_setup(self, context): super(NetAppBlockStorage7modeLibrary, self).do_setup(context) - self.volume_list = self.configuration.netapp_volume_list - if self.volume_list: - self.volume_list = self.volume_list.split(',') - self.volume_list = [el.strip() for el in self.volume_list] + self.volume_list = [] self.vfiler = self.configuration.netapp_vfiler @@ -108,6 +107,14 @@ class NetAppBlockStorage7modeLibrary(block_base.NetAppBlockStorageLibrary): else: msg = _("API version could not be determined.") raise exception.VolumeBackendAPIException(data=msg) + + self._refresh_volume_info() + + if not self.volume_list: + msg = _('No pools are available for provisioning volumes. ' + 'Ensure that the configuration option ' + 'netapp_pool_name_search_pattern is set correctly.') + raise exception.NetAppDriverException(msg) super(NetAppBlockStorage7modeLibrary, self).check_for_setup_error() def _create_lun(self, volume_name, lun_name, size, @@ -254,13 +261,12 @@ class NetAppBlockStorage7modeLibrary(block_base.NetAppBlockStorageLibrary): """Retrieve pool (i.e. Data ONTAP volume) stats info from volumes.""" pools = [] - if not self.vols: - return pools for vol in self.vols: - # omit volumes not specified in the config volume_name = vol.get_child_content('name') + + # omit volumes not specified in the config if self.volume_list and volume_name not in self.volume_list: continue @@ -306,6 +312,35 @@ class NetAppBlockStorage7modeLibrary(block_base.NetAppBlockStorageLibrary): return pools + def _get_filtered_pools(self): + """Return available pools filtered by a pool name search pattern.""" + + # Inform deprecation of legacy option. + if self.configuration.safe_get('netapp_volume_list'): + msg = _LW("The option 'netapp_volume_list' is deprecated and " + "will be removed in the future releases. Please use " + "the option 'netapp_pool_name_search_pattern' instead.") + versionutils.report_deprecated_feature(LOG, msg) + + pool_regex = na_utils.get_pool_name_filter_regex(self.configuration) + + filtered_pools = [] + for vol in self.vols: + vol_name = vol.get_child_content('name') + if pool_regex.match(vol_name): + msg = ("Volume '%(vol_name)s' matches against regular " + "expression: %(vol_pattern)s") + LOG.debug(msg, {'vol_name': vol_name, + 'vol_pattern': pool_regex.pattern}) + filtered_pools.append(vol_name) + else: + msg = ("Volume '%(vol_name)s' does not match against regular " + "expression: %(vol_pattern)s") + LOG.debug(msg, {'vol_name': vol_name, + 'vol_pattern': pool_regex.pattern}) + + return filtered_pools + def _get_lun_block_count(self, path): """Gets block counts for the LUN.""" bs = super(NetAppBlockStorage7modeLibrary, @@ -333,6 +368,7 @@ class NetAppBlockStorage7modeLibrary(block_base.NetAppBlockStorageLibrary): return self.vol_refresh_voluntary = False self.vols = self.zapi_client.get_filer_volumes() + self.volume_list = self._get_filtered_pools() self.vol_refresh_time = timeutils.utcnow() except Exception as e: LOG.warning(_LW("Error refreshing volume info. Message: %s"), diff --git a/cinder/volume/drivers/netapp/dataontap/block_cmode.py b/cinder/volume/drivers/netapp/dataontap/block_cmode.py index 7531acaad..896f42bfb 100644 --- a/cinder/volume/drivers/netapp/dataontap/block_cmode.py +++ b/cinder/volume/drivers/netapp/dataontap/block_cmode.py @@ -6,6 +6,7 @@ # Copyright (c) 2014 Andrew Kerr. All rights reserved. # Copyright (c) 2014 Jeff Applewhite. All rights reserved. # Copyright (c) 2015 Tom Barron. All rights reserved. +# Copyright (c) 2015 Goutham Pacha Ravi. All rights reserved. # # Licensed under the Apache License, Version 2.0 (the "License"); you may # not use this file except in compliance with the License. You may obtain @@ -70,12 +71,19 @@ class NetAppBlockStorageCmodeLibrary(block_base.NetAppBlockStorageLibrary): port=self.configuration.netapp_server_port, vserver=self.vserver) - self.ssc_vols = None + self.ssc_vols = {} self.stale_vols = set() def check_for_setup_error(self): """Check that the driver is working and can communicate.""" ssc_cmode.check_ssc_api_permissions(self.zapi_client) + ssc_cmode.refresh_cluster_ssc(self, self.zapi_client.get_connection(), + self.vserver, synchronous=True) + if not self._get_filtered_pools(): + msg = _('No pools are available for provisioning volumes. ' + 'Ensure that the configuration option ' + 'netapp_pool_name_search_pattern is set correctly.') + raise exception.NetAppDriverException(msg) super(NetAppBlockStorageCmodeLibrary, self).check_for_setup_error() self._start_periodic_tasks() @@ -191,10 +199,11 @@ class NetAppBlockStorageCmodeLibrary(block_base.NetAppBlockStorageLibrary): """Retrieve pool (Data ONTAP volume) stats info from SSC volumes.""" pools = [] + if not self.ssc_vols: return pools - for vol in self.ssc_vols['all']: + for vol in self._get_filtered_pools(): pool = dict() pool['pool_name'] = vol.id['name'] pool['QoS_support'] = True @@ -244,6 +253,27 @@ class NetAppBlockStorageCmodeLibrary(block_base.NetAppBlockStorageLibrary): return pools + def _get_filtered_pools(self): + """Return filtered pools given a pool name search pattern.""" + pool_regex = na_utils.get_pool_name_filter_regex(self.configuration) + + filtered_pools = [] + for vol in self.ssc_vols.get('all', []): + vol_name = vol.id['name'] + if pool_regex.match(vol_name): + msg = ("Volume '%(vol_name)s' matches against regular " + "expression: %(vol_pattern)s") + LOG.debug(msg, {'vol_name': vol_name, + 'vol_pattern': pool_regex.pattern}) + filtered_pools.append(vol) + else: + msg = ("Volume '%(vol_name)s' does not match against regular " + "expression: %(vol_pattern)s") + LOG.debug(msg, {'vol_name': vol_name, + 'vol_pattern': pool_regex.pattern}) + + return filtered_pools + @utils.synchronized('update_stale') def _update_stale_vols(self, volume=None, reset=False): """Populates stale vols with vol and returns set copy if reset.""" diff --git a/cinder/volume/drivers/netapp/eseries/library.py b/cinder/volume/drivers/netapp/eseries/library.py index 255713d38..323af2d6c 100644 --- a/cinder/volume/drivers/netapp/eseries/library.py +++ b/cinder/volume/drivers/netapp/eseries/library.py @@ -26,6 +26,7 @@ import uuid from oslo_config import cfg from oslo_log import log as logging +from oslo_log import versionutils from oslo_service import loopingcall from oslo_utils import excutils from oslo_utils import units @@ -62,8 +63,7 @@ class NetAppESeriesLibrary(object): AUTOSUPPORT_INTERVAL_SECONDS = 3600 # hourly VERSION = "1.0.0" REQUIRED_FLAGS = ['netapp_server_hostname', 'netapp_controller_ips', - 'netapp_login', 'netapp_password', - 'netapp_storage_pools'] + 'netapp_login', 'netapp_password'] SLEEP_SECS = 5 HOST_TYPES = {'aix': 'AIX MPIO', 'avt': 'AVT_4M', @@ -177,6 +177,7 @@ class NetAppESeriesLibrary(object): def check_for_setup_error(self): self._check_host_type() self._check_multipath() + self._check_pools() self._check_storage_system() self._start_periodic_tasks() @@ -197,6 +198,14 @@ class NetAppESeriesLibrary(object): {'backend': self._backend_name, 'mpflag': 'use_multipath_for_image_xfer'}) + def _check_pools(self): + """Ensure that the pool listing contains at least one pool""" + if not self._get_storage_pools(): + msg = _('No pools are available for provisioning volumes. ' + 'Ensure that the configuration option ' + 'netapp_pool_name_search_pattern is set correctly.') + raise exception.NetAppDriverException(msg) + def _ensure_multi_attach_host_group_exists(self): try: host_group = self._client.get_host_group_by_name( @@ -1115,17 +1124,34 @@ class NetAppESeriesLibrary(object): return ssc_stats def _get_storage_pools(self): - conf_enabled_pools = [] - for value in self.configuration.netapp_storage_pools.split(','): - if value: - conf_enabled_pools.append(value.strip().lower()) + """Retrieve storage pools that match user-configured search pattern.""" + + # Inform deprecation of legacy option. + if self.configuration.safe_get('netapp_storage_pools'): + msg = _LW("The option 'netapp_storage_pools' is deprecated and " + "will be removed in the future releases. Please use " + "the option 'netapp_pool_name_search_pattern' instead.") + versionutils.report_deprecated_feature(LOG, msg) + + pool_regex = na_utils.get_pool_name_filter_regex(self.configuration) - filtered_pools = [] storage_pools = self._client.list_storage_pools() - for storage_pool in storage_pools: - # Check if pool can be used - if (storage_pool['label'].lower() in conf_enabled_pools): - filtered_pools.append(storage_pool) + + filtered_pools = [] + for pool in storage_pools: + pool_name = pool['label'] + + if pool_regex.match(pool_name): + msg = ("Pool '%(pool_name)s' matches against regular " + "expression: %(pool_pattern)s") + LOG.debug(msg, {'pool_name': pool_name, + 'pool_pattern': pool_regex.pattern}) + filtered_pools.append(pool) + else: + msg = ("Pool '%(pool_name)s' does not match against regular " + "expression: %(pool_pattern)s") + LOG.debug(msg, {'pool_name': pool_name, + 'pool_pattern': pool_regex.pattern}) return filtered_pools diff --git a/cinder/volume/drivers/netapp/options.py b/cinder/volume/drivers/netapp/options.py index 377b451cd..f4df96702 100644 --- a/cinder/volume/drivers/netapp/options.py +++ b/cinder/volume/drivers/netapp/options.py @@ -80,14 +80,6 @@ netapp_provisioning_opts = [ 'the volume creation request. Note: this option ' 'is deprecated and will be removed in favor of ' '"reserved_percentage" in the Mitaka release.')), - cfg.StrOpt('netapp_volume_list', - default=None, - help=('This option is only utilized when the storage protocol ' - 'is configured to use iSCSI or FC. This option is used ' - 'to restrict provisioning to the specified controller ' - 'volumes. Specify the value of this option to be a ' - 'comma separated list of NetApp controller volume names ' - 'to be used for provisioning.')), cfg.StrOpt('netapp_lun_space_reservation', default='enabled', choices=['enabled', 'disabled'], @@ -164,13 +156,6 @@ netapp_eseries_opts = [ default=None, help=('Password for the NetApp E-Series storage array.'), secret=True), - cfg.StrOpt('netapp_storage_pools', - default=None, - help=('This option is used to restrict provisioning to the ' - 'specified storage pools. Only dynamic disk pools are ' - 'currently supported. Specify the value of this option to' - ' be a comma separated list of disk pool names to be used' - ' for provisioning.')), cfg.BoolOpt('netapp_enable_multiattach', default=False, help='This option specifies whether the driver should allow ' @@ -200,7 +185,19 @@ netapp_san_opts = [ help=('This option defines the type of operating system for' ' all initiators that can access a LUN. This information' ' is used when mapping LUNs to individual hosts or' - ' groups of hosts.'))] + ' groups of hosts.')), + cfg.StrOpt('netapp_pool_name_search_pattern', + deprecated_opts=[cfg.DeprecatedOpt(name='netapp_volume_list'), + cfg.DeprecatedOpt(name='netapp_storage_pools') + ], + default="(.+)", + help=('This option is used to restrict provisioning to the ' + 'specified pools. Specify the value of ' + 'this option to be a regular expression which will be ' + 'applied to the names of objects from the storage ' + 'backend which represent pools in Cinder. This option ' + 'is only utilized when the storage protocol is ' + 'configured to use iSCSI or FC.')), ] CONF = cfg.CONF CONF.register_opts(netapp_proxy_opts) diff --git a/cinder/volume/drivers/netapp/utils.py b/cinder/volume/drivers/netapp/utils.py index 81966eb09..b6a13b04d 100644 --- a/cinder/volume/drivers/netapp/utils.py +++ b/cinder/volume/drivers/netapp/utils.py @@ -24,6 +24,7 @@ NetApp drivers to achieve the desired functionality. import decimal import platform +import re import socket from oslo_concurrency import processutils as putils @@ -247,6 +248,31 @@ def get_qos_policy_group_name_from_info(qos_policy_group_info): return None +def get_pool_name_filter_regex(configuration): + """Build the regex for filtering pools by name + + :param configuration: The volume driver configuration + :raise InvalidConfigurationValue: if configured regex pattern is invalid + :return: A compiled regex for filtering pool names + """ + + # If the configuration parameter is specified as an empty string + # (interpreted as matching all pools), we replace it here with + # (.+) to be explicit with CSV compatibility support implemented below. + pool_patterns = configuration.netapp_pool_name_search_pattern or '(.+)' + + # Strip whitespace from start/end and then 'or' all regex patterns + pool_patterns = '|'.join(['^' + pool_pattern.strip('^$ \t') + '$' for + pool_pattern in pool_patterns.split(',')]) + + try: + return re.compile(pool_patterns) + except re.error: + raise exception.InvalidConfigurationValue( + option='netapp_pool_name_search_pattern', + value=configuration.netapp_pool_name_search_pattern) + + def get_valid_qos_policy_group_info(volume, extra_specs=None): """Given a volume, return information for QOS provisioning.""" info = dict(legacy=None, spec=None)