From feebe73ede6239f2a6b605f25c67bde258b3e22c Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Wed, 6 Jul 2016 14:40:11 -0500 Subject: [PATCH] Fix the way handlers are compiled and found/notified * Instead of rebuilding the handler list all over the place, we now compile the handlers at the point the play is post-validated so that the view of the play in the PlayIterator contains the definitive list * Assign the dep_chain to the handlers as they're compiling, just as we do for regular tasks (cherry picked from commit 930d0905074ccf6537d0ecf14259a7da6a9559fb) --- lib/ansible/executor/task_queue_manager.py | 7 ++----- lib/ansible/playbook/play.py | 2 +- lib/ansible/playbook/role/__init__.py | 18 +++++++++++++++--- lib/ansible/plugins/strategy/__init__.py | 21 ++++++++------------- 4 files changed, 26 insertions(+), 22 deletions(-) diff --git a/lib/ansible/executor/task_queue_manager.py b/lib/ansible/executor/task_queue_manager.py index 651ee20d89b..5415411c315 100644 --- a/lib/ansible/executor/task_queue_manager.py +++ b/lib/ansible/executor/task_queue_manager.py @@ -121,10 +121,6 @@ class TaskQueueManager: inventory hostnames for those hosts triggering the handler. ''' - handlers = play.handlers - for role in play.roles: - handlers.extend(role._handler_blocks) - # Zero the dictionary first by removing any entries there. # Proxied dicts don't support iteritems, so we have to use keys() self._notified_handlers.clear() @@ -139,7 +135,7 @@ class TaskQueueManager: return temp_list handler_list = [] - for handler_block in handlers: + for handler_block in play.handlers: handler_list.extend(_process_block(handler_block)) # then initialize it with the given handler list @@ -210,6 +206,7 @@ class TaskQueueManager: new_play = play.copy() new_play.post_validate(templar) + new_play.handlers = new_play.compile_roles_handlers() + new_play.handlers self.hostvars = HostVars( inventory=self._inventory, diff --git a/lib/ansible/playbook/play.py b/lib/ansible/playbook/play.py index a85ab7fe648..2031465eef8 100644 --- a/lib/ansible/playbook/play.py +++ b/lib/ansible/playbook/play.py @@ -265,7 +265,7 @@ class Play(Base, Taggable, Become): if len(self.roles) > 0: for r in self.roles: - block_list.extend(r.get_handler_blocks()) + block_list.extend(r.get_handler_blocks(play=self)) return block_list diff --git a/lib/ansible/playbook/role/__init__.py b/lib/ansible/playbook/role/__init__.py index 6793feb343c..3f1908c754a 100644 --- a/lib/ansible/playbook/role/__init__.py +++ b/lib/ansible/playbook/role/__init__.py @@ -301,12 +301,24 @@ class Role(Base, Become, Conditional, Taggable): def get_task_blocks(self): return self._task_blocks[:] - def get_handler_blocks(self): + def get_handler_blocks(self, play, dep_chain=None): block_list = [] + + # update the dependency chain here + if dep_chain is None: + dep_chain = [] + new_dep_chain = dep_chain + [self] + for dep in self.get_direct_dependencies(): - dep_blocks = dep.get_handler_blocks() + dep_blocks = dep.get_handler_blocks(play=play, dep_chain=new_dep_chain) block_list.extend(dep_blocks) - block_list.extend(self._handler_blocks) + + for task_block in self._handler_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 def has_run(self, host): diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py index 23dcec9f3d8..2ba53aef478 100644 --- a/lib/ansible/plugins/strategy/__init__.py +++ b/lib/ansible/plugins/strategy/__init__.py @@ -348,22 +348,17 @@ class StrategyBase: # dependency chain of the current task (if it's from a role), otherwise # we just look through the list of handlers in the current play/all # roles and use the first one that matches the notify name - target_handler = None - if original_task._role: - target_handler = search_handler_blocks(original_task._role.get_handler_blocks()) - if target_handler is None: - target_handler = search_handler_blocks(iterator._play.handlers) + target_handler = search_handler_blocks(iterator._play.handlers) if target_handler is None: raise AnsibleError("The requested handler '%s' was not found in any of the known handlers" % handler_name) - # FIXME: this should be an error now in 2.1+ - if target_handler not in self._notified_handlers: - self._notified_handlers[target_handler] = [] - - if original_host not in self._notified_handlers[target_handler]: - self._notified_handlers[target_handler].append(original_host) - # FIXME: should this be a callback? - display.vv("NOTIFIED HANDLER %s" % (handler_name,)) + if target_handler in self._notified_handlers: + if original_host not in self._notified_handlers[target_handler]: + self._notified_handlers[target_handler].append(original_host) + # FIXME: should this be a callback? + display.vv("NOTIFIED HANDLER %s" % (handler_name,)) + else: + raise AnsibleError("The requested handler '%s' was not found in the main handlers list" % handler_name) elif result[0] == 'register_host_var': # essentially the same as 'set_host_var' below, however we