Merge pull request #6287 from risaacson/fix_hash_in_var

Fix inventory for test_dir_inventory, merge PRs from GH-5749, cleanup some formatting.

lib/ansible/inventory/ini.py

@@ -23,10 +23,12 @@ from ansible.inventory.group import Group
 from ansible.inventory.expand_hosts import detect_range
 from ansible.inventory.expand_hosts import expand_hostname_range
 from ansible import errors
+import ansible.utils as utils
 import shlex
 import re
 import ast
 
+
 class InventoryParser(object):
     """
     Host inventory for ansible.
@@ -47,7 +49,6 @@ class InventoryParser(object):
         self._parse_group_variables()
         return self.groups
 
-
     # [webservers]
     # alpha
     # beta:2345
@@ -65,7 +66,14 @@ class InventoryParser(object):
         active_group_name = 'ungrouped'
 
         for line in self.lines:
-            line = line.split("#")[0].strip()
+
+            # Split off any comments that are not contained in a variable.
+            if "#" in line:
+                line = utils.split_unquoted_hash(line)
+
+            # Clean up the end of the line.
+            line = line.strip()
+
             if line.startswith("[") and line.endswith("]"):
                 active_group_name = line.replace("[", "").replace("]", "")
                 if line.find(":vars") != -1 or line.find(":children") != -1:
@@ -101,14 +109,12 @@ class InventoryParser(object):
                       (hostname.find("]") == -1 and hostname.find(":") != -1)):
                     (hostname, port) = hostname.rsplit(":", 1)
 
-                hostnames = []
                 if detect_range(hostname):
                     hostnames = expand_hostname_range(hostname)
                 else:
                     hostnames = [hostname]
 
                 for hn in hostnames:
-                    host = None
                     if hn in self.hosts:
                         host = self.hosts[hn]
                     else:
@@ -122,11 +128,20 @@ class InventoryParser(object):
                                 (k, v) = t.split("=", 1)
                             except ValueError, e:
                                 raise errors.AnsibleError("Invalid ini entry: %s - %s" % (t, str(e)))
+                            # I am not sure where a variable with a hash needs to be evaluated via ast.
+                            # If an instance comes up this is the condition we need to modify.
+                            if "#" in v:
+                                host.set_variable(k, v)
+                            else:
                                 try:
                                     host.set_variable(k, ast.literal_eval(v))
-                            except:
-                                # most likely a string that literal_eval
-                                # doesn't like, so just set it
+                                # Using explicit exceptions.
+                                # Likely a string that literal_eval does not like. We wil then just set it.
+                                except ValueError:
+                                    # For some reason this was thought to be malformed.
+                                    host.set_variable(k, v)
+                                except SyntaxError:
+                                    # Is this a hash with an equals at the end?
                                     host.set_variable(k, v)
                     self.groups[active_group_name].add_host(host)
 
@@ -157,7 +172,6 @@ class InventoryParser(object):
                 else:
                     group.add_child_group(kid_group)
 
-
     # [webservers:vars]
     # http_port=1234
     # maxRequestsPerChild=200

lib/ansible/utils/__init__.py

@@ -1071,3 +1071,37 @@ def random_password(length=20, chars=C.DEFAULT_PASSWORD_CHARS):
             password.append(new_char)
 
     return ''.join(password)
+
+
+def split_unquoted_hash(line):
+    '''
+     Carve off comments from a line which are not contained in quotes and a part of an assignment.
+    '''
+
+    # We would really like to have this using a regex to make it less code. For instance:
+    # line = re.split('(?<!=["|\'].*)#(?!.*?["|\']).*', line)[0]
+    # this has the problem that it comes back with a "sre_constants.error: look-behind requires fixed-width pattern"
+
+    if "#" in line:
+        split_line = line.split("#")
+        instances = len(split_line) - 1
+        if instances > 0:
+            marker = 0
+            while marker < instances:
+                if ("=\"" in split_line[marker] and "\"" in split_line[marker + 1]) or (
+                        "='" in split_line[marker] and "'" in split_line[marker + 1]):
+                    marker += 1
+                else:
+                    if marker == 0:
+                        line = split_line[marker]
+                    else:
+                        # We have multiple fragments that we need to combine back together.
+                        # rekram is us reversing that work we did with marker.
+                        rekram = 0
+                        new_line = split_line[rekram]
+                        while marker > rekram:
+                            rekram += 1
+                            new_line = new_line + "#" + split_line[rekram]
+                        line = new_line
+                    break
+        return line

test/units/TestInventory.py

@@ -5,6 +5,7 @@ from nose.tools import raises
 from ansible import errors
 from ansible.inventory import Inventory
 
+
 class TestInventory(unittest.TestCase):
 
     def setUp(self):
@@ -275,7 +276,6 @@ class TestInventory(unittest.TestCase):
 
     def test_complex_enumeration(self):
 
-
         expected1 = ['rtp_b']
         expected2 = ['rtp_a', 'rtp_b']
         expected3 = ['rtp_a', 'rtp_b', 'rtp_c', 'tri_a', 'tri_b', 'tri_c']
@@ -342,8 +342,8 @@ class TestInventory(unittest.TestCase):
 
         expected_hosts = ['jupiter', 'saturn', 'zeus', 'hera', 'poseidon', 'thor', 'odin', 'loki']
 
-        print "Expected: %s"%(expected_hosts)
-        print "Got     : %s"%(hosts)
+        print "Expected: %s" % expected_hosts
+        print "Got     : %s" % hosts
         assert sorted(hosts) == sorted(expected_hosts)
 
     def test_script_all(self):
@@ -417,15 +417,24 @@ class TestInventory(unittest.TestCase):
         auth = inventory.get_variables('neptun')['auth']
         assert auth == 'YWRtaW46YWRtaW4='
 
-    # test disabled as needs to be updated to model desired behavior
-    #
-    #def test_dir_inventory(self):
-    #    inventory = self.dir_inventory()
-    #    vars = inventory.get_variables('zeus')
-    #
-    #   print "VARS=%s" % vars
-    #
-    #    assert vars == {'inventory_hostname': 'zeus',
-    #                    'inventory_hostname_short': 'zeus',
-    #                    'group_names': ['greek', 'major-god', 'ungrouped'],
-    #                    'var_a': '1#2'}
+    def test_dir_inventory(self):
+        inventory = self.dir_inventory()
+
+        host_vars = inventory.get_variables('zeus')
+
+        expected_vars = {'inventory_hostname': 'zeus',
+                         'inventory_hostname_short': 'zeus',
+                         'group_names': ['greek', 'major-god', 'ungrouped'],
+                         'var_a': '2#3'}
+
+        print "HOST     VARS=%s" % host_vars
+        print "EXPECTED VARS=%s" % expected_vars
+
+        assert host_vars == expected_vars
+
+    def test_dir_inventory_multiple_groups(self):
+        inventory = self.dir_inventory()
+        group_greek = inventory.get_hosts('greek')
+        actual_host_names = [host.name for host in group_greek]
+        print "greek : %s " % actual_host_names
+        assert actual_host_names == ['zeus', 'morpheus']

test/units/inventory_test_data/inventory_dir/0hosts

@@ -1,3 +1,3 @@
-zeus var_a=2
+zeus var_a=0
 morpheus
 thor

test/units/inventory_test_data/inventory_dir/3comments

@@ -1,5 +1,8 @@
+[titan]
+cronus var_a="a#b" var_b="b#c" var_c="c#d" # Is this overkill?
+
 [major-god] # group with inline comments
-zeus var_a="1#2" # host with inline comments and "#" in the var string
+zeus var_a="2#3" # host with inline comments and "#" in the var string
 # A comment
 thor