From a9bc395407fcc897c689911842c4bd277c5728fc Mon Sep 17 00:00:00 2001 From: Felipe Rodrigues Date: Fri, 5 Jun 2020 15:28:01 +0000 Subject: [PATCH] [NetApp] Fix svm scoped account When the NetApp backend starts, it needs to know whether the `revert_to_snapshot` support exist. So, it was retrieving the licenses and checking if the `SnapRestore` is included. Using a scoped account, the backend cannot retrieve that information, though. So, this patch solves it by sending a fake operation `revert_to_snapshot` that must fail. Analyzing the given error, it sets the backend support field. Closes-Bug: #1882590 Change-Id: Ib71a6cec939288498e48736f129fbfdacaabe9da (cherry picked from commit 6f58af1ae39b4151db5feaa72deecafb9365ca16) (cherry picked from commit b5e541a6284200d28c7e6298ec46cb1f2f1eb9b9) (cherry picked from commit b252cb801d547c9a9717d705303c3206a079c27f) (cherry picked from commit 036908dd59684e9f5f44e305535cdc81dc563f2c) --- .../netapp/dataontap/cluster_mode/lib_base.py | 36 ++++++++-- .../dataontap/cluster_mode/test_lib_base.py | 68 ++++++++++++++----- .../share/drivers/netapp/dataontap/fakes.py | 2 +- ...ix-svm-scoped-netapp-85b53830135f7558.yaml | 6 ++ 4 files changed, 86 insertions(+), 26 deletions(-) create mode 100644 releasenotes/notes/bug-1882590-fix-svm-scoped-netapp-85b53830135f7558.yaml diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py index 4ba8d7c0e4..322ffb1b9d 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py @@ -30,6 +30,7 @@ from oslo_log import log from oslo_service import loopingcall from oslo_utils import timeutils from oslo_utils import units +from oslo_utils import uuidutils import six from manila.common import constants @@ -124,6 +125,7 @@ class NetAppCmodeFileStorageLibrary(object): self._clients = {} self._ssc_stats = {} self._have_cluster_creds = None + self._revert_to_snapshot_support = False self._cluster_info = {} self._app_version = kwargs.get('app_version', 'unknown') @@ -140,6 +142,9 @@ class NetAppCmodeFileStorageLibrary(object): if self._have_cluster_creds is True: self._set_cluster_info() + self._licenses = self._get_licenses() + self._revert_to_snapshot_support = self._check_snaprestore_license() + # Performance monitoring library self._perf_library = performance.PerformanceLibrary(self._client) @@ -151,7 +156,6 @@ class NetAppCmodeFileStorageLibrary(object): @na_utils.trace def check_for_setup_error(self): - self._licenses = self._get_licenses() self._start_periodic_tasks() def _get_vserver(self, share_server=None): @@ -255,10 +259,30 @@ class NetAppCmodeFileStorageLibrary(object): @na_utils.trace def _check_snaprestore_license(self): """Check if snaprestore license is enabled.""" - if not self._licenses: - self._licenses = self._client.get_licenses() + if self._have_cluster_creds: + return 'snaprestore' in self._licenses + else: + # NOTE: (felipe_rodrigues): workaround to find out whether the + # backend has the license: since without cluster credentials it + # cannot retrieve the ontap licenses, it sends a fake ONTAP + # "snapshot-restore-volume" request which is only available when + # the license exists. By the got error, it checks whether license + # is installed or not. + try: + self._client.restore_snapshot( + "fake_%s" % uuidutils.generate_uuid(dashed=False), "") + except netapp_api.NaApiError as e: + no_license = 'is not licensed' + LOG.debug('Fake restore_snapshot request failed: %s', e) + return not (e.code == netapp_api.EAPIERROR and + no_license in e.message) - return 'snaprestore' in self._licenses + # since it passed an empty snapshot, it should never get here + msg = _("Caught an unexpected behavior: the fake restore to " + "snapshot request using 'fake' volume and empty string " + "snapshot as argument has not failed.") + LOG.exception(msg) + raise exception.NetAppException(msg) @na_utils.trace def _get_aggregate_node(self, aggregate_name): @@ -331,8 +355,6 @@ class NetAppCmodeFileStorageLibrary(object): netapp_flexvol_encryption = self._cluster_info.get( 'nve_support', False) - revert_to_snapshot_support = self._check_snaprestore_license() - for aggr_name in sorted(aggregates): reserved_percentage = self.configuration.reserved_share_percentage @@ -362,7 +384,7 @@ class NetAppCmodeFileStorageLibrary(object): 'thin_provisioning': [True, False], 'snapshot_support': True, 'create_share_from_snapshot_support': True, - 'revert_to_snapshot_support': revert_to_snapshot_support, + 'revert_to_snapshot_support': self._revert_to_snapshot_support, } # Add storage service catalog data. diff --git a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py index 6beeaf288e..94c4219ee4 100644 --- a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py +++ b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py @@ -118,8 +118,16 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): self.mock_object( self.library._client, 'check_for_cluster_credentials', mock.Mock(return_value=True)) + self.mock_object( + self.library, '_check_snaprestore_license', + mock.Mock(return_value=True)) + self.mock_object( + self.library, + '_get_licenses', + mock.Mock(return_value=fake.LICENSES)) self.library.do_setup(self.context) + self.assertEqual(fake.LICENSES, self.library._licenses) mock_get_api_client.assert_called_once_with() (self.library._client.check_for_cluster_credentials. assert_called_once_with()) @@ -127,6 +135,9 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): self.mock_object(self.library._client, 'check_for_cluster_credentials', mock.Mock(return_value=True)) + self.mock_object( + self.library, '_check_snaprestore_license', + mock.Mock(return_value=True)) mock_set_cluster_info = self.mock_object( self.library, '_set_cluster_info') self.library.do_setup(self.context) @@ -138,17 +149,10 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): fake.CLUSTER_NODES) def test_check_for_setup_error(self): - - self.library._licenses = [] - self.mock_object(self.library, - '_get_licenses', - mock.Mock(return_value=['fake_license'])) mock_start_periodic_tasks = self.mock_object(self.library, '_start_periodic_tasks') - self.library.check_for_setup_error() - self.assertEqual(['fake_license'], self.library._licenses) mock_start_periodic_tasks.assert_called_once_with() def test_get_vserver(self): @@ -333,7 +337,8 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): assert_called_once_with(fake.AGGREGATES)) self.assertDictEqual(fake.AGGREGATE_CAPACITIES, result) - def test_check_snaprestore_license_notfound(self): + def test_check_snaprestore_license_admin_notfound(self): + self.library._have_cluster_creds = True licenses = list(fake.LICENSES) licenses.remove('snaprestore') self.mock_object(self.client, @@ -342,13 +347,44 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): result = self.library._check_snaprestore_license() self.assertIs(False, result) - def test_check_snaprestore_license_found(self): - self.mock_object(self.client, - 'get_licenses', - mock.Mock(return_value=fake.LICENSES)) + def test_check_snaprestore_license_admin_found(self): + self.library._have_cluster_creds = True + self.library._licenses = fake.LICENSES result = self.library._check_snaprestore_license() self.assertIs(True, result) + def test_check_snaprestore_license_svm_scoped_notfound(self): + self.library._have_cluster_creds = False + self.mock_object(self.library._client, + 'restore_snapshot', + mock.Mock(side_effect=netapp_api.NaApiError( + code=netapp_api.EAPIERROR, + message=fake.NO_SNAPRESTORE_LICENSE))) + result = self.library._check_snaprestore_license() + self.assertIs(False, result) + + def test_check_snaprestore_license_svm_scoped_found(self): + self.library._have_cluster_creds = False + self.mock_object(self.library._client, + 'restore_snapshot', + mock.Mock(side_effect=netapp_api.NaApiError( + code=netapp_api.EAPIERROR, + message='Other error'))) + result = self.library._check_snaprestore_license() + self.assertIs(True, result) + + def test_check_snaprestore_license_svm_scoped_found_exception(self): + self.mock_object(lib_base.LOG, 'exception') + self.library._have_cluster_creds = False + self.mock_object(self.library._client, + 'restore_snapshot', + mock.Mock(return_value=None)) + + self.assertRaises( + exception.NetAppException, + self.library._check_snaprestore_license) + lib_base.LOG.exception.assert_called_once() + def test_get_aggregate_node_cluster_creds(self): self.library._have_cluster_creds = True @@ -449,13 +485,11 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): self.library, '_get_aggregate_space', mock.Mock(return_value=fake.AGGREGATE_CAPACITIES)) self.library._have_cluster_creds = True + self.library._revert_to_snapshot_support = True self.library._cluster_info = fake.CLUSTER_INFO self.library._ssc_stats = fake.SSC_INFO self.library._perf_library.get_node_utilization_for_pool = ( mock.Mock(side_effect=[30.0, 42.0])) - self.mock_object(self.library, - '_check_snaprestore_license', - mock.Mock(return_value=True)) result = self.library._get_pools(filter_function='filter', goodness_function='goodness') @@ -468,13 +502,11 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): self.library, '_get_aggregate_space', mock.Mock(return_value=fake.AGGREGATE_CAPACITIES_VSERVER_CREDS)) self.library._have_cluster_creds = False + self.library._revert_to_snapshot_support = True self.library._cluster_info = fake.CLUSTER_INFO self.library._ssc_stats = fake.SSC_INFO_VSERVER_CREDS self.library._perf_library.get_node_utilization_for_pool = ( mock.Mock(side_effect=[50.0, 50.0])) - self.mock_object(self.library, - '_check_snaprestore_license', - mock.Mock(return_value=True)) result = self.library._get_pools() diff --git a/manila/tests/share/drivers/netapp/dataontap/fakes.py b/manila/tests/share/drivers/netapp/dataontap/fakes.py index c1b8da52b7..240915270c 100644 --- a/manila/tests/share/drivers/netapp/dataontap/fakes.py +++ b/manila/tests/share/drivers/netapp/dataontap/fakes.py @@ -18,7 +18,7 @@ import copy from manila.common import constants import manila.tests.share.drivers.netapp.fakes as na_fakes - +NO_SNAPRESTORE_LICENSE = '"SnapRestore" is not licensed in the cluster.' BACKEND_NAME = 'fake_backend_name' DRIVER_NAME = 'fake_driver_name' APP_VERSION = 'fake_app_vsersion' diff --git a/releasenotes/notes/bug-1882590-fix-svm-scoped-netapp-85b53830135f7558.yaml b/releasenotes/notes/bug-1882590-fix-svm-scoped-netapp-85b53830135f7558.yaml new file mode 100644 index 0000000000..d588552c34 --- /dev/null +++ b/releasenotes/notes/bug-1882590-fix-svm-scoped-netapp-85b53830135f7558.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixed `bug #1882590 `_ + that caused an error on starting a NetApp backend when using the SVM + scoped account.