From 393412dc6492945c3f726de0ce4b8bb4128953ea Mon Sep 17 00:00:00 2001 From: David Shrewsbury Date: Fri, 28 Aug 2020 13:23:30 -0400 Subject: [PATCH] Fix play stats when rescue block is a child block (#70922) (#71334) * check run state of current block only * Add changelog and test * Add test for issue 29047 (cherry picked from commit f2f6c3463234a59410e4d5bfe320dbff2490d9c4) --- .../fragments/70922-fix-block-in-rescue.yml | 2 ++ lib/ansible/executor/play_iterator.py | 11 +++++++ lib/ansible/plugins/strategy/__init__.py | 7 +++- .../targets/blocks/block_in_rescue.yml | 33 +++++++++++++++++++ .../integration/targets/blocks/issue29047.yml | 4 +++ .../targets/blocks/issue29047_tasks.yml | 13 ++++++++ test/integration/targets/blocks/runme.sh | 10 ++++++ 7 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 changelogs/fragments/70922-fix-block-in-rescue.yml create mode 100644 test/integration/targets/blocks/block_in_rescue.yml create mode 100644 test/integration/targets/blocks/issue29047.yml create mode 100644 test/integration/targets/blocks/issue29047_tasks.yml diff --git a/changelogs/fragments/70922-fix-block-in-rescue.yml b/changelogs/fragments/70922-fix-block-in-rescue.yml new file mode 100644 index 00000000000..7900452977a --- /dev/null +++ b/changelogs/fragments/70922-fix-block-in-rescue.yml @@ -0,0 +1,2 @@ +bugfixes: + - Fix statistics reporting when rescue block contains another block (issue https://github.com/ansible/ansible/issues/61253). diff --git a/lib/ansible/executor/play_iterator.py b/lib/ansible/executor/play_iterator.py index d6b1bf9f474..1a53f3e7016 100644 --- a/lib/ansible/executor/play_iterator.py +++ b/lib/ansible/executor/play_iterator.py @@ -514,6 +514,17 @@ class PlayIterator: return self.get_active_state(state.always_child_state) return state + def is_any_block_rescuing(self, state): + ''' + Given the current HostState state, determines if the current block, or any child blocks, + are in rescue mode. + ''' + if state.run_state == self.ITERATING_RESCUE: + return True + if state.tasks_child_state is not None: + return self.is_any_block_rescuing(state.tasks_child_state) + return False + def get_original_task(self, host, task): # now a noop because we've changed the way we do caching return (None, None) diff --git a/lib/ansible/plugins/strategy/__init__.py b/lib/ansible/plugins/strategy/__init__.py index 1236986256c..51fe18cb9f8 100644 --- a/lib/ansible/plugins/strategy/__init__.py +++ b/lib/ansible/plugins/strategy/__init__.py @@ -569,7 +569,12 @@ class StrategyBase: if iterator.is_failed(original_host) and state and state.run_state == iterator.ITERATING_COMPLETE: self._tqm._failed_hosts[original_host.name] = True - if state and iterator.get_active_state(state).run_state == iterator.ITERATING_RESCUE: + # Use of get_active_state() here helps detect proper state if, say, we are in a rescue + # block from an included file (include_tasks). In a non-included rescue case, a rescue + # that starts with a new 'block' will have an active state of ITERATING_TASKS, so we also + # check the current state block tree to see if any blocks are rescuing. + if state and (iterator.get_active_state(state).run_state == iterator.ITERATING_RESCUE or + iterator.is_any_block_rescuing(state)): self._tqm._stats.increment('rescued', original_host.name) self._variable_manager.set_nonpersistent_facts( original_host.name, diff --git a/test/integration/targets/blocks/block_in_rescue.yml b/test/integration/targets/blocks/block_in_rescue.yml new file mode 100644 index 00000000000..15360304b53 --- /dev/null +++ b/test/integration/targets/blocks/block_in_rescue.yml @@ -0,0 +1,33 @@ +- hosts: localhost + gather_facts: no + tasks: + - block: + - name: "EXPECTED FAILURE" + fail: + msg: "fail to test single level block in rescue" + rescue: + - block: + - debug: + msg: Rescued! + + - block: + - name: "EXPECTED FAILURE" + fail: + msg: "fail to test multi-level block in rescue" + rescue: + - block: + - block: + - debug: + msg: Rescued! + + - name: "Outer block" + block: + - name: "Inner block" + block: + - name: "EXPECTED FAILURE" + fail: + msg: "fail to test multi-level block" + rescue: + - name: "Rescue block" + block: + - debug: msg="Inner block rescue" diff --git a/test/integration/targets/blocks/issue29047.yml b/test/integration/targets/blocks/issue29047.yml new file mode 100644 index 00000000000..9743773c8cd --- /dev/null +++ b/test/integration/targets/blocks/issue29047.yml @@ -0,0 +1,4 @@ +- hosts: localhost + gather_facts: no + tasks: + - include_tasks: issue29047_tasks.yml diff --git a/test/integration/targets/blocks/issue29047_tasks.yml b/test/integration/targets/blocks/issue29047_tasks.yml new file mode 100644 index 00000000000..3470d8672de --- /dev/null +++ b/test/integration/targets/blocks/issue29047_tasks.yml @@ -0,0 +1,13 @@ +--- +- name: "EXPECTED FAILURE" + block: + - fail: + msg: "EXPECTED FAILURE" + rescue: + - name: Assert that ansible_failed_task is defined + assert: + that: ansible_failed_task is defined + + - name: Assert that ansible_failed_result is defined + assert: + that: ansible_failed_result is defined diff --git a/test/integration/targets/blocks/runme.sh b/test/integration/targets/blocks/runme.sh index edba0c5275f..4f3db1db757 100755 --- a/test/integration/targets/blocks/runme.sh +++ b/test/integration/targets/blocks/runme.sh @@ -83,3 +83,13 @@ set -e cat rc_test.out [ $exit_code -eq 0 ] rm -f rc_test_out + +# https://github.com/ansible/ansible/issues/29047 +ansible-playbook -vv issue29047.yml -i ../../inventory + +# https://github.com/ansible/ansible/issues/61253 +ansible-playbook -vv block_in_rescue.yml -i ../../inventory > rc_test.out +cat rc_test.out +[ "$(grep -c 'rescued=3' rc_test.out)" -eq 1 ] +[ "$(grep -c 'failed=0' rc_test.out)" -eq 1 ] +rm -f rc_test.out