From f10703d8b0dab00357c92840d958832f779e8450 Mon Sep 17 00:00:00 2001 From: Clinton Knight Date: Fri, 12 Dec 2014 15:30:26 -0500 Subject: [PATCH] Replace the NetApp driver proxy layer with a proper factory. The proxy implementation required a hack in the Abstract Base Classes project in Cinder core. This factory implementation removes the need for the hack. It also improves logging, as the actual driver object is provided to the volume manager instead of the driver proxy. Change-Id: I28f4798b9e799547d1696d2f135d3302b3c553fe Closes-Bug: 1405060 --- cinder/test.py | 1 + cinder/tests/test_netapp.py | 19 +-- cinder/tests/test_netapp_eseries_iscsi.py | 8 +- .../volume/drivers/netapp/test_common.py | 127 ++++++++++++++ cinder/volume/drivers/netapp/common.py | 157 +++++------------- 5 files changed, 180 insertions(+), 132 deletions(-) create mode 100644 cinder/tests/volume/drivers/netapp/test_common.py diff --git a/cinder/test.py b/cinder/test.py index 2906d67a4..3d98f5ac5 100644 --- a/cinder/test.py +++ b/cinder/test.py @@ -258,6 +258,7 @@ class TestCase(testtools.TestCase): patcher = mock.patch.object(obj, attr_name, new_attr, **kwargs) patcher.start() self.addCleanup(patcher.stop) + return new_attr # Useful assertions def assertDictMatch(self, d1, d2, approx_equal=False, tolerance=0.001): diff --git a/cinder/tests/test_netapp.py b/cinder/tests/test_netapp.py index 6c02e3270..664de2ae7 100644 --- a/cinder/tests/test_netapp.py +++ b/cinder/tests/test_netapp.py @@ -745,21 +745,6 @@ class NetAppDriverNegativeTestCase(test.TestCase): except exception.InvalidInput: pass - def test_non_netapp_driver(self): - self.mock_object(utils, 'OpenStackInfo') - configuration = create_configuration() - common.netapp_unified_plugin_registry['test_family'] =\ - {'iscsi': 'cinder.volume.drivers.arbitrary.IscsiDriver'} - configuration.netapp_storage_family = 'test_family' - configuration.netapp_storage_protocol = 'iscsi' - try: - common.NetAppDriver(configuration=configuration) - raise AssertionError('Non NetApp driver is getting instantiated.') - except exception.InvalidInput: - pass - finally: - common.netapp_unified_plugin_registry.pop('test_family') - class FakeDirect7MODEServerHandler(FakeHTTPRequestHandler): """HTTP handler that fakes enough stuff to allow the driver to run.""" @@ -1213,8 +1198,8 @@ class NetAppDirect7modeISCSIDriverTestCase_NV( self.driver.volume_list = [] def test_connect(self): - self.driver.driver.library.zapi_client = mock.MagicMock() - self.driver.driver.library.zapi_client.get_ontapi_version.\ + self.driver.library.zapi_client = mock.MagicMock() + self.driver.library.zapi_client.get_ontapi_version.\ return_value = (1, 20) self.driver.check_for_setup_error() diff --git a/cinder/tests/test_netapp_eseries_iscsi.py b/cinder/tests/test_netapp_eseries_iscsi.py index aeb6720d9..9ba083500 100644 --- a/cinder/tests/test_netapp_eseries_iscsi.py +++ b/cinder/tests/test_netapp_eseries_iscsi.py @@ -853,10 +853,10 @@ class NetAppEseriesISCSIDriverTestCase(test.TestCase): fake_pool['volumeGroupRef'] = 'foo' fake_pools = [fake_pool] storage_pools.return_value = fake_pools - drv = self.driver - storage_vol = drv.driver._create_volume(self.fake_eseries_pool_label, - self.fake_eseries_volume_label, - self.fake_size_gb) + storage_vol = self.driver._create_volume( + self.fake_eseries_pool_label, + self.fake_eseries_volume_label, + self.fake_size_gb) log_info.assert_called_once_with("Created volume with label %s.", self.fake_eseries_volume_label) self.assertEqual('CorrectVolume', storage_vol) diff --git a/cinder/tests/volume/drivers/netapp/test_common.py b/cinder/tests/volume/drivers/netapp/test_common.py new file mode 100644 index 000000000..347333f3b --- /dev/null +++ b/cinder/tests/volume/drivers/netapp/test_common.py @@ -0,0 +1,127 @@ +# Copyright (c) 2014 Clinton Knight. 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 +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import mock +import six + +from cinder import exception +from cinder import test +import cinder.tests.volume.drivers.netapp.fakes as na_fakes +import cinder.volume.drivers.netapp.common as na_common +import cinder.volume.drivers.netapp.dataontap.fc_cmode as fc_cmode +import cinder.volume.drivers.netapp.utils as na_utils + + +class NetAppDriverFactoryTestCase(test.TestCase): + + def setUp(self): + super(NetAppDriverFactoryTestCase, self).setUp() + self.mock_object(na_common, 'LOG') + + def test_new(self): + + self.mock_object(na_utils.OpenStackInfo, 'info', + mock.Mock(return_value='fake_info')) + mock_create_driver = self.mock_object(na_common.NetAppDriver, + 'create_driver') + + config = na_fakes.create_configuration() + config.netapp_storage_family = 'fake_family' + config.netapp_storage_protocol = 'fake_protocol' + + kwargs = {'configuration': config} + na_common.NetAppDriver(**kwargs) + + kwargs['app_version'] = 'fake_info' + mock_create_driver.assert_called_with('fake_family', 'fake_protocol', + *(), **kwargs) + + def test_new_missing_config(self): + + self.mock_object(na_utils.OpenStackInfo, 'info') + self.mock_object(na_common.NetAppDriver, 'create_driver') + + self.assertRaises(exception.InvalidInput, na_common.NetAppDriver, **{}) + + def test_new_missing_family(self): + + self.mock_object(na_utils.OpenStackInfo, 'info') + self.mock_object(na_common.NetAppDriver, 'create_driver') + + config = na_fakes.create_configuration() + config.netapp_storage_protocol = 'fake_protocol' + config.netapp_storage_family = None + + kwargs = {'configuration': config} + self.assertRaises(exception.InvalidInput, + na_common.NetAppDriver, + **kwargs) + + def test_new_missing_protocol(self): + + self.mock_object(na_utils.OpenStackInfo, 'info') + self.mock_object(na_common.NetAppDriver, 'create_driver') + + config = na_fakes.create_configuration() + config.netapp_storage_protocol = None + config.netapp_storage_family = 'fake_family' + + kwargs = {'configuration': config} + self.assertRaises(exception.InvalidInput, + na_common.NetAppDriver, + **kwargs) + + def test_create_driver(self): + + def get_full_class_name(obj): + return obj.__module__ + '.' + obj.__class__.__name__ + + kwargs = {'configuration': na_fakes.create_configuration(), + 'app_version': 'fake_info'} + + registry = na_common.NETAPP_UNIFIED_DRIVER_REGISTRY + + for family in six.iterkeys(registry): + for protocol, full_class_name in six.iteritems(registry[family]): + driver = na_common.NetAppDriver.create_driver( + family, protocol, **kwargs) + self.assertEqual(full_class_name, get_full_class_name(driver)) + + def test_create_driver_case_insensitive(self): + + kwargs = {'configuration': na_fakes.create_configuration(), + 'app_version': 'fake_info'} + + driver = na_common.NetAppDriver.create_driver('ONTAP_CLUSTER', 'FC', + **kwargs) + + self.assertIsInstance(driver, fc_cmode.NetAppCmodeFibreChannelDriver) + + def test_create_driver_invalid_family(self): + + kwargs = {'configuration': na_fakes.create_configuration(), + 'app_version': 'fake_info'} + + self.assertRaises(exception.InvalidInput, + na_common.NetAppDriver.create_driver, + 'kardashian', 'iscsi', **kwargs) + + def test_create_driver_invalid_protocol(self): + + kwargs = {'configuration': na_fakes.create_configuration(), + 'app_version': 'fake_info'} + + self.assertRaises(exception.InvalidInput, + na_common.NetAppDriver.create_driver, + 'ontap_7mode', 'carrier_pigeon', **kwargs) \ No newline at end of file diff --git a/cinder/volume/drivers/netapp/common.py b/cinder/volume/drivers/netapp/common.py index 70415b73d..5fa387c77 100644 --- a/cinder/volume/drivers/netapp/common.py +++ b/cinder/volume/drivers/netapp/common.py @@ -1,4 +1,3 @@ -# Copyright (c) 2012 NetApp, Inc. All rights reserved. # Copyright (c) 2014 Navneet Singh. All rights reserved. # Copyright (c) 2014 Clinton Knight. All rights reserved. # @@ -16,162 +15,98 @@ """ Unified driver for NetApp storage systems. -Supports call to multiple storage systems of different families and protocols. +Supports multiple storage systems of different families and protocols. """ + from oslo.utils import importutils from cinder import exception from cinder.i18n import _, _LI from cinder.openstack.common import log as logging -from cinder.volume import driver from cinder.volume.drivers.netapp.options import netapp_proxy_opts from cinder.volume.drivers.netapp import utils as na_utils LOG = logging.getLogger(__name__) - -# NOTE(singn): Holds family:{protocol:driver} registration information. -# Plug in new families and protocols to support new drivers. -# No other code modification required. - DATAONTAP_PATH = 'cinder.volume.drivers.netapp.dataontap' ESERIES_PATH = 'cinder.volume.drivers.netapp.eseries' -netapp_unified_plugin_registry =\ - {'ontap_cluster': - { - 'iscsi': DATAONTAP_PATH + '.iscsi_cmode.NetAppCmodeISCSIDriver', - 'nfs': DATAONTAP_PATH + '.nfs_cmode.NetAppCmodeNfsDriver', - 'fc': DATAONTAP_PATH + '.fc_cmode.NetAppCmodeFibreChannelDriver' - }, - 'ontap_7mode': - { - 'iscsi': DATAONTAP_PATH + '.iscsi_7mode.NetApp7modeISCSIDriver', - 'nfs': DATAONTAP_PATH + '.nfs_7mode.NetApp7modeNfsDriver', - 'fc': DATAONTAP_PATH + '.fc_7mode.NetApp7modeFibreChannelDriver' - }, - 'eseries': - { - 'iscsi': ESERIES_PATH + '.iscsi.NetAppEseriesISCSIDriver' - }, - } +# Add new drivers here, no other code changes required. +NETAPP_UNIFIED_DRIVER_REGISTRY = { + 'ontap_cluster': + { + 'iscsi': DATAONTAP_PATH + '.iscsi_cmode.NetAppCmodeISCSIDriver', + 'nfs': DATAONTAP_PATH + '.nfs_cmode.NetAppCmodeNfsDriver', + 'fc': DATAONTAP_PATH + '.fc_cmode.NetAppCmodeFibreChannelDriver' + }, + 'ontap_7mode': + { + 'iscsi': DATAONTAP_PATH + '.iscsi_7mode.NetApp7modeISCSIDriver', + 'nfs': DATAONTAP_PATH + '.nfs_7mode.NetApp7modeNfsDriver', + 'fc': DATAONTAP_PATH + '.fc_7mode.NetApp7modeFibreChannelDriver' + }, + 'eseries': + { + 'iscsi': ESERIES_PATH + '.iscsi.NetAppEseriesISCSIDriver' + }} class NetAppDriver(object): """"NetApp unified block storage driver. - Acts as a mediator to NetApp storage drivers. - Proxies requests based on the storage family and protocol configured. - Override the proxy driver method by adding method in this driver. + Acts as a factory to create NetApp storage drivers based on the + storage family and protocol configured. """ REQUIRED_FLAGS = ['netapp_storage_family', 'netapp_storage_protocol'] - def __init__(self, *args, **kwargs): - super(NetAppDriver, self).__init__() - - app_version = na_utils.OpenStackInfo().info() - LOG.info(_LI('OpenStack OS Version Info: %(info)s') % { - 'info': app_version}) + def __new__(cls, *args, **kwargs): - self.configuration = kwargs.get('configuration', None) - if not self.configuration: + config = kwargs.get('configuration', None) + if not config: raise exception.InvalidInput( - reason=_("Required configuration not found")) + reason=_('Required configuration not found')) - self.configuration.append_config_values(netapp_proxy_opts) - na_utils.check_flags(self.REQUIRED_FLAGS, self.configuration) + config.append_config_values(netapp_proxy_opts) + na_utils.check_flags(NetAppDriver.REQUIRED_FLAGS, config) + app_version = na_utils.OpenStackInfo().info() + LOG.info(_LI('OpenStack OS Version Info: %(info)s') % { + 'info': app_version}) kwargs['app_version'] = app_version - self.driver = NetAppDriverFactory.create_driver( - self.configuration.netapp_storage_family, - self.configuration.netapp_storage_protocol, - *args, **kwargs) - - def __setattr__(self, name, value): - """Sets the attribute.""" - if getattr(self, 'driver', None): - self.driver.__setattr__(name, value) - return - object.__setattr__(self, name, value) - - def __getattr__(self, name): - """"Gets the attribute.""" - drv = object.__getattribute__(self, 'driver') - return getattr(drv, name) - - -class NetAppDriverFactory(object): - """Factory to instantiate appropriate NetApp driver.""" + return NetAppDriver.create_driver(config.netapp_storage_family, + config.netapp_storage_protocol, + *args, **kwargs) @staticmethod def create_driver(storage_family, storage_protocol, *args, **kwargs): """"Creates an appropriate driver based on family and protocol.""" - fmt = {'storage_family': storage_family.lower(), - 'storage_protocol': storage_protocol.lower()} + storage_family = storage_family.lower() + storage_protocol = storage_protocol.lower() + + fmt = {'storage_family': storage_family, + 'storage_protocol': storage_protocol} LOG.info(_LI('Requested unified config: %(storage_family)s and ' - '%(storage_protocol)s') % fmt) + '%(storage_protocol)s.') % fmt) - family_meta = netapp_unified_plugin_registry.get(storage_family) + family_meta = NETAPP_UNIFIED_DRIVER_REGISTRY.get(storage_family) if family_meta is None: raise exception.InvalidInput( - reason=_('Storage family %s is not supported') + reason=_('Storage family %s is not supported.') % storage_family) driver_loc = family_meta.get(storage_protocol) if driver_loc is None: raise exception.InvalidInput( - reason=_('Protocol %(storage_protocol)s is not supported' - ' for storage family %(storage_family)s') % fmt) + reason=_('Protocol %(storage_protocol)s is not supported ' + 'for storage family %(storage_family)s.') % fmt) - NetAppDriverFactory.check_netapp_driver(driver_loc) kwargs = kwargs or {} kwargs['netapp_mode'] = 'proxy' driver = importutils.import_object(driver_loc, *args, **kwargs) - LOG.info(_LI('NetApp driver of family %(storage_family)s and protocol' - ' %(storage_protocol)s loaded') % fmt) + LOG.info(_LI('NetApp driver of family %(storage_family)s and protocol ' + '%(storage_protocol)s loaded.') % fmt) return driver - - @staticmethod - def check_netapp_driver(location): - """Checks if the driver requested is a netapp driver.""" - if location.find(".netapp.") == -1: - raise exception.InvalidInput( - reason=_("Only loading netapp drivers supported.")) - - -class Deprecated(driver.VolumeDriver): - """Deprecated driver for NetApp. - - This driver is used for mapping deprecated - drivers to itself in manager. It prevents cinder - from getting errored out in case of upgrade scenarios - and also suggests further steps. - """ - - def __init__(self, *args, **kwargs): - self._log_deprecated_warn() - - def _log_deprecated_warn(self): - """Logs appropriate warning and suggestion.""" - - link = "https://communities.netapp.com/groups/openstack" - msg = _("The configured NetApp driver is deprecated." - " Please refer the link to resolve the issue '%s'.") - LOG.warning(msg % link) - - def check_for_setup_error(self): - pass - - def ensure_export(self, context, volume): - pass - - def get_volume_stats(self, refresh=False): - """Return the current state of the volume service. If 'refresh' is - True, run the update first. - """ - self._log_deprecated_warn() - return None -- 2.45.2