From 3d46cc2884cd842555ed5f762f87748cd5519927 Mon Sep 17 00:00:00 2001
From: s-hertel <shertel@redhat.com>
Date: Mon, 24 Jun 2019 12:23:38 -0400
Subject: [PATCH 1/7] Remove inter-object references from Host and Group
 objects and fix deserializing bug

---
 lib/ansible/inventory/data.py            | 294 +++++++++++++++++++++--
 lib/ansible/inventory/group.py           | 155 +-----------
 lib/ansible/inventory/host.py            |  50 +---
 lib/ansible/inventory/manager.py         |  11 +-
 lib/ansible/plugins/strategy/__init__.py |   3 +-
 lib/ansible/vars/manager.py              |   2 +-
 6 files changed, 292 insertions(+), 223 deletions(-)

diff --git a/lib/ansible/inventory/data.py b/lib/ansible/inventory/data.py
index c87d9385642b2e..83491aee001b5b 100644
--- a/lib/ansible/inventory/data.py
+++ b/lib/ansible/inventory/data.py
@@ -21,6 +21,8 @@
 
 import sys
 
+from itertools import chain
+
 from ansible import constants as C
 from ansible.errors import AnsibleError
 from ansible.inventory.group import Group
@@ -33,7 +35,235 @@
 display = Display()
 
 
