From 62feba3d1511129179eaf0d6be841f204813c7fc Mon Sep 17 00:00:00 2001
From: Alan Rominger <arominge@redhat.com>
Date: Wed, 20 Feb 2019 10:41:25 -0500
Subject: [PATCH] support deterministic host ordering from group ancestors
 (#44067)

Fixes #44065
---
 lib/ansible/inventory/group.py             | 35 +++++++++++++---------
 test/units/plugins/inventory/test_group.py | 27 +++++++++++++++++
 2 files changed, 48 insertions(+), 14 deletions(-)

diff --git a/lib/ansible/inventory/group.py b/lib/ansible/inventory/group.py
index 859781cc4d..52f69af63c 100644
--- a/lib/ansible/inventory/group.py
+++ b/lib/ansible/inventory/group.py
@@ -82,7 +82,7 @@ class Group:
             g.deserialize(parent_data)
             self.parent_groups.append(g)
 
-    def _walk_relationship(self, rel):
+    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,
@@ -98,21 +98,34 @@ class Group:
         '''
         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)
-            unprocessed = set(chain.from_iterable(
-                getattr(g, rel) for g in unprocessed
-            ))
-            unprocessed.difference_update(seen)
+            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):
-        return self._walk_relationship('child_groups')
+    def get_descendants(self, **kwargs):
+        return self._walk_relationship('child_groups', **kwargs)
 
     @property
     def host_names(self):
@@ -215,7 +228,7 @@ class Group:
 
         hosts = []
         seen = {}
-        for kid in self.get_descendants():
+        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:
@@ -223,12 +236,6 @@ class Group:
                     if self.name == 'all' and kk.implicit:
                         continue
                     hosts.append(kk)
-        for mine in self.hosts:
-            if mine not in seen:
-                seen[mine] = 1
-                if self.name == 'all' and mine.implicit:
-                    continue
-                hosts.append(mine)
         return hosts
 
     def get_vars(self):
diff --git a/test/units/plugins/inventory/test_group.py b/test/units/plugins/inventory/test_group.py
index 5c995cdd2a..097445cf6c 100644
--- a/test/units/plugins/inventory/test_group.py
+++ b/test/units/plugins/inventory/test_group.py
@@ -76,6 +76,33 @@ class TestGroup(unittest.TestCase):
         with self.assertRaises(AnsibleError):
             C.add_child_group(A)
 
+    def test_direct_host_ordering(self):
+        """Hosts are returned in order they are added
+        """
+        group = Group('A')
+        # host names not added in alphabetical order
+        host_name_list = ['z', 'b', 'c', 'a', 'p', 'q']
+        expected_hosts = []
+        for host_name in host_name_list:
+            h = Host(host_name)
+            group.add_host(h)
+            expected_hosts.append(h)
+        assert group.get_hosts() == expected_hosts
+
+    def test_sub_group_host_ordering(self):
+        """With multiple nested groups, asserts that hosts are returned
+        in deterministic order
+        """
+        top_group = Group('A')
+        expected_hosts = []
+        for name in ['z', 'b', 'c', 'a', 'p', 'q']:
+            child = Group('group_{0}'.format(name))
+            top_group.add_child_group(child)
+            host = Host('host_{0}'.format(name))
+            child.add_host(host)
+            expected_hosts.append(host)
+        assert top_group.get_hosts() == expected_hosts
+
     def test_populates_descendant_hosts(self):
         A = Group('A')
         B = Group('B')