From fb797a9e7786a3fde70e7de34ccf9ac3946501b3 Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Fri, 22 Jan 2016 12:44:56 -0500 Subject: [PATCH] Fixing role dependency chain creation The dep chain for roles created during the compile step had bugs, in which the dep chain was overwriten and the original tasks in the role were not assigned a dep chain. This lead to problems in determining whether roles had already run when in a "diamond" structure, and in some cases roles were not correctly getting variables from parents. Fixes #14046 --- lib/ansible/executor/play_iterator.py | 13 ++++++++++++- lib/ansible/playbook/role/__init__.py | 16 +++++++++------- .../roles/test_var_precedence_dep/tasks/main.yml | 2 +- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/lib/ansible/executor/play_iterator.py b/lib/ansible/executor/play_iterator.py index 584cfb0fe67..09caeec2d98 100644 --- a/lib/ansible/executor/play_iterator.py +++ b/lib/ansible/executor/play_iterator.py @@ -49,6 +49,7 @@ class HostState: self.cur_rescue_task = 0 self.cur_always_task = 0 self.cur_role = None + self.cur_dep_chain = None self.run_state = PlayIterator.ITERATING_SETUP self.fail_state = PlayIterator.FAILED_NONE self.pending_setup = False @@ -102,6 +103,8 @@ class HostState: new_state.run_state = self.run_state new_state.fail_state = self.fail_state new_state.pending_setup = self.pending_setup + if self.cur_dep_chain is not None: + new_state.cur_dep_chain = self.cur_dep_chain[:] if self.tasks_child_state is not None: new_state.tasks_child_state = self.tasks_child_state.copy() if self.rescue_child_state is not None: @@ -212,13 +215,21 @@ class PlayIterator: s.pending_setup = False if not task: + old_s = s (s, task) = self._get_next_task_from_state(s, peek=peek) + def _roles_are_different(ra, rb): + if ra != rb: + return True + else: + return old_s.cur_dep_chain != task._block._dep_chain + if task and task._role: # if we had a current role, mark that role as completed - if s.cur_role and task._role != s.cur_role and host.name in s.cur_role._had_task_run and not peek: + if s.cur_role and _roles_are_different(task._role, s.cur_role) and host.name in s.cur_role._had_task_run and not peek: s.cur_role._completed[host.name] = True s.cur_role = task._role + s.cur_dep_chain = task._block._dep_chain if not peek: self._host_states[host.name] = s diff --git a/lib/ansible/playbook/role/__init__.py b/lib/ansible/playbook/role/__init__.py index 10e14b4ac38..f192ea6c945 100644 --- a/lib/ansible/playbook/role/__init__.py +++ b/lib/ansible/playbook/role/__init__.py @@ -323,7 +323,7 @@ class Role(Base, Become, Conditional, Taggable): return host.name in self._completed and not self._metadata.allow_duplicates - def compile(self, play, dep_chain=[]): + def compile(self, play, dep_chain=None): ''' Returns the task list for this role, which is created by first recursively compiling the tasks for all direct dependencies, and @@ -337,18 +337,20 @@ class Role(Base, Become, Conditional, Taggable): block_list = [] # update the dependency chain here + if dep_chain is None: + dep_chain = [] new_dep_chain = dep_chain + [self] deps = self.get_direct_dependencies() for dep in deps: dep_blocks = dep.compile(play=play, dep_chain=new_dep_chain) - for dep_block in dep_blocks: - new_dep_block = dep_block.copy() - new_dep_block._dep_chain = new_dep_chain - new_dep_block._play = play - block_list.append(new_dep_block) + block_list.extend(dep_blocks) - block_list.extend(self._task_blocks) + for task_block in self._task_blocks: + new_task_block = task_block.copy() + new_task_block._dep_chain = new_dep_chain + new_task_block._play = play + block_list.append(new_task_block) return block_list diff --git a/test/integration/roles/test_var_precedence_dep/tasks/main.yml b/test/integration/roles/test_var_precedence_dep/tasks/main.yml index b50f9dfc271..2f8e17096bc 100644 --- a/test/integration/roles/test_var_precedence_dep/tasks/main.yml +++ b/test/integration/roles/test_var_precedence_dep/tasks/main.yml @@ -7,7 +7,7 @@ - assert: that: - 'extra_var == "extra_var"' - - 'param_var == "param_var"' + - 'param_var == "param_var_role1"' - 'vars_var == "vars_var"' - 'vars_files_var == "vars_files_var"' - 'vars_files_var_role == "vars_files_var_dep"'