From efe5bb122e10ab1d54467c6ceb4d0be54ce13f08 Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Mon, 15 Aug 2016 15:21:37 -0500 Subject: [PATCH] Rework the way params are assigned to TaskIncludes when they're dynamic Copying the TaskInclude task (which is the parent) before loading the blocks makes the code much more simple and clean, and fixes a bug introduced during the performance improvement changes (and specifically the change which moved things to a single-parent model). Fixes #17064 (cherry picked from commit f4237b215155f7c65a44675cfcc16ce67866064e) --- lib/ansible/playbook/base.py | 13 ++++++++ lib/ansible/plugins/strategy/__init__.py | 40 +++++++++++------------- 2 files changed, 31 insertions(+), 22 deletions(-) diff --git a/lib/ansible/playbook/base.py b/lib/ansible/playbook/base.py index 294570326ba..220f823f7ae 100644 --- a/lib/ansible/playbook/base.py +++ b/lib/ansible/playbook/base.py @@ -86,6 +86,19 @@ class Base: # and init vars, avoid using defaults in field declaration as it lives across plays self.vars = dict() + def dump_me(self, depth=0): + if depth == 0: + print("DUMPING OBJECT ------------------------------------------------------") + print("%s- %s (%s, id=%s)" % (" " * depth, self.__class__.__name__, self, id(self))) + if hasattr(self, '_block') and self.__class__.__name__ == 'Task' and self._block: + self._block.dump_me(depth+2) + for attr_name in ('_parent_block', '_task_include'): + if hasattr(self, attr_name): + attr = getattr(self, attr_name) + if attr is not None: + attr.dump_me(depth+2) + if hasattr(self, '_play') and self._play: + self._play.dump_me(depth+2) # The following three functions are used to programatically define data # descriptors (aka properties) for the Attributes of all of the playbook diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py index edb17ff8bd4..4f867ff49c5 100644 --- a/lib/ansible/plugins/strategy/__init__.py +++ b/lib/ansible/plugins/strategy/__init__.py @@ -575,11 +575,28 @@ class StrategyBase: elif not isinstance(data, list): raise AnsibleError("included task files must contain a list of tasks") + ti_copy = included_file._task.copy() + temp_vars = ti_copy.vars.copy() + temp_vars.update(included_file._args) + # pop tags out of the include args, if they were specified there, and assign + # them to the include. If the include already had tags specified, we raise an + # error so that users know not to specify them both ways + tags = included_file._task.vars.pop('tags', []) + if isinstance(tags, string_types): + tags = tags.split(',') + if len(tags) > 0: + if len(included_file._task.tags) > 0: + raise AnsibleParserError("Include tasks should not specify tags in more than one way (both via args and directly on the task). Mixing tag specify styles is prohibited for whole import hierarchy, not only for single import statement", + obj=included_file._task._ds) + display.deprecated("You should not specify tags in the include parameters. All tags should be specified using the task-level option") + included_file._task.tags = tags + ti_copy.vars = temp_vars + block_list = load_list_of_blocks( data, play=included_file._task._block._play, parent_block=None, - task_include=included_file._task, + task_include=ti_copy, role=included_file._task._role, use_handlers=is_handler, loader=self._loader, @@ -602,27 +619,6 @@ class StrategyBase: self._tqm.send_callback('v2_runner_on_failed', tr) return [] - # set the vars for this task from those specified as params to the include - for b in block_list: - # first make a copy of the including task, so that each has a unique copy to modify - b._task_include = b._task_include.copy() - # then we create a temporary set of vars to ensure the variable reference is unique - temp_vars = b._task_include.vars.copy() - temp_vars.update(included_file._args.copy()) - # pop tags out of the include args, if they were specified there, and assign - # them to the include. If the include already had tags specified, we raise an - # error so that users know not to specify them both ways - tags = temp_vars.pop('tags', []) - if isinstance(tags, string_types): - tags = tags.split(',') - if len(tags) > 0: - if len(b._task_include.tags) > 0: - raise AnsibleParserError("Include tasks should not specify tags in more than one way (both via args and directly on the task). Mixing tag specify styles is prohibited for whole import hierarchy, not only for single import statement", - obj=included_file._task._ds) - display.deprecated("You should not specify tags in the include parameters. All tags should be specified using the task-level option") - b._task_include.tags = tags - b._task_include.vars = temp_vars - # finally, send the callback and return the list of blocks loaded self._tqm.send_callback('v2_playbook_on_include', included_file) display.debug("done processing included file")