EMC VMAX - Fix for randomly selecting a portgroup

Before now the utils function was always returning the first
portgroup in the list.  This fix ensures a port group is
randomly selected from the list, where more than one value
is in the list.

Closes-Bug: #1501919
Change-Id: I49020a192f62bc75bca26ea0d206e9ed7c700195

diff --git a/cinder/tests/unit/test_emc_vmax.py b/cinder/tests/unit/test_emc_vmax.py
index 7a3f321ded2aa3f8ca7a576ce65fffe8f681e51d..d789a85230a9a9b53dd569618e542b7eebbc164c 100644 (file)
--- a/cinder/tests/unit/test_emc_vmax.py
+++ b/cinder/tests/unit/test_emc_vmax.py
@@ -1899,6 +1899,84 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase):
             self.data.test_volume, connector, extraSpecs)
         self.assertLessEqual(64, len(maskingViewDict['sgGroupName']))
 
+    def test_filter_list(self):
+        portgroupnames = ['pg3', 'pg1', 'pg4', 'pg2']
+        portgroupnames = (
+            self.driver.common.utils._filter_list(portgroupnames))
+        self.assertEqual(4, len(portgroupnames))
+        self.assertEqual(['pg1', 'pg2', 'pg3', 'pg4'], sorted(portgroupnames))
+
+        portgroupnames = ['pg1']
+        portgroupnames = (
+            self.driver.common.utils._filter_list(portgroupnames))
+        self.assertEqual(1, len(portgroupnames))
+        self.assertEqual(['pg1'], portgroupnames)
+
+        portgroupnames = ['only_pg', '', '', '', '', '']
+        portgroupnames = (
+            self.driver.common.utils._filter_list(portgroupnames))
+        self.assertEqual(1, len(portgroupnames))
+        self.assertEqual(['only_pg'], portgroupnames)
+
+    def test_get_random_pg_from_list(self):
+        portGroupNames = ['pg1', 'pg2', 'pg3', 'pg4']
+        portGroupName = (
+            self.driver.common.utils._get_random_pg_from_list(portGroupNames))
+        self.assertTrue('pg' in portGroupName)
+
+        portGroupNames = ['pg1']
+        portGroupName = (
+            self.driver.common.utils._get_random_pg_from_list(portGroupNames))
+        self.assertEqual('pg1', portGroupName)
+
+    def test_get_random_portgroup(self):
+        # 4 portgroups
+        data = ("<?xml version='1.0' encoding='UTF-8'?>\n<EMC>\n"
+                "<PortGroups>"
+                "<PortGroup>OS-PG1</PortGroup>\n"
+                "<PortGroup>OS-PG2</PortGroup>\n"
+                "<PortGroup>OS-PG3</PortGroup>\n"
+                "<PortGroup>OS-PG4</PortGroup>\n"
+                "</PortGroups>"
+                "</EMC>")
+        dom = minidom.parseString(data)
+        portgroup = self.driver.common.utils._get_random_portgroup(dom)
+        self.assertTrue('OS-PG' in portgroup)
+
+        # Duplicate portgroups
+        data = ("<?xml version='1.0' encoding='UTF-8'?>\n<EMC>\n"
+                "<PortGroups>"
+                "<PortGroup>OS-PG1</PortGroup>\n"
+                "<PortGroup>OS-PG1</PortGroup>\n"
+                "<PortGroup>OS-PG1</PortGroup>\n"
+                "<PortGroup>OS-PG2</PortGroup>\n"
+                "</PortGroups>"
+                "</EMC>")
+        dom = minidom.parseString(data)
+        portgroup = self.driver.common.utils._get_random_portgroup(dom)
+        self.assertTrue('OS-PG' in portgroup)
+
+    def test_get_random_portgroup_exception(self):
+        # Missing PortGroup values
+        data = ("<?xml version='1.0' encoding='UTF-8'?>\n<EMC>\n"
+                "<PortGroups>"
+                "<PortGroup></PortGroup>\n"
+                "<PortGroup></PortGroup>\n"
+                "</PortGroups>"
+                "</EMC>")
+        dom = minidom.parseString(data)
+        self.assertRaises(exception.VolumeBackendAPIException,
+                          self.driver.common.utils._get_random_portgroup, dom)
+
+        # Missing portgroups
+        data = ("<?xml version='1.0' encoding='UTF-8'?>\n<EMC>\n"
+                "<PortGroups>"
+                "</PortGroups>"
+                "</EMC>")
+        dom = minidom.parseString(data)
+        self.assertRaises(exception.VolumeBackendAPIException,
+                          self.driver.common.utils._get_random_portgroup, dom)
+
     def test_generate_unique_trunc_pool(self):
         pool_under_16_chars = 'pool_under_16'
         pool1 = self.driver.utils.generate_unique_trunc_pool(

diff --git a/cinder/volume/drivers/emc/emc_vmax_fc.py b/cinder/volume/drivers/emc/emc_vmax_fc.py
index a30b8f88a277d3afec54e60699c2a8997c7d5674..f8b44945bd0618ebddd918bfb8e362611e2bf69b 100644 (file)
--- a/cinder/volume/drivers/emc/emc_vmax_fc.py
+++ b/cinder/volume/drivers/emc/emc_vmax_fc.py
@@ -42,10 +42,11 @@ class EMCVMAXFCDriver(driver.FibreChannelDriver):
         2.2.2 - Update Consistency Group
         2.2.3 - Pool aware scheduler(multi-pool) support
         2.2.4 - Create CG from CG snapshot
-        2.3   - Name change for MV and SG for FAST (bug #1515181)
+        2.3.0 - Name change for MV and SG for FAST (bug #1515181)
+              - Fix for randomly choosing port group. (bug #1501919)
     """
 
-    VERSION = "2.3"
+    VERSION = "2.3.0"
 
     def __init__(self, *args, **kwargs):
 

diff --git a/cinder/volume/drivers/emc/emc_vmax_iscsi.py b/cinder/volume/drivers/emc/emc_vmax_iscsi.py
index 30a819a71906176d417300b74836ee81dacb62c4..3161a51ab0fb05359e4be87e20311f602ff359bc 100644 (file)
--- a/cinder/volume/drivers/emc/emc_vmax_iscsi.py
+++ b/cinder/volume/drivers/emc/emc_vmax_iscsi.py
@@ -50,10 +50,11 @@ class EMCVMAXISCSIDriver(driver.ISCSIDriver):
         2.2.2 - Update Consistency Group
         2.2.3 - Pool aware scheduler(multi-pool) support
         2.2.4 - Create CG from CG snapshot
-        2.3   - Name change for MV and SG for FAST (bug #1515181)
+        2.3.0 - Name change for MV and SG for FAST (bug #1515181)
+              - Fix for randomly choosing port group. (bug #1501919)
     """
 
-    VERSION = "2.3"
+    VERSION = "2.3.0"
 
     def __init__(self, *args, **kwargs):
 

diff --git a/cinder/volume/drivers/emc/emc_vmax_utils.py b/cinder/volume/drivers/emc/emc_vmax_utils.py
index cd139dfc8f0e3441908a6bd24accd588bca4b401..217a1d44865438fb7d9cbafa7605bb66ea1a6d85 100644 (file)
--- a/cinder/volume/drivers/emc/emc_vmax_utils.py
+++ b/cinder/volume/drivers/emc/emc_vmax_utils.py
@@ -2042,27 +2042,47 @@ class EMCVMAXUtils(object):
         portGroupElements = element.getElementsByTagName('PortGroup')
         if portGroupElements and len(portGroupElements) > 0:
             portGroupNames = []
-            for __ in portGroupElements:
-                portGroupName = self._process_tag(
-                    element, 'PortGroup')
-                if portGroupName:
-                    portGroupNames.append(portGroupName)
-
-            LOG.debug("portGroupNames: %(portGroupNames)s.",
-                      {'portGroupNames': portGroupNames})
-            numPortGroups = len(portGroupNames)
-            if numPortGroups > 0:
-                selectedPortGroupName = (
-                    portGroupNames[random.randint(0, numPortGroups - 1)])
-                LOG.debug("Returning selected PortGroup: "
-                          "%(selectedPortGroupName)s.",
-                          {'selectedPortGroupName': selectedPortGroupName})
-                return selectedPortGroupName
-
-        exception_message = (_("No PortGroup elements found in config file."))
+            for portGroupElement in portGroupElements:
+                if portGroupElement.childNodes:
+                    portGroupName = portGroupElement.childNodes[0].nodeValue
+                    if portGroupName:
+                        portGroupNames.append(portGroupName.strip())
+            portGroupNames = EMCVMAXUtils._filter_list(portGroupNames)
+            if len(portGroupNames) > 0:
+                return EMCVMAXUtils._get_random_pg_from_list(portGroupNames)
+
+        exception_message = (_("No Port Group elements found in config file."))
         LOG.error(exception_message)
         raise exception.VolumeBackendAPIException(data=exception_message)
 
+    @staticmethod
+    def _get_random_pg_from_list(portgroupnames):
+        """From list of portgroup, choose one randomly
+
+        :param portGroupNames: list of available portgroups
+        :returns: portGroupName - the random portgroup
+        """
+        portgroupname = (
+            portgroupnames[random.randint(0, len(portgroupnames) - 1)])
+
+        LOG.info(_LI("Returning random Port Group: "
+                     "%(portGroupName)s."),
+                 {'portGroupName': portgroupname})
+
+        return portgroupname
+
+    @staticmethod
+    def _filter_list(portgroupnames):
+        """Clean up the port group list
+
+        :param portgroupnames: list of available portgroups
+        :returns: portgroupnames - cleaned up list
+        """
+        portgroupnames = filter(None, portgroupnames)
+        # Convert list to set to remove duplicate portgroups
+        portgroupnames = list(set(portgroupnames))
+        return portgroupnames
+
     def _get_serial_number(self, arrayInfo):
         """If we don't have a pool then we just get the serial number.