Merge pull request #2535 from smunilla/BZ1368414_fix

Fix openshift_node_labels in generated hosts file

utils/src/ooinstall/openshift_ansible.py

@@ -36,7 +36,6 @@ HOST_VARIABLES_MAP = {
     'public_ip': 'openshift_public_ip',
     'hostname': 'openshift_hostname',
     'public_hostname': 'openshift_public_hostname',
-    'node_labels': 'openshift_node_labels',
     'containerized': 'containerized',
 }
 
@@ -200,6 +199,9 @@ def write_host(host, role, inventory, schedulable=None):
         for variable, value in host.other_variables.iteritems():
             facts += " {}={}".format(variable, value)
 
+    if host.node_labels and role == 'node':
+        facts += ' openshift_node_labels="{}"'.format(host.node_labels)
+
     # Distinguish between three states, no schedulability specified (use default),
     # explicitly set to True, or explicitly set to False:
     if role != 'node' or schedulable is None:

utils/test/oo_config_tests.py

@@ -2,6 +2,7 @@
 # repo. We will work on these over time.
 # pylint: disable=bad-continuation,missing-docstring,no-self-use,invalid-name
 
+import cStringIO
 import os
 import unittest
 import tempfile
@@ -9,6 +10,7 @@ import shutil
 import yaml
 
 from ooinstall.oo_config import OOConfig, Host, OOConfigInvalidHostError
+import ooinstall.openshift_ansible
 
 SAMPLE_CONFIG = """
 variant: openshift-enterprise
@@ -224,3 +226,81 @@ class HostTests(OOInstallFixture):
             'public_hostname': 'a.example.com',
         }
         self.assertRaises(OOConfigInvalidHostError, Host, **yaml_props)
+
+    def test_inventory_file_quotes_node_labels(self):
+        """Verify a host entry wraps openshift_node_labels value in double quotes"""
+        yaml_props = {
+            'ip': '192.168.0.1',
+            'hostname': 'a.example.com',
+            'connect_to': 'a-private.example.com',
+            'public_ip': '192.168.0.1',
+            'public_hostname': 'a.example.com',
+            'new_host': True,
+            'roles': ['node'],
+            'node_labels': {
+                'region': 'infra'
+            },
+
+        }
+
+        new_node = Host(**yaml_props)
+        inventory = cStringIO.StringIO()
+        # This is what the 'write_host' function generates. write_host
+        # has no return value, it just writes directly to the file
+        # 'inventory' which in this test-case is a StringIO object
+        ooinstall.openshift_ansible.write_host(
+            new_node,
+            'node',
+            inventory,
+            schedulable=True)
+        # read the value of what was written to the inventory "file"
+        legacy_inventory_line = inventory.getvalue()
+
+        # Given the `yaml_props` above we should see a line like this:
+        #     openshift_node_labels="{'region': 'infra'}"
+        node_labels_expected = '''openshift_node_labels="{'region': 'infra'}"'''  # Quotes around the hash
+        node_labels_bad = '''openshift_node_labels={'region': 'infra'}'''  # No quotes around the hash
+
+        # The good line is present in the written inventory line
+        self.assertIn(node_labels_expected, legacy_inventory_line)
+        # An unquoted version is not present
+        self.assertNotIn(node_labels_bad, legacy_inventory_line)
+
+
+    # def test_new_write_inventory_same_as_legacy(self):
+    #     """Verify the original write_host function produces the same output as the new method"""
+    #     yaml_props = {
+    #         'ip': '192.168.0.1',
+    #         'hostname': 'a.example.com',
+    #         'connect_to': 'a-private.example.com',
+    #         'public_ip': '192.168.0.1',
+    #         'public_hostname': 'a.example.com',
+    #         'new_host': True,
+    #         'roles': ['node'],
+    #         'other_variables': {
+    #             'zzz': 'last',
+    #             'foo': 'bar',
+    #             'aaa': 'first',
+    #         },
+    #     }
+
+    #     new_node = Host(**yaml_props)
+    #     inventory = cStringIO.StringIO()
+
+    #     # This is what the original 'write_host' function will
+    #     # generate. write_host has no return value, it just writes
+    #     # directly to the file 'inventory' which in this test-case is
+    #     # a StringIO object
+    #     ooinstall.openshift_ansible.write_host(
+    #         new_node,
+    #         'node',
+    #         inventory,
+    #         schedulable=True)
+    #     legacy_inventory_line = inventory.getvalue()
+
+    #     # This is what the new method in the Host class generates
+    #     new_inventory_line = new_node.inventory_string('node', schedulable=True)
+
+    #     self.assertEqual(
+    #         legacy_inventory_line,
+    #         new_inventory_line)