From 775bc1110ea245dd8c9be8b92c91b3d748a27ab2 Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Thu, 4 Jul 2024 09:33:37 +0200 Subject: [PATCH] linear: fix included handlers executing in lockstep (#83209) Fixes #83019 --- .../83019-linear-handlers-lockstep-fix.yml | 2 + lib/ansible/plugins/strategy/linear.py | 66 ++++++------------- ...handlers_lockstep_83019-include-nested.yml | 3 + .../handlers_lockstep_83019-include.yml | 6 ++ .../handlers/handlers_lockstep_83019.yml | 8 +++ test/integration/targets/handlers/runme.sh | 3 + 6 files changed, 43 insertions(+), 45 deletions(-) create mode 100644 changelogs/fragments/83019-linear-handlers-lockstep-fix.yml create mode 100644 test/integration/targets/handlers/handlers_lockstep_83019-include-nested.yml create mode 100644 test/integration/targets/handlers/handlers_lockstep_83019-include.yml create mode 100644 test/integration/targets/handlers/handlers_lockstep_83019.yml diff --git a/changelogs/fragments/83019-linear-handlers-lockstep-fix.yml b/changelogs/fragments/83019-linear-handlers-lockstep-fix.yml new file mode 100644 index 00000000000..5ee00904199 --- /dev/null +++ b/changelogs/fragments/83019-linear-handlers-lockstep-fix.yml @@ -0,0 +1,2 @@ +bugfixes: + - "linear strategy: fix handlers included via ``include_tasks`` handler to be executed in lockstep (https://github.com/ansible/ansible/issues/83019)" diff --git a/lib/ansible/plugins/strategy/linear.py b/lib/ansible/plugins/strategy/linear.py index d9e5d425ac8..3c974e91954 100644 --- a/lib/ansible/plugins/strategy/linear.py +++ b/lib/ansible/plugins/strategy/linear.py @@ -31,7 +31,6 @@ DOCUMENTATION = ''' from ansible import constants as C from ansible.errors import AnsibleError, AnsibleAssertionError, AnsibleParserError -from ansible.executor.play_iterator import IteratingStates from ansible.module_utils.common.text.converters import to_text from ansible.playbook.handler import Handler from ansible.playbook.included_file import IncludedFile @@ -46,12 +45,6 @@ display = Display() class StrategyModule(StrategyBase): - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - - # used for the lockstep to indicate to run handlers - self._in_handlers = False - def _get_next_task_lockstep(self, hosts, iterator): ''' Returns a list of (host, task) tuples, where the task may @@ -73,52 +66,35 @@ class StrategyModule(StrategyBase): if not state_task_per_host: return [(h, None) for h in hosts] - if self._in_handlers and not any(filter( - lambda rs: rs == IteratingStates.HANDLERS, - (s.run_state for s, dummy in state_task_per_host.values())) - ): - self._in_handlers = False - - if self._in_handlers: - lowest_cur_handler = min( - s.cur_handlers_task for s, t in state_task_per_host.values() - if s.run_state == IteratingStates.HANDLERS - ) - else: - task_uuids = [t._uuid for s, t in state_task_per_host.values()] - _loop_cnt = 0 - while _loop_cnt <= 1: - try: - cur_task = iterator.all_tasks[iterator.cur_task] - except IndexError: - # pick up any tasks left after clear_host_errors - iterator.cur_task = 0 - _loop_cnt += 1 - else: - iterator.cur_task += 1 - if cur_task._uuid in task_uuids: - break + task_uuids = {t._uuid for s, t in state_task_per_host.values()} + _loop_cnt = 0 + while _loop_cnt <= 1: + try: + cur_task = iterator.all_tasks[iterator.cur_task] + except IndexError: + # pick up any tasks left after clear_host_errors + iterator.cur_task = 0 + _loop_cnt += 1 else: - # prevent infinite loop - raise AnsibleAssertionError( - 'BUG: There seems to be a mismatch between tasks in PlayIterator and HostStates.' - ) + iterator.cur_task += 1 + if cur_task._uuid in task_uuids: + break + else: + # prevent infinite loop + raise AnsibleAssertionError( + 'BUG: There seems to be a mismatch between tasks in PlayIterator and HostStates.' + ) host_tasks = [] for host, (state, task) in state_task_per_host.items(): - if ((self._in_handlers and lowest_cur_handler == state.cur_handlers_task) or - (not self._in_handlers and cur_task._uuid == task._uuid)): + if cur_task._uuid == task._uuid: iterator.set_state_for_host(host.name, state) host_tasks.append((host, task)) else: host_tasks.append((host, noop_task)) - # once hosts synchronize on 'flush_handlers' lockstep enters - # '_in_handlers' phase where handlers are run instead of tasks - # until at least one host is in IteratingStates.HANDLERS - if (not self._in_handlers and cur_task.action in C._ACTION_META and - cur_task.args.get('_raw_params') == 'flush_handlers'): - self._in_handlers = True + if cur_task.action in C._ACTION_META and cur_task.args.get('_raw_params') == 'flush_handlers': + iterator.all_tasks[iterator.cur_task:iterator.cur_task] = [h for b in iterator._play.handlers for h in b.block] return host_tasks @@ -310,7 +286,7 @@ class StrategyModule(StrategyBase): final_block = new_block.filter_tagged_tasks(task_vars) display.debug("done filtering new block on tags") - included_tasks.extend(final_block.get_tasks()) + included_tasks.extend(final_block.get_tasks()) for host in hosts_left: if host in included_file._hosts: diff --git a/test/integration/targets/handlers/handlers_lockstep_83019-include-nested.yml b/test/integration/targets/handlers/handlers_lockstep_83019-include-nested.yml new file mode 100644 index 00000000000..bc763b9fe20 --- /dev/null +++ b/test/integration/targets/handlers/handlers_lockstep_83019-include-nested.yml @@ -0,0 +1,3 @@ +- name: handler1 + debug: + msg: handler1 diff --git a/test/integration/targets/handlers/handlers_lockstep_83019-include.yml b/test/integration/targets/handlers/handlers_lockstep_83019-include.yml new file mode 100644 index 00000000000..06acb3c96a2 --- /dev/null +++ b/test/integration/targets/handlers/handlers_lockstep_83019-include.yml @@ -0,0 +1,6 @@ +- include_tasks: handlers_lockstep_83019-include-nested.yml + when: inventory_hostname == "A" + +- name: handler2 + debug: + msg: handler2 diff --git a/test/integration/targets/handlers/handlers_lockstep_83019.yml b/test/integration/targets/handlers/handlers_lockstep_83019.yml new file mode 100644 index 00000000000..f7cf6b5a87f --- /dev/null +++ b/test/integration/targets/handlers/handlers_lockstep_83019.yml @@ -0,0 +1,8 @@ +- hosts: A,B + gather_facts: false + tasks: + - command: echo + notify: handler + handlers: + - name: handler + include_tasks: handlers_lockstep_83019-include.yml diff --git a/test/integration/targets/handlers/runme.sh b/test/integration/targets/handlers/runme.sh index 9250fc8fb34..6b4e8fb3b31 100755 --- a/test/integration/targets/handlers/runme.sh +++ b/test/integration/targets/handlers/runme.sh @@ -219,3 +219,6 @@ ansible-playbook 82241.yml -i inventory.handlers "$@" 2>&1 | tee out.txt ansible-playbook handlers_lockstep_82307.yml -i inventory.handlers "$@" 2>&1 | tee out.txt [ "$(grep out.txt -ce 'TASK \[handler2\]')" = "0" ] + +ansible-playbook handlers_lockstep_83019.yml -i inventory.handlers "$@" 2>&1 | tee out.txt +[ "$(grep out.txt -ce 'TASK \[handler1\]')" = "0" ]