From eb2d9640e2b09553c70a4726c1e3a0e5fa4db025 Mon Sep 17 00:00:00 2001 From: Clinton Knight Date: Thu, 29 Sep 2016 19:39:36 -0400 Subject: [PATCH] NetApp cDOT driver should not report untenable pools The NetApp cDOT driver now explicitly filters root aggregates from the pools reported to the manila scheduler if the driver is operating with cluster credentials. Change-Id: I659edada559e50d2332790025c65fae265a27c3d Closes-Bug: #1624526 --- .../netapp/dataontap/client/client_cmode.py | 48 ++++++++- .../dataontap/cluster_mode/lib_multi_svm.py | 2 +- .../dataontap/cluster_mode/lib_single_svm.py | 8 +- .../drivers/netapp/dataontap/client/fakes.py | 102 ++++++++++++++---- .../dataontap/client/test_client_cmode.py | 72 ++++++++++++- .../cluster_mode/test_lib_multi_svm.py | 10 +- .../cluster_mode/test_lib_single_svm.py | 23 +++- .../share/drivers/netapp/dataontap/fakes.py | 1 + ...lter-root-aggregates-c30ac5064d530b86.yaml | 6 ++ 9 files changed, 234 insertions(+), 38 deletions(-) create mode 100644 releasenotes/notes/bug-1624526-netapp-cdot-filter-root-aggregates-c30ac5064d530b86.yaml diff --git a/manila/share/drivers/netapp/dataontap/client/client_cmode.py b/manila/share/drivers/netapp/dataontap/client/client_cmode.py index 715be386..90cc3434 100644 --- a/manila/share/drivers/netapp/dataontap/client/client_cmode.py +++ b/manila/share/drivers/netapp/dataontap/client/client_cmode.py @@ -444,7 +444,51 @@ class NetAppCmodeClient(client_base.NetAppBaseClient): return sorted(ports, key=sort_key, reverse=True) @na_utils.trace - def list_aggregates(self): + def list_root_aggregates(self): + """Get names of all aggregates that contain node root volumes.""" + + desired_attributes = { + 'aggr-attributes': { + 'aggregate-name': None, + 'aggr-raid-attributes': { + 'has-local-root': None, + 'has-partner-root': None, + }, + }, + } + aggrs = self._get_aggregates(desired_attributes=desired_attributes) + + root_aggregates = [] + for aggr in aggrs: + aggr_name = aggr.get_child_content('aggregate-name') + aggr_raid_attrs = aggr.get_child_by_name('aggr-raid-attributes') + + local_root = strutils.bool_from_string( + aggr_raid_attrs.get_child_content('has-local-root')) + partner_root = strutils.bool_from_string( + aggr_raid_attrs.get_child_content('has-partner-root')) + + if local_root or partner_root: + root_aggregates.append(aggr_name) + + return root_aggregates + + @na_utils.trace + def list_non_root_aggregates(self): + """Get names of all aggregates that don't contain node root volumes.""" + + query = { + 'aggr-attributes': { + 'aggr-raid-attributes': { + 'has-local-root': 'false', + 'has-partner-root': 'false', + } + }, + } + return self._list_aggregates(query=query) + + @na_utils.trace + def _list_aggregates(self, query=None): """Get names of all aggregates.""" try: api_args = { @@ -454,6 +498,8 @@ class NetAppCmodeClient(client_base.NetAppBaseClient): }, }, } + if query: + api_args['query'] = query result = self.send_iter_request('aggr-get-iter', api_args) aggr_list = result.get_child_by_name( 'attributes-list').get_children() diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py index 9f6f44dd..a4140d43 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py @@ -100,7 +100,7 @@ class NetAppCmodeMultiSVMFileStorageLibrary( @na_utils.trace def _find_matching_aggregates(self): """Find all aggregates match pattern.""" - aggregate_names = self._client.list_aggregates() + aggregate_names = self._client.list_non_root_aggregates() pattern = self.configuration.netapp_aggregate_name_search_pattern return [aggr_name for aggr_name in aggregate_names if re.match(pattern, aggr_name)] diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_single_svm.py b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_single_svm.py index 05dc7f83..c4e3c0a1 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_single_svm.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_single_svm.py @@ -115,9 +115,15 @@ class NetAppCmodeSingleSVMFileStorageLibrary( """Find all aggregates match pattern.""" vserver_client = self._get_api_client(vserver=self._vserver) aggregate_names = vserver_client.list_vserver_aggregates() + + root_aggregate_names = [] + if self._have_cluster_creds: + root_aggregate_names = self._client.list_root_aggregates() + pattern = self.configuration.netapp_aggregate_name_search_pattern return [aggr_name for aggr_name in aggregate_names - if re.match(pattern, aggr_name)] + if re.match(pattern, aggr_name) and + aggr_name not in root_aggregate_names] @na_utils.trace def get_network_allocations_number(self): diff --git a/manila/tests/share/drivers/netapp/dataontap/client/fakes.py b/manila/tests/share/drivers/netapp/dataontap/client/fakes.py index 74198e0a..3f000922 100644 --- a/manila/tests/share/drivers/netapp/dataontap/client/fakes.py +++ b/manila/tests/share/drivers/netapp/dataontap/client/fakes.py @@ -38,6 +38,7 @@ VSERVER_NAME_2 = 'fake_vserver_2' ADMIN_VSERVER_NAME = 'fake_admin_vserver' NODE_VSERVER_NAME = 'fake_node_vserver' NFS_VERSIONS = ['nfs3', 'nfs4.0'] +ROOT_AGGREGATE_NAMES = ('root_aggr1', 'root_aggr2') ROOT_VOLUME_AGGREGATE_NAME = 'fake_root_aggr' ROOT_VOLUME_NAME = 'fake_root_volume' SHARE_AGGREGATE_NAME = 'fake_aggr1' @@ -791,34 +792,21 @@ AGGR_GET_NAMES_RESPONSE = etree.XML(""" - - - /%(aggr1)s/plex0 - - - /%(aggr1)s/plex0/rg0 - - - - + + %(root1)s + + + + + %(root2)s + + + %(aggr1)s - - - /%(aggr2)s/plex0 - - - /%(aggr2)s/plex0/rg0 - - - /%(aggr2)s/plex0/rg1 - - - - %(aggr2)s @@ -826,6 +814,8 @@ AGGR_GET_NAMES_RESPONSE = etree.XML(""" 2 """ % { + 'root1': ROOT_AGGREGATE_NAMES[0], + 'root2': ROOT_AGGREGATE_NAMES[1], 'aggr1': SHARE_AGGREGATE_NAMES[0], 'aggr2': SHARE_AGGREGATE_NAMES[1], }) @@ -1268,6 +1258,72 @@ AGGR_GET_ITER_SSC_RESPONSE = etree.XML(""" """ % {'aggr1': SHARE_AGGREGATE_NAMES[0]}) +AGGR_GET_ITER_ROOT_AGGR_RESPONSE = etree.XML(""" + + + + + true + false + + %(root1)s + + + + true + false + + %(root2)s + + + + false + false + + %(aggr1)s + + + + false + false + + %(aggr2)s + + + 6 + +""" % { + 'root1': ROOT_AGGREGATE_NAMES[0], + 'root2': ROOT_AGGREGATE_NAMES[1], + 'aggr1': SHARE_AGGREGATE_NAMES[0], + 'aggr2': SHARE_AGGREGATE_NAMES[1], +}) + +AGGR_GET_ITER_NON_ROOT_AGGR_RESPONSE = etree.XML(""" + + + + + false + false + + %(aggr1)s + + + + false + false + + %(aggr2)s + + + 6 + +""" % { + 'aggr1': SHARE_AGGREGATE_NAMES[0], + 'aggr2': SHARE_AGGREGATE_NAMES[1], +}) + VOLUME_GET_NAME_RESPONSE = etree.XML(""" diff --git a/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py b/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py index 6deb0e6f..9a53e57f 100644 --- a/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py +++ b/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py @@ -796,16 +796,80 @@ class NetAppClientCmodeTestCase(test.TestCase): self.assertSequenceEqual(fake.SORTED_PORTS_ALL_SPEEDS, result) + def test_list_root_aggregates(self): + + api_response = netapp_api.NaElement( + fake.AGGR_GET_ITER_ROOT_AGGR_RESPONSE) + self.mock_object(self.client, + 'send_iter_request', + mock.Mock(return_value=api_response)) + + result = self.client.list_root_aggregates() + + aggr_get_iter_args = { + 'desired-attributes': { + 'aggr-attributes': { + 'aggregate-name': None, + 'aggr-raid-attributes': { + 'has-local-root': None, + 'has-partner-root': None, + }, + }, + } + } + self.assertSequenceEqual(fake.ROOT_AGGREGATE_NAMES, result) + self.client.send_iter_request.assert_has_calls([ + mock.call('aggr-get-iter', aggr_get_iter_args)]) + + def test_list_non_root_aggregates(self): + + api_response = netapp_api.NaElement( + fake.AGGR_GET_ITER_NON_ROOT_AGGR_RESPONSE) + self.mock_object(self.client, + 'send_iter_request', + mock.Mock(return_value=api_response)) + + result = self.client.list_non_root_aggregates() + + aggr_get_iter_args = { + 'query': { + 'aggr-attributes': { + 'aggr-raid-attributes': { + 'has-local-root': 'false', + 'has-partner-root': 'false', + } + }, + }, + 'desired-attributes': { + 'aggr-attributes': { + 'aggregate-name': None, + }, + }, + } + self.assertSequenceEqual(fake.SHARE_AGGREGATE_NAMES, result) + self.client.send_iter_request.assert_has_calls([ + mock.call('aggr-get-iter', aggr_get_iter_args)]) + def test_list_aggregates(self): api_response = netapp_api.NaElement(fake.AGGR_GET_NAMES_RESPONSE) self.mock_object(self.client, - 'send_request', + 'send_iter_request', mock.Mock(return_value=api_response)) - result = self.client.list_aggregates() + result = self.client._list_aggregates() - self.assertSequenceEqual(fake.SHARE_AGGREGATE_NAMES, result) + aggr_get_iter_args = { + 'desired-attributes': { + 'aggr-attributes': { + 'aggregate-name': None, + }, + }, + } + self.assertSequenceEqual( + fake.ROOT_AGGREGATE_NAMES + fake.SHARE_AGGREGATE_NAMES, result) + self.client.send_iter_request.assert_has_calls([ + mock.call('aggr-get-iter', aggr_get_iter_args)]) def test_list_aggregates_not_found(self): @@ -815,7 +879,7 @@ class NetAppClientCmodeTestCase(test.TestCase): mock.Mock(return_value=api_response)) self.assertRaises(exception.NetAppException, - self.client.list_aggregates) + self.client._list_aggregates) def test_list_vserver_aggregates(self): diff --git a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_multi_svm.py b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_multi_svm.py index ea2f519f..edce5d2b 100644 --- a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_multi_svm.py +++ b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_multi_svm.py @@ -186,14 +186,16 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): def test_find_matching_aggregates(self): - self.mock_object(self.client, - 'list_aggregates', - mock.Mock(return_value=fake.AGGREGATES)) - + mock_list_non_root_aggregates = self.mock_object( + self.client, 'list_non_root_aggregates', + mock.Mock(return_value=fake.AGGREGATES)) self.library.configuration.netapp_aggregate_name_search_pattern = ( '.*_aggr_1') + result = self.library._find_matching_aggregates() + self.assertListEqual([fake.AGGREGATES[0]], result) + mock_list_non_root_aggregates.assert_called_once_with() def test_setup_server(self): diff --git a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_single_svm.py b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_single_svm.py index 405ea00c..48a6f658 100644 --- a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_single_svm.py +++ b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_single_svm.py @@ -15,6 +15,7 @@ Unit tests for the NetApp Data ONTAP cDOT single-SVM storage driver library. """ +import ddt import mock from oslo_log import log @@ -26,6 +27,7 @@ from manila import test import manila.tests.share.drivers.netapp.dataontap.fakes as fake +@ddt.ddt class NetAppFileStorageLibraryTestCase(test.TestCase): def setUp(self): @@ -164,19 +166,32 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): self.assertTrue(mock_vserver_client.prune_deleted_snapshots.called) self.assertTrue(mock_super.called) - def test_find_matching_aggregates(self): + @ddt.data(True, False) + def test_find_matching_aggregates(self, have_cluster_creds): + self.library._have_cluster_creds = have_cluster_creds + aggregates = fake.AGGREGATES + fake.ROOT_AGGREGATES mock_vserver_client = mock.Mock() - mock_vserver_client.list_vserver_aggregates.return_value = ( - fake.AGGREGATES) + mock_vserver_client.list_vserver_aggregates.return_value = aggregates self.mock_object(self.library, '_get_api_client', mock.Mock(return_value=mock_vserver_client)) + mock_client = mock.Mock() + mock_client.list_root_aggregates.return_value = fake.ROOT_AGGREGATES + self.library._client = mock_client self.library.configuration.netapp_aggregate_name_search_pattern = ( '.*_aggr_1') + result = self.library._find_matching_aggregates() - self.assertListEqual([fake.AGGREGATES[0]], result) + + if have_cluster_creds: + self.assertListEqual([fake.AGGREGATES[0]], result) + mock_client.list_root_aggregates.assert_called_once_with() + else: + self.assertListEqual([fake.AGGREGATES[0], fake.ROOT_AGGREGATES[0]], + result) + self.assertFalse(mock_client.list_root_aggregates.called) def test_get_network_allocations_number(self): self.assertEqual(0, self.library.get_network_allocations_number()) diff --git a/manila/tests/share/drivers/netapp/dataontap/fakes.py b/manila/tests/share/drivers/netapp/dataontap/fakes.py index 71449e9d..a311f692 100644 --- a/manila/tests/share/drivers/netapp/dataontap/fakes.py +++ b/manila/tests/share/drivers/netapp/dataontap/fakes.py @@ -54,6 +54,7 @@ FREE_CAPACITY = 10000000000 TOTAL_CAPACITY = 20000000000 AGGREGATE = 'manila_aggr_1' AGGREGATES = ('manila_aggr_1', 'manila_aggr_2') +ROOT_AGGREGATES = ('root_aggr_1', 'root_aggr_2') ROOT_VOLUME_AGGREGATE = 'manila1' ROOT_VOLUME = 'root' CLUSTER_NODE = 'cluster1_01' diff --git a/releasenotes/notes/bug-1624526-netapp-cdot-filter-root-aggregates-c30ac5064d530b86.yaml b/releasenotes/notes/bug-1624526-netapp-cdot-filter-root-aggregates-c30ac5064d530b86.yaml new file mode 100644 index 00000000..f92c3eed --- /dev/null +++ b/releasenotes/notes/bug-1624526-netapp-cdot-filter-root-aggregates-c30ac5064d530b86.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - The NetApp cDOT driver now explicitly filters root aggregates + from the pools reported to the manila scheduler if the driver + is operating with cluster credentials. +