-class InventoryData(object):
+class InventoryHostManager(object):
+    """
+    Manager for inventory host relatives
+    """
+    def host_populate_ancestors(self, host_name, additions=None):
+        """
+        :type host_name: str
+        :type additions: list[str]
+        """
+        host = self.hosts[host_name]
+        if additions is None:
+            for group_name in host.groups:
+                for ancestor_name in self.group_get_ancestors(group_name):
+                    if ancestor_name not in host.groups:
+                        host.groups.append(ancestor_name)
+        else:
+            for group_name in additions:
+                if group_name not in host.groups:
+                    host.groups.append(group_name)
+
+    def host_add_group(self, host_name, group_name):
+        """
+        :type host_name: str
+        :type group_name: str
+        """
+        self.host_populate_ancestors(host_name)
+        if group_name not in self.hosts[host_name].groups:
+            self.hosts[host_name].groups.append(group_name)
+
+    def host_remove_group(self, host_name, group_name):
+        """
+        :type host_name: str
+        :type group_name: str
+        """
+        # Remove references to a group and its ancestors from a host
+        host = self.hosts[host_name]
+
+        if group_name in host.groups:
+            host.groups.remove(group_name)
+
+        # Update ancestry
+        for old_group in self.group_get_ancestors(group_name):
+            if old_group.name != 'all':
+                for child_group_name in host.groups:
+                    if old_group in self.group_get_ancestors(child_group_name):
+                        break
+                    else:
+                        self.host_remove_group(host_name, old_group.name)
+
+
+class InventoryGroupManager(object):
+    """
+    Manager for inventory group relatives
+    """
+
+    def group_walk_relationship(self, group, rel, include_self=False, preserve_ordering=False):
+        '''
+        :type group: str
+        :type rel: str
+        :rtype: list[str] | set(str)
+
+        Given `rel` that is an iterable property of Group,
+        consitituting a directed acyclic graph among all groups,
+        Returns a set of all groups in full tree
+        A   B    C
+        |  / |  /
+        | /  | /
+        D -> E
+        |  /    vertical connections
+        | /     are directed upward
+        F
+        Called with group F, returns set of (A, B, C, D, E)
+        '''
+        seen = set([])
+        unprocessed = set(getattr(self.groups[group], rel))
+        if include_self:
+            unprocessed.add(group)
+        if preserve_ordering:
+            ordered = [group] if include_self else []
+            ordered.extend(getattr(self.groups[group], rel))
+
+        while unprocessed:
+            seen.update(unprocessed)
+            new_unprocessed = set([])
+
+            for new_item in chain.from_iterable(getattr(self.groups[g], rel) for g in unprocessed):
+                new_unprocessed.add(new_item)
+                if preserve_ordering:
+                    if new_item not in seen:
+                        ordered.append(new_item)
+
+            new_unprocessed.difference_update(seen)
+            unprocessed = new_unprocessed
+
+        if preserve_ordering:
+            return ordered
+        return seen
+
+    def group_remove_host(self, group_name, host_name):
+        """
+        :type group_name: str
+        :type host_name: str
+        """
+        group = self.groups[group_name]
+        if host_name in group.host_names:
+            group.hosts.remove(host_name)
+            group._hosts.remove(host_name)
+            self.group_clear_hosts_cache(group_name)
+
+    def group_add_host(self, group_name, host_name):
+        """
+        :type group_name: str
+        :type host_name: str
+        """
+        group = self.groups[group_name]
+        if host_name not in group.host_names:
+            group.hosts.append(host_name)
+            group._hosts.add(host_name)
+            self.group_clear_hosts_cache(group_name)
+
+    def group_get_ancestors(self, group_name):
+        """
+        :type group_name: str
+        :rtype: set(str)
+        """
+        return self.group_walk_relationship(group_name, 'parent_groups')
+
+    def group_get_descendants(self, group_name, **kwargs):
+        """
+        :type group_name: str
+        :rtype: list[str] | set(str)
+        """
+        return self.group_walk_relationship(group_name, 'child_groups', **kwargs)
+
+    def group_add_child_group(self, group, parent):
+        """
+        :type group: str
+        :type parent: str
+        """
+
+        if parent == group:
+            raise Exception("can't add group to itself")
+
+        # don't add if it's already there
+        if group not in self.groups[parent].child_groups:
+            # prepare list of group's new ancestors this edge creates
+            start_ancestors = self.group_get_ancestors(group)
+            new_ancestors = self.group_get_ancestors(parent)
+            if group in new_ancestors:
+                raise AnsibleError("Adding group '%s' as child to '%s' creates a recursive dependency loop." % (to_native(group), to_native(parent)))
+            new_ancestors.add(parent)
+            new_ancestors.difference_update(start_ancestors)
+            self.groups[parent].child_groups.append(group)
+
+            # update the depth of the child
+            self.groups[group].depth = max([self.groups[parent].depth + 1, self.groups[group].depth])
+
+            # update the depth of the grandchildren
+            self.group_check_children_depth(group)
+
+            # now add group to child's parent_groups list, but only if there
+            # isn't already a group with the same name
+            if parent not in self.groups[group].parent_groups:
+                self.groups[group].parent_groups.append(parent)
+                for h in self.group_get_hosts(group):
+                    self.host_populate_ancestors(h, additions=new_ancestors)
+
+            self.group_clear_hosts_cache(parent)
+
+    def group_check_children_depth(self, group_name):
+        """
+        :type group_name: str
+        """
+        group = self.groups[group_name]
+        depth = group.depth
+        start_depth = group.depth  # group.depth could change over loop
+        seen = set([])
+        unprocessed = set(group.child_groups)
+
+        while unprocessed:
+            seen.update(unprocessed)
+            depth += 1
+            to_process = unprocessed.copy()
+            unprocessed = set([])
+            for i in to_process:
+                g = self.groups[i]
+                if g.depth < depth:
+                    g.depth = depth
+                    unprocessed.update(g.child_groups)
+            if depth - start_depth > len(seen):
+                raise AnsibleError("The group named '%s' has a recursive dependency loop." % to_native(group.name))
+
+    def group_clear_hosts_cache(self, group_name):
+        """
+        :type group_name: str
+        """
+        self.groups[group_name]._hosts_cache = None
+        for ancestor_name in self.group_get_ancestors(group_name):
+            self.groups[ancestor_name]._hosts_cache = None
+
+    def group_get_hosts(self, group_name):
+        """
+        :type group_name: str
+        :rtype: list[str]
+        Returns the cache of hosts associated with the group and any of its descenants
+        """
+        if self.groups[group_name]._hosts_cache is None:
+            self.groups[group_name]._hosts_cache = self._group_get_hosts(group_name)
+        return self.groups[group_name]._hosts_cache
+
+    def _group_get_hosts(self, group_name):
+        """
+        :type group_name: str
+        :rtype: list[str]
+        Returns all of the hosts from the group's descendants
+        """
+        hosts = []
+        seen = set()
+        for child in self.group_get_descendants(group_name, include_self=True, preserve_ordering=True):
+            for host_name in self.groups[child].hosts:
+                if host_name not in seen:
+                    seen.add(host_name)
+                    if group_name == 'all' and self.hosts[host_name].implicit:
+                        continue
+                    hosts.append(host_name)
+        return hosts
+
+
+class InventoryData(InventoryHostManager, InventoryGroupManager):
     """
     Holds inventory data (host and group objects).
     Using it's methods should guarantee expected relationships and data.
@@ -58,6 +288,35 @@ def __init__(self):
             self.add_group(group)
         self.add_child('all', 'ungrouped')
 
+    def __eq__(self, inventory):
+        return not self.__ne__(inventory)
+
+    def __ne__(self, inventory):
+        if not isinstance(inventory, InventoryData):
+            return True
+
+        current_inventory = self.serialize()
+        comparison_inventory = inventory.serialize()
+
+        # Compare top level of inventory
+        if current_inventory['local'] != comparison_inventory['local']:
+            return True
+        if sorted(current_inventory['hosts'].keys()) != sorted(comparison_inventory['hosts'].keys()):
+            return True
+        if sorted(current_inventory['groups'].keys()) != sorted(comparison_inventory['groups'].keys()):
+            return True
+
+        # Compare inventory entity relationships and variables
+        for host in current_inventory['hosts']:
+            if current_inventory['hosts'][host].serialize() != comparison_inventory['hosts'][host].serialize():
+                return True
+        for group in current_inventory['groups']:
+            if current_inventory['groups'][group].serialize() != comparison_inventory['groups'][group].serialize():
+                return True
+
+        # Inventories are identical
+        return False
+
     def serialize(self):
         self._groups_dict_cache = None
         data = {
@@ -112,7 +371,7 @@ def reconcile_inventory(self):
             group_names.add(group.name)
 
             # ensure all groups inherit from 'all'
-            if group.name != 'all' and not group.get_ancestors():
+            if group.name != 'all' and not self.group_get_ancestors(group.name):
                 self.add_child('all', group.name)
 
         host_names = set()
@@ -125,7 +384,7 @@ def reconcile_inventory(self):
             if self.groups['ungrouped'] in mygroups:
                 # clear ungrouped of any incorrectly stored by parser
                 if set(mygroups).difference(set([self.groups['all'], self.groups['ungrouped']])):
-                    self.groups['ungrouped'].remove_host(host)
+                    self.group_remove_host('ungrouped', host.name)
 
             elif not host.implicit:
                 # add ungrouped hosts to ungrouped, except implicit
@@ -175,17 +434,17 @@ def add_group(self, group):
 
         return group
 
-    def remove_group(self, group):
+    def remove_group(self, group_name):
+        # Remove references to the group from any hosts first
+        # If the only group object is deleted before doing so the ancestors for the host could be inaccurate
+        for host_name in self.hosts:
+            self.host_remove_group(host_name, group_name)
 
-        if group in self.groups:
-            del self.groups[group]
-            display.debug("Removed group %s from inventory" % group)
+        if group_name in self.groups:
+            del self.groups[group_name]
+            display.debug("Removed group %s from inventory" % group_name)
             self._groups_dict_cache = {}
 
-        for host in self.hosts:
-            h = self.hosts[host]
-            h.remove_group(group)
-
     def add_host(self, host, group=None, port=None):
         ''' adds a host to inventory and possibly a group if not there already '''
 
@@ -223,7 +482,8 @@ def add_host(self, host, group=None, port=None):
                 h = self.hosts[host]
 
             if g:
-                g.add_host(h)
+                self.group_add_host(group, host)
+                self.host_add_group(h.name, g.name)
                 self._groups_dict_cache = {}
                 display.debug("Added host %s to group %s" % (host, group))
         else:
@@ -237,8 +497,7 @@ def remove_host(self, host):
             del self.hosts[host.name]
 
         for group in self.groups:
-            g = self.groups[group]
-            g.remove_host(host)
+            self.group_remove_host(group, host)
 
     def set_variable(self, entity, varname, value):
         ''' sets a variable for an inventory object '''
@@ -259,9 +518,10 @@ def add_child(self, group, child):
         if group in self.groups:
             g = self.groups[group]
             if child in self.groups:
-                g.add_child_group(self.groups[child])
+                self.group_add_child_group(child, group)
             elif child in self.hosts:
-                g.add_host(self.hosts[child])
+                self.group_add_host(group, child)
+                self.host_add_group(child, group)
             else:
                 raise AnsibleError("%s is not a known host nor group" % child)
             self._groups_dict_cache = {}
@@ -275,6 +535,6 @@ def get_groups_dict(self):
         """
         if not self._groups_dict_cache:
             for (group_name, group) in iteritems(self.groups):
-                self._groups_dict_cache[group_name] = [h.name for h in group.get_hosts()]
+                self._groups_dict_cache[group_name] = self.group_get_hosts(group_name)
 
         return self._groups_dict_cache
diff --git a/lib/ansible/inventory/group.py b/lib/ansible/inventory/group.py
index 6d5e19ef592b50..98c34c81b968d1 100644
--- a/lib/ansible/inventory/group.py
+++ b/lib/ansible/inventory/group.py
@@ -17,8 +17,6 @@
 from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
-from itertools import chain
-
 from ansible import constants as C
 from ansible.errors import AnsibleError
 from ansible.module_utils._text import to_native, to_text
