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
This commit is contained in:
Joe Cropper 2015-02-18 23:55:04 -06:00
parent 8bda008dd0
commit f4f46e9720
5 changed files with 39 additions and 31 deletions

View File

@ -36,7 +36,8 @@ target_list = ['20240002ac000a50']
class TestFCZoneManager(test.TestCase): 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() super(TestFCZoneManager, self).setUp()
config = conf.Configuration(None) config = conf.Configuration(None)
config.fc_fabric_names = fabric_name config.fc_fabric_names = fabric_name
@ -53,9 +54,10 @@ class TestFCZoneManager(test.TestCase):
self.driver = Mock(FCZoneDriver) self.driver = Mock(FCZoneDriver)
def __init__(self, *args, **kwargs): 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')\ with mock.patch.object(self.zm.driver, 'add_connection')\
as add_connection_mock: as add_connection_mock:
self.zm.driver.get_san_context.return_value = fabric_map 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, add_connection_mock.assert_called_once_with(fabric_name,
init_target_map) 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')\ with mock.patch.object(self.zm.driver, 'add_connection')\
as add_connection_mock: as add_connection_mock:
add_connection_mock.side_effect = exception.FCZoneDriverException add_connection_mock.side_effect = exception.FCZoneDriverException
self.assertRaises(exception.ZoneManagerException, self.assertRaises(exception.ZoneManagerException,
self.zm.add_connection, init_target_map) 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')\ with mock.patch.object(self.zm.driver, 'delete_connection')\
as delete_connection_mock: as delete_connection_mock:
self.zm.driver.get_san_context.return_value = fabric_map 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, delete_connection_mock.assert_called_once_with(fabric_name,
init_target_map) 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')\ with mock.patch.object(self.zm.driver, 'delete_connection')\
as del_connection_mock: as del_connection_mock:
del_connection_mock.side_effect = exception.FCZoneDriverException del_connection_mock.side_effect = exception.FCZoneDriverException

View File

@ -41,10 +41,11 @@ class TestVolumeDriver(test.TestCase):
self.driver = None self.driver = None
def __init__(self, *args, **kwargs): 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') @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 utils_mock.return_value = True
with mock.patch.object(fc_zone_manager.ZoneManager, 'add_connection')\ with mock.patch.object(fc_zone_manager.ZoneManager, 'add_connection')\
as add_zone_mock: as add_zone_mock:
@ -66,8 +67,9 @@ class TestVolumeDriver(test.TestCase):
self.driver.no_zone_initialize_connection(None, None) self.driver.no_zone_initialize_connection(None, None)
assert not add_zone_mock.called assert not add_zone_mock.called
@mock.patch('oslo_config.cfg._is_opt_registered', return_value=False)
@mock.patch.object(utils, 'require_driver_initialized') @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 utils_mock.return_value = True
with mock.patch.object(fc_zone_manager.ZoneManager, with mock.patch.object(fc_zone_manager.ZoneManager,
'delete_connection') as remove_zone_mock: 'delete_connection') as remove_zone_mock:

View File

@ -82,8 +82,8 @@ class FCSanLookupService(fc_common.FCCommon):
lookup_service, configuration=self.configuration) lookup_service, configuration=self.configuration)
else: else:
msg = _("Lookup service not configured. Config option for " msg = _("Lookup service not configured. Config option for "
"fc_san_lookup_service need to specify a concrete " "fc_san_lookup_service needs to specify a concrete "
"implementation of lookup service") "implementation of the lookup service.")
LOG.error(msg) LOG.error(msg)
raise exception.FCSanLookupServiceException(msg) raise exception.FCSanLookupServiceException(msg)
try: try:

View File

