Brocade: Python 3 support

Builtin method `filter` changed behavior from Python 2, where it
returned a list, to Python 3, where it returns an interable, and the
Brocade driver was not ready to handle this behavior which results
in:

- Always evaluating to True in `if filtered_members`
- TypeError: 'filter' object is not subscriptable
- RuntimeError: dictionary changed size during iteration

This patch fixes all these to make the driver Python 3 compatible.

Extensive changes to test_brcd_fc_zone_driver.py were necessary to fix
the code that was incorrectly setting the configuration options for the
tests, which resulted in always running with default values.

Code has been tested on a DevStack deployment running Python 3.7 on
Fedora 31 with a Brocade 300 running FOS 7.1.0c against 3PAR FC array
and with `fc_southbound_protocol = HTTP` in the fabric group
configuration.

Configuration `fc_southbound_protocol = REST_HTTP` could not be tested
as its only supported in FOS 8.2.1 and later which available hardware
doesn't support.

Closes-Bug: #1888548
Change-Id: Id9807871f3c183e4195f01aa28c458d7551de9ed

cinder/tests/unit/zonemanager/test_brcd_fc_zone_driver.py

@@ -18,7 +18,6 @@
 """Unit tests for Brocade fc zone driver."""
 from unittest import mock
 
-from oslo_config import cfg
 from oslo_utils import importutils
 import paramiko
 import requests
@@ -26,21 +25,19 @@ import requests
 from cinder import exception
 from cinder.tests.unit import test
 from cinder.volume import configuration as conf
+from cinder.zonemanager.drivers.brocade import brcd_fabric_opts as fabric_opts
 from cinder.zonemanager.drivers.brocade import brcd_fc_zone_driver as driver
+from cinder.zonemanager import fc_zone_manager as zmanager
+
+_zone_name = 'openstack_fab1_10008c7cff523b0120240002ac000a50'
+_zone_name_initiator_mode = 'openstack_fab1_10008c7cff523b01'
+WWNS = ['10:00:8c:7c:ff:52:3b:01', '20:24:00:02:ac:00:0a:50']
 
 _active_cfg_before_add = {}
-_active_cfg_before_delete = {
-    'zones': {
-        'openstack10008c7cff523b0120240002ac000a50': (
-            ['10:00:8c:7c:ff:52:3b:01',
-             '20:24:00:02:ac:00:0a:50']), 't_zone': ['1,0']},
-        'active_zone_config': 'cfg1'}
 _activate = True
-_zone_name = 'openstack10008c7cff523b0120240002ac000a50'
 _target_ns_map = {'100000051e55a100': ['20240002ac000a50']}
 _initiator_ns_map = {'100000051e55a100': ['10008c7cff523b01']}
-_zone_map_to_add = {'openstack10008c7cff523b0120240002ac000a50': (
+_zone_map_to_add = {_zone_name: WWNS}
-    ['10:00:8c:7c:ff:52:3b:01', '20:24:00:02:ac:00:0a:50'])}
 
 _initiator_target_map = {'10008c7cff523b01': ['20240002ac000a50']}
 _device_map_to_verify = {
@@ -52,44 +49,43 @@ _fabric_wwn = '100000051e55a100'
 
 class BrcdFcZoneDriverBaseTest(object):
 
+    def _set_conf_overrides(self, group, **kwargs):
+        for name, value in kwargs.items():
+            self.override_config(name, value, group)
+
     def setup_config(self, is_normal, mode):
-        fc_test_opts = [
+        self.override_config('zoning_mode', 'fabric')
-            cfg.StrOpt('fc_fabric_address_BRCD_FAB_1', default='10.24.48.213',
-                       help='FC Fabric names'),
-        ]
-        configuration = conf.Configuration(fc_test_opts)
-        # fill up config
-        configuration.zoning_mode = 'fabric'
-        configuration.zone_driver = ('cinder.tests.unit.zonemanager.'
-                                     'test_brcd_fc_zone_driver.'
-                                     'FakeBrcdFCZoneDriver')
-        configuration.brcd_sb_connector = ('cinder.tests.unit.zonemanager.'
-                                           'test_brcd_fc_zone_driver'
-                                           '.FakeBrcdFCZoneClientCLI')
-        configuration.zoning_policy = 'initiator-target'
-        configuration.zone_activate = True
-        configuration.zone_name_prefix = 'openstack'
-        configuration.fc_san_lookup_service = ('cinder.tests.unit.zonemanager.'
-                                               'test_brcd_fc_zone_driver.'
-                                               'FakeBrcdFCSanLookupService')
 
-        configuration.fc_fabric_names = 'BRCD_FAB_1'
+        fabric_group_name = 'BRCD_FAB_1'
-        configuration.fc_fabric_address_BRCD_FAB_1 = '10.24.48.213'
+        self._set_conf_overrides(
-        configuration.fc_southbound_connector = 'CLI'
+            'fc-zone-manager',
-        if is_normal:
+            zone_driver=('cinder.tests.unit.zonemanager.'
-            configuration.fc_fabric_user_BRCD_FAB_1 = 'admin'
+                         'test_brcd_fc_zone_driver.FakeBrcdFCZoneDriver'),
-        else:
+            brcd_sb_connector=('cinder.tests.unit.zonemanager.'
-            configuration.fc_fabric_user_BRCD_FAB_1 = 'invaliduser'
+                               'test_brcd_fc_zone_driver.'
-        configuration.fc_fabric_password_BRCD_FAB_1 = 'password'
+                               'FakeBrcdFCZoneClientCLI'),
+            zoning_policy='initiator-target',
+            fc_san_lookup_service=('cinder.tests.unit.zonemanager.'
+                                   'test_brcd_fc_zone_driver.'
+                                   'FakeBrcdFCSanLookupService'),
+            fc_fabric_names=fabric_group_name,
+        )
 
-        if mode == 1:
+        # Ensure that we have the fabric_name group
-            configuration.zoning_policy_BRCD_FAB_1 = 'initiator-target'
+        conf.Configuration(fabric_opts.brcd_zone_opts, fabric_group_name)
-        elif mode == 2:
+        self._set_conf_overrides(
-            configuration.zoning_policy_BRCD_FAB_1 = 'initiator'
+            fabric_group_name,
-        else:
+            fc_fabric_address='10.24.48.213',
-            configuration.zoning_policy_BRCD_FAB_1 = 'initiator-target'
+            fc_fabric_password='password',
-        configuration.zone_activate_BRCD_FAB_1 = True
+            fc_fabric_user='admin' if is_normal else 'invaliduser',
-        configuration.zone_name_prefix_BRCD_FAB_1 = 'openstack_fab1'
+            zoning_policy='initiator' if mode == 2 else 'initiator-target',
+            zone_activate=True,
+            zone_name_prefix='openstack_fab1_',
+            fc_southbound_protocol='SSH',
+        )
+
+        configuration = conf.Configuration(zmanager.zone_manager_opts,
+                                           'fc-zone-manager')
         return configuration
 
 
@@ -125,10 +121,6 @@ class TestBrcdFcZoneDriver(BrcdFcZoneDriverBaseTest, test.TestCase):
         )
         return client
 
