Cleaning up default group creation in inventory parsers

lib/ansible/inventory/__init__.py

@@ -86,12 +86,14 @@ class Inventory(object):
 
         self.parser = None
 
-        # Always create the 'all' group, even if host_list is
-        # empty: in this case we will subsequently an the implicit
-        # 'localhost' to it.
+        # Always create the 'all' and 'ungrouped' groups, even if host_list is
+        # empty: in this case we will subsequently an the implicit 'localhost' to it.
 
+        ungrouped = Group(name='ungrouped')
         all = Group('all')
-        self.groups = [ all ]
+        all.add_child_group(ungrouped)
+
+        self.groups = dict(all=all, ungrouped=ungrouped)
 
         if host_list is None:
             pass
@@ -104,9 +106,9 @@ class Inventory(object):
             if self._loader.is_directory(host_list):
                 # Ensure basedir is inside the directory
                 host_list = os.path.join(self.host_list, "")
-                self.parser = InventoryDirectory(loader=self._loader, filename=host_list)
+                self.parser = InventoryDirectory(loader=self._loader, groups=self.groups, filename=host_list)
             else:
-                self.parser = get_file_parser(host_list, self._loader)
+                self.parser = get_file_parser(host_list, self.groups, self._loader)
                 vars_loader.add_directory(self.basedir(), with_subdir=True)
 
             if self.parser:
@@ -393,13 +395,7 @@ class Inventory(object):
         new_host.set_variable("ansible_python_interpreter", sys.executable)
         new_host.set_variable("ansible_connection", "local")
         new_host.ipv4_address = '127.0.0.1'
-
-        ungrouped = self.get_group("ungrouped")
-        if ungrouped is None:
-            self.add_group(Group('ungrouped'))
-            ungrouped = self.get_group('ungrouped')
-            self.get_group('all').add_child_group(ungrouped)
-        ungrouped.add_host(new_host)
+        self.get_group("ungrouped").add_host(new_host)
         return new_host
 
     def clear_pattern_cache(self):

lib/ansible/inventory/dir.py

@@ -34,7 +34,7 @@ from ansible.inventory.script import InventoryScript
 
 __all__ = ['get_file_parser']
 
-def get_file_parser(hostsfile, loader):
+def get_file_parser(hostsfile, groups, loader):
     # check to see if the specified file starts with a
     # shebang (#!/), so if an error is raised by the parser
     # class we can show a more apropos error
@@ -55,7 +55,7 @@ def get_file_parser(hostsfile, loader):
 
     if loader.is_executable(hostsfile):
         try:
