preserve add_host/group_by on refresh (#77944)

* preserve add_host/group_by on meta: refresh_inventory

Co-authored-by: Jordan Borean <jborean93@gmail.com>

9 files changed, 239 insertions, 90 deletions

diff --git a/changelogs/fragments/fix_inv_refresh.yml b/changelogs/fragments/fix_inv_refresh.yml
new file mode 100644
index 0000000000..28a741d4b6
--- /dev/null
+++ b/changelogs/fragments/fix_inv_refresh.yml
@@ -0,0 +1,2 @@
+bugfixes:
+  - '"meta: refresh_inventory" does not clobber entries added by add_host/group_by anymore.'
diff --git a/lib/ansible/inventory/manager.py b/lib/ansible/inventory/manager.py
index c55bbe615d..400bc6b2b5 100644
--- a/lib/ansible/inventory/manager.py
+++ b/lib/ansible/inventory/manager.py
@@ -166,9 +166,12 @@ class InventoryManager(object):
         if parse:
             self.parse_sources(cache=cache)
 
+        self._cached_dynamic_hosts = []
+        self._cached_dynamic_grouping = []
+
     @property
     def localhost(self):
-        return self._inventory.localhost
+        return self._inventory.get_host('localhost')
 
     @property
     def groups(self):
@@ -343,6 +346,11 @@ class InventoryManager(object):
         self.clear_caches()
         self._inventory = InventoryData()
         self.parse_sources(cache=False)
+        for host in self._cached_dynamic_hosts:
+            self.add_dynamic_host(host, {'refresh': True})
+        for host, result in self._cached_dynamic_grouping:
+            result['refresh'] = True
+            self.add_dynamic_group(host, result)
 
     def _match_list(self, items, pattern_str):
         # compile patterns
@@ -648,3 +656,97 @@ class InventoryManager(object):
 
     def clear_pattern_cache(self):
         self._pattern_cache = {}
+
+    def add_dynamic_host(self, host_info, result_item):
+        '''
+        Helper function to add a new host to inventory based on a task result.
+        '''
+
+        changed = False
+        if not result_item.get('refresh'):
+            self._cached_dynamic_hosts.append(host_info)
+
+        if host_info:
+            host_name = host_info.get('host_name')
+
+            # Check if host in inventory, add if not
+            if host_name not in self.hosts:
+                self.add_host(host_name, 'all')
+                changed = True
+            new_host = self.hosts.get(host_name)
+
+            # Set/update the vars for this host
+            new_host_vars = new_host.get_vars()
+            new_host_combined_vars = combine_vars(new_host_vars, host_info.get('host_vars', dict()))
+            if new_host_vars != new_host_combined_vars:
+                new_host.vars = new_host_combined_vars
+                changed = True
+
+            new_groups = host_info.get('groups', [])
+            for group_name in new_groups:
+                if group_name not in self.groups:
+                    group_name = self._inventory.add_group(group_name)
+                    changed = True
+                new_group = self.groups[group_name]
+                if new_group.add_host(self.hosts[host_name]):
+                    changed = True
+
+            # reconcile inventory, ensures inventory rules are followed
+            if changed:
+                self.reconcile_inventory()
+
+            result_item['changed'] = changed
+
+    def add_dynamic_group(self, host, result_item):
+        '''
+        Helper function to add a group (if it does not exist), and to assign the
+        specified host to that group.
+        '''
+
+        changed = False
+
+        if not result_item.get('refresh'):
+            self._cached_dynamic_grouping.append((host, result_item))
+
+        # the host here is from the executor side, which means it was a
+        # serialized/cloned copy and we'll need to look up the proper
+        # host object from the master inventory
+        real_host = self.hosts.get(host.name)
+        if real_host is None:
+            if host.name == self.localhost.name:
+                real_host = self.localhost
+            elif not result_item.get('refresh'):
+                raise AnsibleError('%s cannot be matched in inventory' % host.name)
+            else:
+                # host was removed from inventory during refresh, we should not process
+                return
+
+        group_name = result_item.get('add_group')
+        parent_group_names = result_item.get('parent_groups', [])
+
+        if group_name not in self.groups:
+            group_name = self.add_group(group_name)
+
+        for name in parent_group_names:
+            if name not in self.groups:
+                # create the new group and add it to inventory
+                self.add_group(name)
+                changed = True
+
+        group = self._inventory.groups[group_name]
+        for parent_group_name in parent_group_names:
+            parent_group = self.groups[parent_group_name]
+            new = parent_group.add_child_group(group)
+            if new and not changed:
+                changed = True
+
+        if real_host not in group.get_hosts():
+            changed = group.add_host(real_host)
+
+        if group not in real_host.get_groups():
+            changed = real_host.add_group(group)
+
+        if changed:
+            self.reconcile_inventory()
+
+        result_item['changed'] = changed
diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py
index d1dadebf9c..1d703ac6a0 100644
--- a/lib/ansible/plugins/strategy/__init__.py
+++ b/lib/ansible/plugins/strategy/__init__.py
@@ -43,7 +43,7 @@ from ansible.executor.process.worker import WorkerProcess
 from ansible.executor.task_result import TaskResult
 from ansible.executor.task_queue_manager import CallbackSend
 from ansible.module_utils.six import string_types
-from ansible.module_utils._text import to_text, to_native
+from ansible.module_utils._text import to_text
 from ansible.module_utils.connection import Connection, ConnectionError
 from ansible.playbook.conditional import Conditional
 from ansible.playbook.handler import Handler
@@ -697,11 +697,14 @@ class StrategyBase:
                     if 'add_host' in result_item:
                         # this task added a new host (add_host module)
                         new_host_info = result_item.get('add_host', dict())
-                        self._add_host(new_host_info, result_item)
+                        self._inventory.add_dynamic_host(new_host_info, result_item)
+                        # ensure host is available for subsequent plays
+                        if result_item.get('changed') and new_host_info['host_name'] not in self._hosts_cache_all:
+                            self._hosts_cache_all.append(new_host_info['host_name'])
 
                     elif 'add_group' in result_item:
                         # this task added a new group (group_by module)
-                        self._add_group(original_host, result_item)
+                        self._inventory.add_dynamic_group(original_host, result_item)
 
                     if 'add_host' in result_item or 'add_group' in result_item:
                         item_vars = _get_item_vars(result_item, original_task)
@@ -871,92 +874,6 @@ class StrategyBase:
 
         return ret_results
 
-    def _add_host(self, host_info, result_item):
-        '''
-        Helper function to add a new host to inventory based on a task result.
-        '''
-
-        changed = False
-
-        if host_info:
-            host_name = host_info.get('host_name')
-
-            # Check if host in inventory, add if not
-            if host_name not in self._inventory.hosts:
-                self._inventory.add_host(host_name, 'all')
-                self._hosts_cache_all.append(host_name)
-                changed = True
-            new_host = self._inventory.hosts.get(host_name)
-
-            # Set/update the vars for this host
-            new_host_vars = new_host.get_vars()
-            new_host_combined_vars = combine_vars(new_host_vars, host_info.get('host_vars', dict()))
-            if new_host_vars != new_host_combined_vars:
-                new_host.vars = new_host_combined_vars
-                changed = True
-
-            new_groups = host_info.get('groups', [])
-            for group_name in new_groups:
-                if group_name not in self._inventory.groups:
-                    group_name = self._inventory.add_group(group_name)
-                    changed = True
-                new_group = self._inventory.groups[group_name]
-                if new_group.add_host(self._inventory.hosts[host_name]):
-                    changed = True
-
-            # reconcile inventory, ensures inventory rules are followed
-            if changed:
-                self._inventory.reconcile_inventory()
-
-            result_item['changed'] = changed
-
-    def _add_group(self, host, result_item):
-        '''
-        Helper function to add a group (if it does not exist), and to assign the
-        specified host to that group.
-        '''
-
-        changed = False
-
-        # the host here is from the executor side, which means it was a
-        # serialized/cloned copy and we'll need to look up the proper
-        # host object from the master inventory
-        real_host = self._inventory.hosts.get(host.name)
-        if real_host is None:
-            if host.name == self._inventory.localhost.name:
-                real_host = self._inventory.localhost
-            else:
-                raise AnsibleError('%s cannot be matched in inventory' % host.name)
-        group_name = result_item.get('add_group')
-        parent_group_names = result_item.get('parent_groups', [])
-
-        if group_name not in self._inventory.groups:
-            group_name = self._inventory.add_group(group_name)
-
-        for name in parent_group_names:
-            if name not in self._inventory.groups:
-                # create the new group and add it to inventory
-                self._inventory.add_group(name)
-                changed = True
-
-        group = self._inventory.groups[group_name]
-        for parent_group_name in parent_group_names:
-            parent_group = self._inventory.groups[parent_group_name]
-            new = parent_group.add_child_group(group)
-            if new and not changed:
-                changed = True
-
-        if real_host not in group.get_hosts():
-            changed = group.add_host(real_host)
-
-        if group not in real_host.get_groups():
-            changed = real_host.add_group(group)
-
-        if changed:
-            self._inventory.reconcile_inventory()
-
-        result_item['changed'] = changed
-
     def _copy_included_file(self, included_file):
         '''
         A proven safe and performant way to create a copy of an included file
diff --git a/test/integration/targets/meta_tasks/inventory_new.yml b/test/integration/targets/meta_tasks/inventory_new.yml
new file mode 100644
index 0000000000..6d6dec7c00
--- /dev/null
+++ b/test/integration/targets/meta_tasks/inventory_new.yml
@@ -0,0 +1,8 @@
+all:
+  hosts:
+    two:
+      parity: even
+    three:
+      parity: odd
+    four:
+      parity: even
diff --git a/test/integration/targets/meta_tasks/inventory_old.yml b/test/integration/targets/meta_tasks/inventory_old.yml
new file mode 100644
index 0000000000..42d9bdd7a5
--- /dev/null
+++ b/test/integration/targets/meta_tasks/inventory_old.yml
@@ -0,0 +1,8 @@
+all:
+  hosts:
+    one:
+      parity: odd
+    two:
+      parity: even
+    three:
+      parity: odd
diff --git a/test/integration/targets/meta_tasks/inventory_refresh.yml b/test/integration/targets/meta_tasks/inventory_refresh.yml
new file mode 120000
index 0000000000..2ecdd0aedb
--- /dev/null
+++ b/test/integration/targets/meta_tasks/inventory_refresh.yml
@@ -0,0 +1 @@
+inventory_old.yml
\ No newline at end of file
diff --git a/test/integration/targets/meta_tasks/refresh.yml b/test/integration/targets/meta_tasks/refresh.yml
new file mode 100644
index 0000000000..ac24b7da93
--- /dev/null
+++ b/test/integration/targets/meta_tasks/refresh.yml
@@ -0,0 +1,38 @@
+- hosts: all
+  gather_facts: false
+  tasks:
+    - block:
+      - name: check initial state
+        assert:
+          that:
+            - "'one' in ansible_play_hosts"
+            - "'two' in ansible_play_hosts"
+            - "'three' in ansible_play_hosts"
+            - "'four' not in ansible_play_hosts"
+        run_once: true
+
+      - name: change symlink
+        file: src=./inventory_new.yml dest=./inventory_refresh.yml state=link force=yes follow=false
+        delegate_to: localhost
+        run_once: true
+
+      - name: refresh the inventory to new source
+        meta: refresh_inventory
+
+      always:
+        - name: revert symlink, invenotry was already reread or failed
+          file: src=./inventory_old.yml dest=./inventory_refresh.yml state=link force=yes follow=false
+          delegate_to: localhost
+          run_once: true
+
+- hosts: all
+  gather_facts: false
+  tasks:
+      - name: check refreshed state
+        assert:
+          that:
+            - "'one' not in ansible_play_hosts"
+            - "'two' in ansible_play_hosts"
+            - "'three' in ansible_play_hosts"
+            - "'four' in ansible_play_hosts"
+        run_once: true
diff --git a/test/integration/targets/meta_tasks/refresh_preserve_dynamic.yml b/test/integration/targets/meta_tasks/refresh_preserve_dynamic.yml
new file mode 100644
index 0000000000..7766a382a9
--- /dev/null
+++ b/test/integration/targets/meta_tasks/refresh_preserve_dynamic.yml
@@ -0,0 +1,69 @@
+- hosts: all
+  gather_facts: false
+  tasks:
+      - name: check initial state
+        assert:
+          that:
+            - "'one' in ansible_play_hosts"
+            - "'two' in ansible_play_hosts"
+            - "'three' in ansible_play_hosts"
+            - "'four' not in ansible_play_hosts"
+        run_once: true
+
+      - name: add a host
+        add_host:
+          name: yolo
+          parity: null
+
+      - name: group em
+        group_by:
+          key: '{{parity}}'
+
+- hosts: all
+  gather_facts: false
+  tasks:
+    - name: test and ensure we restore symlink
+      run_once: true
+      block:
+      - name: check added host state
+        assert:
+          that:
+            - "'yolo' in ansible_play_hosts"
+            - "'even' in groups"
+            - "'odd' in groups"
+            - "'two' in groups['even']"
+            - "'three' in groups['odd']"
+
+      - name: change symlink
+        file: src=./inventory_new.yml dest=./inventory_refresh.yml state=link force=yes follow=false
+        delegate_to: localhost
+
+      - name: refresh the inventory to new source
+        meta: refresh_inventory
+
+      always:
+        - name: revert symlink, invenotry was already reread or failed
+          file: src=./inventory_old.yml dest=./inventory_refresh.yml state=link force=yes follow=false
+          delegate_to: localhost
+
+- hosts: all
+  gather_facts: false
+  tasks:
+      - name: check refreshed state
+        assert:
+          that:
+            - "'one' not in ansible_play_hosts"
+            - "'two' in ansible_play_hosts"
+            - "'three' in ansible_play_hosts"
+            - "'four' in ansible_play_hosts"
+        run_once: true
+
+      - name: check added host state
+        assert:
+          that:
+            - "'yolo' in ansible_play_hosts"
+            - "'even' in groups"
+            - "'odd' in groups"
+            - "'two' in groups['even']"
+            - "'three' in groups['odd']"
+        run_once: true
diff --git a/test/integration/targets/meta_tasks/runme.sh b/test/integration/targets/meta_tasks/runme.sh
index c29579bf1d..bee6636314 100755
--- a/test/integration/targets/meta_tasks/runme.sh
+++ b/test/integration/targets/meta_tasks/runme.sh
@@ -72,3 +72,7 @@ for test_strategy in linear free; do
   [ "$(grep -c "META: ending batch" <<< "$out" )" -eq 2 ]
   grep -qv 'Failed to end_batch' <<< "$out"
 done
+
+# test refresh
+ansible-playbook -i inventory_refresh.yml refresh.yml "$@"
+ansible-playbook -i inventory_refresh.yml refresh_preserve_dynamic.yml "$@"