-    def fake_get_san_context(self, target_wwn_list):
-        fabric_map = {}
-        return fabric_map
-
     @mock.patch.object(driver.BrcdFCZoneDriver, '_get_southbound_client')
     def test_add_connection(self, get_southbound_client_mock):
         """Normal flow for i-t mode."""
@@ -139,11 +131,17 @@ class TestBrcdFcZoneDriver(BrcdFcZoneDriverBaseTest, test.TestCase):
         self.driver.add_connection('BRCD_FAB_1', _initiator_target_map)
         self.assertIn(_zone_name, GlobalVars._zone_state)
 
+    def _active_cfg_before_delete(self, mode):
+        zone_name = _zone_name if mode == 1 else _zone_name_initiator_mode
+        return {'zones': {zone_name: WWNS, 't_zone': ['1,0']},
+                'active_zone_config': 'cfg1'}
+
     @mock.patch.object(driver.BrcdFCZoneDriver, '_get_southbound_client')
     def test_delete_connection(self, get_southbound_client_mock):
         GlobalVars._is_normal_test = True
+        GlobalVars._zone_state.append(_zone_name)
         get_southbound_client_mock.return_value = self.get_client("CLI")
-        GlobalVars._active_cfg = _active_cfg_before_delete
+        GlobalVars._active_cfg = self._active_cfg_before_delete(mode=1)
         self.driver.delete_connection(
             'BRCD_FAB_1', _initiator_target_map)
         self.assertNotIn(_zone_name, GlobalVars._zone_state)
@@ -156,32 +154,35 @@ class TestBrcdFcZoneDriver(BrcdFcZoneDriverBaseTest, test.TestCase):
         GlobalVars._active_cfg = _active_cfg_before_add
         self.setup_driver(self.setup_config(True, 2))
         self.driver.add_connection('BRCD_FAB_1', _initiator_target_map)
-        self.assertIn(_zone_name, GlobalVars._zone_state)
+        self.assertIn(_zone_name_initiator_mode, GlobalVars._zone_state)
 
     @mock.patch.object(driver.BrcdFCZoneDriver, '_get_southbound_client')
     def test_delete_connection_for_initiator_mode(self,
                                                   get_southbound_client_mk):
         GlobalVars._is_normal_test = True
+        GlobalVars._zone_state.append(_zone_name_initiator_mode)
         get_southbound_client_mk.return_value = self.get_client("HTTPS")
-        GlobalVars._active_cfg = _active_cfg_before_delete
+        GlobalVars._active_cfg = self._active_cfg_before_delete(mode=2)
         self.setup_driver(self.setup_config(True, 2))
         self.driver.delete_connection(
             'BRCD_FAB_1', _initiator_target_map)
-        self.assertNotIn(_zone_name, GlobalVars._zone_state)
+        self.assertNotIn(_zone_name_initiator_mode, GlobalVars._zone_state)
 
-    def test_add_connection_for_invalid_fabric(self):
+    @mock.patch('cinder.zonemanager.drivers.brocade.brcd_fc_zone_client_cli.'
+                'BrcdFCZoneClientCLI.__init__', side_effect=Exception)
+    def test_add_connection_for_invalid_fabric(self, create_client_mock):
         """Test abnormal flows."""
