Fix image properties parsing

It removes useless spaces and now conforms with oslo.conf dict.
New helpers and unit tests are available.

Change-Id: I20335c5e5c3840cc872e158c6072d97a3f2cb98c
Signed-off-by: Cédric Ollivier <cedric.ollivier@orange.com>
(cherry picked from commit 904377462f58464937d4cafeb8c432f024a85bd4)

6 files changed, 85 insertions, 46 deletions

diff --git a/functest/core/singlevm.py b/functest/core/singlevm.py
index d1806eb12..b3a1417f6 100644
--- a/functest/core/singlevm.py
+++ b/functest/core/singlevm.py
@@ -24,6 +24,7 @@ from xtesting.core import testcase
 from functest.core import tenantnetwork
 from functest.utils import config
 from functest.utils import env
+from functest.utils import functest_utils
 
 
 class VmReady1(tenantnetwork.TenantNetwork1):
@@ -76,9 +77,8 @@ class VmReady1(tenantnetwork.TenantNetwork1):
         extra_properties = self.extra_properties.copy()
         if env.get('IMAGE_PROPERTIES'):
             extra_properties.update(
-                dict((k.strip(), v.strip()) for k, v in (
-                    item.split(': ') for item in env.get(
-                        'IMAGE_PROPERTIES').split(','))))
+                functest_utils.convert_ini_to_dict(
+                    env.get('IMAGE_PROPERTIES')))
         extra_properties.update(
             getattr(config.CONF, '{}_extra_properties'.format(
                 self.case_name), {}))
@@ -112,9 +112,8 @@ class VmReady1(tenantnetwork.TenantNetwork1):
         extra_alt_properties = self.extra_alt_properties.copy()
         if env.get('IMAGE_PROPERTIES'):
             extra_alt_properties.update(
-                dict((k.strip(), v.strip()) for k, v in (
-                    item.split(': ') for item in env.get(
-                        'IMAGE_PROPERTIES').split(','))))
+                functest_utils.convert_ini_to_dict(
+                    env.get('IMAGE_PROPERTIES')))
         extra_alt_properties.update(
             getattr(config.CONF, '{}_extra_alt_properties'.format(
                 self.case_name), {}))
diff --git a/functest/opnfv_tests/openstack/tempest/conf_utils.py b/functest/opnfv_tests/openstack/tempest/conf_utils.py
index fc36a72ed..60d7f3218 100644
--- a/functest/opnfv_tests/openstack/tempest/conf_utils.py
+++ b/functest/opnfv_tests/openstack/tempest/conf_utils.py
@@ -19,12 +19,12 @@ import os
 import subprocess
 
 import pkg_resources
-import six
 from six.moves import configparser
 import yaml
 
 from functest.utils import config
 from functest.utils import env
+from functest.utils import functest_utils
 
 
 RALLY_CONF_PATH = "/etc/rally/rally.conf"
@@ -201,13 +201,13 @@ def configure_tempest_update_params(
                'ServerGroupAntiAffinityFilter', 'ServerGroupAffinityFilter']
     rconfig.set(
         'compute-feature-enabled', 'scheduler_available_filters',
-        convert_list_to_ini(filters))
+        functest_utils.convert_list_to_ini(filters))
     if os.environ.get('OS_REGION_NAME'):
         rconfig.set('identity', 'region', os.environ.get('OS_REGION_NAME'))
     if env.get("NEW_USER_ROLE").lower() != "member":
         rconfig.set(
             'auth', 'tempest_roles',
-            convert_list_to_ini([env.get("NEW_USER_ROLE")]))
+            functest_utils.convert_list_to_ini([env.get("NEW_USER_ROLE")]))
     if not json.loads(env.get("USE_DYNAMIC_CREDENTIALS").lower()):
         rconfig.set('auth', 'use_dynamic_credentials', False)
         account_file = os.path.join(
@@ -269,16 +269,3 @@ def configure_verifier(deployment_dir):
                      tempest_conf_file)
         return None
     return tempest_conf_file
-
-
-def convert_dict_to_ini(value):
-    "Convert dict to oslo.conf input"
-    assert isinstance(value, dict)
-    return ",".join("{}:{}".format(
-        key, val) for (key, val) in six.iteritems(value))
-
-
-def convert_list_to_ini(value):
-    "Convert list to oslo.conf input"
-    assert isinstance(value, list)
-    return ",".join("{}".format(val) for val in value)
diff --git a/functest/opnfv_tests/openstack/tempest/tempest.py b/functest/opnfv_tests/openstack/tempest/tempest.py
index c2ff98f6c..910b54615 100644
--- a/functest/opnfv_tests/openstack/tempest/tempest.py
+++ b/functest/opnfv_tests/openstack/tempest/tempest.py
@@ -27,6 +27,7 @@ from functest.core import singlevm
 from functest.opnfv_tests.openstack.tempest import conf_utils
 from functest.utils import config
 from functest.utils import env
+from functest.utils import functest_utils
 
 LOGGER = logging.getLogger(__name__)
 
@@ -384,15 +385,14 @@ class TempestCommon(singlevm.VmReady2):
         extra_properties = self.extra_properties.copy()
         if env.get('IMAGE_PROPERTIES'):
             extra_properties.update(
-                dict((k.strip(), v.strip()) for k, v in (
-                    item.split(': ') for item in env.get(
-                        'IMAGE_PROPERTIES').split(','))))
+                functest_utils.convert_ini_to_dict(
+                    env.get('IMAGE_PROPERTIES')))
         extra_properties.update(
             getattr(config.CONF, '{}_extra_properties'.format(
                 self.case_name), {}))
         rconfig.set(
             'scenario', 'img_properties',
-            conf_utils.convert_dict_to_ini(extra_properties))
+            functest_utils.convert_dict_to_ini(extra_properties))
         with open(self.conf_file, 'wb') as config_file:
             rconfig.write(config_file)
 
