Repository: ansible/ansible
Pull request: Template run_once for handlers
##### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
As we do for tasks.

Fixes #27237
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->

##### ISSUE TYPE
<!--- Pick one below and delete the rest -->
- Bugfix Pull Request

##### COMPONENT NAME
<!--- Write the short name of the module, plugin, task or feature below -->
handlers
`lib/ansible/plugins/strategy/__init__.py`


Problem statement:
issue with handlers notification and Jinja2 default value in run_once
##### ISSUE TYPE
 - Bug Report

##### COMPONENT NAME
core

##### ANSIBLE VERSION
```
ansible 2.3.1.0
  config file = 
  configured module search path = Default w/o overrides
  python version = 2.7.12 (default, Nov 19 2016, 06:48:10) [GCC 5.4.0 20160609]
```

##### CONFIGURATION
<!---
Mention any settings you have changed/added/removed in ansible.cfg
(or using the ANSIBLE_* environment variables).
-->

##### OS / ENVIRONMENT
<!---
Mention the OS you are running Ansible from, and the OS you are
managing, or say “N/A” for anything that is not platform-specific.
Also mention the specific version of what you are trying to control,
e.g. if this is a network bug the version of firmware on the network device.
-->
Ubuntu 16.04

##### SUMMARY
The behavior of handlers notification is inconsistent if we add a role with run_once parameter using default value for undefined variables.

##### STEPS TO REPRODUCE
<!---
For bugs, show exactly how to reproduce the problem, using a minimal test-case.
For new features, show how the feature would be used.
-->

./test-inventory
```yaml
[all]
192.168.50.10
192.168.50.20
192.168.50.30
```

./roles/test-role/tasks/main.yml
```yaml
---
- name: test task
  command: /bin/true
  notify:
    - test handler
```
./roles/test-role/handlers/main.yml
```yaml
- name: test handler
  command: /bin/true
```
./test-playbook.yml
```yaml
---
- hosts: all
  roles:
    - { role: test-role, run_once: "{{ testvar | default(false) }}" }
```
<!--- You can also paste gist.github.com links for larger files -->

##### EXPECTED RESULTS
<!--- What did you expect to happen when running the steps above? -->
if we remove the default Jinja2's filter
```
    - { role: test-role, run_once: false }
```
we have the expected result 
the handler is well launched on all hosts :
```
PLAY [all] *************************************************************************************************************************************

TASK [Gathering Facts] *************************************************************************************************************************
ok: [192.168.50.10]
ok: [192.168.50.20]
ok: [192.168.50.30]

TASK [test-role : test task] *******************************************************************************************************************
changed: [192.168.50.30]
changed: [192.168.50.20]
changed: [192.168.50.10]

RUNNING HANDLER [test-role : test handler] *****************************************************************************************************
changed: [192.168.50.20]
changed: [192.168.50.30]
changed: [192.168.50.10]

PLAY RECAP *************************************************************************************************************************************
192.168.50.10              : ok=3    changed=2    unreachable=0    failed=0   
192.168.50.20              : ok=3    changed=2    unreachable=0    failed=0   
192.168.50.30              : ok=3    changed=2    unreachable=0    failed=0   
```

##### ACTUAL RESULTS
the handler is only launched on one host : 
```
PLAY [all] *************************************************************************************************************************************

TASK [Gathering Facts] *************************************************************************************************************************
ok: [192.168.50.20]
ok: [192.168.50.30]
ok: [192.168.50.10]

TASK [test-role : test task] *******************************************************************************************************************
changed: [192.168.50.20]
changed: [192.168.50.30]
changed: [192.168.50.10]

RUNNING HANDLER [test-role : test handler] *****************************************************************************************************
changed: [192.168.50.20]

PLAY RECAP *************************************************************************************************************************************
192.168.50.10              : ok=2    changed=1    unreachable=0    failed=0   
192.168.50.20              : ok=3    changed=2    unreachable=0    failed=0   
192.168.50.30              : ok=2    changed=1    unreachable=0    failed=0   

```
##### TEST IN PREVIOUS VERSIONS
it was ok on ansible 1.9.6: 
```
ansible --version
ansible 1.9.6
  configured module search path = None
```
```
PLAY [all] ******************************************************************** 

GATHERING FACTS *************************************************************** 
ok: [192.168.50.10]
ok: [192.168.50.30]
ok: [192.168.50.20]

TASK: [test-role | test task] ************************************************* 
changed: [192.168.50.30]
changed: [192.168.50.10]
changed: [192.168.50.20]

NOTIFIED: [test-role | test handler] ****************************************** 
changed: [192.168.50.20]
changed: [192.168.50.10]
changed: [192.168.50.30]

PLAY RECAP ******************************************************************** 
192.168.50.10              : ok=3    changed=2    unreachable=0    failed=0   
192.168.50.20              : ok=3    changed=2    unreachable=0    failed=0   
192.168.50.30              : ok=3    changed=2    unreachable=0    failed=0   
```
it was different on 2.0.0.2 (same issue at task level) :
```
ansible 2.0.0.2
  config file = 
  configured module search path = Default w/o overrides
```
```
PLAY ***************************************************************************

TASK [setup] *******************************************************************
ok: [192.168.50.20]
ok: [192.168.50.30]
ok: [192.168.50.10]

TASK [test-role : test task] ***************************************************
changed: [192.168.50.10]

RUNNING HANDLER [test-role : test handler] *************************************
changed: [192.168.50.10]

PLAY RECAP *********************************************************************
192.168.50.10              : ok=3    changed=2    unreachable=0    failed=0   
192.168.50.20              : ok=1    changed=0    unreachable=0    failed=0   
192.168.50.30              : ok=1    changed=0    unreachable=0    failed=0   
```

