From 6730f81024bdd6eb56b30763aabb574e2ff6040c Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Thu, 14 Sep 2017 15:56:12 -0500 Subject: [PATCH] Correctly create include_role blocks when using loops (#30372) Also fixes instances where conditionals or other variables may result in hosts executing lists of tasks of differing sizes. Fixes #18748 --- lib/ansible/playbook/included_file.py | 94 ++++++++++++++------------ lib/ansible/plugins/strategy/free.py | 13 +++- lib/ansible/plugins/strategy/linear.py | 47 ++++--------- 3 files changed, 76 insertions(+), 78 deletions(-) diff --git a/lib/ansible/playbook/included_file.py b/lib/ansible/playbook/included_file.py index 07a0d235f84..1c5d30c6cdd 100644 --- a/lib/ansible/playbook/included_file.py +++ b/lib/ansible/playbook/included_file.py @@ -34,11 +34,12 @@ except ImportError: class IncludedFile: - def __init__(self, filename, args, task): + def __init__(self, filename, args, task, is_role=False): self._filename = filename self._args = args self._task = task self._hosts = [] + self._is_role = is_role def add_host(self, host): if host not in self._hosts: @@ -65,7 +66,7 @@ class IncludedFile: original_host = res._host original_task = res._task - if original_task.action in ('include', 'include_tasks'): + if original_task.action in ('include', 'include_tasks', 'include_role'): if original_task.loop: if 'results' not in res._result: continue @@ -75,7 +76,7 @@ class IncludedFile: 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: + if 'skipped' in include_result and include_result['skipped'] or 'failed' in include_result and include_result['failed']: continue task_vars = variable_manager.get_vars(play=iterator._play, host=original_host, task=original_task) @@ -88,49 +89,52 @@ class IncludedFile: if loop_var in include_result: task_vars[loop_var] = include_variables[loop_var] = include_result[loop_var] - include_file = None - if original_task: - if original_task.static: - continue - - if original_task._parent: - # handle relative includes by walking up the list of parent include - # tasks and checking the relative result to see if it exists - parent_include = original_task._parent - cumulative_path = None - while parent_include is not None: - if not isinstance(parent_include, TaskInclude): - parent_include = parent_include._parent - continue - if isinstance(parent_include, IncludeRole): - parent_include_dir = parent_include._role_path - else: - parent_include_dir = os.path.dirname(templar.template(parent_include.args.get('_raw_params'))) - if cumulative_path is not None and not os.path.isabs(cumulative_path): - cumulative_path = os.path.join(parent_include_dir, cumulative_path) - else: - cumulative_path = parent_include_dir + if original_task.action in ('include', 'include_tasks'): + include_file = None + if original_task: + if original_task.static: + continue + + if original_task._parent: + # handle relative includes by walking up the list of parent include + # tasks and checking the relative result to see if it exists + parent_include = original_task._parent + cumulative_path = None + while parent_include is not None: + if not isinstance(parent_include, TaskInclude): + parent_include = parent_include._parent + continue + if isinstance(parent_include, IncludeRole): + parent_include_dir = parent_include._role_path + else: + parent_include_dir = os.path.dirname(templar.template(parent_include.args.get('_raw_params'))) + if cumulative_path is not None and not os.path.isabs(cumulative_path): + cumulative_path = os.path.join(parent_include_dir, cumulative_path) + else: + cumulative_path = parent_include_dir + include_target = templar.template(include_result['include']) + if original_task._role: + new_basedir = os.path.join(original_task._role._role_path, 'tasks', cumulative_path) + include_file = loader.path_dwim_relative(new_basedir, 'tasks', include_target) + else: + include_file = loader.path_dwim_relative(loader.get_basedir(), cumulative_path, include_target) + + if os.path.exists(include_file): + break + else: + parent_include = parent_include._parent + + if include_file is None: + if original_task._role: include_target = templar.template(include_result['include']) - if original_task._role: - new_basedir = os.path.join(original_task._role._role_path, 'tasks', cumulative_path) - include_file = loader.path_dwim_relative(new_basedir, 'tasks', include_target) - else: - include_file = loader.path_dwim_relative(loader.get_basedir(), cumulative_path, include_target) - - if os.path.exists(include_file): - break - else: - parent_include = parent_include._parent - - if include_file is None: - if original_task._role: - include_target = templar.template(include_result['include']) - include_file = loader.path_dwim_relative(original_task._role._role_path, 'tasks', include_target) - else: - include_file = loader.path_dwim(include_result['include']) - - include_file = templar.template(include_file) - inc_file = IncludedFile(include_file, include_variables, original_task) + include_file = loader.path_dwim_relative(original_task._role._role_path, 'tasks', include_target) + else: + include_file = loader.path_dwim(include_result['include']) + + include_file = templar.template(include_file) + inc_file = IncludedFile(include_file, include_variables, original_task) + else: + inc_file = IncludedFile("role", include_variables, original_task, is_role=True) try: pos = included_files.index(inc_file) diff --git a/lib/ansible/plugins/strategy/free.py b/lib/ansible/plugins/strategy/free.py index c398c19cc49..7b79639efa5 100644 --- a/lib/ansible/plugins/strategy/free.py +++ b/lib/ansible/plugins/strategy/free.py @@ -193,7 +193,18 @@ class StrategyModule(StrategyBase): for included_file in included_files: display.debug("collecting new blocks for %s" % included_file) try: - new_blocks = self._load_included_file(included_file, iterator=iterator) + if included_file._is_role: + new_ir = included_file._task.copy() + new_ir.vars.update(included_file._args) + + new_blocks, handler_blocks = new_ir.get_block_list( + play=iterator._play, + variable_manager=self._variable_manager, + loader=self._loader, + ) + self._tqm.update_handler_list([handler for handler_block in handler_blocks for handler in handler_block.block]) + else: + new_blocks = self._load_included_file(included_file, iterator=iterator) except AnsibleError as e: for host in included_file._hosts: iterator.mark_host_failed(host) diff --git a/lib/ansible/plugins/strategy/linear.py b/lib/ansible/plugins/strategy/linear.py index 7ecc75b58f7..71d5448f8e6 100644 --- a/lib/ansible/plugins/strategy/linear.py +++ b/lib/ansible/plugins/strategy/linear.py @@ -290,38 +290,8 @@ class StrategyModule(StrategyBase): display.debug("done queuing things up, now waiting for results queue to drain") if self._pending_results > 0: results += self._wait_on_pending_results(iterator) - host_results.extend(results) - all_role_blocks = [] - for hr in results: - # handle include_role - if hr._task.action == 'include_role': - loop_var = None - if hr._task.loop: - loop_var = 'item' - if hr._task.loop_control: - loop_var = hr._task.loop_control.loop_var or 'item' - include_results = hr._result.get('results', []) - else: - include_results = [hr._result] - - for include_result in include_results: - if 'skipped' in include_result and include_result['skipped'] or 'failed' in include_result and include_result['failed']: - continue - - display.debug("generating all_blocks data for role") - new_ir = hr._task.copy() - new_ir.vars.update(include_result.get('include_variables', dict())) - if loop_var and loop_var in include_result: - new_ir.vars[loop_var] = include_result[loop_var] - - blocks, handler_blocks = new_ir.get_block_list(play=iterator._play, variable_manager=self._variable_manager, loader=self._loader) - all_role_blocks.extend(blocks) - self._tqm.update_handler_list([handler for handler_block in handler_blocks for handler in handler_block.block]) - - if len(all_role_blocks) > 0: - for host in hosts_left: - iterator.add_tasks(host, all_role_blocks) + host_results.extend(results) try: included_files = IncludedFile.process_include_results( @@ -339,6 +309,8 @@ class StrategyModule(StrategyBase): include_failure = False if len(included_files) > 0: display.debug("we have included files to process") + + # A noop task for use in padding dynamic includes noop_task = Task() noop_task.action = 'meta' noop_task.args['_raw_params'] = 'noop' @@ -352,7 +324,18 @@ 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, iterator=iterator) + if included_file._is_role: + new_ir = included_file._task.copy() + new_ir.vars.update(included_file._args) + + new_blocks, handler_blocks = new_ir.get_block_list( + play=iterator._play, + variable_manager=self._variable_manager, + loader=self._loader, + ) + self._tqm.update_handler_list([handler for handler_block in handler_blocks for handler in handler_block.block]) + else: + new_blocks = self._load_included_file(included_file, iterator=iterator) display.debug("iterating over new_blocks loaded from include file") for new_block in new_blocks: