Merge "Don't override extra specs with config in VMAX"

cinder/tests/test_emc_vmax.py

@@ -1470,6 +1470,34 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase):
         doc.writexml(f)
         f.close()
 
+    def create_fake_config_file_no_fast_with_interval(self):
+
+        doc = minidom.Document()
+        emc = doc.createElement("EMC")
+        doc.appendChild(emc)
+        doc = self.add_array_info(doc, emc)
+        doc = self.add_interval_only(doc, emc)
+        filename = 'cinder_emc_config_ISCSINoFAST.xml'
+        self.config_file_path = self.tempdir + '/' + filename
+
+        f = open(self.config_file_path, 'w')
+        doc.writexml(f)
+        f.close()
+
+    def create_fake_config_file_no_fast_with_retries(self):
+
+        doc = minidom.Document()
+        emc = doc.createElement("EMC")
+        doc.appendChild(emc)
+        doc = self.add_array_info(doc, emc)
+        doc = self.add_retries_only(doc, emc)
+        filename = 'cinder_emc_config_ISCSINoFAST.xml'
+        self.config_file_path = self.tempdir + '/' + filename
+
+        f = open(self.config_file_path, 'w')
+        doc.writexml(f)
+        f.close()
+
     def add_array_info(self, doc, emc):
         array = doc.createElement("Array")
         arraytext = doc.createTextNode("1234567891011")
@@ -1532,6 +1560,20 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase):
         retries.appendChild(retriestext)
         return doc
 
+    def add_interval_only(self, doc, emc):
+        interval = doc.createElement("Interval")
+        intervaltext = doc.createTextNode("20")
+        emc.appendChild(interval)
+        interval.appendChild(intervaltext)
+        return doc
+
+    def add_retries_only(self, doc, emc):
+        retries = doc.createElement("Retries")
+        retriestext = doc.createTextNode("70")
+        emc.appendChild(retries)
+        retries.appendChild(retriestext)
+        return doc
+
     # fix for https://bugs.launchpad.net/cinder/+bug/1364232
     def create_fake_config_file_1364232(self):
         filename = 'cinder_emc_config_1364232.xml'
@@ -1724,7 +1766,10 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase):
         volumeInstanceName = (
             conn.EnumerateInstanceNames("EMC_StorageVolume")[0])
         volumeName = "1403160-Vol"
-        extraSpecs = self.driver.common.extraSpecs
+        extraSpecs = {'volume_backend_name': 'ISCSINoFAST'}
+        extraSpecs = (
+            self.driver.common._get_job_extra_specs(self.config_file_path,
+                                                    extraSpecs))
 
         # Deleting Storage Group failed
         self.assertRaises(
@@ -2036,34 +2081,50 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase):
             'OS-PORTGROUP' in
             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(
+    def test_intervals_and_retries_override(
-        emc_vmax_common.EMCVMAXCommon,
+            self):
-        '_get_pool_and_storage_system',
-        return_value=(None, EMCVMAXCommonData.storage_system))
-    @mock.patch.object(
-        volume_types,
-        'get_volume_type_extra_specs',
-        return_value={'volume_backend_name': 'ISCSINoFAST'})
-    def test_intervals_and_retries(
-            self, _mock_volume_type, mock_storage_system):
-        save_config_path = self.config_file_path
         self.create_fake_config_file_no_fast_with_add_ons()
-        self.driver.create_volume(self.data.test_volume_v2)
+        extraSpecs = {'volume_backend_name': 'ISCSINoFAST'}
-        extraSpecs = self.driver.common.extraSpecs
+        extraSpecs = (
+            self.driver.common._get_job_extra_specs(self.config_file_path,
+                                                    extraSpecs))
         self.assertEqual(40,
                          self.driver.utils._get_max_job_retries(extraSpecs))
         self.assertEqual(5,
                          self.driver.utils._get_interval_in_secs(extraSpecs))
 