Hints from issue discussion:
For jinja2 filters you might want to use False as false can end up being interpreted as a string, when using it directly in YAML it will be interpreted as a boolean though.
@bcoca thank for your feedback
the issue still occurs with `    - { role: test-role, run_once: "{{ testvar | default(False) }}" }`
```
PLAY [all] *************************************************************************************************************************************

TASK [Gathering Facts] *************************************************************************************************************************
ok: [192.168.50.10]
ok: [192.168.50.20]
ok: [192.168.50.30]

TASK [test-role : test task] *******************************************************************************************************************
changed: [192.168.50.20]
changed: [192.168.50.10]
changed: [192.168.50.30]

RUNNING HANDLER [test-role : test handler] *****************************************************************************************************
changed: [192.168.50.20]

PLAY RECAP *************************************************************************************************************************************
192.168.50.10              : ok=2    changed=1    unreachable=0    failed=0   
192.168.50.20              : ok=3    changed=2    unreachable=0    failed=0   
192.168.50.30              : ok=2    changed=1    unreachable=0    failed=0   

```

Commit message: Add changelog

Patch to review:
diff --git a/changelogs/fragments/27237-handlers-template-run_once.yaml b/changelogs/fragments/27237-handlers-template-run_once.yaml
new file mode 100644
index 00000000000000..3e15287fdb890a
--- /dev/null
+++ b/changelogs/fragments/27237-handlers-template-run_once.yaml
@@ -0,0 +1,2 @@
+bugfixes:
+  - Fix handlers to allow for templated values in run_once (https://github.com/ansible/ansible/issues/27237)
diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py
index d182c456bd995f..bdf5c5dd08e7f4 100644
--- a/lib/ansible/plugins/strategy/__init__.py
+++ b/lib/ansible/plugins/strategy/__init__.py
@@ -846,9 +846,7 @@ def run_handlers(self, iterator, play_context):
             #        we consider the ability of meta tasks to flush handlers
             for handler in handler_block.block:
                 if handler.notified_hosts:
-                    handler_vars = self._variable_manager.get_vars(play=iterator._play, task=handler)
-                    handler_name = handler.get_name()
-                    result = self._do_handler_run(handler, handler_name, iterator=iterator, play_context=play_context)
+                    result = self._do_handler_run(handler, handler.get_name(), iterator=iterator, play_context=play_context)
                     if not result:
                         break
         return result
@@ -874,7 +872,9 @@ def _do_handler_run(self, handler, handler_name, iterator, play_context, notifie
         run_once = False
         try:
             action = action_loader.get(handler.action, class_only=True)
-            if handler.run_once or getattr(action, 'BYPASS_HOST_LOOP', False):
+            handler_vars = self._variable_manager.get_vars(play=iterator._play, task=handler)
+            templar = Templar(loader=self._loader, variables=handler_vars)
+            if templar.template(handler.run_once) or getattr(action, 'BYPASS_HOST_LOOP', False):
                 run_once = True
         except KeyError:
             # we don't care here, because the action may simply not have a
diff --git a/test/integration/targets/handlers/runme.sh b/test/integration/targets/handlers/runme.sh
index 711f91ee006f63..1bddc079c9e1b8 100755
--- a/test/integration/targets/handlers/runme.sh
+++ b/test/integration/targets/handlers/runme.sh
@@ -75,3 +75,10 @@ set -e
 
 # https://github.com/ansible/ansible/issues/47287
 [ "$(ansible-playbook test_handlers_including_task.yml -i ../../inventory -v "$@" | egrep -o 'failed=[0-9]+')" = "failed=0" ]
+
+# https://github.com/ansible/ansible/issues/27237
+set +e
+result="$(ansible-playbook test_handlers_template_run_once.yml -i inventory.handlers "$@" 2>&1)"
+set -e
+grep -q "handler A" <<< "$result"
+grep -q "handler B" <<< "$result"
diff --git a/test/integration/targets/handlers/test_handlers_template_run_once.yml b/test/integration/targets/handlers/test_handlers_template_run_once.yml
new file mode 100644
index 00000000000000..6edc32e235c84b
--- /dev/null
+++ b/test/integration/targets/handlers/test_handlers_template_run_once.yml
@@ -0,0 +1,12 @@
+- hosts: A,B
+  gather_facts: no
+  tasks:
+    - debug:
+      changed_when: true
+      notify:
+        - handler
+  handlers:
+    - name: handler
+      debug:
+        msg: "handler {{ inventory_hostname }}"
+      run_once: "{{ testvar | default(False) }}"


Code review comments:
diff_hunk: @@ -874,7 +872,9 @@ def _do_handler_run(self, handler, handler_name, iterator, play_context, notifie
         run_once = False
         try:
             action = action_loader.get(handler.action, class_only=True)
-            if handler.run_once or getattr(action, 'BYPASS_HOST_LOOP', False):
+            handler_vars = self._variable_manager.get_vars(play=iterator._play, task=handler)
+            templar = Templar(loader=self._loader, variables=handler_vars)
path: lib/ansible/plugins/strategy/__init__.py
text: @user1:
instead of forcing templating here, when you don't have task_vars yet, just set run_once based on 'bypass' and then template below in 890 with full task_vars. That way you avoid a 2nd very expensive call for vars and actually get the complete ones (with host).

@author:
ah, missed that. Thanks!