-            parser = InventoryScript(loader=loader, filename=hostsfile)
+            parser = InventoryScript(loader=loader, groups=groups, filename=hostsfile)
             processed = True
         except Exception as e:
             myerr.append("The file %s is marked as executable, but failed to execute correctly. " % hostsfile + \
@@ -64,7 +64,7 @@ def get_file_parser(hostsfile, loader):
 
     if not processed:
         try:
-            parser = InventoryINIParser(loader=loader, filename=hostsfile)
+            parser = InventoryINIParser(loader=loader, groups=groups, filename=hostsfile)
             processed = True
         except Exception as e:
             if shebang_present and not loader.is_executable(hostsfile):
@@ -81,13 +81,13 @@ def get_file_parser(hostsfile, loader):
 class InventoryDirectory(object):
     ''' Host inventory parser for ansible using a directory of inventories. '''
 
-    def __init__(self, loader, filename=C.DEFAULT_HOST_LIST):
+    def __init__(self, loader, groups=dict(), filename=C.DEFAULT_HOST_LIST):
         self.names = os.listdir(filename)
         self.names.sort()
         self.directory = filename
         self.parsers = []
         self.hosts = {}
-        self.groups = {}
+        self.groups = groups
 
         self._loader = loader
 

lib/ansible/inventory/ini.py

@@ -38,7 +38,7 @@ class InventoryParser(object):
     with their associated hosts and variable settings.
     """
 
-    def __init__(self, loader, filename=C.DEFAULT_HOST_LIST):
+    def __init__(self, loader, groups=dict(), filename=C.DEFAULT_HOST_LIST):
         self._loader = loader
         self.filename = filename
 
@@ -47,10 +47,7 @@ class InventoryParser(object):
 
         self.hosts = {}
         self.patterns = {}
-        self.groups = dict(
-            all = Group(name='all'),
-            ungrouped = Group(name='ungrouped')
-        )
+        self.groups = groups
 
         # Read in the hosts, groups, and variables defined in the
         # inventory file.
@@ -64,15 +61,6 @@ class InventoryParser(object):
 
         self._parse(data)
 
-        # Finally, add all top-level groups (including 'ungrouped') as
-        # children of 'all'.
-
-        for group in self.groups.values():
-            if group.depth == 0 and group.name != 'all':
-                self.groups['all'].add_child_group(group)
-
-        # Note: we could discard self.hosts after this point.
-
     def _raise_error(self, message):
         raise AnsibleError("%s:%d: " % (self.filename, self.lineno) + message)
 
@@ -186,6 +174,15 @@ class InventoryParser(object):
             elif decl['state'] == 'children':
                 raise AnsibleError("%s:%d: Section [%s:children] includes undefined group: %s" % (self.filename, decl['line'], decl['parent'], decl['name']))
 
+        # Finally, add all top-level groups as children of 'all'.
+        # We exclude ungrouped here because it was already added as a child of
+        # 'all' at the time it was created.
+
+        for group in self.groups.values():
+            if group.depth == 0 and group.name not in ('all', 'ungrouped'):
+                self.groups['all'].add_child_group(group)
+
+
     def _parse_group_name(self, line):
         '''
         Takes a single line and tries to parse it as a group name. Returns the

lib/ansible/inventory/script.py

@@ -36,9 +36,10 @@ from ansible.module_utils.basic import json_dict_bytes_to_unicode
 class InventoryScript:
     ''' Host inventory parser for ansible using external inventory scripts. '''
 
-    def __init__(self, loader, filename=C.DEFAULT_HOST_LIST):
+    def __init__(self, loader, groups=dict(), filename=C.DEFAULT_HOST_LIST):
 
         self._loader = loader
+        self.groups = groups
 
         # Support inventory scripts that are not prefixed with some
         # path information but happen to be in the current working
@@ -57,7 +58,7 @@ class InventoryScript:
         self.data = stdout
         # see comment about _meta below
         self.host_vars_from_top = None
-        self.groups = self._parse(stderr)
+        self._parse(stderr)
 
 
     def _parse(self, err):
@@ -77,11 +78,7 @@ class InventoryScript:
 
         self.raw  = json_dict_bytes_to_unicode(self.raw)
 
-        all       = Group('all')
-        groups    = dict(all=all)
-        group     = None
-
-
+        group = None
         for (group_name, data) in self.raw.items():
 
             # in Ansible 1.3 and later, a "_meta" subelement may contain
@@ -95,10 +92,10 @@ class InventoryScript:
                     self.host_vars_from_top = data['hostvars']
                     continue
 
-            if group_name != all.name:
-                group = groups[group_name] = Group(group_name)
-            else:
-                group = all
+            if group_name not in self.groups:
+                group = self.groups[group_name] = Group(group_name)
+
+            group = self.groups[group_name]
             host = None
 
             if not isinstance(data, dict):
@@ -124,10 +121,7 @@ class InventoryScript:
                         "data for variables:\n %s" % (group_name, data))
 
                 for k, v in iteritems(data['vars']):
-                    if group.name == all.name:
-                        all.set_variable(k, v)
-                    else:
-                        group.set_variable(k, v)
+                    group.set_variable(k, v)
 
         # Separate loop to ensure all groups are defined
         for (group_name, data) in self.raw.items():
@@ -135,14 +129,16 @@ class InventoryScript:
                 continue
             if isinstance(data, dict) and 'children' in data:
                 for child_name in data['children']:
-                    if child_name in groups:
-                        groups[group_name].add_child_group(groups[child_name])
+                    if child_name in self.groups:
+                        self.groups[group_name].add_child_group(self.groups[child_name])
 
-        for group in groups.values():
-            if group.depth == 0 and group.name != 'all':
-                all.add_child_group(group)
+        # Finally, add all top-level groups as children of 'all'.
+        # We exclude ungrouped here because it was already added as a child of
+        # 'all' at the time it was created.
 
-        return groups
+        for group in self.groups.values():
+            if group.depth == 0 and group.name not in ('all', 'ungrouped'):
+                self.groups['all'].add_child_group(group)
 
     def get_host_variables(self, host):
         """ Runs <script> --host <hostname> to determine additional host variables """