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

diff --git a/cinder/tests/zonemanager/test_fc_zone_manager.py b/cinder/tests/zonemanager/test_fc_zone_manager.py
index f1b2099473db1bb1d4615f77a0c9c63df711af5d..fa3f7c6547d71edc3136f89c240e126c02be7d65 100644 (file)
--- 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 6f975188874fa1c0c64fded53d7ffa6682a18af8..fe4c919d6274e029c17c3a0db2760aae5a442952 100644 (file)
--- 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 48b0df691355f4ff58cbaaf7fadc2a6e30c896d5..99acc84c8863afd543edd59666b5a0642d44f875 100644 (file)
--- 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 f0070156972eea7447380cd026f595da751a29e3..f508cc519f353194ae81887f7ade73e11b53698a 100644 (file)
--- 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 dbe67ac56eb9a0104778497cf7d5264c0ce69aa4..fb2dba5257199082df2e98929fb6fb5e1940241d 100644 (file)
--- 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)