-        GlobalVars._is_normal_test = True
         GlobalVars._active_cfg = _active_cfg_before_add
-        GlobalVars._is_normal_test = False
         self.setup_driver(self.setup_config(False, 1))
         self.assertRaises(exception.FCZoneDriverException,
                           self.driver.add_connection,
                           'BRCD_FAB_1',
                           _initiator_target_map)
 
-    def test_delete_connection_for_invalid_fabric(self):
+    @mock.patch('cinder.zonemanager.drivers.brocade.brcd_fc_zone_client_cli.'
-        GlobalVars._active_cfg = _active_cfg_before_delete
+                'BrcdFCZoneClientCLI.__init__', side_effect=Exception)
+    def test_delete_connection_for_invalid_fabric(self, create_client_mock):
+        GlobalVars._active_cfg = self._active_cfg_before_delete(mode=1)
         GlobalVars._is_normal_test = False
         self.setup_driver(self.setup_config(False, 1))
         self.assertRaises(exception.FCZoneDriverException,
@@ -189,6 +190,22 @@ class TestBrcdFcZoneDriver(BrcdFcZoneDriverBaseTest, test.TestCase):
                           'BRCD_FAB_1',
                           _initiator_target_map)
 
+    @mock.patch.object(driver.BrcdFCZoneDriver, '_get_southbound_client')
+    def test_get_san_context(self, client_mock):
+        GlobalVars._is_normal_test = True
+        self.setup_driver(self.setup_config(True, 1))
+        get_ns_mock = client_mock.return_value.get_nameserver_info
+        wwn = '20:24:00:02:ac:00:0a:50'
+        get_ns_mock.return_value = [wwn]
+        expected = {'BRCD_FAB_1': ['20240002ac000a50']}
+
+        res = self.driver.get_san_context([WWNS[0], wwn.upper()])
+
+        client_mock.assert_called_once_with('BRCD_FAB_1')
+        client_mock.return_value.cleanup.assert_called_once_with()
+        get_ns_mock.assert_called_once_with()
+        self.assertEqual(expected, res)
+
 
 class FakeClient(object):
     def get_active_zone_set(self):

cinder/tests/unit/zonemanager/test_brcd_http_fc_zone_client.py

@@ -750,6 +750,13 @@ class TestBrcdHttpFCZoneClient(client.BrcdHTTPFCZoneClient, test.TestCase):
                 delete_zones_info,
                 active_cfg))
 
+        cfgs = {'openstack_cfg': 'openstack50060b0000c26604201900051ee8e329'}
+        res = self.delete_zones_cfgs(cfgs,
+                                     zones_to_delete.copy(),
+                                     delete_zones_info,
+                                     active_cfg)
+        self.assertEqual((zones, {}, ''), res)
+
         cfgs = {'openstack_cfg': 'zone2'}
         zones = {'zone2': '20:01:00:05:33:0e:96:14;20:00:00:05:33:0e:93:11'}
         delete_zones_info = valid_zone_name + ";zone1"

cinder/zonemanager/drivers/brocade/brcd_fc_zone_driver.py

@@ -331,14 +331,14 @@ class BrcdFCZoneDriver(fc_zone_driver.FCZoneDriver):
                     # Check to see if there are other zone members
                     # in the zone besides the initiator and
                     # the targets being removed.
-                    filtered_members = filter(
+                    has_members = any(
-                        lambda x: x not in zone_members,
+                        x for x in cfgmap_from_fabric['zones'][zone_name]
-                        cfgmap_from_fabric['zones'][zone_name])
+                        if x not in zone_members)
 
                     # If there are other zone members, proceed with
                     # zone update to remove the targets.  Otherwise,
                     # delete the zone.
-                    if filtered_members:
+                    if has_members:
                         zone_members.remove(formatted_initiator)
                         # Verify that the zone members in target list
                         # are listed in zone definition.  If not, remove
@@ -440,9 +440,8 @@ class BrcdFCZoneDriver(fc_zone_driver.FCZoneDriver):
                     raise exception.FCZoneDriverException(msg)
                 finally:
                     conn.cleanup()
-                visible_targets = filter(
+                visible_targets = [x for x in nsinfo
-                    lambda x: x in formatted_target_list,
+                                   if x in formatted_target_list]
-                    nsinfo)
 
                 if visible_targets:
                     LOG.info("Filtered targets for SAN is: %(targets)s",

cinder/zonemanager/drivers/brocade/brcd_http_fc_zone_client.py

@@ -805,7 +805,7 @@ class BrcdHTTPFCZoneClient(object):
                 zones.pop(zone)
                 # iterated all the cfgs, but need to check since in SSH only
                 # active cfg is iterated
-                for k, v in cfgs.items():
+                for k, v in list(cfgs.items()):
                     v = v.split(";")
                     if zone in v:
                         # remove the zone from the cfg string

releasenotes/notes/brocade_py3-15647dbe3981d44b.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Add Python 3 support to the Brocade Zone Manager driver.
+    (bug #1888548).