From 9cf655f02a38353b5dd429a847cf5b489d25dc93 Mon Sep 17 00:00:00 2001 From: James Cammarata Date: Sun, 12 Jun 2016 18:03:18 -0500 Subject: [PATCH] Track notified handlers by object rather than simply their name Due to the fact that roles may be instantiated with different sets of params (multiple inclusions of the same role or via role dependencies), simply tracking notified handlers by name does not work. This patch changes the way we track handler notifications by using the handler object itself instead of just the name, allowing for multiple internal instances. Normally this would be bad, but we also modify the way we search for handlers by first looking at the notifying tasks dependency chain (ensuring that roles find their own handlers first) and then at the main list of handlers, using the first match it finds. This patch also modifies the way we setup the internal list of handlers, which should allow us to correctly identify if a notified handler exists more easily. Fixes #15084 --- lib/ansible/executor/task_queue_manager.py | 11 ++- lib/ansible/playbook/task.py | 2 +- lib/ansible/plugins/strategy/__init__.py | 73 ++++++++++++------- .../plugins/strategies/test_strategy_base.py | 21 +++++- 4 files changed, 75 insertions(+), 32 deletions(-) diff --git a/lib/ansible/executor/task_queue_manager.py b/lib/ansible/executor/task_queue_manager.py index 8fe74043287e35..d1eb08550901e8 100644 --- a/lib/ansible/executor/task_queue_manager.py +++ b/lib/ansible/executor/task_queue_manager.py @@ -114,13 +114,17 @@ def _initialize_processes(self, num): self._result_prc = ResultProcess(self._final_q, self._workers) self._result_prc.start() - def _initialize_notified_handlers(self, handlers): + def _initialize_notified_handlers(self, play): ''' Clears and initializes the shared notified handlers dict with entries for each handler in the play, which is an empty array that will contain 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() for key in self._notified_handlers.keys(): @@ -141,7 +145,8 @@ def _process_block(b): # then initialize it with the handler names from the handler list for handler in handler_list: - self._notified_handlers[handler.get_name()] = [] + if handler not in self._notified_handlers: + self._notified_handlers[handler] = [] def load_callbacks(self): ''' @@ -226,7 +231,7 @@ def run(self, play): self.send_callback('v2_playbook_on_play_start', new_play) # initialize the shared dictionary containing the notified handlers - self._initialize_notified_handlers(new_play.handlers) + self._initialize_notified_handlers(new_play) # load the specified strategy (or the default linear one) strategy = strategy_loader.get(new_play.strategy, self) diff --git a/lib/ansible/playbook/task.py b/lib/ansible/playbook/task.py index d6eabf366c62d0..309579c85a8137 100644 --- a/lib/ansible/playbook/task.py +++ b/lib/ansible/playbook/task.py @@ -105,7 +105,7 @@ def get_path(self): def get_name(self): ''' return the name of the task ''' - if self._role and self.name: + if self._role and self.name and ("%s : " % self._role._role_name) not in self.name: return "%s : %s" % (self._role.get_name(), self.name) elif self.name: return self.name diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py index 2881bf419ec438..41af1efdf8c030 100644 --- a/lib/ansible/plugins/strategy/__init__.py +++ b/lib/ansible/plugins/strategy/__init__.py @@ -318,11 +318,51 @@ def get_original_host(host): original_host = get_original_host(task_result._host) original_task = iterator.get_original_task(original_host, task_result._task) - if handler_name not in self._notified_handlers: - self._notified_handlers[handler_name] = [] - if original_host not in self._notified_handlers[handler_name]: - self._notified_handlers[handler_name].append(original_host) + def search_handler_blocks(handler_blocks): + for handler_block in handler_blocks: + for handler_task in handler_block.block: + handler_vars = self._variable_manager.get_vars(loader=self._loader, play=iterator._play, task=handler_task) + templar = Templar(loader=self._loader, variables=handler_vars) + try: + # first we check with the full result of get_name(), which may + # include the role name (if the handler is from a role). If that + # is not found, we resort to the simple name field, which doesn't + # have anything extra added to it. + target_handler_name = templar.template(handler_task.name) + if target_handler_name == handler_name: + return handler_task + else: + target_handler_name = templar.template(handler_task.get_name()) + if target_handler_name == handler_name: + return handler_task + except (UndefinedError, AnsibleUndefinedVariable): + # We skip this handler due to the fact that it may be using + # a variable in the name that was conditionally included via + # set_fact or some other method, and we don't want to error + # out unnecessarily + continue + return None + + # Find the handler using the above helper. First we look up the + # 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) + 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,)) elif result[0] == 'register_host_var': @@ -572,25 +612,8 @@ def run_handlers(self, iterator, play_context): # but this may take some work in the iterator and gets tricky when # we consider the ability of meta tasks to flush handlers for handler in handler_block.block: - handler_vars = self._variable_manager.get_vars(loader=self._loader, play=iterator._play, task=handler) - templar = Templar(loader=self._loader, variables=handler_vars) - try: - # first we check with the full result of get_name(), which may - # include the role name (if the handler is from a role). If that - # is not found, we resort to the simple name field, which doesn't - # have anything extra added to it. - handler_name = templar.template(handler.name) - if handler_name not in self._notified_handlers: - handler_name = templar.template(handler.get_name()) - except (UndefinedError, AnsibleUndefinedVariable): - # We skip this handler due to the fact that it may be using - # a variable in the name that was conditionally included via - # set_fact or some other method, and we don't want to error - # out unnecessarily - continue - - if handler_name in self._notified_handlers and len(self._notified_handlers[handler_name]): - result = self._do_handler_run(handler, handler_name, iterator=iterator, play_context=play_context) + if handler in self._notified_handlers and len(self._notified_handlers[handler]): + result = self._do_handler_run(handler, handler.get_name(), iterator=iterator, play_context=play_context) if not result: break return result @@ -608,7 +631,7 @@ def _do_handler_run(self, handler, handler_name, iterator, play_context, notifie handler.name = saved_name if notified_hosts is None: - notified_hosts = self._notified_handlers[handler_name] + notified_hosts = self._notified_handlers[handler] run_once = False try: @@ -671,7 +694,7 @@ def _do_handler_run(self, handler, handler_name, iterator, play_context, notifie continue # wipe the notification list - self._notified_handlers[handler_name] = [] + self._notified_handlers[handler] = [] display.debug("done running handlers, result is: %s" % result) return result diff --git a/test/units/plugins/strategies/test_strategy_base.py b/test/units/plugins/strategies/test_strategy_base.py index 338bcc1fd6a7ab..132f3eb847bf5f 100644 --- a/test/units/plugins/strategies/test_strategy_base.py +++ b/test/units/plugins/strategies/test_strategy_base.py @@ -171,7 +171,10 @@ def _queue_get(*args, **kwargs): mock_tqm._stats = MagicMock() mock_tqm._stats.increment.return_value = None + mock_play = MagicMock() + mock_iterator = MagicMock() + mock_iterator._play = mock_play mock_iterator.mark_host_failed.return_value = None mock_iterator.get_next_task_for_host.return_value = (None, None) @@ -184,6 +187,17 @@ def _queue_get(*args, **kwargs): mock_task._role = None mock_task.ignore_errors = False + mock_handler_task = MagicMock(Handler) + mock_handler_task.action = 'foo' + mock_handler_task.get_name.return_value = "test handler" + mock_handler_task.has_triggered.return_value = False + + mock_handler_block = MagicMock() + mock_handler_block.block = [mock_handler_task] + mock_play.handlers = [mock_handler_block] + + mock_tqm._notified_handlers = {mock_handler_task: []} + mock_group = MagicMock() mock_group.add_host.return_value = None @@ -281,8 +295,8 @@ def _get_group(group_name): self.assertEqual(len(results), 0) self.assertEqual(strategy_base._pending_results, 1) self.assertIn('test01', strategy_base._blocked_hosts) - self.assertIn('test handler', strategy_base._notified_handlers) - self.assertIn(mock_host, strategy_base._notified_handlers['test handler']) + self.assertIn(mock_handler_task, strategy_base._notified_handlers) + self.assertIn(mock_host, strategy_base._notified_handlers[mock_handler_task]) queue_items.append(('set_host_var', mock_host, mock_task, None, 'foo', 'bar')) results = strategy_base._process_pending_results(iterator=mock_iterator) @@ -379,13 +393,14 @@ def fake_run(*args): passwords=None, ) tqm._initialize_processes(3) + tqm._initialize_notified_handlers(mock_play) tqm.hostvars = dict() try: strategy_base = StrategyBase(tqm=tqm) strategy_base._inventory = mock_inventory - strategy_base._notified_handlers = {"test handler": [mock_host]} + strategy_base._notified_handlers = {mock_handler_task: [mock_host]} task_result = TaskResult(Host('host01'), Handler(), dict(changed=False)) tqm._final_q.put(('host_task_ok', task_result))