@@ -89,18 +87,15 @@ def __setstate__(self, data):
         return self.deserialize(data)
 
     def serialize(self):
-        parent_groups = []
-        for parent in self.parent_groups:
-            parent_groups.append(parent.serialize())
-
         self._hosts = None
 
         result = dict(
             name=self.name,
             vars=self.vars.copy(),
-            parent_groups=parent_groups,
+            parent_groups=self.parent_groups,
             depth=self.depth,
             hosts=self.hosts,
+            child_groups=self.child_groups,
         )
 
         return result
@@ -112,57 +107,8 @@ def deserialize(self, data):
         self.depth = data.get('depth', 0)
         self.hosts = data.get('hosts', [])
         self._hosts = None
-
-        parent_groups = data.get('parent_groups', [])
-        for parent_data in parent_groups:
-            g = Group()
-            g.deserialize(parent_data)
-            self.parent_groups.append(g)
-
-    def _walk_relationship(self, rel, include_self=False, preserve_ordering=False):
-        '''
-        Given `rel` that is an iterable property of Group,
-        consitituting a directed acyclic graph among all groups,
-        Returns a set of all groups in full tree
-        A   B    C
-        |  / |  /
-        | /  | /
-        D -> E
-        |  /    vertical connections
-        | /     are directed upward
-        F
-        Called on F, returns set of (A, B, C, D, E)
-        '''
-        seen = set([])
-        unprocessed = set(getattr(self, rel))
-        if include_self:
-            unprocessed.add(self)
-        if preserve_ordering:
-            ordered = [self] if include_self else []
-            ordered.extend(getattr(self, rel))
-
-        while unprocessed:
-            seen.update(unprocessed)
-            new_unprocessed = set([])
-
-            for new_item in chain.from_iterable(getattr(g, rel) for g in unprocessed):
-                new_unprocessed.add(new_item)
-                if preserve_ordering:
-                    if new_item not in seen:
-                        ordered.append(new_item)
-
-            new_unprocessed.difference_update(seen)
-            unprocessed = new_unprocessed
-
-        if preserve_ordering:
-            return ordered
-        return seen
-
-    def get_ancestors(self):
-        return self._walk_relationship('parent_groups')
-
-    def get_descendants(self, **kwargs):
-        return self._walk_relationship('child_groups', **kwargs)
+        self.child_groups = data.get('child_groups', [])
+        self.parent_groups = data.get('parent_groups', [])
 
     @property
     def host_names(self):
@@ -173,73 +119,6 @@ def host_names(self):
     def get_name(self):
         return self.name
 
-    def add_child_group(self, group):
-
-        if self == group:
-            raise Exception("can't add group to itself")
-
-        # don't add if it's already there
-        if group not in self.child_groups:
-
-            # prepare list of group's new ancestors this edge creates
-            start_ancestors = group.get_ancestors()
-            new_ancestors = self.get_ancestors()
-            if group in new_ancestors:
-                raise AnsibleError("Adding group '%s' as child to '%s' creates a recursive dependency loop." % (to_native(group.name), to_native(self.name)))
-            new_ancestors.add(self)
-            new_ancestors.difference_update(start_ancestors)
-
-            self.child_groups.append(group)
-
-            # update the depth of the child
-            group.depth = max([self.depth + 1, group.depth])
-
-            # update the depth of the grandchildren
-            group._check_children_depth()
-
-            # now add self to child's parent_groups list, but only if there
-            # isn't already a group with the same name
-            if self.name not in [g.name for g in group.parent_groups]:
-                group.parent_groups.append(self)
-                for h in group.get_hosts():
-                    h.populate_ancestors(additions=new_ancestors)
-
-            self.clear_hosts_cache()
-
-    def _check_children_depth(self):
-
-        depth = self.depth
-        start_depth = self.depth  # self.depth could change over loop
-        seen = set([])
-        unprocessed = set(self.child_groups)
-
-        while unprocessed:
-            seen.update(unprocessed)
-            depth += 1
-            to_process = unprocessed.copy()
-            unprocessed = set([])
-            for g in to_process:
-                if g.depth < depth:
-                    g.depth = depth
-                    unprocessed.update(g.child_groups)
-            if depth - start_depth > len(seen):
-                raise AnsibleError("The group named '%s' has a recursive dependency loop." % to_native(self.name))
-
-    def add_host(self, host):
-        if host.name not in self.host_names:
-            self.hosts.append(host)
-            self._hosts.add(host.name)
-            host.add_group(self)
-            self.clear_hosts_cache()
-
-    def remove_host(self, host):
-
-        if host.name in self.host_names:
-            self.hosts.remove(host)
-            self._hosts.remove(host.name)
-            host.remove_group(self)
-            self.clear_hosts_cache()
-
     def set_variable(self, key, value):
 
         if key == 'ansible_group_priority':
@@ -247,32 +126,6 @@ def set_variable(self, key, value):
         else:
             self.vars[key] = value
 
-    def clear_hosts_cache(self):
-
-        self._hosts_cache = None
-        for g in self.get_ancestors():
-            g._hosts_cache = None
-
-    def get_hosts(self):
-
-        if self._hosts_cache is None:
-            self._hosts_cache = self._get_hosts()
-        return self._hosts_cache
-
-    def _get_hosts(self):
-
-        hosts = []
-        seen = {}
-        for kid in self.get_descendants(include_self=True, preserve_ordering=True):
-            kid_hosts = kid.hosts
-            for kk in kid_hosts:
-                if kk not in seen:
-                    seen[kk] = 1
-                    if self.name == 'all' and kk.implicit:
-                        continue
-                    hosts.append(kk)
-        return hosts
-
     def get_vars(self):
         return self.vars.copy()
 
diff --git a/lib/ansible/inventory/host.py b/lib/ansible/inventory/host.py
index 432727394541dd..c0b4e33d0d09eb 100644
--- a/lib/ansible/inventory/host.py
+++ b/lib/ansible/inventory/host.py
@@ -54,16 +54,12 @@ def __repr__(self):
         return self.get_name()
 
     def serialize(self):
-        groups = []
-        for group in self.groups:
-            groups.append(group.serialize())
-
         return dict(
             name=self.name,
             vars=self.vars.copy(),
             address=self.address,
             uuid=self._uuid,
-            groups=groups,
+            groups=self.groups,
             implicit=self.implicit,
         )
 
@@ -75,12 +71,7 @@ def deserialize(self, data):
         self.address = data.get('address', '')
         self._uuid = data.get('uuid', None)
         self.implicit = data.get('implicit', False)
-
-        groups = data.get('groups', [])
-        for group_data in groups:
-            g = Group()
-            g.deserialize(group_data)
-            self.groups.append(g)
+        self.groups = data.get('groups', [])
 
     def __init__(self, name=None, port=None, gen_uuid=True):
 
@@ -101,41 +92,6 @@ def __init__(self, name=None, port=None, gen_uuid=True):
     def get_name(self):
         return self.name
 
