From 8e4e0c86c32d802389e4718e5610fb06765e4308 Mon Sep 17 00:00:00 2001 From: Ryan Liang Date: Tue, 27 Dec 2016 10:04:20 +0800 Subject: [PATCH] Unity: Add support to set IO ports in option Add option `unity_io_ports`, which is a list of strings set by users to specify which FC/iSCSI ports can be used by the driver. The format is "spa_iom_0_fc0" for FC and "spa_eth0" for iSCSI. Unix-style glob expression is supported, like "spa_*". DocImpact Implements: blueprint unity-add-io-port-option Change-Id: Ifd7873b65cb06605b72af216b87846117f874868 --- .../drivers/dell_emc/unity/test_adapter.py | 66 ++++++++++++++- .../drivers/dell_emc/unity/test_client.py | 40 +++++++-- .../volume/drivers/dell_emc/unity/adapter.py | 84 ++++++++++++++++--- .../volume/drivers/dell_emc/unity/client.py | 29 +++++-- .../volume/drivers/dell_emc/unity/driver.py | 8 +- cinder/volume/drivers/dell_emc/unity/utils.py | 23 +++++ .../add-io-ports-option-c751d1bd395dd614.yaml | 3 + 7 files changed, 221 insertions(+), 32 deletions(-) create mode 100644 releasenotes/notes/add-io-ports-option-c751d1bd395dd614.yaml diff --git a/cinder/tests/unit/volume/drivers/dell_emc/unity/test_adapter.py b/cinder/tests/unit/volume/drivers/dell_emc/unity/test_adapter.py index 1e787b0b7f1..190414a6c59 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/unity/test_adapter.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/unity/test_adapter.py @@ -32,7 +32,9 @@ from cinder.volume.drivers.dell_emc.unity import adapter ######################## class MockConfig(object): def __init__(self): + self.config_group = 'test_backend' self.unity_storage_pool_names = ['pool1', 'pool2'] + self.unity_io_ports = None self.reserved_percentage = 5 self.max_over_subscription_ratio = 300 self.volume_backend_name = 'backend' @@ -129,12 +131,13 @@ class MockClient(object): raise ex.DetachIsCalled() @staticmethod - def get_iscsi_target_info(): + def get_iscsi_target_info(allowed_ports=None): return [{'portal': '1.2.3.4:1234', 'iqn': 'iqn.1-1.com.e:c.a.a0'}, {'portal': '1.2.3.5:1234', 'iqn': 'iqn.1-1.com.e:c.a.a1'}] @staticmethod - def get_fc_target_info(host=None, logged_in_only=False): + def get_fc_target_info(host=None, logged_in_only=False, + allowed_ports=None): if host and host.name == 'no_target': ret = [] else: @@ -154,6 +157,15 @@ class MockClient(object): if size_gib <= 0: raise ex.ExtendLunError + @staticmethod + def get_fc_ports(): + return test_client.MockResourceList(ids=['spa_iom_0_fc0', + 'spa_iom_0_fc1']) + + @staticmethod + def get_ethernet_ports(): + return test_client.MockResourceList(ids=['spa_eth0', 'spb_eth0']) + class MockLookupService(object): @staticmethod @@ -171,7 +183,9 @@ class MockLookupService(object): def mock_adapter(driver_clz): ret = driver_clz() ret._client = MockClient() - ret.do_setup(MockDriver(), MockConfig()) + with mock.patch('cinder.volume.drivers.dell_emc.unity.adapter.' + 'CommonAdapter.validate_ports'): + ret.do_setup(MockDriver(), MockConfig()) ret.lookup_service = MockLookupService() return ret @@ -420,6 +434,26 @@ class CommonAdapterTest(unittest.TestCase): self.assertRaises(exception.VolumeBackendAPIException, f) + def test_normalize_config(self): + config = MockConfig() + config.unity_storage_pool_names = [' pool_1 ', '', ' '] + config.unity_io_ports = [' spa_eth2 ', '', ' '] + normalized = self.adapter.normalize_config(config) + self.assertEqual(['pool_1'], normalized.unity_storage_pool_names) + self.assertEqual(['spa_eth2'], normalized.unity_io_ports) + + def test_normalize_config_raise(self): + with self.assertRaisesRegexp(exception.InvalidConfigurationValue, + 'unity_storage_pool_names'): + config = MockConfig() + config.unity_storage_pool_names = ['', ' '] + self.adapter.normalize_config(config) + with self.assertRaisesRegexp(exception.InvalidConfigurationValue, + 'unity_io_ports'): + config = MockConfig() + config.unity_io_ports = ['', ' '] + self.adapter.normalize_config(config) + class FCAdapterTest(unittest.TestCase): def setUp(self): @@ -488,6 +522,32 @@ class FCAdapterTest(unittest.TestCase): target_wwn = ['100000051e55a100', '100000051e55a121'] self.assertListEqual(target_wwn, data['target_wwn']) + def test_validate_ports_whitelist_none(self): + ports = self.adapter.validate_ports(None) + self.assertEqual(set(('spa_iom_0_fc0', 'spa_iom_0_fc1')), set(ports)) + + def test_validate_ports(self): + ports = self.adapter.validate_ports(['spa_iom_0_fc0']) + self.assertEqual(set(('spa_iom_0_fc0',)), set(ports)) + + def test_validate_ports_asterisk(self): + ports = self.adapter.validate_ports(['spa*']) + self.assertEqual(set(('spa_iom_0_fc0', 'spa_iom_0_fc1')), set(ports)) + + def test_validate_ports_question_mark(self): + ports = self.adapter.validate_ports(['spa_iom_0_fc?']) + self.assertEqual(set(('spa_iom_0_fc0', 'spa_iom_0_fc1')), set(ports)) + + def test_validate_ports_no_matched(self): + with self.assertRaisesRegexp(exception.InvalidConfigurationValue, + 'unity_io_ports'): + self.adapter.validate_ports(['spc_invalid']) + + def test_validate_ports_unmatched_whitelist(self): + with self.assertRaisesRegexp(exception.InvalidConfigurationValue, + 'unity_io_ports'): + self.adapter.validate_ports(['spa_iom*', 'spc_invalid']) + class ISCSIAdapterTest(unittest.TestCase): def setUp(self): diff --git a/cinder/tests/unit/volume/drivers/dell_emc/unity/test_client.py b/cinder/tests/unit/volume/drivers/dell_emc/unity/test_client.py index a4c70007b32..d00f96cbd6a 100644 --- a/cinder/tests/unit/volume/drivers/dell_emc/unity/test_client.py +++ b/cinder/tests/unit/volume/drivers/dell_emc/unity/test_client.py @@ -48,6 +48,10 @@ class MockResource(object): self.max_kbps = None self.pool_name = 'Pool0' + @property + def id(self): + return self._id + def get_id(self): return self._id @@ -141,11 +145,11 @@ class MockResource(object): path1.is_logged_in = False path2 = MockResource('%s_path_2' % self.name) path2.is_logged_in = True - return [path0, path1] + return MockResourceList.create(path0, path1) @property def fc_port(self): - ret = MockResource() + ret = MockResource(_id='spa_iom_0_fc0') ret.wwn = '00:11:22:33:44:55:66:77:88:99:AA:BB:CC:DD:EE:FF' return ret @@ -163,8 +167,11 @@ class MockResource(object): class MockResourceList(object): - def __init__(self, names): - self.resources = [MockResource(name) for name in names] + def __init__(self, names=None, ids=None): + if names is not None: + self.resources = [MockResource(name=name) for name in names] + elif ids is not None: + self.resources = [MockResource(_id=_id) for _id in ids] @staticmethod def create(*rsc_list): @@ -185,6 +192,12 @@ class MockResourceList(object): def __getattr__(self, item): return [getattr(i, item) for i in self.resources] + def shadow_copy(self, **kwargs): + if list(filter(None, kwargs.values())): + return MockResourceList.create(self.resources[0]) + else: + return self + class MockSystem(object): def __init__(self): @@ -222,7 +235,7 @@ class MockSystem(object): portal0.ip_address = '1.1.1.1' portal1 = MockResource('p1') portal1.ip_address = '1.1.1.2' - return [portal0, portal1] + return MockResourceList.create(portal0, portal1) @staticmethod def get_fc_port(): @@ -230,7 +243,7 @@ class MockSystem(object): port0.wwn = '00:11:22:33:44:55:66:77:88:99:AA:BB:CC:DD:EE:FF' port1 = MockResource('fcp1') port1.wwn = '00:11:22:33:44:55:66:77:88:99:AA:BB:CC:DD:FF:EE' - return [port0, port1] + return MockResourceList.create(port0, port1) @staticmethod def create_io_limit_policy(name, max_iops=None, max_kbps=None): @@ -393,15 +406,30 @@ class ClientTest(unittest.TestCase): {'iqn': 'iqn.1-1.com.e:c.p1.0', 'portal': '1.1.1.2:3260'}] self.assertListEqual(expected, ret) + def test_get_iscsi_target_info_allowed_ports(self): + ret = self.client.get_iscsi_target_info(allowed_ports=['spa_eth0']) + expected = [{'iqn': 'iqn.1-1.com.e:c.p0.0', 'portal': '1.1.1.1:3260'}] + self.assertListEqual(expected, ret) + def test_get_fc_target_info_without_host(self): ret = self.client.get_fc_target_info() self.assertListEqual(['8899AABBCCDDEEFF', '8899AABBCCDDFFEE'], ret) + def test_get_fc_target_info_without_host_but_allowed_ports(self): + ret = self.client.get_fc_target_info(allowed_ports=['spa_fc0']) + self.assertListEqual(['8899AABBCCDDEEFF'], ret) + def test_get_fc_target_info_with_host(self): host = MockResource('host0') ret = self.client.get_fc_target_info(host, True) self.assertListEqual(['8899AABBCCDDEEFF', '8899AABBCCDDEEFF'], ret) + def test_get_fc_target_info_with_host_and_allowed_ports(self): + host = MockResource('host0') + ret = self.client.get_fc_target_info(host, True, + allowed_ports=['spb_iom_0_fc0']) + self.assertListEqual([], ret) + def test_get_io_limit_policy_none(self): ret = self.client.get_io_limit_policy(None) self.assertIsNone(ret) diff --git a/cinder/volume/drivers/dell_emc/unity/adapter.py b/cinder/volume/drivers/dell_emc/unity/adapter.py index 6c03b2e908d..07c5e44bf77 100644 --- a/cinder/volume/drivers/dell_emc/unity/adapter.py +++ b/cinder/volume/drivers/dell_emc/unity/adapter.py @@ -41,6 +41,7 @@ class CommonAdapter(object): def __init__(self, version=None): self.version = version self.driver = None + self.config = None self.configured_pool_names = None self.reserved_percentage = None self.max_over_subscription_ratio = None @@ -54,24 +55,75 @@ class CommonAdapter(object): self._serial_number = None self.storage_pools_map = None self._client = None + self.allowed_ports = None def do_setup(self, driver, conf): self.driver = driver - self.configured_pool_names = conf.unity_storage_pool_names - self.reserved_percentage = conf.reserved_percentage - self.max_over_subscription_ratio = conf.max_over_subscription_ratio - self.volume_backend_name = (conf.safe_get('volume_backend_name') or - self.driver_name) - self.ip = conf.san_ip - self.username = conf.san_login - self.password = conf.san_password + self.config = self.normalize_config(conf) + self.configured_pool_names = self.config.unity_storage_pool_names + self.reserved_percentage = self.config.reserved_percentage + self.max_over_subscription_ratio = ( + self.config.max_over_subscription_ratio) + self.volume_backend_name = ( + self.config.safe_get('volume_backend_name') or self.driver_name) + self.ip = self.config.san_ip + self.username = self.config.san_login + self.password = self.config.san_password # Unity currently not support to upload certificate. # Once it supports, enable the verify. self.array_cert_verify = False - self.array_ca_cert_path = conf.driver_ssl_cert_path + self.array_ca_cert_path = self.config.driver_ssl_cert_path self.storage_pools_map = self.get_managed_pools() + self.allowed_ports = self.validate_ports(self.config.unity_io_ports) + + def normalize_config(self, config): + config.unity_storage_pool_names = utils.remove_empty( + '%s.unity_storage_pool_names' % config.config_group, + config.unity_storage_pool_names) + + config.unity_io_ports = utils.remove_empty( + '%s.unity_io_ports' % config.config_group, + config.unity_io_ports) + return config + + def get_all_ports(self): + raise NotImplementedError() + + def validate_ports(self, ports_whitelist): + all_ports = self.get_all_ports() + # After normalize_config, `ports_whitelist` could be only None or valid + # list in which the items are stripped. + if ports_whitelist is None: + return all_ports.id + + # For iSCSI port, the format is 'spa_eth0', and 'spa_iom_0_fc0' for FC. + # Unix style glob like 'spa_*' is supported. + whitelist = set(ports_whitelist) + + matched, _ignored, unmatched_whitelist = utils.match_any(all_ports.id, + whitelist) + if not matched: + LOG.error(_LE('No matched ports filtered by all patterns: %s'), + whitelist) + raise exception.InvalidConfigurationValue( + option='%s.unity_io_ports' % self.config.config_group, + value=self.config.unity_io_ports) + + if unmatched_whitelist: + LOG.error(_LE('No matched ports filtered by below patterns: %s'), + unmatched_whitelist) + raise exception.InvalidConfigurationValue( + option='%s.unity_io_ports' % self.config.config_group, + value=self.config.unity_io_ports) + + LOG.info(_LI('These ports %(matched)s will be used based on ' + 'the option unity_io_ports: %(config)s'), + {'matched': matched, + 'config': self.config.unity_io_ports}) + return matched + @property def verify_cert(self): verify_cert = self.array_cert_verify @@ -436,11 +488,14 @@ class ISCSIAdapter(CommonAdapter): driver_name = 'UnityISCSIDriver' driver_volume_type = 'iscsi' + def get_all_ports(self): + return self.client.get_ethernet_ports() + def get_connector_uids(self, connector): return utils.extract_iscsi_uids(connector) def get_connection_info(self, hlu, host, connector): - targets = self.client.get_iscsi_target_info() + targets = self.client.get_iscsi_target_info(self.allowed_ports) if not targets: msg = _("There is no accessible iSCSI targets on the system.") raise exception.VolumeBackendAPIException(data=msg) @@ -471,6 +526,9 @@ class FCAdapter(CommonAdapter): super(FCAdapter, self).do_setup(driver, config) self.lookup_service = utils.create_lookup_service() + def get_all_ports(self): + return self.client.get_fc_ports() + def get_connector_uids(self, connector): return utils.extract_fc_uids(connector) @@ -480,7 +538,8 @@ class FCAdapter(CommonAdapter): def get_connection_info(self, hlu, host, connector): targets = self.client.get_fc_target_info( - host, logged_in_only=(not self.auto_zone_enabled)) + host, logged_in_only=(not self.auto_zone_enabled), + allowed_ports=self.allowed_ports) if not targets: msg = _("There is no accessible fibre channel targets on the " @@ -507,7 +566,8 @@ class FCAdapter(CommonAdapter): } host = self.client.get_host(connector['host']) if len(host.host_luns) == 0: - targets = self.client.get_fc_target_info(logged_in_only=True) + targets = self.client.get_fc_target_info( + logged_in_only=True, allowed_ports=self.allowed_ports) ret['data'] = self._get_fc_zone_info(connector['wwpns'], targets) diff --git a/cinder/volume/drivers/dell_emc/unity/client.py b/cinder/volume/drivers/dell_emc/unity/client.py index e4f4210fd4f..fc5313fca7a 100644 --- a/cinder/volume/drivers/dell_emc/unity/client.py +++ b/cinder/volume/drivers/dell_emc/unity/client.py @@ -229,13 +229,21 @@ class UnityClient(object): def get_host(self, name): return self.system.get_host(name=name) - def get_iscsi_target_info(self): + def get_ethernet_ports(self): + return self.system.get_ethernet_port() + + def get_iscsi_target_info(self, allowed_ports=None): portals = self.system.get_iscsi_portal() + portals = portals.shadow_copy(port_ids=allowed_ports) return [{'portal': utils.convert_ip_to_portal(p.ip_address), 'iqn': p.iscsi_node.name} for p in portals] - def get_fc_target_info(self, host=None, logged_in_only=False): + def get_fc_ports(self): + return self.system.get_fc_port() + + def get_fc_target_info(self, host=None, logged_in_only=False, + allowed_ports=None): """Get the ports WWN of FC on array. :param host: the host to which the FC port is registered. @@ -246,15 +254,18 @@ class UnityClient(object): This function removes the colons and returns the last 16 bits: 5006016C09200925. """ - ports = [] if logged_in_only: - for host_initiator in host.fc_host_initiators: - paths = host_initiator.paths or [] - for path in paths: - if path.is_logged_in: - ports.append(path.fc_port) + ports = [] + for paths in filter(None, host.fc_host_initiators.paths): + paths = paths.shadow_copy(is_logged_in=True) + # `paths.fc_port` is just a list, not a UnityFcPortList, + # so use filter instead of shadow_copy here. + ports.extend(filter(lambda p: (allowed_ports is None or + p.get_id() in allowed_ports), + paths.fc_port)) else: - ports = self.system.get_fc_port() + ports = self.get_fc_ports() + ports = ports.shadow_copy(port_ids=allowed_ports) return [po.wwn.replace(':', '')[16:] for po in ports] def create_io_limit_policy(self, name, max_iops=None, max_kbps=None): diff --git a/cinder/volume/drivers/dell_emc/unity/driver.py b/cinder/volume/drivers/dell_emc/unity/driver.py index 3fcf7ab4e99..8224744334a 100644 --- a/cinder/volume/drivers/dell_emc/unity/driver.py +++ b/cinder/volume/drivers/dell_emc/unity/driver.py @@ -31,8 +31,12 @@ CONF = cfg.CONF UNITY_OPTS = [ cfg.ListOpt('unity_storage_pool_names', default=None, - help='A comma-separated list of storage pool names ' - 'to be used.')] + help='A comma-separated list of storage pool names to be ' + 'used.'), + cfg.ListOpt('unity_io_ports', + default=None, + help='A comma-separated list of iSCSI or FC ports to be used. ' + 'Each port can be Unix-style glob expressions.')] CONF.register_opts(UNITY_OPTS) diff --git a/cinder/volume/drivers/dell_emc/unity/utils.py b/cinder/volume/drivers/dell_emc/unity/utils.py index 5255462f006..e73943a4092 100644 --- a/cinder/volume/drivers/dell_emc/unity/utils.py +++ b/cinder/volume/drivers/dell_emc/unity/utils.py @@ -18,6 +18,7 @@ from __future__ import division import contextlib import functools from oslo_log import log as logging +from oslo_utils import fnmatch from oslo_utils import units import six @@ -261,3 +262,25 @@ def get_backend_qos_specs(volume): QOS_MAX_IOPS: max_iops, QOS_MAX_BWS: max_bws, } + + +def remove_empty(option, value_list): + if value_list is not None: + value_list = list(filter(None, map(str.strip, value_list))) + if not value_list: + raise exception.InvalidConfigurationValue(option=option, + value=value_list) + return value_list + + +def match_any(full, patterns): + matched = list( + filter(lambda x: any(fnmatch.fnmatchcase(x, p) for p in patterns), + full)) + unmatched = list( + filter(lambda x: not any(fnmatch.fnmatchcase(x, p) for p in patterns), + full)) + unmatched_patterns = list( + filter(lambda p: not any(fnmatch.fnmatchcase(x, p) for x in full), + patterns)) + return matched, unmatched, unmatched_patterns diff --git a/releasenotes/notes/add-io-ports-option-c751d1bd395dd614.yaml b/releasenotes/notes/add-io-ports-option-c751d1bd395dd614.yaml new file mode 100644 index 00000000000..beadbe26b95 --- /dev/null +++ b/releasenotes/notes/add-io-ports-option-c751d1bd395dd614.yaml @@ -0,0 +1,3 @@ +--- +features: + - Add support to configure IO ports option in Dell EMC Unity driver.