From ef8c9798d3399605284f3357069769b67330fd81 Mon Sep 17 00:00:00 2001 From: Will Thames Date: Thu, 20 Jul 2017 06:02:32 +1000 Subject: [PATCH] include_role handlers bug fix (#26335) * Ensure that include_role properly fires handlers include_role needs to ensure that any handlers included with the role are added to the _notified_handler and _listening_handler lists of the TaskQueueManager, otherwise it fails when trying to run the handler. Additionally, the handler needs to be added to the PlayIterator's `_uuid_cache` or it fails after running the handler Add more uuid debug statements - this code was hard to debug with existing debug statements, so add more uuid information at little additional output cost. Fixes #18411 * Add tests for include_role handlers Tests for #18411 --- lib/ansible/executor/process/worker.py | 6 +-- lib/ansible/executor/task_executor.py | 2 +- lib/ansible/executor/task_queue_manager.py | 6 ++- lib/ansible/playbook/helpers.py | 3 +- lib/ansible/playbook/role_include.py | 6 +-- lib/ansible/plugins/strategy/__init__.py | 4 ++ lib/ansible/plugins/strategy/linear.py | 4 +- test/integration/targets/handlers/aliases | 1 + .../handlers/main.yml | 5 ++ .../test_handlers_include_role/meta/main.yml | 1 + .../test_handlers_include_role/tasks/main.yml | 47 +++++++++++++++++++ test/integration/targets/handlers/runme.sh | 3 ++ .../handlers/test_handlers_include_role.yml | 8 ++++ 13 files changed, 86 insertions(+), 10 deletions(-) create mode 100644 test/integration/targets/handlers/roles/test_handlers_include_role/handlers/main.yml create mode 100644 test/integration/targets/handlers/roles/test_handlers_include_role/meta/main.yml create mode 100644 test/integration/targets/handlers/roles/test_handlers_include_role/tasks/main.yml create mode 100644 test/integration/targets/handlers/test_handlers_include_role.yml diff --git a/lib/ansible/executor/process/worker.py b/lib/ansible/executor/process/worker.py index b8c7a5b981..f4d26caf2a 100644 --- a/lib/ansible/executor/process/worker.py +++ b/lib/ansible/executor/process/worker.py @@ -118,7 +118,7 @@ class WorkerProcess(multiprocessing.Process): self._rslt_q ).run() - display.debug("done running TaskExecutor() for %s/%s" % (self._host, self._task)) + display.debug("done running TaskExecutor() for %s/%s [%s]" % (self._host, self._task, self._task._uuid)) self._host.vars = dict() self._host.groups = [] task_result = TaskResult( @@ -129,9 +129,9 @@ class WorkerProcess(multiprocessing.Process): ) # put the result on the result queue - display.debug("sending task result") + display.debug("sending task result for task %s" % self._task._uuid) self._rslt_q.put(task_result) - display.debug("done sending task result") + display.debug("done sending task result for task %s" % self._task._uuid) except AnsibleConnectionFailure: self._host.vars = dict() diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py index 69012e7e54..f4e96e2065 100644 --- a/lib/ansible/executor/task_executor.py +++ b/lib/ansible/executor/task_executor.py @@ -81,7 +81,7 @@ class TaskExecutor: returned as a dict. ''' - display.debug("in run()") + display.debug("in run() - task %s" % self._task._uuid) try: try: diff --git a/lib/ansible/executor/task_queue_manager.py b/lib/ansible/executor/task_queue_manager.py index f5a6886e1c..49b84364bb 100644 --- a/lib/ansible/executor/task_queue_manager.py +++ b/lib/ansible/executor/task_queue_manager.py @@ -137,10 +137,13 @@ class TaskQueueManager: handler_list = [] for handler_block in play.handlers: handler_list.extend(_process_block(handler_block)) - # then initialize it with the given handler list + self.update_handler_list(handler_list) + + def update_handler_list(self, handler_list): for handler in handler_list: if handler._uuid not in self._notified_handlers: + display.debug("Adding handler %s to notified list" % handler.name) self._notified_handlers[handler._uuid] = [] if handler.listen: listeners = handler.listen @@ -149,6 +152,7 @@ class TaskQueueManager: for listener in listeners: if listener not in self._listening_handlers: self._listening_handlers[listener] = [] + display.debug("Adding handler %s to listening list" % handler.name) self._listening_handlers[listener].append(handler._uuid) def load_callbacks(self): diff --git a/lib/ansible/playbook/helpers.py b/lib/ansible/playbook/helpers.py index abfa4088d4..2cdc26a165 100644 --- a/lib/ansible/playbook/helpers.py +++ b/lib/ansible/playbook/helpers.py @@ -319,7 +319,8 @@ def load_list_of_tasks(ds, play, block=None, role=None, task_include=None, use_h if is_static: # uses compiled list from object - t = task_list.extend(ir.get_block_list(variable_manager=variable_manager, loader=loader)) + blocks, _ = ir.get_block_list(variable_manager=variable_manager, loader=loader) + t = task_list.extend(blocks) else: # passes task object itself for latter generation of list t = task_list.append(ir) diff --git a/lib/ansible/playbook/role_include.py b/lib/ansible/playbook/role_include.py index bfa6950402..a2309b914f 100644 --- a/lib/ansible/playbook/role_include.py +++ b/lib/ansible/playbook/role_include.py @@ -87,9 +87,9 @@ class IncludeRole(TaskInclude): b._parent = self # updated available handlers in play - myplay.handlers = myplay.handlers + actual_role.get_handler_blocks(play=myplay) - - return blocks + handlers = actual_role.get_handler_blocks(play=myplay) + myplay.handlers = myplay.handlers + handlers + return blocks, handlers @staticmethod def load(data, block=None, role=None, task_include=None, variable_manager=None, loader=None): diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py index 786ec66447..ea8a94ea59 100644 --- a/lib/ansible/plugins/strategy/__init__.py +++ b/lib/ansible/plugins/strategy/__init__.py @@ -450,6 +450,8 @@ class StrategyBase: target_handler = search_handler_blocks_by_name(handler_name, iterator._play.handlers) if target_handler is not None: found = True + if target_handler._uuid not in self._notified_handlers: + self._notified_handlers[target_handler._uuid] = [] if original_host not in self._notified_handlers[target_handler._uuid]: self._notified_handlers[target_handler._uuid].append(original_host) # FIXME: should this be a callback? @@ -761,6 +763,8 @@ class StrategyBase: host_results = [] for host in notified_hosts: if not handler.has_triggered(host) and (not iterator.is_failed(host) or play_context.force_handlers): + if handler._uuid not in iterator._task_uuid_cache: + iterator._task_uuid_cache[handler._uuid] = handler task_vars = self._variable_manager.get_vars(play=iterator._play, host=host, task=handler) self.add_tqm_variables(task_vars, play=iterator._play) self._queue_task(host, handler, task_vars, play_context) diff --git a/lib/ansible/plugins/strategy/linear.py b/lib/ansible/plugins/strategy/linear.py index c2d3609ebe..76bb946ab8 100644 --- a/lib/ansible/plugins/strategy/linear.py +++ b/lib/ansible/plugins/strategy/linear.py @@ -315,7 +315,9 @@ class StrategyModule(StrategyBase): if loop_var and loop_var in include_result: new_ir.vars[loop_var] = include_result[loop_var] - all_role_blocks.extend(new_ir.get_block_list(play=iterator._play, variable_manager=self._variable_manager, loader=self._loader)) + 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: diff --git a/test/integration/targets/handlers/aliases b/test/integration/targets/handlers/aliases index 79d8b9285e..73027b2573 100644 --- a/test/integration/targets/handlers/aliases +++ b/test/integration/targets/handlers/aliases @@ -1 +1,2 @@ posix/ci/group3 +handlers diff --git a/test/integration/targets/handlers/roles/test_handlers_include_role/handlers/main.yml b/test/integration/targets/handlers/roles/test_handlers_include_role/handlers/main.yml new file mode 100644 index 0000000000..0261f93569 --- /dev/null +++ b/test/integration/targets/handlers/roles/test_handlers_include_role/handlers/main.yml @@ -0,0 +1,5 @@ +- name: set handler fact + set_fact: + handler_called: True +- name: test handler + debug: msg="handler called" diff --git a/test/integration/targets/handlers/roles/test_handlers_include_role/meta/main.yml b/test/integration/targets/handlers/roles/test_handlers_include_role/meta/main.yml new file mode 100644 index 0000000000..32cf5dda7e --- /dev/null +++ b/test/integration/targets/handlers/roles/test_handlers_include_role/meta/main.yml @@ -0,0 +1 @@ +dependencies: [] diff --git a/test/integration/targets/handlers/roles/test_handlers_include_role/tasks/main.yml b/test/integration/targets/handlers/roles/test_handlers_include_role/tasks/main.yml new file mode 100644 index 0000000000..fbc3d1c5a5 --- /dev/null +++ b/test/integration/targets/handlers/roles/test_handlers_include_role/tasks/main.yml @@ -0,0 +1,47 @@ +# This file is part of Ansible +# +# Ansible is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Ansible is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Ansible. If not, see . + + +- name: reset handler_called variable to false for all hosts + set_fact: + handler_called: False + tags: scenario1 + +- name: notify the handler for host A only + shell: echo + notify: + - set handler fact + when: inventory_hostname == 'A' + tags: scenario1 + +- name: force handler execution now + meta: "flush_handlers" + tags: scenario1 + +- debug: var=handler_called + tags: scenario1 + +- name: validate the handler only ran on one host + assert: + that: + - "inventory_hostname == 'A' and handler_called == True or handler_called == False" + tags: scenario1 + +# item below is passed in by the playbook that calls this +- name: 'test notify with loop' + debug: msg='a task' + changed_when: item == 1 + notify: test handler + tags: scenario2 diff --git a/test/integration/targets/handlers/runme.sh b/test/integration/targets/handlers/runme.sh index bafdbcce30..726d8094c7 100755 --- a/test/integration/targets/handlers/runme.sh +++ b/test/integration/targets/handlers/runme.sh @@ -42,6 +42,9 @@ ansible-playbook test_listening_handlers.yml -i inventory.handlers -v "$@" [ "$(ansible-playbook test_handlers_include.yml -i ../../inventory -v "$@" --tags role_include_handlers \ | egrep -o 'RUNNING HANDLER \[test_handlers_include : .*?]')" = "RUNNING HANDLER [test_handlers_include : test handler]" ] +[ "$(ansible-playbook test_handlers_include_role.yml -i ../../inventory -v "$@" \ +| egrep -o 'RUNNING HANDLER \[test_handlers_include_role : .*?]')" = "RUNNING HANDLER [test_handlers_include_role : test handler]" ] + # Notify handler listen ansible-playbook test_handlers_listen.yml -i inventory.handlers -v "$@" diff --git a/test/integration/targets/handlers/test_handlers_include_role.yml b/test/integration/targets/handlers/test_handlers_include_role.yml new file mode 100644 index 0000000000..77e6b53ada --- /dev/null +++ b/test/integration/targets/handlers/test_handlers_include_role.yml @@ -0,0 +1,8 @@ +- name: verify that play can include handler + hosts: testhost + tasks: + - include_role: + name: test_handlers_include_role + with_items: + - 1 + - 2