-    def populate_ancestors(self, additions=None):
-        # populate ancestors
-        if additions is None:
-            for group in self.groups:
-                self.add_group(group)
-        else:
-            for group in additions:
-                if group not in self.groups:
-                    self.groups.append(group)
-
-    def add_group(self, group):
-
-        # populate ancestors first
-        for oldg in group.get_ancestors():
-            if oldg not in self.groups:
-                self.groups.append(oldg)
-
-        # actually add group
-        if group not in self.groups:
-            self.groups.append(group)
-
-    def remove_group(self, group):
-
-        if group in self.groups:
-            self.groups.remove(group)
-
-            # remove exclusive ancestors, xcept all!
-            for oldg in group.get_ancestors():
-                if oldg.name != 'all':
-                    for childg in self.groups:
-                        if oldg in childg.get_ancestors():
-                            break
-                    else:
-                        self.remove_group(oldg)
-
     def set_variable(self, key, value):
         self.vars[key] = value
 
@@ -146,7 +102,7 @@ def get_magic_vars(self):
         results = {}
         results['inventory_hostname'] = self.name
         results['inventory_hostname_short'] = self.name.split('.')[0]
-        results['group_names'] = sorted([g.name for g in self.get_groups() if g.name != 'all'])
+        results['group_names'] = sorted([g for g in self.get_groups() if g != 'all'])
 
         return results
 
diff --git a/lib/ansible/inventory/manager.py b/lib/ansible/inventory/manager.py
index e10f56876e5e5b..5503f027bb6347 100644
--- a/lib/ansible/inventory/manager.py
+++ b/lib/ansible/inventory/manager.py
@@ -333,7 +333,7 @@ def _match_list(self, items, pattern_str):
     def get_hosts(self, pattern="all", ignore_limits=False, ignore_restrictions=False, order=None):
         """
         Takes a pattern or list of patterns and returns a list of matching
-        inventory host names, taking into account any active restrictions
+        inventory host objects, taking into account any active restrictions
         or applied subsets
         """
 
