New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add stats on rescued/ignored tasks #48418

Open
wants to merge 13 commits into
base: devel
from

Conversation

@mkrizek
Contributor

mkrizek commented Nov 9, 2018

SUMMARY

PLEASE PING THE TOWER TEAM BEFORE MERGING

Following up on the work that @jimi-c has done in his improved_stats branch jimi-c@2832d08.

This PR adds two counters to PLAY RECAP:

  • ignored for tasks that failed but had set ignore_errors: yes,
  • rescued for tasks that failed initially but the rescue section was executed; note that rescued tasks are no longer counted as failed as before this PR

Fixes #31245
Fixes #20346

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

callbacks

ANSIBLE VERSION
2.8
ADDITIONAL INFORMATION
@mkrizek

This comment has been minimized.

Contributor

mkrizek commented Nov 9, 2018

cc @AlanCoding

@ansibot

This comment was marked as resolved.

Contributor

ansibot commented Nov 9, 2018

Hi @mkrizek, thank you for submitting this pull-request!

click here for bot help

@mkrizek mkrizek force-pushed the mkrizek:improved_stats branch from ef9929c to 4cbc2e0 Dec 5, 2018

@ansibot ansibot removed the stale_ci label Dec 5, 2018

@ansibot

This comment has been minimized.

Contributor

ansibot commented Dec 5, 2018

cc @akatch
click here for bot help

@@ -31,5 +31,5 @@ TASK [Second free task] ********************************************************
changed: [testhost]
PLAY RECAP *********************************************************************
testhost : ok=10 changed=7 unreachable=0 failed=0 skipped=1
testhost : ok=10 changed=7 unreachable=0 failed=0 rescued=0 ignored=1

This comment has been minimized.

@bcoca

bcoca Dec 5, 2018

Member

we might want to change to failed=Y (ignored=X) so the 'sum of tasks' seems to stay the same, these are not at the same level as the general counters, they are subsets of the existing categories

@mkrizek mkrizek changed the title from [WIP] Add stats on rescue/ignored tasks to [WIP] Add stats on rescued/ignored tasks Dec 5, 2018

@@ -328,25 +328,27 @@ def v2_playbook_on_stats(self, stats):
t = stats.summarize(h)
self._display.display(
u"%s : %s %s %s %s %s %s" % (
u"%s : %s %s %s %s %s %s %s" % (

This comment has been minimized.

@bcoca

bcoca Dec 5, 2018

Member

I was thinking that we should show them as subgroups, otherwise task numbers don't match up:

 u"%s : %s(%s) %s %s %s(%s) %s " %``
...
colorize(u'ok', t['ok'], C.COLOR_OK), 
colorize(u'rescued', t['rescued'], C.COLOR_OK),
...
 colorize(u'failed', t['failures'], C.COLOR_ERROR),
colorize(u'ignored', t['ignored'], C.COLOR_WARN),

This comment has been minimized.

@mkrizek

mkrizek Dec 6, 2018

Contributor

But rescued tasks are not counted as ok so unsure ok=X (rescued=Y) makes sense? Not sure failed=X (ignored=Y) makes sense either since those two are mutually exclusive, you can have either one or the other.

See this playbook and its recap:

- hosts: localhost
  gather_facts: no
  tasks:
    - name: ok1
      debug:

    - name: ok2 and changed
      debug:
      changed_when: True

    - name: skipped
      debug:
      when: False

    - block:
        - name: rescued
          fail:
      rescue:
        - name: ok3
          debug:

    - name: ok4 and ignored
      fail:
      ignore_errors: yes

    - name: failed
      fail:

localhost : ok=4 changed=1 unreachable=0 failed=1 skipped=1 rescued=1 ignored=1

Task numbers don't match up because changed and ignored are counted as ok as well.

This comment has been minimized.

@bcoca

bcoca Dec 13, 2018

Member

I seem to have confused the accounting, so ok (ignored) .. i assumed rescued was 'failed but rescued' but it seems to account for 'rescue tasks run'?

@ansibot

This comment has been minimized.

Contributor

ansibot commented Dec 6, 2018

@mkrizek mkrizek changed the title from [WIP] Add stats on rescued/ignored tasks to Add stats on rescued/ignored tasks Dec 7, 2018

@mkrizek mkrizek added this to Needs Review - This is when code is complete and needs review. in Ansible 2.8 Dec 7, 2018

@@ -125,6 +125,8 @@ Plugins
has changed to ``%USERPROFILE%\.ansible_async``. To control this path now, either set the ``ansible_async_dir``
variable or the ``async_dir`` value in the ``powershell`` section of the config ini.
* The ``default`` callback plugin now has two additional stat counters in play recap: ``ignored`` for tasks that failed but had set ``ignore_errors: yes`` and ``rescued`` for tasks that failed initially but the rescue section was executed. Note that ``rescued`` tasks are no longer counted as ``failed`` as in Ansible 2.7 (and earlier).

This comment has been minimized.

@acozine

acozine Dec 12, 2018

Contributor

I'd start with the part users will see, then describe how it works:

"Play recap now counts ignored and rescued tasks as well as ok, changed, skipped and failed tasks, thanks to two additional stat counters in the default callback plugin. Tasks that fail and have ignore_errors: yes set are listed as ignored. Tasks that fail and then execute a rescue section are listed as rescued. Note that ignored tasks are also included in the total count of failed tasks, but rescued tasks are not."

This comment has been minimized.

@mkrizek

mkrizek Dec 13, 2018

Contributor

Thanks for re-wording. Couple of notes though:

  • "ok, changed, skipped and failed tasks" - if we're listing all currently existing counters, should we mention unreachable too?
  • "Note that ignored tasks are also included in the total count of failed tasks" - this isn't true as ignored and failed are mutually exclusive. So I'd leave the last sentence as currently is in the PR (or re-word it) since it clarifies what has changed from previous versions?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment