Ansible: fix lowercasing issue with ConfigParser

by default ConfigParser will lowercase everything,
unless you override optionxform.

also sort key value in inventory line for consistency

https://docs.python.org/3/library/configparser.html#configparser.ConfigParser.optionxform

Transforms the option name option as found in an input file or as passed in by
client code to the form that should be used in the internal structures. The
default implementation returns a lower-case version of option; subclasses may
override this or client code can set an attribute of this name on instances to
affect this behavior.

You don’t need to subclass the parser to use this method, you can also set it
on an instance, to a function that takes a string argument and returns a
string. Setting it to str, for example, would make option names case sensitive:

cfgparser = ConfigParser()
cfgparser.optionxform = str

Note that when reading configuration files, whitespace around the option names
is stripped before optionxform() is called.

YARDSTICK-833

Change-Id: Ia1810b0c77922d84e11c9e538540b38816338593
Signed-off-by: Ross Brattain <ross.b.brattain@intel.com>
(cherry picked from commit 3e93bb8ff3ef9ff454d6be13295198dbeac75df7)

diff --git a/tests/unit/common/test_ansible_common.py b/tests/unit/common/test_ansible_common.py
index a1eaf96..1ef8eee 100644 (file)
--- a/tests/unit/common/test_ansible_common.py
+++ b/tests/unit/common/test_ansible_common.py
@@ -23,6 +23,7 @@ import mock
 import unittest
 
 from six.moves.configparser import ConfigParser
+from six.moves import StringIO
 
 from yardstick.common import ansible_common
 
@@ -30,19 +31,18 @@ PREFIX = 'yardstick.common.ansible_common'
 
 
 class OverwriteDictTestCase(unittest.TestCase):
-
     def test_overwrite_dict_cfg(self):
         c = ConfigParser(allow_no_value=True)
         d = {
             "section_a": "empty_value",
-            "section_b": {"key_c": "val_d", "key_d": "val_d"},
+            "section_b": {"key_c": "Val_d", "key_d": "VAL_D"},
             "section_c": ["key_c", "key_d"],
         }
         ansible_common.overwrite_dict_to_cfg(c, d)
         # Python3 and Python2 convert empty values into None or ''
         # we don't really care but we need to compare correctly for unittest
         self.assertTrue(c.has_option("section_a", "empty_value"))
-        self.assertEqual(sorted(c.items("section_b")), [('key_c', 'val_d'), ('key_d', 'val_d')])
+        self.assertEqual(sorted(c.items("section_b")), [('key_c', 'Val_d'), ('key_d', 'VAL_D')])
         self.assertTrue(c.has_option("section_c", "key_c"))
         self.assertTrue(c.has_option("section_c", "key_d"))
 
@@ -50,23 +50,23 @@ class OverwriteDictTestCase(unittest.TestCase):
 class FilenameGeneratorTestCase(unittest.TestCase):
     @mock.patch('{}.NamedTemporaryFile'.format(PREFIX))
     def test__handle_existing_file(self, mock_tmp):
-        f = ansible_common.FileNameGenerator._handle_existing_file("/dev/null")
+        ansible_common.FileNameGenerator._handle_existing_file("/dev/null")
 
     def test_get_generator_from_file(self):
-        f = ansible_common.FileNameGenerator.get_generator_from_filename("/dev/null", "", "", "")
+        ansible_common.FileNameGenerator.get_generator_from_filename("/dev/null", "", "", "")
 
     def test_get_generator_from_file_middle(self):
-        f = ansible_common.FileNameGenerator.get_generator_from_filename("/dev/null", "", "",
-                                                                         "null")
+        ansible_common.FileNameGenerator.get_generator_from_filename("/dev/null", "", "",
+                                                                     "null")
 
     def test_get_generator_from_file_prefix(self):
-        f = ansible_common.FileNameGenerator.get_generator_from_filename("/dev/null", "", "null",
-                                                                         "middle")
+        ansible_common.FileNameGenerator.get_generator_from_filename("/dev/null", "", "null",
+                                                                     "middle")
 
 
 class AnsibleNodeTestCase(unittest.TestCase):
     def test_ansible_node(self):
-        a = ansible_common.AnsibleNode()
+        ansible_common.AnsibleNode()
 
     def test_ansible_node_len(self):
         a = ansible_common.AnsibleNode()
@@ -104,42 +104,51 @@ class AnsibleNodeTestCase(unittest.TestCase):
 
 class AnsibleNodeDictTestCase(unittest.TestCase):
     def test_ansible_node_dict(self):
-        n = ansible_common.AnsibleNode()
-        a = ansible_common.AnsibleNodeDict(n, {})
+        n = ansible_common.AnsibleNode
+        ansible_common.AnsibleNodeDict(n, {})
 
     def test_ansible_node_dict_len(self):
-        n = ansible_common.AnsibleNode()
+        n = ansible_common.AnsibleNode
         a = ansible_common.AnsibleNodeDict(n, {})
         len(a)
 
     def test_ansible_node_dict_repr(self):
-        n = ansible_common.AnsibleNode()
+        n = ansible_common.AnsibleNode
         a = ansible_common.AnsibleNodeDict(n, {})
         repr(a)
 
     def test_ansible_node_dict_iter(self):
-        n = ansible_common.AnsibleNode()
+        n = ansible_common.AnsibleNode
         a = ansible_common.AnsibleNodeDict(n, {})
         for _ in a:
             pass
 
     def test_ansible_node_dict_get(self):
-        n = ansible_common.AnsibleNode()
+        n = ansible_common.AnsibleNode
         a = ansible_common.AnsibleNodeDict(n, {})
         self.assertIsNone(a.get(""))
 
     def test_gen_inventory_lines_for_all_of_type(self):
-        n = ansible_common.AnsibleNode()
+        n = ansible_common.AnsibleNode
         a = ansible_common.AnsibleNodeDict(n, {})
         self.assertEqual(a.gen_inventory_lines_for_all_of_type(""), [])
 
+    def test_gen_inventory_lines(self):
+        n = ansible_common.AnsibleNode
+        a = ansible_common.AnsibleNodeDict(n, [{
+            "name": "name", "user": "user", "password": "PASS",
+            "role": "role",
+        }])
+        self.assertEqual(a.gen_all_inventory_lines(),
+                         ["name ansible_ssh_pass=PASS ansible_user=user"])
+
 
 class AnsibleCommonTestCase(unittest.TestCase):
     def test_get_timeouts(self):
         self.assertAlmostEquals(ansible_common.AnsibleCommon.get_timeout(-100), 1200.0)
 
     def test__init__(self):
-        a = ansible_common.AnsibleCommon({})
+        ansible_common.AnsibleCommon({})
 
     def test_reset(self):
         a = ansible_common.AnsibleCommon({})
@@ -150,9 +159,16 @@ class AnsibleCommonTestCase(unittest.TestCase):
         self.assertRaises(OSError, a.do_install, '', '')
 
     def test_gen_inventory_dict(self):
-        a = ansible_common.AnsibleCommon({})
-        a.inventory_dict = {}
-        self.assertIsNone(a.gen_inventory_ini_dict())
+        nodes = [{
+            "name": "name", "user": "user", "password": "PASS",
+            "role": "role",
+        }]
+        a = ansible_common.AnsibleCommon(nodes)
+        a.gen_inventory_ini_dict()
+        self.assertEqual(a.inventory_dict, {
+            'nodes': ['name ansible_ssh_pass=PASS ansible_user=user'],
+            'role': ['name']
+        })
 
     def test_deploy_dir(self):
         a = ansible_common.AnsibleCommon({})
@@ -176,6 +192,25 @@ class AnsibleCommonTestCase(unittest.TestCase):
         finally:
             os.rmdir(d)
 
+    @mock.patch('{}.NamedTemporaryFile'.format(PREFIX))
+    @mock.patch('{}.open'.format(PREFIX))
+    def test__gen_ansible_inventory_file(self, mock_open, mock_tmp):
+        nodes = [{
+            "name": "name", "user": "user", "password": "PASS",
+            "role": "role",
+        }]
+        d = tempfile.mkdtemp()
+        try:
+            a = ansible_common.AnsibleCommon(nodes)
+            a.gen_inventory_ini_dict()
+            inv_context = a._gen_ansible_inventory_file(d)
+            with inv_context:
+                c = StringIO()
+                inv_context.write_func(c)
+                self.assertIn("ansible_ssh_pass=PASS", c.getvalue())
+        finally:
+            os.rmdir(d)
+
     @mock.patch('{}.NamedTemporaryFile'.format(PREFIX))
     @mock.patch('{}.open'.format(PREFIX))
     def test__gen_ansible_playbook_file_list_multiple(self, mock_open, mock_tmp):

diff --git a/yardstick/common/ansible_common.py b/yardstick/common/ansible_common.py
index 0cafa97..9a4426b 100644 (file)
--- a/yardstick/common/ansible_common.py
+++ b/yardstick/common/ansible_common.py
@@ -298,8 +298,9 @@ class AnsibleNode(MutableMapping):
     def gen_inventory_line(self):
         inventory_params = self.get_inventory_params()
         # use format to convert ints
+        # sort to ensure consistent key value ordering
         formatted_args = (u"{}={}".format(*entry) for entry in
-                          inventory_params.items())
+                          sorted(inventory_params.items()))
         line = u" ".join(chain([self['name']], formatted_args))
         return line
 
@@ -472,6 +473,8 @@ class AnsibleCommon(object):
         prefix = '_'.join([self.prefix, prefix, 'inventory'])
         ini_temp_file = IniMapTemporaryFile(directory=directory, prefix=prefix)
         inventory_config = ConfigParser.ConfigParser(allow_no_value=True)
+        # disable default lowercasing
+        inventory_config.optionxform = str
         return ini_temp_file.make_context(self.inventory_dict, write_func,
                                           descriptor='inventory')