Ensure handlers run when meta tasks are defined and add handler integration tests

Fixes #6678
Fixes #6670

lib/ansible/playbook/__init__.py

@@ -611,8 +611,12 @@ class PlayBook(object):
 
             for task in play.tasks():
 
-                # skip handlers until play is finished
                 if task.meta is not None:
+                    # meta tasks can force handlers to run mid-play
+                    if task.meta == 'flush_handlers':
+                        self.run_handlers(play)
+
+                    # skip calling the handler till the play is finished
                     continue
 
                 # only run the task if the requested tags match
@@ -661,14 +665,20 @@ class PlayBook(object):
                         task_errors = True
                         break
                     else:
+                        self.callbacks.on_no_hosts_remaining()
                         return False
 
-        if task_errors and not self.force_handlers:
-            return False
-        else:
+            # lift restrictions after each play finishes
             self.inventory.lift_also_restriction()
-            if not self.run_handlers(play):
+
+            if task_errors and not self.force_handlers:
+                # if there were failed tasks and handler execution
+                # is not forced, quit the play with an error
                 return False
+            else:
+                # no errors, go ahead and execute all handlers
+                if not self.run_handlers(play):
+                    return False
 
         return True
 
@@ -679,38 +689,35 @@ class PlayBook(object):
         for task in play.tasks():
             if task.meta is not None:
 
-                # meta tasks are an internalism and are not valid for end-user playbook usage
-                # here a meta task is a placeholder that signals handlers should be run
-
-                if task.meta == 'flush_handlers':
-                    fired_names = {}
-                    for handler in play.handlers():
-                        if len(handler.notified_by) > 0:
-                            self.inventory.restrict_to(handler.notified_by)
-
-                            # Resolve the variables first
-                            handler_name = template(play.basedir, handler.name, handler.module_vars)
-                            if handler_name not in fired_names:
-                                self._run_task(play, handler, True)
-                            # prevent duplicate handler includes from running more than once
-                            fired_names[handler_name] = 1
-
-                            host_list = self._trim_unavailable_hosts(play._play_hosts)
-                            if handler.any_errors_fatal and len(host_list) < hosts_count:
-                                play.max_fail_pct = 0
-                            if (hosts_count - len(host_list)) > int((play.max_fail_pct)/100.0 * hosts_count):
-                                host_list = None
-                            if not host_list and not self.force_handlers:
-                                self.callbacks.on_no_hosts_remaining()
-                                return False
-
-                            self.inventory.lift_restriction()
-                            new_list = handler.notified_by[:]
-                            for host in handler.notified_by:
-                                if host in on_hosts:
-                                    while host in new_list:
-                                        new_list.remove(host)
-                            handler.notified_by = new_list
+                fired_names = {}
+                for handler in play.handlers():
+                    if len(handler.notified_by) > 0:
+                        self.inventory.restrict_to(handler.notified_by)
+
+                        # Resolve the variables first
+                        handler_name = template(play.basedir, handler.name, handler.module_vars)
+                        if handler_name not in fired_names:
+                            self._run_task(play, handler, True)
+                        # prevent duplicate handler includes from running more than once
+                        fired_names[handler_name] = 1
+
+                        host_list = self._trim_unavailable_hosts(play._play_hosts)
+                        if handler.any_errors_fatal and len(host_list) < hosts_count:
+                            play.max_fail_pct = 0
+                        if (hosts_count - len(host_list)) > int((play.max_fail_pct)/100.0 * hosts_count):
+                            host_list = None
+                        if not host_list and not self.force_handlers:
+                            self.callbacks.on_no_hosts_remaining()
+                            return False
+
+                        self.inventory.lift_restriction()
+                        new_list = handler.notified_by[:]
+                        for host in handler.notified_by:
+                            if host in on_hosts:
+                                while host in new_list:
+                                    new_list.remove(host)
+                        handler.notified_by = new_list
+
+                continue
 
-                    continue
         return True

test/integration/Makefile

@@ -14,7 +14,7 @@ else
 CREDENTIALS_ARG =
 endif
 
-all: non_destructive destructive check_mode test_hash
+all: non_destructive destructive check_mode test_hash test_handlers
 
 non_destructive:
 	ansible-playbook non_destructive.yml -i $(INVENTORY) -e @$(VARS_FILE) $(CREDENTIALS_ARG) -v $(TEST_FLAGS)
@@ -25,6 +25,9 @@ destructive:
 check_mode:
 	ansible-playbook check_mode.yml -i $(INVENTORY) -e @$(VARS_FILE) $(CREDENTIALS_ARG) -v --check $(TEST_FLAGS)
 
+test_handlers:
+	ansible-playbook test_handlers.yml -i inventory.handlers -e @$(VARS_FILE) $(CREDENTIALS_ARG) -v $(TEST_FLAGS)
+
 test_hash:
 	ANSIBLE_HASH_BEHAVIOUR=replace ansible-playbook test_hash.yml -i $(INVENTORY) $(CREDENTIALS_ARG) -v -e '{"test_hash":{"extra_args":"this is an extra arg"}}'
 	ANSIBLE_HASH_BEHAVIOUR=merge ansible-playbook test_hash.yml -i $(INVENTORY) $(CREDENTIALS_ARG) -v -e '{"test_hash":{"extra_args":"this is an extra arg"}}'

test/integration/inventory.handlers

@@ -0,0 +1,6 @@
+[testgroup]
+A
+B
+C
+D
+E

test/integration/roles/test_handlers_meta/handlers/main.yml

@@ -0,0 +1,7 @@
+- name: set_handler_fact_1
+  set_fact: 
+    handler1_called: True
+
+- name: set_handler_fact_2
+  set_fact:
+    handler2_called: True

test/integration/roles/test_handlers_meta/meta/main.yml

@@ -0,0 +1,3 @@
+dependencies: 
+  - prepare_tests
+

test/integration/roles/test_handlers_meta/tasks/main.yml

@@ -0,0 +1,41 @@
+# test code for the async keyword
+# (c) 2014, James Tanner <tanner.jc@gmail.com>
+
+# 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 <http://www.gnu.org/licenses/>.
+
+- name: notify the first handler
+  shell: echo
+  notify: 
+    - set_handler_fact_1
+
+- name: force handler execution now
+  meta: "flush_handlers"
+
+- name: assert handler1 ran and not handler2 
+  assert:
+    that:
+        - "handler1_called is defined"
+        - "handler2_called is not defined"
+
+- name: reset handler1_called
+  set_fact:
+    handler1_called: False
+
+- name: notify the second handler
+  shell: echo
+  notify: 
+    - set_handler_fact_2
+

test/integration/test_handlers.yml

@@ -0,0 +1,24 @@
+---
+- name: run handlers
+  hosts: A
+  gather_facts: False
+  connection: local
+  roles:
+  - { role: test_handlers_meta }
+
+- name: verify final handler was run
+  hosts: A
+  gather_facts: False
+  connection: local
+  tasks:
+    - name: verify handler2 ran
+      assert: 
+        that:
+            - "not hostvars[inventory_hostname]['handler1_called']"
+            - "'handler2_called' in hostvars[inventory_hostname]"
+
+#- hosts: testgroup
+#  gather_facts: False
+#  connection: local
+#  roles:
+#  - { role: test_handlers_meta }