diff --git a/changelogs/fragments/any_errors_fatal-fixes.yml b/changelogs/fragments/any_errors_fatal-fixes.yml new file mode 100644 index 00000000000..b0fc4e78a01 --- /dev/null +++ b/changelogs/fragments/any_errors_fatal-fixes.yml @@ -0,0 +1,6 @@ +bugfixes: + - Fix for when ``any_errors_fatal`` was ignored if error occured in a block with always (https://github.com/ansible/ansible/issues/31543) + - Fix ``force_handlers`` not working with ``any_errors_fatal`` (https://github.com/ansible/ansible/issues/36308) + - Fix tasks in always section not being executed for nested blocks with ``any_errors_fatal`` (https://github.com/ansible/ansible/issues/73246) + - "``any_errors_fatal`` should fail all hosts and rescue all of them when a ``rescue`` section is specified (https://github.com/ansible/ansible/issues/80981)" + - Fix issues when tasks withing nested blocks wouldn't run when ``force_handlers`` is set (https://github.com/ansible/ansible/issues/81533) diff --git a/lib/ansible/plugins/strategy/linear.py b/lib/ansible/plugins/strategy/linear.py index f644d8037e2..14d45d405da 100644 --- a/lib/ansible/plugins/strategy/linear.py +++ b/lib/ansible/plugins/strategy/linear.py @@ -31,7 +31,7 @@ DOCUMENTATION = ''' from ansible import constants as C from ansible.errors import AnsibleError, AnsibleAssertionError, AnsibleParserError -from ansible.executor.play_iterator import IteratingStates, FailedStates +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 @@ -354,25 +354,16 @@ class StrategyModule(StrategyBase): failed_hosts = [] unreachable_hosts = [] for res in results: - # execute_meta() does not set 'failed' in the TaskResult - # so we skip checking it with the meta tasks and look just at the iterator - if (res.is_failed() or res._task.action in C._ACTION_META) and iterator.is_failed(res._host): + if res.is_failed(): failed_hosts.append(res._host.name) elif res.is_unreachable(): unreachable_hosts.append(res._host.name) - # if any_errors_fatal and we had an error, mark all hosts as failed - if any_errors_fatal and (len(failed_hosts) > 0 or len(unreachable_hosts) > 0): - dont_fail_states = frozenset([IteratingStates.RESCUE, IteratingStates.ALWAYS]) + if any_errors_fatal and (failed_hosts or unreachable_hosts): for host in hosts_left: - (s, dummy) = iterator.get_next_task_for_host(host, peek=True) - # the state may actually be in a child state, use the get_active_state() - # method in the iterator to figure out the true active state - s = iterator.get_active_state(s) - if s.run_state not in dont_fail_states or \ - s.run_state == IteratingStates.RESCUE and s.fail_state & FailedStates.RESCUE != 0: + if host.name not in failed_hosts: self._tqm._failed_hosts[host.name] = True - result |= self._tqm.RUN_FAILED_BREAK_PLAY + iterator.mark_host_failed(host) display.debug("done checking for any_errors_fatal") display.debug("checking for max_fail_percentage") diff --git a/test/integration/targets/any_errors_fatal/31543.yml b/test/integration/targets/any_errors_fatal/31543.yml new file mode 100644 index 00000000000..19b0ba879e2 --- /dev/null +++ b/test/integration/targets/any_errors_fatal/31543.yml @@ -0,0 +1,12 @@ +- hosts: testhost,testhost2 + gather_facts: false + any_errors_fatal: true + tasks: + - block: + - fail: + when: inventory_hostname == 'testhost' + always: + - debug: + + - debug: + msg: SHOULD NOT HAPPEN diff --git a/test/integration/targets/any_errors_fatal/36308.yml b/test/integration/targets/any_errors_fatal/36308.yml new file mode 100644 index 00000000000..ec5d1ae7848 --- /dev/null +++ b/test/integration/targets/any_errors_fatal/36308.yml @@ -0,0 +1,14 @@ +- hosts: testhost + gather_facts: false + any_errors_fatal: true + force_handlers: true + tasks: + - command: echo + notify: + - handler1 + + - fail: + handlers: + - name: handler1 + debug: + msg: handler1 ran diff --git a/test/integration/targets/any_errors_fatal/73246.yml b/test/integration/targets/any_errors_fatal/73246.yml new file mode 100644 index 00000000000..50f20d9715e --- /dev/null +++ b/test/integration/targets/any_errors_fatal/73246.yml @@ -0,0 +1,11 @@ +- hosts: testhost + gather_facts: false + any_errors_fatal: true + tasks: + - block: + - block: + - fail: + always: + - block: + - debug: + msg: PASSED diff --git a/test/integration/targets/any_errors_fatal/80981.yml b/test/integration/targets/any_errors_fatal/80981.yml new file mode 100644 index 00000000000..51cf8dfe146 --- /dev/null +++ b/test/integration/targets/any_errors_fatal/80981.yml @@ -0,0 +1,17 @@ +- hosts: testhost,testhost2 + gather_facts: false + any_errors_fatal: true + tasks: + - block: + - fail: + when: inventory_hostname == "testhost" + - name: any_errors_fatal fails all hosts when any of them fails + debug: + msg: SHOULD NOT HAPPEN + rescue: + - name: Rescues both hosts + debug: + msg: rescue + - name: You can recover from fatal errors by adding a rescue section to the block. + debug: + msg: recovered diff --git a/test/integration/targets/any_errors_fatal/runme.sh b/test/integration/targets/any_errors_fatal/runme.sh index c54ea8d5e0b..58f0ddfcafa 100755 --- a/test/integration/targets/any_errors_fatal/runme.sh +++ b/test/integration/targets/any_errors_fatal/runme.sh @@ -35,3 +35,17 @@ for test_name in test_include_role test_include_tasks; do exit 1 fi done + +ansible-playbook -i inventory "$@" 31543.yml | tee out.txt +[ "$(grep -c 'SHOULD NOT HAPPEN' out.txt)" -eq 0 ] + +ansible-playbook -i inventory "$@" 36308.yml | tee out.txt +[ "$(grep -c 'handler1 ran' out.txt)" -eq 1 ] + +ansible-playbook -i inventory "$@" 73246.yml | tee out.txt +[ "$(grep -c 'PASSED' out.txt)" -eq 1 ] + +ansible-playbook -i inventory "$@" 80981.yml | tee out.txt +[ "$(grep -c 'SHOULD NOT HAPPEN' out.txt)" -eq 0 ] +[ "$(grep -c 'rescue' out.txt)" -eq 2 ] +[ "$(grep -c 'recovered' out.txt)" -eq 2 ] diff --git a/test/integration/targets/handlers/force_handlers_blocks_81533-1.yml b/test/integration/targets/handlers/force_handlers_blocks_81533-1.yml new file mode 100644 index 00000000000..20605e451e0 --- /dev/null +++ b/test/integration/targets/handlers/force_handlers_blocks_81533-1.yml @@ -0,0 +1,16 @@ +- hosts: A,B + gather_facts: false + force_handlers: true + tasks: + - fail: + when: inventory_hostname == "A" + + - run_once: true + block: + - debug: + msg: task1 + - debug: + msg: task2 + + - debug: + msg: hosts_left diff --git a/test/integration/targets/handlers/force_handlers_blocks_81533-2.yml b/test/integration/targets/handlers/force_handlers_blocks_81533-2.yml new file mode 100644 index 00000000000..4284a3751c3 --- /dev/null +++ b/test/integration/targets/handlers/force_handlers_blocks_81533-2.yml @@ -0,0 +1,12 @@ +- hosts: A,B + gather_facts: false + force_handlers: true + tasks: + - fail: + when: inventory_hostname == "A" + + - meta: clear_host_errors + when: false + + - debug: + msg: hosts_left diff --git a/test/integration/targets/handlers/runme.sh b/test/integration/targets/handlers/runme.sh index 757200de385..e26fdd7a1be 100755 --- a/test/integration/targets/handlers/runme.sh +++ b/test/integration/targets/handlers/runme.sh @@ -198,3 +198,11 @@ ansible-playbook test_include_tasks_in_include_role.yml "$@" 2>&1 | tee out.txt ansible-playbook test_run_once.yml -i inventory.handlers "$@" 2>&1 | tee out.txt [ "$(grep out.txt -ce 'handler ran once')" = "1" ] + +ansible-playbook force_handlers_blocks_81533-1.yml -i inventory.handlers "$@" 2>&1 | tee out.txt +[ "$(grep out.txt -ce 'task1')" = "1" ] +[ "$(grep out.txt -ce 'task2')" = "1" ] +[ "$(grep out.txt -ce 'hosts_left')" = "1" ] + +ansible-playbook force_handlers_blocks_81533-2.yml -i inventory.handlers "$@" 2>&1 | tee out.txt +[ "$(grep out.txt -ce 'hosts_left')" = "1" ]