From 1e5ccb326fa49046da7a06e42734836b99e2d5cc Mon Sep 17 00:00:00 2001 From: David Shrewsbury Date: Mon, 15 Mar 2021 15:39:59 -0400 Subject: [PATCH] Allow for searching handler subdir for included tasks (#73809) * Allow for searching handler subdir for included tasks --- .../fragments/73809-search-handler-subdir.yml | 2 + lib/ansible/playbook/included_file.py | 98 ++++++++++--------- .../handlers/A.yml | 1 + .../handlers/main.yml | 5 + .../tasks/B.yml | 1 + test/integration/targets/handlers/runme.sh | 3 + .../test_role_handlers_including_tasks.yml | 18 ++++ 7 files changed, 81 insertions(+), 47 deletions(-) create mode 100644 changelogs/fragments/73809-search-handler-subdir.yml create mode 100644 test/integration/targets/handlers/roles/test_role_handlers_include_tasks/handlers/A.yml create mode 100644 test/integration/targets/handlers/roles/test_role_handlers_include_tasks/handlers/main.yml create mode 100644 test/integration/targets/handlers/roles/test_role_handlers_include_tasks/tasks/B.yml create mode 100644 test/integration/targets/handlers/test_role_handlers_including_tasks.yml diff --git a/changelogs/fragments/73809-search-handler-subdir.yml b/changelogs/fragments/73809-search-handler-subdir.yml new file mode 100644 index 00000000000..22228dff5c0 --- /dev/null +++ b/changelogs/fragments/73809-search-handler-subdir.yml @@ -0,0 +1,2 @@ +bugfixes: + - A handler defined within a role will now search handlers subdir for included tasks (issue https://github.com/ansible/ansible/issues/71222). diff --git a/lib/ansible/playbook/included_file.py b/lib/ansible/playbook/included_file.py index 2d209deb600..36108c07e03 100644 --- a/lib/ansible/playbook/included_file.py +++ b/lib/ansible/playbook/included_file.py @@ -24,6 +24,7 @@ import os from ansible import constants as C from ansible.errors import AnsibleError from ansible.module_utils._text import to_text +from ansible.playbook.handler import Handler from ansible.playbook.task_include import TaskInclude from ansible.playbook.role_include import IncludeRole from ansible.template import Templar @@ -114,60 +115,63 @@ class IncludedFile: if original_task.action in C._ACTION_ALL_INCLUDE_TASKS: include_file = None - if original_task: - if original_task.static: - continue + if original_task.static: + continue - if original_task._parent: - # handle relative includes by walking up the list of parent include - # tasks and checking the relative result to see if it exists - parent_include = original_task._parent - cumulative_path = None - while parent_include is not None: - if not isinstance(parent_include, TaskInclude): - parent_include = parent_include._parent - continue - if isinstance(parent_include, IncludeRole): - parent_include_dir = parent_include._role_path - else: + if original_task._parent: + # handle relative includes by walking up the list of parent include + # tasks and checking the relative result to see if it exists + parent_include = original_task._parent + cumulative_path = None + while parent_include is not None: + if not isinstance(parent_include, TaskInclude): + parent_include = parent_include._parent + continue + if isinstance(parent_include, IncludeRole): + parent_include_dir = parent_include._role_path + else: + try: + parent_include_dir = os.path.dirname(templar.template(parent_include.args.get('_raw_params'))) + except AnsibleError as e: + parent_include_dir = '' + display.warning( + 'Templating the path of the parent %s failed. The path to the ' + 'included file may not be found. ' + 'The error was: %s.' % (original_task.action, to_text(e)) + ) + if cumulative_path is not None and not os.path.isabs(cumulative_path): + cumulative_path = os.path.join(parent_include_dir, cumulative_path) + else: + cumulative_path = parent_include_dir + include_target = templar.template(include_result['include']) + if original_task._role: + new_basedir = os.path.join(original_task._role._role_path, 'tasks', cumulative_path) + candidates = [loader.path_dwim_relative(original_task._role._role_path, 'tasks', include_target), + loader.path_dwim_relative(new_basedir, 'tasks', include_target)] + for include_file in candidates: try: - parent_include_dir = os.path.dirname(templar.template(parent_include.args.get('_raw_params'))) - except AnsibleError as e: - parent_include_dir = '' - display.warning( - 'Templating the path of the parent %s failed. The path to the ' - 'included file may not be found. ' - 'The error was: %s.' % (original_task.action, to_text(e)) - ) - if cumulative_path is not None and not os.path.isabs(cumulative_path): - cumulative_path = os.path.join(parent_include_dir, cumulative_path) - else: - cumulative_path = parent_include_dir - include_target = templar.template(include_result['include']) - if original_task._role: - new_basedir = os.path.join(original_task._role._role_path, 'tasks', cumulative_path) - candidates = [loader.path_dwim_relative(original_task._role._role_path, 'tasks', include_target), - loader.path_dwim_relative(new_basedir, 'tasks', include_target)] - for include_file in candidates: - try: - # may throw OSError - os.stat(include_file) - # or select the task file if it exists - break - except OSError: - pass - else: - include_file = loader.path_dwim_relative(loader.get_basedir(), cumulative_path, include_target) + # may throw OSError + os.stat(include_file) + # or select the task file if it exists + break + except OSError: + pass + else: + include_file = loader.path_dwim_relative(loader.get_basedir(), cumulative_path, include_target) - if os.path.exists(include_file): - break - else: - parent_include = parent_include._parent + if os.path.exists(include_file): + break + else: + parent_include = parent_include._parent if include_file is None: if original_task._role: include_target = templar.template(include_result['include']) - include_file = loader.path_dwim_relative(original_task._role._role_path, 'tasks', include_target) + include_file = loader.path_dwim_relative( + original_task._role._role_path, + 'handlers' if isinstance(original_task, Handler) else 'tasks', + include_target, + is_role=True) else: include_file = loader.path_dwim(include_result['include']) diff --git a/test/integration/targets/handlers/roles/test_role_handlers_include_tasks/handlers/A.yml b/test/integration/targets/handlers/roles/test_role_handlers_include_tasks/handlers/A.yml new file mode 100644 index 00000000000..b7520c77cbf --- /dev/null +++ b/test/integration/targets/handlers/roles/test_role_handlers_include_tasks/handlers/A.yml @@ -0,0 +1 @@ +- debug: msg="handler with tasks from A.yml called" diff --git a/test/integration/targets/handlers/roles/test_role_handlers_include_tasks/handlers/main.yml b/test/integration/targets/handlers/roles/test_role_handlers_include_tasks/handlers/main.yml new file mode 100644 index 00000000000..ba2e8f69c20 --- /dev/null +++ b/test/integration/targets/handlers/roles/test_role_handlers_include_tasks/handlers/main.yml @@ -0,0 +1,5 @@ +- name: role-based handler from handler subdir + include_tasks: A.yml + +- name: role-based handler from tasks subdir + include_tasks: B.yml diff --git a/test/integration/targets/handlers/roles/test_role_handlers_include_tasks/tasks/B.yml b/test/integration/targets/handlers/roles/test_role_handlers_include_tasks/tasks/B.yml new file mode 100644 index 00000000000..956c9754847 --- /dev/null +++ b/test/integration/targets/handlers/roles/test_role_handlers_include_tasks/tasks/B.yml @@ -0,0 +1 @@ +- debug: msg="handler with tasks from B.yml called" diff --git a/test/integration/targets/handlers/runme.sh b/test/integration/targets/handlers/runme.sh index 59c81bcec09..cefa926b5fd 100755 --- a/test/integration/targets/handlers/runme.sh +++ b/test/integration/targets/handlers/runme.sh @@ -87,6 +87,9 @@ set -e # https://github.com/ansible/ansible/issues/47287 [ "$(ansible-playbook test_handlers_including_task.yml -i ../../inventory -v "$@" | grep -E -o 'failed=[0-9]+')" = "failed=0" ] +# https://github.com/ansible/ansible/issues/71222 +ansible-playbook test_role_handlers_including_tasks.yml -i ../../inventory -v "$@" + # https://github.com/ansible/ansible/issues/27237 set +e result="$(ansible-playbook test_handlers_template_run_once.yml -i inventory.handlers "$@" 2>&1)" diff --git a/test/integration/targets/handlers/test_role_handlers_including_tasks.yml b/test/integration/targets/handlers/test_role_handlers_including_tasks.yml new file mode 100644 index 00000000000..47d8f00ac20 --- /dev/null +++ b/test/integration/targets/handlers/test_role_handlers_including_tasks.yml @@ -0,0 +1,18 @@ +--- +- name: Verify a role handler can include other tasks from handlers and tasks subdirs + hosts: testhost + roles: + - test_role_handlers_include_tasks + + tasks: + - name: notify a role-based handler (include tasks from handler subdir) + debug: + msg: notifying role handler + changed_when: yes + notify: role-based handler from handler subdir + + - name: notify a role-based handler (include tasks from tasks subdir) + debug: + msg: notifying another role handler + changed_when: yes + notify: role-based handler from tasks subdir