@@ -385,7 +385,7 @@ def get_hosts(self, pattern="all", ignore_limits=False, ignore_restrictions=Fals
 
     def _evaluate_patterns(self, patterns):
         """
-        Takes a list of patterns and returns a list of matching host names,
+        Takes a list of patterns and returns a list of matching host objects,
         taking into account any negative and intersection patterns.
         """
 
@@ -408,7 +408,7 @@ def _evaluate_patterns(self, patterns):
 
     def _match_one_pattern(self, pattern):
         """
-        Takes a single pattern and returns a list of matching host names.
+        Takes a single pattern and returns a list of matching host objects.
         Ignores intersection (&) and exclusion (!) specifiers.
 
         The pattern may be:
@@ -523,7 +523,7 @@ def _apply_subscript(self, hosts, subscript):
 
     def _enumerate_matches(self, pattern):
         """
-        Returns a list of host names matching the given pattern according to the
+        Returns a list of host objects matching the given pattern according to the
         rules explained above in _match_one_pattern.
         """
 
@@ -532,7 +532,8 @@ def _enumerate_matches(self, pattern):
         matching_groups = self._match_list(self._inventory.groups, pattern)
         if matching_groups:
             for groupname in matching_groups:
-                results.extend(self._inventory.groups[groupname].get_hosts())
+                group_hosts = [self._inventory.hosts[h] for h in self._inventory.group_get_hosts(groupname)]
+                results.extend(group_hosts)
 
         # check hosts if no groups matched or it is a regex/glob pattern
         if not matching_groups or pattern.startswith('~') or any(special in pattern for special in ('.', '?', '*', '[')):
diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py
index 6a38edc2828b3a..170bb6267bd167 100644
--- a/lib/ansible/plugins/strategy/__init__.py
+++ b/lib/ansible/plugins/strategy/__init__.py
@@ -738,8 +738,7 @@ def _add_group(self, host, result_item):
                 changed = True
         group = self._inventory.groups[group_name]
         for parent_group_name in parent_group_names:
-            parent_group = self._inventory.groups[parent_group_name]
-            parent_group.add_child_group(group)
+            self._inventory.group_add_child_group(group_name, parent_group_name)
 
         if real_host.name not in group.get_hosts():
             group.add_host(real_host)
diff --git a/lib/ansible/vars/manager.py b/lib/ansible/vars/manager.py
index 9a2b6f4845adbb..a714c1106b45ae 100644
--- a/lib/ansible/vars/manager.py
+++ b/lib/ansible/vars/manager.py
@@ -202,7 +202,7 @@ def get_vars(self, play=None, host=None, task=None, include_hostvars=True, inclu
         if host:
             # THE 'all' group and the rest of groups for a host, used below
             all_group = self._inventory.groups.get('all')
-            host_groups = sort_groups([g for g in host.get_groups() if g.name not in ['all']])
+            host_groups = sort_groups([self._inventory.groups.get(g) for g in host.groups if g not in ['all']])
 
             def _get_plugin_vars(plugin, path, entities):
                 data = {}

From 439fb779475c36534937d9cb7703cf85fc425b76 Mon Sep 17 00:00:00 2001
From: s-hertel <shertel@redhat.com>
Date: Mon, 24 Jun 2019 12:24:02 -0400
Subject: [PATCH 2/7] Update ansible-inventory to not rely on inter-object
 references

---
 lib/ansible/cli/inventory.py | 69 ++++++++++++++++++------------------
 1 file changed, 35 insertions(+), 34 deletions(-)

diff --git a/lib/ansible/cli/inventory.py b/lib/ansible/cli/inventory.py
index 81540db82e7251..df983f2c544b71 100644
--- a/lib/ansible/cli/inventory.py
+++ b/lib/ansible/cli/inventory.py
@@ -241,8 +241,10 @@ def _get_host_variables(self, host):
         return self._remove_internal(hostvars)
 
     def _get_group(self, gname):
-        group = self.inventory.groups.get(gname)
-        return group
+        return self.inventory.groups.get(gname)
+
+    def _get_host(self, hname):
+        return self.inventory.hosts.get(hname)
 
     @staticmethod
     def _remove_internal(dump):
@@ -278,13 +280,13 @@ def _graph_group(self, group, depth=0):
 
         result = [self._graph_name('@%s:' % group.name, depth)]
         depth = depth + 1
-        for kid in sorted(group.child_groups, key=attrgetter('name')):
-            result.extend(self._graph_group(kid, depth))
+        for kid in sorted(group.child_groups):
+            result.extend(self._graph_group(self._get_group(kid), depth))
 
         if group.name != 'all':
-            for host in sorted(group.hosts, key=attrgetter('name')):
-                result.append(self._graph_name(host.name, depth))
-                result.extend(self._show_vars(self._get_host_variables(host), depth + 1))
+            for host in sorted(group.hosts):
+                result.append(self._graph_name(host, depth))
+                result.extend(self._show_vars(self._get_host_variables(self._get_host(host)), depth + 1))
 
         result.extend(self._show_vars(self._get_group_variables(group), depth))
 
@@ -306,13 +308,13 @@ def format_group(group):
             results = {}
             results[group.name] = {}
             if group.name != 'all':
-                results[group.name]['hosts'] = [h.name for h in sorted(group.hosts, key=attrgetter('name'))]
+                results[group.name]['hosts'] = sorted(group.hosts)
             results[group.name]['children'] = []
-            for subgroup in sorted(group.child_groups, key=attrgetter('name')):
-                results[group.name]['children'].append(subgroup.name)
-                if subgroup.name not in seen:
-                    results.update(format_group(subgroup))
-                    seen.add(subgroup.name)
+            for subgroup in sorted(group.child_groups):
+                results[group.name]['children'].append(subgroup)
+                if subgroup not in seen:
+                    results.update(format_group(self._get_group(subgroup)))
+                    seen.add(subgroup)
             if context.CLIARGS['export']:
                 results[group.name]['vars'] = self._get_group_variables(group)
 
@@ -326,8 +328,7 @@ def format_group(group):
 
         # populate meta
         results['_meta'] = {'hostvars': {}}
-        hosts = self.inventory.get_hosts()
-        for host in hosts:
+        for name, host in self.inventory.hosts.items():
             hvars = self._get_host_variables(host)
             if hvars:
                 results['_meta']['hostvars'][host.name] = hvars
@@ -346,19 +347,19 @@ def format_group(group):
 
             # subgroups
             results[group.name]['children'] = {}
-            for subgroup in sorted(group.child_groups, key=attrgetter('name')):
-                if subgroup.name != 'all':
-                    results[group.name]['children'].update(format_group(subgroup))
+            for subgroup in sorted(group.child_groups):
+                if subgroup != 'all':
+                    results[group.name]['children'].update(format_group(self._get_group(subgroup)))
 
             # hosts for group
             results[group.name]['hosts'] = {}
             if group.name != 'all':
-                for h in sorted(group.hosts, key=attrgetter('name')):
+                for h in sorted(group.hosts):
                     myvars = {}
-                    if h.name not in seen:  # avoid defining host vars more than once
-                        seen.append(h.name)
-                        myvars = self._get_host_variables(host=h)
-                    results[group.name]['hosts'][h.name] = myvars
+                    if h not in seen: # avoid defining host vars more than once
+                        seen.append(h)
+                        myvars = self._get_host_variables(host=self._get_host(h))
+                    results[group.name]['hosts'][h] = myvars
 
             if context.CLIARGS['export']:
                 gvars = self._get_group_variables(group)
@@ -373,31 +374,31 @@ def format_group(group):
 
     def toml_inventory(self, top):
         seen = set()
-        has_ungrouped = bool(next(g.hosts for g in top.child_groups if g.name == 'ungrouped'))
+        has_ungrouped = bool(next(self._get_group(g).hosts for g in top.child_groups if g == 'ungrouped'))
 
         def format_group(group):
             results = {}
             results[group.name] = {}
 
             results[group.name]['children'] = []
-            for subgroup in sorted(group.child_groups, key=attrgetter('name')):
-                if subgroup.name == 'ungrouped' and not has_ungrouped:
+            for subgroup in sorted(group.child_groups):
+                if subgroup == 'ungrouped' and not has_ungrouped:
                     continue
                 if group.name != 'all':
-                    results[group.name]['children'].append(subgroup.name)
-                results.update(format_group(subgroup))
+                    results[group.name]['children'].append(subgroup)
+                results.update(format_group(self._get_group(subgroup)))
 
             if group.name != 'all':
-                for host in sorted(group.hosts, key=attrgetter('name')):
-                    if host.name not in seen:
-                        seen.add(host.name)
-                        host_vars = self._get_host_variables(host=host)
+                for host in sorted(group.hosts):
+                    if host not in seen:
+                        seen.add(host)
+                        host_vars = self._get_host_variables(host=self._get_host(host))
                     else:
                         host_vars = {}
                     try:
-                        results[group.name]['hosts'][host.name] = host_vars
+                        results[group.name]['hosts'][host] = host_vars
                     except KeyError:
-                        results[group.name]['hosts'] = {host.name: host_vars}
+                        results[group.name]['hosts'] = {host: host_vars}
 
             if context.CLIARGS['export']:
                 results[group.name]['vars'] = self._get_group_variables(group)

From d1beb865fa98f2c1f724139f6b3ea0c4546fe499 Mon Sep 17 00:00:00 2001
From: s-hertel <shertel@redhat.com>
Date: Mon, 24 Jun 2019 12:27:50 -0400
Subject: [PATCH 3/7] Restore inventory to last good state if a source is
 partially parsed and fails

---
 lib/ansible/inventory/manager.py | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/lib/ansible/inventory/manager.py b/lib/ansible/inventory/manager.py
index 5503f027bb6347..2ca7457055aa26 100644
--- a/lib/ansible/inventory/manager.py
+++ b/lib/ansible/inventory/manager.py
@@ -26,6 +26,7 @@
 import itertools
 import traceback
 
+from copy import deepcopy
 from operator import attrgetter
 from random import shuffle
 
@@ -243,6 +244,8 @@ def parse_source(self, source, cache=False):
         else:
             # left with strings or files, let plugins figure it out
 
+            inventory_copy = None
+
             # set so new hosts can use for inventory_file/dir vasr
             self._inventory.current_source = source
 
@@ -260,6 +263,10 @@ def parse_source(self, source, cache=False):
                     plugin_wants = False
 
                 if plugin_wants:
+                    # No need to deepcopy inventory unless it has changed
+                    if inventory_copy == None:
+                        inventory_copy = deepcopy(self._inventory)
+
                     try:
                         # FIXME in case plugin fails 1/2 way we have partial inventory
                         plugin.parse(self._inventory, self._loader, source, cache=cache)
@@ -269,6 +276,7 @@ def parse_source(self, source, cache=False):
                             # some plugins might not implement caching
                             pass
                         parsed = True
+                        inventory_copy = None
                         display.vvv('Parsed %s inventory source with %s plugin' % (source, plugin_name))
                         break
                     except AnsibleParserError as e:
@@ -279,6 +287,11 @@ def parse_source(self, source, cache=False):
                         display.debug('%s failed while attempting to parse %s' % (plugin_name, source))
                         tb = ''.join(traceback.format_tb(sys.exc_info()[2]))
                         failures.append({'src': source, 'plugin': plugin_name, 'exc': AnsibleError(e), 'tb': tb})
+                    finally:
+                        if not parsed and self._inventory != inventory_copy:
+                            display.warning(u'\n restoring inventory because %s was partially parsed by %s' % (source, plugin_name))
+                            self._inventory = inventory_copy
+                            inventory_copy = None
                 else:
                     display.vvv("%s declined parsing %s as it did not pass its verify_file() method" % (plugin_name, source))
             else:

From 41a2edfb145f9c6b768fb19fc0ea4721a4b61ba0 Mon Sep 17 00:00:00 2001
From: s-hertel <shertel@redhat.com>
Date: Mon, 24 Jun 2019 12:28:21 -0400
Subject: [PATCH 4/7] Add integration tests

---
 .../targets/inventory_data/aliases            |  1 +
 .../targets/inventory_data/groups_of_groups/a | 13 ++++
 .../targets/inventory_data/groups_of_groups/b |  8 +++
 .../one_valid_and_invalid_source/a_parses     | 18 +++++
 .../b_fails_before_partial_parsing            |  2 +
 .../a_parses_first_and_fails                  |  7 ++
 .../b_parses_second_and_succeeds              | 18 +++++
 .../a_succeeds                                | 15 ++++
 .../b_adds_var_then_fails                     |  8 +++
 .../targets/inventory_data/runme.sh           | 68 +++++++++++++++++++
 .../source_with_duplication/a_parses          | 10 +++
 .../b_parses_with_duplicates                  | 10 +++
 .../inventory_data/test_groups_of_groups.yml  | 27 ++++++++
 13 files changed, 205 insertions(+)
 create mode 100644 test/integration/targets/inventory_data/aliases
 create mode 100644 test/integration/targets/inventory_data/groups_of_groups/a
 create mode 100644 test/integration/targets/inventory_data/groups_of_groups/b
 create mode 100644 test/integration/targets/inventory_data/one_valid_and_invalid_source/a_parses
 create mode 100644 test/integration/targets/inventory_data/one_valid_and_invalid_source/b_fails_before_partial_parsing
 create mode 100644 test/integration/targets/inventory_data/partially_bad_source/a_parses_first_and_fails
 create mode 100644 test/integration/targets/inventory_data/partially_bad_source/b_parses_second_and_succeeds
 create mode 100644 test/integration/targets/inventory_data/partially_bad_source_test_remove_var/a_succeeds
 create mode 100644 test/integration/targets/inventory_data/partially_bad_source_test_remove_var/b_adds_var_then_fails
 create mode 100755 test/integration/targets/inventory_data/runme.sh
 create mode 100644 test/integration/targets/inventory_data/source_with_duplication/a_parses
 create mode 100644 test/integration/targets/inventory_data/source_with_duplication/b_parses_with_duplicates
 create mode 100644 test/integration/targets/inventory_data/test_groups_of_groups.yml

diff --git a/test/integration/targets/inventory_data/aliases b/test/integration/targets/inventory_data/aliases
new file mode 100644
index 00000000000000..765b70da7963c6
--- /dev/null
+++ b/test/integration/targets/inventory_data/aliases
@@ -0,0 +1 @@
+shippable/posix/group2
diff --git a/test/integration/targets/inventory_data/groups_of_groups/a b/test/integration/targets/inventory_data/groups_of_groups/a
new file mode 100644
index 00000000000000..e864972c9ab9b7
--- /dev/null
+++ b/test/integration/targets/inventory_data/groups_of_groups/a
@@ -0,0 +1,13 @@
+[east1]
+node01
+node02
+node02
+
+[east2]
+node03
+node04
+node05
+
+[all_east:children]
+east1
+east2
diff --git a/test/integration/targets/inventory_data/groups_of_groups/b b/test/integration/targets/inventory_data/groups_of_groups/b
new file mode 100644
index 00000000000000..9705fffc544c7f
--- /dev/null
+++ b/test/integration/targets/inventory_data/groups_of_groups/b
@@ -0,0 +1,8 @@
+[testgroupinbadsource]
+testhostinbadsource
+
+[testhostinbadsource:vars]
+testvar=testvalue
+
+[badgroup:vars]
+exists=False
diff --git a/test/integration/targets/inventory_data/one_valid_and_invalid_source/a_parses b/test/integration/targets/inventory_data/one_valid_and_invalid_source/a_parses
new file mode 100644
index 00000000000000..a2482f3f1bd5c7
--- /dev/null
+++ b/test/integration/targets/inventory_data/one_valid_and_invalid_source/a_parses
@@ -0,0 +1,18 @@
+[all:vars]
+foo=bar
+
+[group_a:children]
+masters
+nodes
+
+[masters]
+master1
+master2
+
+[nodes]
+node1 tags="{'Name':'a'}"
+node2 tags="{'Name':'b'}"
+node3 tags="{'Name':'c'}"
+
+[masters:vars]
+master_only=True
diff --git a/test/integration/targets/inventory_data/one_valid_and_invalid_source/b_fails_before_partial_parsing b/test/integration/targets/inventory_data/one_valid_and_invalid_source/b_fails_before_partial_parsing
new file mode 100644
index 00000000000000..63f9ed3ca31661
--- /dev/null
+++ b/test/integration/targets/inventory_data/one_valid_and_invalid_source/b_fails_before_partial_parsing
@@ -0,0 +1,2 @@
+[undefinedgroup:invalidtype]
+nope=True
diff --git a/test/integration/targets/inventory_data/partially_bad_source/a_parses_first_and_fails b/test/integration/targets/inventory_data/partially_bad_source/a_parses_first_and_fails
new file mode 100644
index 00000000000000..20e89ad022e3de
--- /dev/null
+++ b/test/integration/targets/inventory_data/partially_bad_source/a_parses_first_and_fails
@@ -0,0 +1,7 @@
+[all:vars]
+ansible_become=yes
+ansible_ssh_user=testuser
+scp_if_ssh=True
+
+[group_a:vars]
+containerized=True
diff --git a/test/integration/targets/inventory_data/partially_bad_source/b_parses_second_and_succeeds b/test/integration/targets/inventory_data/partially_bad_source/b_parses_second_and_succeeds
new file mode 100644
index 00000000000000..a2482f3f1bd5c7
--- /dev/null
+++ b/test/integration/targets/inventory_data/partially_bad_source/b_parses_second_and_succeeds
@@ -0,0 +1,18 @@
+[all:vars]
+foo=bar
+
+[group_a:children]
+masters
+nodes
+
+[masters]
+master1
+master2
+
+[nodes]
+node1 tags="{'Name':'a'}"
+node2 tags="{'Name':'b'}"
+node3 tags="{'Name':'c'}"
+
+[masters:vars]
+master_only=True
diff --git a/test/integration/targets/inventory_data/partially_bad_source_test_remove_var/a_succeeds b/test/integration/targets/inventory_data/partially_bad_source_test_remove_var/a_succeeds
new file mode 100644
index 00000000000000..46d0844fa35286
--- /dev/null
+++ b/test/integration/targets/inventory_data/partially_bad_source_test_remove_var/a_succeeds
@@ -0,0 +1,15 @@
+[testgroup:children]
+masters
+nodes
+
+[masters]
+master1
+master2
+
+[nodes]
+node1 tags="{'Name':'a'}"
+node2 tags="{'Name':'b'}"
+node3 tags="{'Name':'c'}"
+
+[masters:vars]
+master_only=True
diff --git a/test/integration/targets/inventory_data/partially_bad_source_test_remove_var/b_adds_var_then_fails b/test/integration/targets/inventory_data/partially_bad_source_test_remove_var/b_adds_var_then_fails
new file mode 100644
index 00000000000000..8f0ff39a915dd7
--- /dev/null
+++ b/test/integration/targets/inventory_data/partially_bad_source_test_remove_var/b_adds_var_then_fails
@@ -0,0 +1,8 @@
+[node1:vars]
+host_var=True
+
+[testgroup:vars]
+group_var=True
+
+[undefinedgroup:vars]
+fail=True
diff --git a/test/integration/targets/inventory_data/runme.sh b/test/integration/targets/inventory_data/runme.sh
new file mode 100755
index 00000000000000..7a469a174ccb0c
--- /dev/null
+++ b/test/integration/targets/inventory_data/runme.sh
@@ -0,0 +1,68 @@
+#!/usr/bin/env bash
+
+set -ux
+set +e
+
+# Check that one source fails, as expected
+ANSIBLE_INVENTORY_ANY_UNPARSED_IS_FAILED=yes ansible-inventory -i partially_bad_source --list
+if [[ $? -ne 1 ]]; then
+    exit 1
+fi
+
+set -e
+
+# Check that a warning is given when inventory is restored from an incomplete state
+ANSIBLE_INVENTORY_UNPARSED_FAILED=yes ANSIBLE_INVENTORY_ENABLED=ini ansible-inventory -i partially_bad_source --list 2>&1 | tee out.txt
+if [[ "$(grep -c out.txt -e 'restoring inventory')" -ne 1 ]] ; then
+    echo "Did not get warning when an inventory source was partially parsed"
+    exit 1
+fi
+
+# Check that a warning is only given when inventory incorrectly changes, not just when a source fails to be parsed
+ANSIBLE_INVENTORY_UNPARSED_FAILED=yes ansible-inventory -i one_valid_and_invalid_source/ --list 2>&1 | tee out.txt
+if [[ "$(grep -c out.txt -e 'restoring inventory')" -ne 0 ]] ; then
+    echo "Got a warning for an inventory source that should not have partially modified inventory"
+    exit 1
+fi
+
+# Check that vars added to hosts and groups are removed if the source does not completely parse
+ANSIBLE_INVENTORY_UNPARED_FAILED=yes ansible-inventory -i partially_bad_source_test_remove_var/ --list | tee out.txt
+if [[ "$(grep -c out.txt -e 'group_var')" -ne 0 ]]
+   [[ "$(grep -c out.txt -e 'host_var')" -ne 0 ]] ; then
+    echo "Found a residual variable from a failed inventory source"
+    exit 1
+elif [[ "$(grep -c out.txt -e 'master_only')" -ne 2 ]] ||
+     [[ "$(grep -c out.txt -e 'tags')" -ne 3 ]] ; then
+    echo "Expected variables from successful source are missing"
+    exit 1
+fi
+
+# Check that inventory only contains data from successful sources
+ANSIBLE_INVENTORY_UNPARSED_FAILED=yes ansible-inventory -i partially_bad_source --list | tee out.txt
+
+if [[ "$(grep -c out.txt -e 'ansible_become')" -ne 0 ]] ||
+   [[ "$(grep -c out.txt -e 'ansible_ssh_user')" -ne 0 ]] ||
+   [[ "$(grep -c out.txt -e 'scp_if_ssh')" -ne 0 ]] ||
+   [[ "$(grep -c out.txt -e 'containerized')" -ne 0 ]] ; then
+    echo "Found a hostvar from a failed inventory source"
+    exit 1
+elif [[ "$(grep -c out.txt -e 'foo')" -ne 5 ]] ||
+     [[ "$(grep -c out.txt -e 'master_only')" -ne 2 ]] ||
+     [[ "$(grep -c out.txt -e 'tags')" -ne 3 ]] ; then
+    echo "Did not find expected hostvars from successful inventory source"
+    exit 1
+fi
+
+# Test that the restored inventory after partially parsing a bad source matches using only the good source
+ANSIBLE_INVENTORY_ANY_UNPARSED_FAILED=yes ansible-playbook -i groups_of_groups/a test_groups_of_groups.yml
+ANSIBLE_INVENTORY_UNPARSED_FAILED=yes ansible-playbook -i groups_of_groups test_groups_of_groups.yml
+
+# Check that displayed inventory does not duplicate hosts per group
+ANSIBLE_INVENTORY_ANY_UNPARSED_IS_FAILED=yes ansible-inventory -i source_with_duplication --graph | tee out.txt
+if [[ "$(grep -c out.txt -e 'master0')" -ne 1 ]] ||
+   [[ "$(grep -c out.txt -e 'node0')" -ne 1 ]] ||
+   [[ "$(grep -c out.txt -e 'node1')" -ne 1 ]] ||
+   [[ "$(grep -c out.txt -e 'node2')" -ne 1 ]] ; then
+    echo "Did not find expected unique hosts per group"
+    exit 1
+fi
diff --git a/test/integration/targets/inventory_data/source_with_duplication/a_parses b/test/integration/targets/inventory_data/source_with_duplication/a_parses
new file mode 100644
index 00000000000000..5cc49e99f5603f
--- /dev/null
+++ b/test/integration/targets/inventory_data/source_with_duplication/a_parses
@@ -0,0 +1,10 @@
+[masters]
+master0
+
+[nodes]
+node0
+node1
+node2
+
+[nodes:vars]
+foo=bar
diff --git a/test/integration/targets/inventory_data/source_with_duplication/b_parses_with_duplicates b/test/integration/targets/inventory_data/source_with_duplication/b_parses_with_duplicates
new file mode 100644
index 00000000000000..5cc49e99f5603f
--- /dev/null
+++ b/test/integration/targets/inventory_data/source_with_duplication/b_parses_with_duplicates
@@ -0,0 +1,10 @@
+[masters]
+master0
+
+[nodes]
+node0
+node1
+node2
+
+[nodes:vars]
+foo=bar
diff --git a/test/integration/targets/inventory_data/test_groups_of_groups.yml b/test/integration/targets/inventory_data/test_groups_of_groups.yml
new file mode 100644
index 00000000000000..56f075366b3ee2
--- /dev/null
+++ b/test/integration/targets/inventory_data/test_groups_of_groups.yml
@@ -0,0 +1,27 @@
+---
+- hosts: localhost
+  connection: local
+  gather_facts: no
+  tasks:
+  - debug: var=groups
+
+  - name: assert that group and hosts from the partially failed source are not in inventory
+    assert:
+      that:
+        - "groups.testgroupinbadsource is undefined"
+        - "hostvars.testhostinbadsource is undefined"
+        - "groups.all.testhostinbadsource is undefined"
+
+  - name: assert the groups and hosts from the viable source exist
+    assert:
+      that:
+        - "groups.all == groups.all_east"
+        - "groups.all == ['node01', 'node02', 'node03', 'node04', 'node05']"
+        - "groups.east1 == ['node01', 'node02']"
+        - "groups.east2 == ['node03', 'node04', 'node05']"
+        - "groups.ungrouped == []"
+
+  - assert:
+      that:
+        - "hostvars.{{ item }} is defined"
+    loop: "{{ groups.all }}"

From e678db251090e9c3c4e8697945e31a73a5c78165 Mon Sep 17 00:00:00 2001
From: s-hertel <shertel@redhat.com>
Date: Mon, 24 Jun 2019 13:16:06 -0400
Subject: [PATCH 5/7] fix sanity

---
 lib/ansible/cli/inventory.py     | 2 +-
 lib/ansible/inventory/data.py    | 1 +
 lib/ansible/inventory/group.py   | 2 +-
 lib/ansible/inventory/manager.py | 2 +-
 4 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lib/ansible/cli/inventory.py b/lib/ansible/cli/inventory.py
index df983f2c544b71..a083931b37876e 100644
--- a/lib/ansible/cli/inventory.py
+++ b/lib/ansible/cli/inventory.py
@@ -356,7 +356,7 @@ def format_group(group):
             if group.name != 'all':
                 for h in sorted(group.hosts):
                     myvars = {}
-                    if h not in seen: # avoid defining host vars more than once
+                    if h not in seen:  # avoid defining host vars more than once
                         seen.append(h)
                         myvars = self._get_host_variables(host=self._get_host(h))
                     results[group.name]['hosts'][h] = myvars
diff --git a/lib/ansible/inventory/data.py b/lib/ansible/inventory/data.py
index 83491aee001b5b..3f48518917fa50 100644
--- a/lib/ansible/inventory/data.py
+++ b/lib/ansible/inventory/data.py
@@ -27,6 +27,7 @@
 from ansible.errors import AnsibleError
 from ansible.inventory.group import Group
 from ansible.inventory.host import Host
+from ansible.module_utils._text import to_native
 from ansible.module_utils.six import iteritems, string_types
 from ansible.utils.display import Display
 from ansible.utils.vars import combine_vars
diff --git a/lib/ansible/inventory/group.py b/lib/ansible/inventory/group.py
index 98c34c81b968d1..f22d3fabe35b7d 100644
--- a/lib/ansible/inventory/group.py
+++ b/lib/ansible/inventory/group.py
@@ -19,7 +19,7 @@
 
 from ansible import constants as C
 from ansible.errors import AnsibleError
-from ansible.module_utils._text import to_native, to_text
+from ansible.module_utils._text import to_text
 
 from ansible.utils.display import Display
 
diff --git a/lib/ansible/inventory/manager.py b/lib/ansible/inventory/manager.py
index 2ca7457055aa26..1020ea3e850571 100644
--- a/lib/ansible/inventory/manager.py
+++ b/lib/ansible/inventory/manager.py
@@ -264,7 +264,7 @@ def parse_source(self, source, cache=False):
 
                 if plugin_wants:
                     # No need to deepcopy inventory unless it has changed
-                    if inventory_copy == None:
+                    if inventory_copy is None:
                         inventory_copy = deepcopy(self._inventory)
 
                     try:

From 871926e662a5b5a0b997256e10dfc204fc1a2cf4 Mon Sep 17 00:00:00 2001
From: s-hertel <shertel@redhat.com>
Date: Mon, 24 Jun 2019 15:03:40 -0400
Subject: [PATCH 6/7] fix strategy/__init__

---
 lib/ansible/plugins/strategy/__init__.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py
index 170bb6267bd167..ca045bbea723f6 100644
--- a/lib/ansible/plugins/strategy/__init__.py
+++ b/lib/ansible/plugins/strategy/__init__.py
@@ -706,7 +706,7 @@ def _add_host(self, host_info, iterator):
                 if group_name not in self._inventory.groups:
                     self._inventory.add_group(group_name)
                 new_group = self._inventory.groups[group_name]
-                new_group.add_host(self._inventory.hosts[host_name])
+                self._inventory.group_add_host(group_name, host_name)
 
             # reconcile inventory, ensures inventory rules are followed
             self._inventory.reconcile_inventory()
@@ -741,11 +741,11 @@ def _add_group(self, host, result_item):
             self._inventory.group_add_child_group(group_name, parent_group_name)
 
         if real_host.name not in group.get_hosts():
-            group.add_host(real_host)
+            self._inventory.group_add_host(group_name, real_host.name)
             changed = True
 
         if group_name not in host.get_groups():
-            real_host.add_group(group)
+            self._inventory.host_add_group(real_host.name, group_name)
             changed = True
 
         if changed:

From 020993391e0962c6fa68035ac2bc05c52d14bfcd Mon Sep 17 00:00:00 2001
From: s-hertel <shertel@redhat.com>
Date: Tue, 25 Jun 2019 16:18:15 -0400
Subject: [PATCH 7/7] Oops, fix using InventoryManager

---
 lib/ansible/plugins/strategy/__init__.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py
index ca045bbea723f6..22d28b9b994f1c 100644
--- a/lib/ansible/plugins/strategy/__init__.py
+++ b/lib/ansible/plugins/strategy/__init__.py
@@ -705,8 +705,7 @@ def _add_host(self, host_info, iterator):
             for group_name in new_groups:
                 if group_name not in self._inventory.groups:
                     self._inventory.add_group(group_name)
-                new_group = self._inventory.groups[group_name]
-                self._inventory.group_add_host(group_name, host_name)
+                self._inventory.add_host(host_name, group=group_name)
 
             # reconcile inventory, ensures inventory rules are followed
             self._inventory.reconcile_inventory()
@@ -738,14 +737,15 @@ def _add_group(self, host, result_item):
                 changed = True
         group = self._inventory.groups[group_name]
         for parent_group_name in parent_group_names:
-            self._inventory.group_add_child_group(group_name, parent_group_name)
+            # Use InventoryData object to add child
+            self._inventory._inventory.add_child(parent_group_name, group_name)
 
-        if real_host.name not in group.get_hosts():
-            self._inventory.group_add_host(group_name, real_host.name)
+        if real_host.name not in self._inventory._inventory.group_get_hosts(group_name):
+            self._inventory.add_host(real_host.name, group=group_name)
             changed = True
 
         if group_name not in host.get_groups():
-            self._inventory.host_add_group(real_host.name, group_name)
+            self._inventory.add_host(real_host.name, group=group_name)
             changed = True
 
         if changed: