From f4f46e972004eca66e4751a836156c342e448621 Mon Sep 17 00:00:00 2001 From: Joe Cropper Date: Wed, 18 Feb 2015 23:55:04 -0600 Subject: [PATCH] Fix some message nits in the ZoneManager This patch addresses some nit comments I've received on some of the ZoneManager's conf options (e.g., not documenting the valid zoning strategies) and some other minor message format items. Cleaned up some globalization items as well (% => ,). Also fixed the UTs since they didn't work at all due to duplicate opt errors based on the UT structure. Change-Id: I84b55710bd8afe4fcdd539fcc49805ba88d13dc4 Closes-Bug: 1423450 --- .../tests/zonemanager/test_fc_zone_manager.py | 17 +++++++---- .../tests/zonemanager/test_volume_driver.py | 8 +++-- cinder/zonemanager/fc_san_lookup_service.py | 4 +-- cinder/zonemanager/fc_zone_manager.py | 29 ++++++++++--------- cinder/zonemanager/utils.py | 12 ++++---- 5 files changed, 39 insertions(+), 31 deletions(-) diff --git a/cinder/tests/zonemanager/test_fc_zone_manager.py b/cinder/tests/zonemanager/test_fc_zone_manager.py index f1b209947..fa3f7c654 100644 --- a/cinder/tests/zonemanager/test_fc_zone_manager.py +++ b/cinder/tests/zonemanager/test_fc_zone_manager.py @@ -36,7 +36,8 @@ target_list = ['20240002ac000a50'] class TestFCZoneManager(test.TestCase): - def setUp(self): + @mock.patch('oslo_config.cfg._is_opt_registered', return_value=False) + def setUp(self, opt_mock): super(TestFCZoneManager, self).setUp() config = conf.Configuration(None) config.fc_fabric_names = fabric_name @@ -53,9 +54,10 @@ class TestFCZoneManager(test.TestCase): self.driver = Mock(FCZoneDriver) def __init__(self, *args, **kwargs): - test.TestCase.__init__(self, *args, **kwargs) + super(TestFCZoneManager, self).__init__(*args, **kwargs) - def test_add_connection(self): + @mock.patch('oslo_config.cfg._is_opt_registered', return_value=False) + def test_add_connection(self, opt_mock): with mock.patch.object(self.zm.driver, 'add_connection')\ as add_connection_mock: self.zm.driver.get_san_context.return_value = fabric_map @@ -64,14 +66,16 @@ class TestFCZoneManager(test.TestCase): add_connection_mock.assert_called_once_with(fabric_name, init_target_map) - def test_add_connection_error(self): + @mock.patch('oslo_config.cfg._is_opt_registered', return_value=False) + def test_add_connection_error(self, opt_mock): with mock.patch.object(self.zm.driver, 'add_connection')\ as add_connection_mock: add_connection_mock.side_effect = exception.FCZoneDriverException self.assertRaises(exception.ZoneManagerException, self.zm.add_connection, init_target_map) - def test_delete_connection(self): + @mock.patch('oslo_config.cfg._is_opt_registered', return_value=False) + def test_delete_connection(self, opt_mock): with mock.patch.object(self.zm.driver, 'delete_connection')\ as delete_connection_mock: self.zm.driver.get_san_context.return_value = fabric_map @@ -80,7 +84,8 @@ class TestFCZoneManager(test.TestCase): delete_connection_mock.assert_called_once_with(fabric_name, init_target_map) - def test_delete_connection_error(self): + @mock.patch('oslo_config.cfg._is_opt_registered', return_value=False) + def test_delete_connection_error(self, opt_mock): with mock.patch.object(self.zm.driver, 'delete_connection')\ as del_connection_mock: del_connection_mock.side_effect = exception.FCZoneDriverException diff --git a/cinder/tests/zonemanager/test_volume_driver.py b/cinder/tests/zonemanager/test_volume_driver.py index 6f9751888..fe4c919d6 100644 --- a/cinder/tests/zonemanager/test_volume_driver.py +++ b/cinder/tests/zonemanager/test_volume_driver.py @@ -41,10 +41,11 @@ class TestVolumeDriver(test.TestCase): self.driver = None def __init__(self, *args, **kwargs): - test.TestCase.__init__(self, *args, **kwargs) + super(TestVolumeDriver, self).__init__(*args, **kwargs) + @mock.patch('oslo_config.cfg._is_opt_registered', return_value=False) @mock.patch.object(utils, 'require_driver_initialized') - def test_initialize_connection_with_decorator(self, utils_mock): + def test_initialize_connection_with_decorator(self, utils_mock, opt_mock): utils_mock.return_value = True with mock.patch.object(fc_zone_manager.ZoneManager, 'add_connection')\ as add_zone_mock: @@ -66,8 +67,9 @@ class TestVolumeDriver(test.TestCase): self.driver.no_zone_initialize_connection(None, None) assert not add_zone_mock.called + @mock.patch('oslo_config.cfg._is_opt_registered', return_value=False) @mock.patch.object(utils, 'require_driver_initialized') - def test_terminate_connection_with_decorator(self, utils_mock): + def test_terminate_connection_with_decorator(self, utils_mock, opt_mock): utils_mock.return_value = True with mock.patch.object(fc_zone_manager.ZoneManager, 'delete_connection') as remove_zone_mock: diff --git a/cinder/zonemanager/fc_san_lookup_service.py b/cinder/zonemanager/fc_san_lookup_service.py index 48b0df691..99acc84c8 100644 --- a/cinder/zonemanager/fc_san_lookup_service.py +++ b/cinder/zonemanager/fc_san_lookup_service.py @@ -82,8 +82,8 @@ class FCSanLookupService(fc_common.FCCommon): lookup_service, configuration=self.configuration) else: msg = _("Lookup service not configured. Config option for " - "fc_san_lookup_service need to specify a concrete " - "implementation of lookup service") + "fc_san_lookup_service needs to specify a concrete " + "implementation of the lookup service.") LOG.error(msg) raise exception.FCSanLookupServiceException(msg) try: diff --git a/cinder/zonemanager/fc_zone_manager.py b/cinder/zonemanager/fc_zone_manager.py index f00701569..f508cc519 100644 --- a/cinder/zonemanager/fc_zone_manager.py +++ b/cinder/zonemanager/fc_zone_manager.py @@ -49,16 +49,17 @@ zone_manager_opts = [ help='FC Zone Driver responsible for zone management'), cfg.StrOpt('zoning_policy', default='initiator-target', - help='Zoning policy configured by user'), + help='Zoning policy configured by user; valid values include ' + '"initiator-target" or "initiator"'), cfg.StrOpt('fc_fabric_names', default=None, - help='Comma separated list of fibre channel fabric names.' + help='Comma separated list of Fibre Channel fabric names.' ' This list of names is used to retrieve other SAN credentials' ' for connecting to each SAN fabric'), cfg.StrOpt('fc_san_lookup_service', default='cinder.zonemanager.drivers.brocade' '.brcd_fc_san_lookup_service.BrcdFCSanLookupService', - help='FC San Lookup Service'), + help='FC SAN Lookup Service'), ] CONF = cfg.CONF @@ -87,7 +88,7 @@ class ZoneManager(fc_common.FCCommon): """Load the driver from the one specified in args, or from flags.""" super(ZoneManager, self).__init__(**kwargs) - self.configuration = kwargs.get('configuration', None) + self.configuration = kwargs.get('configuration') if self.configuration: self.configuration.append_config_values(zone_manager_opts) @@ -129,11 +130,11 @@ class ZoneManager(fc_common.FCCommon): try: for initiator in initiator_target_map.keys(): target_list = initiator_target_map[initiator] - LOG.debug("Target List :%s", {initiator: target_list}) + LOG.debug("Target List: %s", target_list) # get SAN context for the target list fabric_map = self.get_san_context(target_list) - LOG.debug("Fabric Map after context lookup:%s", fabric_map) + LOG.debug("Fabric Map after context lookup: %s", fabric_map) # iterate over each SAN and apply connection control for fabric in fabric_map.keys(): connected_fabric = fabric @@ -143,7 +144,7 @@ class ZoneManager(fc_common.FCCommon): valid_i_t_map = self.get_valid_initiator_target_map( i_t_map, True) LOG.info(_LI("Final filtered map for fabric: %s"), - {fabric: valid_i_t_map}) + valid_i_t_map) # Call driver to add connection control self.driver.add_connection(fabric, valid_i_t_map) @@ -152,8 +153,8 @@ class ZoneManager(fc_common.FCCommon): "over all target list")) except Exception as e: msg = _("Failed adding connection for fabric=%(fabric)s: " - "Error:%(err)s") % {'fabric': connected_fabric, - 'err': e} + "Error: %(err)s") % {'fabric': connected_fabric, + 'err': e} LOG.error(msg) raise exception.ZoneManagerException(reason=msg) @@ -172,8 +173,8 @@ class ZoneManager(fc_common.FCCommon): try: for initiator in initiator_target_map.keys(): target_list = initiator_target_map[initiator] - LOG.info(_LI("Delete connection Target List:%s"), - {initiator: target_list}) + LOG.info(_LI("Delete connection Target List: %s"), + target_list) # get SAN context for the target list fabric_map = self.get_san_context(target_list) @@ -199,8 +200,8 @@ class ZoneManager(fc_common.FCCommon): " target list") except Exception as e: msg = _("Failed removing connection for fabric=%(fabric)s: " - "Error:%(err)s") % {'fabric': connected_fabric, - 'err': e} + "Error: %(err)s") % {'fabric': connected_fabric, + 'err': e} LOG.error(msg) raise exception.ZoneManagerException(reason=msg) @@ -211,7 +212,7 @@ class ZoneManager(fc_common.FCCommon): to list of target WWNs visible to the fabric. """ fabric_map = self.driver.get_san_context(target_wwn_list) - LOG.debug("Got SAN context:%s", fabric_map) + LOG.debug("Got SAN context: %s", fabric_map) return fabric_map def get_valid_initiator_target_map(self, initiator_target_map, diff --git a/cinder/zonemanager/utils.py b/cinder/zonemanager/utils.py index dbe67ac56..fb2dba525 100644 --- a/cinder/zonemanager/utils.py +++ b/cinder/zonemanager/utils.py @@ -33,12 +33,12 @@ LOG.logger.setLevel(logging.DEBUG) def create_zone_manager(): """If zoning is enabled, build the Zone Manager.""" config = Configuration(manager.volume_manager_opts) - LOG.debug("zoning mode %s" % config.safe_get('zoning_mode')) + LOG.debug("Zoning mode: %s", config.safe_get('zoning_mode')) if config.safe_get('zoning_mode') == 'fabric': LOG.debug("FC Zone Manager enabled.") zm = fc_zone_manager.ZoneManager(configuration=config) LOG.info(_LI("Using FC Zone Manager %(zm_version)s," - " Driver %(drv_name)s %(drv_version)s.") % + " Driver %(drv_name)s %(drv_version)s."), {'zm_version': zm.get_version(), 'drv_name': zm.driver.__class__.__name__, 'drv_version': zm.driver.get_version()}) @@ -50,11 +50,11 @@ def create_zone_manager(): def create_lookup_service(): config = Configuration(manager.volume_manager_opts) - LOG.debug("zoning mode %s" % config.safe_get('zoning_mode')) + LOG.debug("Zoning mode: %s", config.safe_get('zoning_mode')) if config.safe_get('zoning_mode') == 'fabric': LOG.debug("FC Lookup Service enabled.") lookup = fc_san_lookup_service.FCSanLookupService(configuration=config) - LOG.info(_LI("Using FC lookup service %s") % lookup.lookup_service) + LOG.info(_LI("Using FC lookup service %s"), lookup.lookup_service) return lookup else: LOG.debug("FC Lookup Service not enabled in cinder.conf.") @@ -86,7 +86,7 @@ def AddFCZone(initialize_connection): init_target_map = conn_info['data']['initiator_target_map'] zm = create_zone_manager() if zm: - LOG.debug("Add FC Zone for mapping '%s'." % + LOG.debug("Add FC Zone for mapping '%s'.", init_target_map) zm.add_connection(init_target_map) @@ -111,7 +111,7 @@ def RemoveFCZone(terminate_connection): init_target_map = conn_info['data']['initiator_target_map'] zm = create_zone_manager() if zm: - LOG.debug("Remove FC Zone for mapping '%s'." % + LOG.debug("Remove FC Zone for mapping '%s'.", init_target_map) zm.delete_connection(init_target_map) -- 2.45.2