-        bExists = os.path.exists(self.config_file_path)
+    def test_intervals_and_retries_default(self):
-        if bExists:
+        extraSpecs = {'volume_backend_name': 'ISCSINoFAST'}
-            os.remove(self.config_file_path)
+        extraSpecs = (
+            self.driver.common._get_job_extra_specs(self.config_file_path,
+                                                    extraSpecs))
+        self.assertEqual(60,
+                         self.driver.utils._get_max_job_retries(extraSpecs))
+        self.assertEqual(10,
+                         self.driver.utils._get_interval_in_secs(extraSpecs))
 
-        self.config_file_path = save_config_path
+    def test_interval_only(self):
+        extraSpecs = {'volume_backend_name': 'ISCSINoFAST'}
+        self.create_fake_config_file_no_fast_with_interval()
+        extraSpecs = (
+            self.driver.common._get_job_extra_specs(self.config_file_path,
+                                                    extraSpecs))
+        self.assertEqual(60,
+                         self.driver.utils._get_max_job_retries(extraSpecs))
+        self.assertEqual(20,
+                         self.driver.utils._get_interval_in_secs(extraSpecs))
+
+    def test_retries_only(self):
+        extraSpecs = {'volume_backend_name': 'ISCSINoFAST'}
+        self.create_fake_config_file_no_fast_with_retries()
+        extraSpecs = (
+            self.driver.common._get_job_extra_specs(self.config_file_path,
+                                                    extraSpecs))
+        self.assertEqual(70,
+                         self.driver.utils._get_max_job_retries(extraSpecs))
+        self.assertEqual(10,
+                         self.driver.utils._get_interval_in_secs(extraSpecs))
 
     @mock.patch.object(
         emc_vmax_utils.EMCVMAXUtils,
@@ -2568,9 +2629,14 @@ class EMCVMAXISCSIDriverNoFastTestCase(test.TestCase):
             self.data.test_ctxt, self.data.test_CG_snapshot)
 
     def _cleanup(self):
-        bExists = os.path.exists(self.config_file_path)
+        if self.config_file_path:
-        if bExists:
+            bExists = os.path.exists(self.config_file_path)
-            os.remove(self.config_file_path)
+            if bExists:
+                os.remove(self.config_file_path)
+        if self.config_file_1364232:
+            bExists = os.path.exists(self.config_file_1364232)
+            if bExists:
+                os.remove(self.config_file_1364232)
         shutil.rmtree(self.tempdir)
 
 

cinder/volume/drivers/emc/emc_vmax_utils.py

@@ -344,7 +344,7 @@ class EMCVMAXUtils(object):
         :param extraSpecs: extraSpecs dict
         :returns: JOB_RETRIES or user defined
         """
-        if extraSpecs:
+        if extraSpecs and RETRIES in extraSpecs:
             jobRetries = extraSpecs[RETRIES]
         else:
             jobRetries = JOB_RETRIES
@@ -356,7 +356,7 @@ class EMCVMAXUtils(object):
         :param extraSpecs: extraSpecs dict
         :returns: INTERVAL_10_SEC or user defined
         """
-        if extraSpecs:
+        if extraSpecs and INTERVAL in extraSpecs:
             intervalInSecs = extraSpecs[INTERVAL]
         else:
             intervalInSecs = INTERVAL_10_SEC
@@ -828,13 +828,13 @@ class EMCVMAXUtils(object):
         If it is not there then the default will be used.
 
         :param fileName: the path and name of the file
-        :returns: string -- interval - the interval in seconds
+        :returns: interval - the interval in seconds
         """
         interval = self._parse_from_file(fileName, 'Interval')
         if interval:
             return interval
         else:
-            LOG.debug("Interval not found in config file.")
+            LOG.debug("Interval not overridden, default of 10 assumed.")
             return None
 
     def parse_retries_from_file(self, fileName):
@@ -843,13 +843,13 @@ class EMCVMAXUtils(object):
         If it is not there then the default will be used.
 
         :param fileName: the path and name of the file
-        :returns: string -- retries - the max number of retries
+        :returns: retries - the max number of retries
         """
         retries = self._parse_from_file(fileName, 'Retries')
         if retries:
             return retries
         else:
-            LOG.debug("Retries not found in config file.")
+            LOG.debug("Retries not overridden, default of 60 assumed.")
             return None
 
     def parse_pool_instance_id(self, poolInstanceId):