diff --git a/functest/tests/unit/openstack/tempest/test_conf_utils.py b/functest/tests/unit/openstack/tempest/test_conf_utils.py
index 316f21be2..19b07ba3e 100644
--- a/functest/tests/unit/openstack/tempest/test_conf_utils.py
+++ b/functest/tests/unit/openstack/tempest/test_conf_utils.py
@@ -158,26 +158,6 @@ class OSTempestConfUtilsTesting(unittest.TestCase):
                 ['rally', 'verify', 'configure-verifier', '--reconfigure',
                  '--id', str(getattr(config.CONF, 'tempest_verifier_name'))])
 
-    def test_convert_dict_to_ini(self):
-        self.assertEqual(
-            conf_utils.convert_dict_to_ini({}), "")
-        self.assertEqual(
-            conf_utils.convert_dict_to_ini({"a": "b"}), "a:b")
-        value = conf_utils.convert_dict_to_ini({"a": "b", "c": "d"})
-        self.assertTrue(value == "a:b,c:d" or value == "c:d,a:b")
-        with self.assertRaises(AssertionError):
-            conf_utils.convert_list_to_ini("")
-
-    def test_convert_list_to_ini(self):
-        self.assertEqual(
-            conf_utils.convert_list_to_ini([]), "")
-        self.assertEqual(
-            conf_utils.convert_list_to_ini(["a"]), "a")
-        self.assertEqual(
-            conf_utils.convert_list_to_ini(["a", "b"]), "a,b")
-        with self.assertRaises(AssertionError):
-            conf_utils.convert_list_to_ini("")
-
 
 if __name__ == "__main__":
     logging.disable(logging.CRITICAL)
diff --git a/functest/tests/unit/utils/test_functest_utils.py b/functest/tests/unit/utils/test_functest_utils.py
index 2b98267c4..a2c768f5d 100644
--- a/functest/tests/unit/utils/test_functest_utils.py
+++ b/functest/tests/unit/utils/test_functest_utils.py
@@ -338,6 +338,47 @@ class FunctestUtilsTesting(unittest.TestCase):
             functest_utils.get_openstack_version(cloud), "Unknown")
         args[0].assert_called_once_with(cloud)
 
+    def test_convert_dict_to_ini(self):
+        self.assertEqual(
+            functest_utils.convert_dict_to_ini({}), "")
+        self.assertEqual(
+            functest_utils.convert_dict_to_ini({"a": "b"}), "a:b")
+        value = functest_utils.convert_dict_to_ini({"a": "b", "c": "d"})
+        self.assertTrue(value == "a:b,c:d" or value == "c:d,a:b")
+        with self.assertRaises(AssertionError):
+            functest_utils.convert_list_to_ini("")
+
+    def test_convert_list_to_ini(self):
+        self.assertEqual(
+            functest_utils.convert_list_to_ini([]), "")
+        self.assertEqual(
+            functest_utils.convert_list_to_ini(["a"]), "a")
+        self.assertEqual(
+            functest_utils.convert_list_to_ini(["a", "b"]), "a,b")
+        with self.assertRaises(AssertionError):
+            functest_utils.convert_list_to_ini("")
+
+    def test_convert_ini_to_dict(self):
+        self.assertEqual(
+            functest_utils.convert_ini_to_dict(""), {})
+        self.assertEqual(
+            functest_utils.convert_ini_to_dict("a:b"), {"a": "b"})
+        self.assertEqual(
+            functest_utils.convert_ini_to_dict(
+                "a:b,c:d"), {"a": "b", "c": "d"})
+        with self.assertRaises(AssertionError):
+            functest_utils.convert_list_to_ini({})
+
+    def test_convert_ini_to_list(self):
+        self.assertEqual(
+            functest_utils.convert_ini_to_list(""), [])
+        self.assertEqual(
+            functest_utils.convert_ini_to_list("a"), ["a"])
+        self.assertEqual(
+            functest_utils.convert_ini_to_list("a,b"), ["a", "b"])
+        with self.assertRaises(AssertionError):
+            functest_utils.convert_ini_to_list([])
+
 
 if __name__ == "__main__":
     logging.disable(logging.CRITICAL)
diff --git a/functest/utils/functest_utils.py b/functest/utils/functest_utils.py
index 1993f1e83..75eefc8dd 100644
--- a/functest/utils/functest_utils.py
+++ b/functest/utils/functest_utils.py
@@ -15,6 +15,8 @@ import subprocess
 import sys
 import yaml
 
+import six
+
 LOGGER = logging.getLogger(__name__)
 
 
@@ -132,3 +134,33 @@ def get_openstack_version(cloud):
     except AssertionError:
         LOGGER.exception("Cannot detect OpenStack version")
         return "Unknown"
+
+
+def convert_dict_to_ini(value):
+    "Convert dict to oslo.conf input"
+    assert isinstance(value, dict)
+    return ",".join("{}:{}".format(
+        key, val) for (key, val) in six.iteritems(value))
+
+
+def convert_list_to_ini(value):
+    "Convert list to oslo.conf input"
+    assert isinstance(value, list)
+    return ",".join("{}".format(val) for val in value)
+
+
+def convert_ini_to_dict(value):
+    "Convert oslo.conf input to dict"
+    assert isinstance(value, str)
+    try:
+        return {k: v for k, v in (x.split(':') for x in value.split(','))}
+    except ValueError:
+        return {}
+
+
+def convert_ini_to_list(value):
+    "Convert list to oslo.conf input"
+    assert isinstance(value, str)
+    if not value:
+        return []
+    return [x for x in value.split(',')]