Moving included file stuff to a proper dedicated class and file (v2)

lib/ansible/playbook/included_file.py

+# (c) 2012-2014, Michael DeHaan <michael.dehaan@gmail.com>
+#
+# This file is part of Ansible
+#
+# Ansible is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# Ansible is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
+
+# Make coding more python3-ish
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+class IncludedFile:
+
+    def __init__(self, filename, args, task):
+        self._filename = filename
+        self._args     = args
+        self._task     = task
+        self._hosts    = []
+
+    def add_host(self, host):
+        if host not in self._hosts:
+            self._hosts.append(host)
+
+    def __eq__(self, other):
+        return other._filename == self._filename and other._args == self._args
+
+    def __repr__(self):
+        return "%s (%s): %s" % (self._filename, self._args, self._hosts)
+
+    @staticmethod
+    def process_include_results(results, tqm, iterator, loader):
+        included_files = []
+
+        for res in results:
+            if res._host in tqm._failed_hosts:
+                raise AnsibleError("host is failed, not including files")
+
+            if res._task.action == 'include':
+                if res._task.loop:
+                    include_results = res._result['results']
+                else:
+                    include_results = [ res._result ]
+
+                for include_result in include_results:
+                    # if the task result was skipped or failed, continue
+                    if 'skipped' in include_result and include_result['skipped'] or 'failed' in include_result:
+                        continue
+
+                    original_task = iterator.get_original_task(res._host, res._task)
+                    if original_task and original_task._role:
+                        include_file = loader.path_dwim_relative(original_task._role._role_path, 'tasks', include_result['include'])
+                    else:
+                        include_file = loader.path_dwim(res._task.args.get('_raw_params'))
+
+                    include_variables = include_result.get('include_variables', dict())
+                    if 'item' in include_result:
+                        include_variables['item'] = include_result['item']
+
+                    inc_file = IncludedFile(include_file, include_variables, original_task)
+
+                    try:
+                        pos = included_files.index(inc_file)
+                        inc_file = included_files[pos]
+                    except ValueError:
+                        included_files.append(inc_file)
+
+                    inc_file.add_host(res._host)
+
+        return included_files

lib/ansible/plugins/strategies/__init__.py

@@ -23,10 +23,9 @@ from six.moves import queue as Queue
 import time
 
 from ansible.errors import *
-
+from ansible.executor.task_result import TaskResult
 from ansible.inventory.host import Host
 from ansible.inventory.group import Group
-
 from ansible.playbook.handler import Handler
 from ansible.playbook.helpers import load_list_of_blocks
 from ansible.playbook.role import ROLE_CACHE, hash_params
@@ -307,12 +306,22 @@ class StrategyBase:
         # and add the host to the group
         new_group.add_host(actual_host)
 
-    def _load_included_file(self, included_file):
+    def _load_included_file(self, included_file, iterator):
         '''
         Loads an included YAML file of tasks, applying the optional set of variables.
         '''
 
-        data = self._loader.load_from_file(included_file._filename)
+        try:
+            data = self._loader.load_from_file(included_file._filename)
+        except AnsibleError, e:
+            for host in included_file._hosts:
+                tr = TaskResult(host=host, task=included_file._task, return_data=dict(failed=True, reason=str(e)))
+                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 []
+
         if not isinstance(data, list):
             raise AnsibleParserError("included task files must contain a list of tasks", obj=included_file._task._ds)
 

lib/ansible/plugins/strategies/linear.py

@@ -22,6 +22,7 @@ __metaclass__ = type
 from ansible.errors import AnsibleError
 from ansible.executor.play_iterator import PlayIterator
 from ansible.playbook.block import Block
+from ansible.playbook.included_file import IncludedFile
 from ansible.playbook.task import Task
 from ansible.plugins import action_loader
 from ansible.plugins.strategies import StrategyBase
@@ -114,7 +115,6 @@ class StrategyModule(StrategyBase):
         # return None for all hosts in the list
         return [(host, None) for host in hosts]
 
-
     def run(self, iterator, connection_info):
         '''
         The linear strategy is simple - get the next task and queue
@@ -208,61 +208,11 @@ class StrategyModule(StrategyBase):
                 results = self._wait_on_pending_results(iterator)
                 host_results.extend(results)
 
-                # FIXME: this needs to be somewhere else
-                class IncludedFile:
-                    def __init__(self, filename, args, task):
-                        self._filename = filename
-                        self._args     = args
-                        self._task     = task
-                        self._hosts    = []
-                    def add_host(self, host):
-                        if host not in self._hosts:
-                            self._hosts.append(host)
-                    def __eq__(self, other):
-                        return other._filename == self._filename and other._args == self._args
-                    def __repr__(self):
-                        return "%s (%s): %s" % (self._filename, self._args, self._hosts)
-
-                # FIXME: this should also be moved to the base class in a method
-                included_files = []
-                for res in host_results:
-                    if res._host in self._tqm._failed_hosts:
-                        return 1
-
-                    if res._task.action == 'include':
-                        if res._task.loop:
-                            include_results = res._result['results']
-                        else:
-                            include_results = [ res._result ]
-
-                        for include_result in include_results:
-                            # if the task result was skipped or failed, continue
-                            if 'skipped' in include_result and include_result['skipped'] or 'failed' in include_result:
-                                continue
-
-                            original_task = iterator.get_original_task(res._host, res._task)
-                            if original_task and original_task._role:
-                                include_file = self._loader.path_dwim_relative(original_task._role._role_path, 'tasks', include_result['include'])
-                            else:
-                                include_file = self._loader.path_dwim(res._task.args.get('_raw_params'))
-
-                            include_variables = include_result.get('include_variables', dict())
-                            if 'item' in include_result:
-                                include_variables['item'] = include_result['item']
-
-                            inc_file = IncludedFile(include_file, include_variables, original_task)
-
-                            try:
-                                pos = included_files.index(inc_file)
-                                inc_file = included_files[pos]
-                            except ValueError:
-                                included_files.append(inc_file)
-
-                            inc_file.add_host(res._host)
+                try:
+                    included_files = IncludedFile.process_include_results(host_results, self._tqm, iterator=iterator, loader=self._loader)
+                except AnsibleError, e:
+                    return 1
 
-                # FIXME: should this be moved into the iterator class? Main downside would be
-                #        that accessing the TQM's callback member would be more difficult, if
-                #        we do want to send callbacks from here
                 if len(included_files) > 0:
                     noop_task = Task()
                     noop_task.action = 'meta'
@@ -274,7 +224,7 @@ class StrategyModule(StrategyBase):
                         # included hosts get the task list while those excluded get an equal-length
                         # list of noop tasks, to make sure that they continue running in lock-step
                         try:
-                            new_blocks = self._load_included_file(included_file)
+                            new_blocks = self._load_included_file(included_file, iterator=iterator)
                         except AnsibleError, e:
                             for host in included_file._hosts:
                                 iterator.mark_host_failed(host)