From: Xing Yang Date: Sat, 13 Sep 2014 00:16:56 +0000 (-0400) Subject: Handle config file with newlines and whitespaces X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=6d59790b9cd42309417ae1866e40a06dcb557abe;p=openstack-build%2Fcinder-build.git Handle config file with newlines and whitespaces EMC VMAX driver does not handle newlines and whitespaces consistently in the XML config file. This patch addressed this issue. Change-Id: I45f61f167feee9174ba2bc3229695b680f8e946d Closes-Bug: #1364232 --- diff --git a/cinder/tests/test_emc_vmax.py b/cinder/tests/test_emc_vmax.py index 536d5a874..eee0025ee 100644 --- a/cinder/tests/test_emc_vmax.py +++ b/cinder/tests/test_emc_vmax.py @@ -925,7 +925,9 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase): self.tempdir = tempfile.mkdtemp() super(EMCVMAXISCSIDriverNoFastTestCase, self).setUp() self.config_file_path = None + self.config_file_1364232 = None self.create_fake_config_file_no_fast() + self.addCleanup(self._cleanup) configuration = mock.Mock() configuration.safe_get.return_value = 'ISCSINoFAST' @@ -945,6 +947,7 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase): driver = EMCVMAXISCSIDriver(configuration=configuration) driver.db = FakeDB() self.driver = driver + self.driver.utils = EMCVMAXUtils(object) def create_fake_config_file_no_fast(self): @@ -1008,6 +1011,28 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase): doc.writexml(f) f.close() + # Create XML config file with newlines and whitespaces + # Bug #1364232 + def create_fake_config_file_1364232(self): + filename = 'cinder_emc_config_1364232.xml' + self.config_file_1364232 = self.tempdir + '/' + filename + text_file = open(self.config_file_1364232, "w") + text_file.write("\n\n" + "10.108.246.202\n" + "5988\n" + "admin\t\n" + "#1Password\n" + "OS-PORTGROUP1-PG" + "OS-PORTGROUP2-PG" + " \n" + "OS-PORTGROUP3-PG" + "OS-PORTGROUP4-PG" + "\n000198700439" + " \n\nFC_SLVR1\n" + "\nSILVER1\n" + "") + text_file.close() + def fake_ecom_connection(self): conn = FakeEcomConnection() return conn @@ -1021,6 +1046,24 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase): def fake_sleep(self, seconds): return + def test_get_volume_stats_1364232(self): + self.create_fake_config_file_1364232() + self.assertEqual('000198700439', + self.driver.utils.parse_array_name_from_file( + self.config_file_1364232)) + self.assertEqual('FC_SLVR1', + self.driver.utils.parse_pool_name_from_file( + self.config_file_1364232)) + self.assertEqual('SILVER1', + self.driver.utils.parse_fast_policy_name_from_file( + self.config_file_1364232)) + self.assertIn('OS-PORTGROUP', + self.driver.utils.parse_file_to_get_port_group_name( + self.config_file_1364232)) + bExists = os.path.exists(self.config_file_1364232) + if bExists: + os.remove(self.config_file_1364232) + @mock.patch.object( EMCVMAXCommon, '_find_storageSystem', @@ -1325,10 +1368,6 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase): os.remove(self.config_file_path) shutil.rmtree(self.tempdir) - def tearDown(self): - self._cleanup() - super(EMCVMAXISCSIDriverNoFastTestCase, self).tearDown() - class EMCVMAXISCSIDriverFastTestCase(test.TestCase): @@ -1340,6 +1379,7 @@ class EMCVMAXISCSIDriverFastTestCase(test.TestCase): super(EMCVMAXISCSIDriverFastTestCase, self).setUp() self.config_file_path = None self.create_fake_config_file_fast() + self.addCleanup(self._cleanup) configuration = mock.Mock() configuration.cinder_emc_config_file = self.config_file_path @@ -1853,10 +1893,6 @@ class EMCVMAXISCSIDriverFastTestCase(test.TestCase): os.remove(self.config_file_path) shutil.rmtree(self.tempdir) - def tearDown(self): - self._cleanup() - super(EMCVMAXISCSIDriverFastTestCase, self).tearDown() - class EMCVMAXFCDriverNoFastTestCase(test.TestCase): def setUp(self): @@ -1867,6 +1903,7 @@ class EMCVMAXFCDriverNoFastTestCase(test.TestCase): super(EMCVMAXFCDriverNoFastTestCase, self).setUp() self.config_file_path = None self.create_fake_config_file_no_fast() + self.addCleanup(self._cleanup) configuration = mock.Mock() configuration.cinder_emc_config_file = self.config_file_path @@ -2259,10 +2296,6 @@ class EMCVMAXFCDriverNoFastTestCase(test.TestCase): os.remove(self.config_file_path) shutil.rmtree(self.tempdir) - def tearDown(self): - self._cleanup() - super(EMCVMAXFCDriverNoFastTestCase, self).tearDown() - class EMCVMAXFCDriverFastTestCase(test.TestCase): @@ -2274,6 +2307,7 @@ class EMCVMAXFCDriverFastTestCase(test.TestCase): super(EMCVMAXFCDriverFastTestCase, self).setUp() self.config_file_path = None self.create_fake_config_file_fast() + self.addCleanup(self._cleanup) configuration = mock.Mock() configuration.cinder_emc_config_file = self.config_file_path @@ -2777,7 +2811,3 @@ class EMCVMAXFCDriverFastTestCase(test.TestCase): if bExists: os.remove(self.config_file_path) shutil.rmtree(self.tempdir) - - def tearDown(self): - self._cleanup() - super(EMCVMAXFCDriverFastTestCase, self).tearDown() diff --git a/cinder/volume/drivers/emc/emc_vmax_common.py b/cinder/volume/drivers/emc/emc_vmax_common.py index b7d7cc99d..08553305e 100644 --- a/cinder/volume/drivers/emc/emc_vmax_common.py +++ b/cinder/volume/drivers/emc/emc_vmax_common.py @@ -652,7 +652,7 @@ class EMCVMAXCommon(object): 'QoS_support': False, 'volume_backend_name': backendName or self.__class__.__name__, 'vendor_name': "EMC", - 'driver_version': '1.0', + 'driver_version': '2.0', 'storage_protocol': 'unknown', 'location_info': location_info} diff --git a/cinder/volume/drivers/emc/emc_vmax_utils.py b/cinder/volume/drivers/emc/emc_vmax_utils.py index 005213afd..ab7e1c410 100644 --- a/cinder/volume/drivers/emc/emc_vmax_utils.py +++ b/cinder/volume/drivers/emc/emc_vmax_utils.py @@ -480,7 +480,6 @@ class EMCVMAXUtils(object): def find_storage_system_name_from_service(self, configService): """Given any service get the storage system name from it. - :param conn: connection to the ecom server :param configService: the configuration service :returns: configService['SystemName'] - storage system name (String) """ @@ -535,7 +534,7 @@ class EMCVMAXUtils(object): NOTE: This exists in common too...will be moving it to other file where both common and masking can access it - :param classname: connection to the ecom server + :param classname: class name for the volume instance :param bindings: volume created from job :returns: foundVolumeInstance - the volume instance @@ -558,18 +557,9 @@ class EMCVMAXUtils(object): :returns: ecomIp - the ecom IP address :returns: ecomPort - the ecom port """ - myFile = open(filename, 'r') - data = myFile.read() - myFile.close() - dom = parseString(data) - ecomIps = dom.getElementsByTagName('EcomServerIp') - if ecomIps is not None and len(ecomIps) > 0: - ecomIp = ecomIps[0].toxml().replace('', '') - ecomIp = ecomIp.replace('', '') - ecomPorts = dom.getElementsByTagName('EcomServerPort') - if ecomPorts is not None and len(ecomPorts) > 0: - ecomPort = ecomPorts[0].toxml().replace('', '') - ecomPort = ecomPort.replace('', '') + + ecomIp = self._parse_from_file(filename, 'EcomServerIp') + ecomPort = self._parse_from_file(filename, 'EcomServerPort') if ecomIp is not None and ecomPort is not None: LOG.debug("Ecom IP: %(ecomIp)s Port: %(ecomPort)s", {'ecomIp': ecomIp, 'ecomPort': ecomPort}) @@ -585,18 +575,9 @@ class EMCVMAXUtils(object): :returns: ecomUser - the ecom user :returns: ecomPasswd - the ecom password """ - myFile = open(filename, 'r') - data = myFile.read() - myFile.close() - dom = parseString(data) - ecomUsers = dom.getElementsByTagName('EcomUserName') - if ecomUsers is not None and len(ecomUsers) > 0: - ecomUser = ecomUsers[0].toxml().replace('', '') - ecomUser = ecomUser.replace('', '') - ecomPasswds = dom.getElementsByTagName('EcomPassword') - if ecomPasswds is not None and len(ecomPasswds) > 0: - ecomPasswd = ecomPasswds[0].toxml().replace('', '') - ecomPasswd = ecomPasswd.replace('', '') + + ecomUser = self._parse_from_file(filename, 'EcomUserName') + ecomPasswd = self._parse_from_file(filename, 'EcomPassword') if ecomUser is not None and ecomPasswd is not None: return ecomUser, ecomPasswd else: @@ -622,11 +603,14 @@ class EMCVMAXUtils(object): portGroupsXml = portGroups[0].toxml() portGroupsXml = portGroupsXml.replace('', '') portGroupsXml = portGroupsXml.replace('', '') - portGroupsXml = portGroupsXml.replace('', '') + portGroupsXml = portGroupsXml.replace('', '|') portGroupsXml = portGroupsXml.replace('', '') - # convert the newline separated string to a list + portGroupsXml = portGroupsXml.replace('\n', '') + portGroupsXml = portGroupsXml.replace('\t', '') + portGroupsXml = portGroupsXml[1:] + # convert the | separated string to a list portGroupNames = ( - [s.strip() for s in portGroupsXml.split('\n') if s]) + [s.strip() for s in portGroupsXml.split('|') if s]) numPortGroups = len(portGroupNames) @@ -639,30 +623,46 @@ class EMCVMAXUtils(object): LOG.error(exception_message) raise exception.VolumeBackendAPIException(data=exception_message) - def parse_fast_policy_name_from_file(self, fileName): - """Parse the fast policy name from config file. If it is not there - then NON FAST is assumed + def _parse_from_file(self, fileName, stringToParse): + """Parse the string from XML. + + Remove newlines, tabs, and trailing spaces. :param fileName: the path and name of the file - :returns: fastPolicyName - the fast policy name + :returns: retString - the returned string """ - fastPolicyName = None + retString = None myFile = open(fileName, 'r') data = myFile.read() myFile.close() dom = parseString(data) - fastPolicy = dom.getElementsByTagName('FastPolicy') - if fastPolicy is not None and len(fastPolicy) > 0: - fastPolicyXml = fastPolicy[0].toxml() - fastPolicyXml = fastPolicyXml.replace('', '') - fastPolicyName = fastPolicyXml.replace('', '') + tag = dom.getElementsByTagName(stringToParse) + if tag is not None and len(tag) > 0: + strXml = tag[0].toxml() + strXml = strXml.replace('<%s>' % stringToParse, '') + strXml = strXml.replace('\n', '') + strXml = strXml.replace('\t', '') + retString = strXml.replace('' % stringToParse, '') + retString = retString.strip() + return retString + + def parse_fast_policy_name_from_file(self, fileName): + """Parse the fast policy name from config file. + + If it is not there, then NON FAST is assumed. + + :param fileName: the path and name of the file + :returns: fastPolicyName - the fast policy name + """ + + fastPolicyName = self._parse_from_file(fileName, 'FastPolicy') + if fastPolicyName: LOG.debug("File %(fileName)s: Fast Policy is %(fastPolicyName)s" % {'fileName': fileName, 'fastPolicyName': fastPolicyName}) - return fastPolicyName else: LOG.info(_("Fast Policy not found.")) - return None + return fastPolicyName def parse_array_name_from_file(self, fileName): """Parse the array name from config file. @@ -673,20 +673,10 @@ class EMCVMAXUtils(object): :param fileName: the path and name of the file :returns: arrayName - the array name """ - arrayName = None - myFile = open(fileName, 'r') - data = myFile.read() - myFile.close() - dom = parseString(data) - array = dom.getElementsByTagName('Array') - if array is not None and len(array) > 0: - arrayXml = array[0].toxml() - arrayXml = arrayXml.replace('', '') - arrayName = arrayXml.replace('', '') - return arrayName - else: + arrayName = self._parse_from_file(fileName, 'Array') + if not arrayName: LOG.debug("Array not found from config file.") - return None + return arrayName def parse_pool_name_from_file(self, fileName): """Parse the pool name from config file. @@ -696,18 +686,8 @@ class EMCVMAXUtils(object): :param fileName: the path and name of the file :returns: poolName - the pool name """ - poolName = None - myFile = open(fileName, 'r') - data = myFile.read() - myFile.close() - dom = parseString(data) - pool = dom.getElementsByTagName('Pool') - if pool is not None and len(pool) > 0: - poolXml = pool[0].toxml() - poolXml = poolXml.replace('', '') - poolName = poolXml.replace('', '') - return poolName - else: + poolName = self._parse_from_file(fileName, 'Pool') + if not poolName: LOG.debug("Pool not found from config file.") return poolName @@ -1148,7 +1128,7 @@ class EMCVMAXUtils(object): self, conn, hardwareIdManagementService): """Get all the hardware ids from an array. - :param conn: connection to the ecom + :param conn: connection to the ecom server :param: hardwareIdManagementService - hardware id management service :returns: hardwareIdInstanceNames - the list of hardware id instance names @@ -1177,11 +1157,15 @@ class EMCVMAXUtils(object): propertiesList = ( ipProtocolEndpointInstance.properties.items()) for properties in propertiesList: - if properties[0] == 'ElementName': + if properties[0] == 'IPv4Address': cimProperties = properties[1] foundIpAddress = cimProperties.value - break - if foundIpAddress is not None: - break + if (foundIpAddress == '127.0.0.1' + or foundIpAddress == '0.0.0.0'): + foundIpAddress = None + else: + break + if foundIpAddress is not None: + break return foundIpAddress