Prevent double failing hosts for includes in loops (#76928)

Fixes #23161

changelogs/fragments/23161-includes-loops-rescue.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - "Properly execute rescue section when an include task fails in all loop iterations (https://github.com/ansible/ansible/issues/23161)"

lib/ansible/plugins/strategy/__init__.py

@@ -826,8 +826,11 @@ class StrategyBase:
     def _load_included_file(self, included_file, iterator, is_handler=False):
         '''
         Loads an included YAML file of tasks, applying the optional set of variables.
-        '''
 
+        Raises AnsibleError exception in case of a failure during including a file,
+        in such case the caller is responsible for marking the host(s) as failed
+        using PlayIterator.mark_host_failed().
+        '''
         display.debug("loading included file: %s" % included_file._filename)
         try:
             data = self._loader.load_from_file(included_file._filename)
@@ -863,15 +866,11 @@ class StrategyBase:
             for r in included_file._results:
                 r._result['failed'] = True
 
-            # mark all of the hosts including this file as failed, send callbacks,
-            # and increment the stats for this host
             for host in included_file._hosts:
                 tr = TaskResult(host=host, task=included_file._task, return_data=dict(failed=True, reason=reason))
-                iterator.mark_host_failed(host)
-                self._tqm._failed_hosts[host.name] = True
                 self._tqm._stats.increment('failures', host.name)
                 self._tqm.send_callback('v2_runner_on_failed', tr)
-            return []
+            raise AnsibleError(reason) from e
 
         # finally, send the callback and return the list of blocks loaded
         self._tqm.send_callback('v2_playbook_on_include', included_file)

lib/ansible/plugins/strategy/free.py

@@ -234,6 +234,7 @@ class StrategyModule(StrategyBase):
 
             if len(included_files) > 0:
                 all_blocks = dict((host, []) for host in hosts_left)
+                failed_includes_hosts = set()
                 for included_file in included_files:
                     display.debug("collecting new blocks for %s" % included_file)
                     is_handler = False
@@ -256,12 +257,12 @@ class StrategyModule(StrategyBase):
                     except AnsibleParserError:
                         raise
                     except AnsibleError as e:
+                        if included_file._is_role:
+                            # include_role does not have on_include callback so display the error
+                            display.error(to_text(e), wrap_text=False)
                         for r in included_file._results:
                             r._result['failed'] = True
-
+                            failed_includes_hosts.add(r._host)
-                        for host in included_file._hosts:
-                            iterator.mark_host_failed(host)
-                        display.warning(to_text(e))
                         continue
 
                     for new_block in new_blocks:
@@ -282,6 +283,10 @@ class StrategyModule(StrategyBase):
                                 all_blocks[host].append(final_block)
                     display.debug("done collecting new blocks for %s" % included_file)
 
+                for host in failed_includes_hosts:
+                    self._tqm._failed_hosts[host.name] = True
+                    iterator.mark_host_failed(host)
+
                 display.debug("adding all collected blocks from %d included file(s) to iterator" % len(included_files))
                 for host in hosts_left:
                     iterator.add_tasks(host, all_blocks[host])

lib/ansible/plugins/strategy/linear.py

@@ -273,6 +273,7 @@ class StrategyModule(StrategyBase):
                     all_blocks = dict((host, []) for host in hosts_left)
                     display.debug("done generating all_blocks data")
                     included_tasks = []
+                    failed_includes_hosts = set()
                     for included_file in included_files:
                         display.debug("processing included file: %s" % included_file._filename)
                         is_handler = False
@@ -324,14 +325,17 @@ class StrategyModule(StrategyBase):
                         except AnsibleParserError:
                             raise
                         except AnsibleError as e:
+                            if included_file._is_role:
+                                # include_role does not have on_include callback so display the error
+                                display.error(to_text(e), wrap_text=False)
                             for r in included_file._results:
                                 r._result['failed'] = True
+                                failed_includes_hosts.add(r._host)
+                            continue
 
-                            for host in included_file._hosts:
+                    for host in failed_includes_hosts:
                         self._tqm._failed_hosts[host.name] = True
                         iterator.mark_host_failed(host)
-                            display.error(to_text(e), wrap_text=False)
-                            continue
 
                     # finally go through all of the hosts and append the
                     # accumulated blocks to their list of tasks

test/integration/targets/includes/includes_loop_rescue.yml

@@ -0,0 +1,29 @@
+- name: "Test rescue/always sections with includes in a loop, strategy={{ strategy }}"
+  hosts: localhost
+  gather_facts: false
+  strategy: "{{ strategy }}"
+  tasks:
+    - block:
+      - include_role:
+          name: "{{ item }}"
+        loop:
+          - a
+          - b
+      rescue:
+        - debug:
+            msg: rescue include_role in a loop
+      always:
+        - debug:
+            msg: always include_role in a loop
+
+    - block:
+      - include_tasks: "{{ item }}"
+        loop:
+          - a
+          - b
+      rescue:
+        - debug:
+            msg: rescue include_tasks in a loop
+      always:
+        - debug:
+            msg: always include_tasks in a loop

test/integration/targets/includes/runme.sh

@@ -11,3 +11,6 @@ set +e
 result="$(ansible-playbook -i ../../inventory include_on_playbook_should_fail.yml -v "$@" 2>&1)"
 set -e
 grep -q "ERROR! 'include' is not a valid attribute for a Play" <<< "$result"
+
+ansible-playbook includes_loop_rescue.yml --extra-vars strategy=linear "$@"
+ansible-playbook includes_loop_rescue.yml --extra-vars strategy=free "$@"