@ -49,16 +49,17 @@ zone_manager_opts = [
help='FC Zone Driver responsible for zone management'), help='FC Zone Driver responsible for zone management'),
cfg.StrOpt('zoning_policy', cfg.StrOpt('zoning_policy',
default='initiator-target', 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', cfg.StrOpt('fc_fabric_names',
default=None, 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' ' This list of names is used to retrieve other SAN credentials'
' for connecting to each SAN fabric'), ' for connecting to each SAN fabric'),
cfg.StrOpt('fc_san_lookup_service', cfg.StrOpt('fc_san_lookup_service',
default='cinder.zonemanager.drivers.brocade' default='cinder.zonemanager.drivers.brocade'
'.brcd_fc_san_lookup_service.BrcdFCSanLookupService', '.brcd_fc_san_lookup_service.BrcdFCSanLookupService',
help='FC San Lookup Service'), help='FC SAN Lookup Service'),
] ]
CONF = cfg.CONF CONF = cfg.CONF
@ -87,7 +88,7 @@ class ZoneManager(fc_common.FCCommon):
"""Load the driver from the one specified in args, or from flags.""" """Load the driver from the one specified in args, or from flags."""
super(ZoneManager, self).__init__(**kwargs) super(ZoneManager, self).__init__(**kwargs)
self.configuration = kwargs.get('configuration', None) self.configuration = kwargs.get('configuration')
if self.configuration: if self.configuration:
self.configuration.append_config_values(zone_manager_opts) self.configuration.append_config_values(zone_manager_opts)
@ -129,7 +130,7 @@ class ZoneManager(fc_common.FCCommon):
try: try:
for initiator in initiator_target_map.keys(): for initiator in initiator_target_map.keys():
target_list = initiator_target_map[initiator] 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 # get SAN context for the target list
fabric_map = self.get_san_context(target_list) fabric_map = self.get_san_context(target_list)
@ -143,7 +144,7 @@ class ZoneManager(fc_common.FCCommon):
valid_i_t_map = self.get_valid_initiator_target_map( valid_i_t_map = self.get_valid_initiator_target_map(
i_t_map, True) i_t_map, True)
LOG.info(_LI("Final filtered map for fabric: %s"), LOG.info(_LI("Final filtered map for fabric: %s"),
{fabric: valid_i_t_map}) valid_i_t_map)
# Call driver to add connection control # Call driver to add connection control
self.driver.add_connection(fabric, valid_i_t_map) self.driver.add_connection(fabric, valid_i_t_map)
@ -173,7 +174,7 @@ class ZoneManager(fc_common.FCCommon):
for initiator in initiator_target_map.keys(): for initiator in initiator_target_map.keys():
target_list = initiator_target_map[initiator] target_list = initiator_target_map[initiator]
LOG.info(_LI("Delete connection Target List: %s"), LOG.info(_LI("Delete connection Target List: %s"),
{initiator: target_list}) target_list)
# get SAN context for the target list # get SAN context for the target list
fabric_map = self.get_san_context(target_list) fabric_map = self.get_san_context(target_list)

View File

@ -33,12 +33,12 @@ LOG.logger.setLevel(logging.DEBUG)
def create_zone_manager(): def create_zone_manager():
"""If zoning is enabled, build the Zone Manager.""" """If zoning is enabled, build the Zone Manager."""
config = Configuration(manager.volume_manager_opts) 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': if config.safe_get('zoning_mode') == 'fabric':
LOG.debug("FC Zone Manager enabled.") LOG.debug("FC Zone Manager enabled.")
zm = fc_zone_manager.ZoneManager(configuration=config) zm = fc_zone_manager.ZoneManager(configuration=config)
LOG.info(_LI("Using FC Zone Manager %(zm_version)s," 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(), {'zm_version': zm.get_version(),
'drv_name': zm.driver.__class__.__name__, 'drv_name': zm.driver.__class__.__name__,
'drv_version': zm.driver.get_version()}) 'drv_version': zm.driver.get_version()})
@ -50,11 +50,11 @@ def create_zone_manager():
def create_lookup_service(): def create_lookup_service():
config = Configuration(manager.volume_manager_opts) 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': if config.safe_get('zoning_mode') == 'fabric':
LOG.debug("FC Lookup Service enabled.") LOG.debug("FC Lookup Service enabled.")
lookup = fc_san_lookup_service.FCSanLookupService(configuration=config) 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 return lookup
else: else:
LOG.debug("FC Lookup Service not enabled in cinder.conf.") 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'] init_target_map = conn_info['data']['initiator_target_map']
zm = create_zone_manager() zm = create_zone_manager()
if zm: if zm:
LOG.debug("Add FC Zone for mapping '%s'." % LOG.debug("Add FC Zone for mapping '%s'.",
init_target_map) init_target_map)
zm.add_connection(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'] init_target_map = conn_info['data']['initiator_target_map']
zm = create_zone_manager() zm = create_zone_manager()
if zm: if zm:
LOG.debug("Remove FC Zone for mapping '%s'." % LOG.debug("Remove FC Zone for mapping '%s'.",
init_target_map) init_target_map)
zm.delete_connection(init_target_map) zm.delete_connection(init_target_map)