Skip to content
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

log which task ended the play #61796

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from
Open

Conversation

ttyS4
Copy link

@ttyS4 ttyS4 commented Sep 4, 2019

Before the change no information was available why a playbook has ended.
By introducing this change information will be available which tells us which
task invoked meta end_play.

SUMMARY

I have playbooks structured in a way that if certain conditions are met the play is ended for good reasons. This can happen in multiple places and also there are multiple checks in one of the playbooks. By adding this change ansible will log which task called the meta action to end the play.

PR #20799 is somewhat related.

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

core

@ansibot ansibot added affects_2.10 This issue/PR affects Ansible v2.10 core_review In order to be merged, this PR must follow the core review workflow. feature This issue/PR relates to a feature request. needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. small_patch support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Sep 4, 2019
@bcoca
Copy link
Member

bcoca commented Sep 4, 2019

you should be able to do the same with a callback and not change it for all users.

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Sep 4, 2019
@ttyS4
Copy link
Author

ttyS4 commented Sep 4, 2019

This is a simplified version of a playbook I use:

% cat test.yaml
---
- hosts: localhost
  vars:
  - auto_reboot_days: 30
    auto_reboot_only_on_weekday: 0
  tasks:
  - name: compare uptime days to reboot threshold
    meta: end_play
    when: auto_reboot_days is not defined or ((( ansible_uptime_seconds|int + 600 ) / 86400 ) < auto_reboot_days )

  - name: check if auto_reboot_only_on_weekday matches
    meta: end_play
    when: (auto_reboot_only_on_weekday is defined) and (ansible_date_time.weekday_number|int != auto_reboot_only_on_weekday)

  - name: reboot and upgrade host
    command: /bin/echo reboot-and-stuff

If I run the latest version of ansible from the git checkout of the devel branch I get this output:

% ansible-playbook test.yaml
 [WARNING]: No inventory was parsed, only implicit localhost is available

 [WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match 'all'


PLAY [localhost] *************************************************************************************************************

TASK [Gathering Facts] *******************************************************************************************************
ok: [localhost]

PLAY RECAP *******************************************************************************************************************
localhost                  : ok=1    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   

If I increase verbosity I get:

% ansible-playbook -vv test.yaml
ansible-playbook 2.10.0.dev0
  config file = None
  configured module search path = [u'/home/cstamas/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /tmp/tmp.l5ZeiGcm6L/ansible/lib/ansible
  executable location = /tmp/tmp.l5ZeiGcm6L/ansible/bin/ansible-playbook
  python version = 2.7.16 (default, Apr  6 2019, 01:42:57) [GCC 8.3.0]
No config file found; using defaults
 [WARNING]: No inventory was parsed, only implicit localhost is available

 [WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match 'all'


PLAYBOOK: test.yaml **********************************************************************************************************
1 plays in test.yaml

PLAY [localhost] *************************************************************************************************************

TASK [Gathering Facts] *******************************************************************************************************
task path: /tmp/tmp.l5ZeiGcm6L/test.yaml:2
ok: [localhost]
META: ran handlers
META: 
META: ending play

PLAY RECAP *******************************************************************************************************************
localhost                  : ok=1    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   

After increasing verbosity it tells the meta end_play triggered the play to finish, but they do not tell where and why it ended. This is what my patch intends to do.

So with my patch I get this:

% ansible-playbook test.yaml
 [WARNING]: No inventory was parsed, only implicit localhost is available

 [WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match 'all'


PLAY [localhost] *************************************************************************************************************

TASK [Gathering Facts] *******************************************************************************************************
ok: [localhost]
META [check if auto_reboot_only_on_weekday matches] is ending play

PLAY RECAP *******************************************************************************************************************
localhost                  : ok=1    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   

with my patch and -vv the output is this:

% ansible-playbook -vv test.yaml
ansible-playbook 2.10.0.dev0
  config file = None
  configured module search path = [u'/home/cstamas/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /tmp/tmp.l5ZeiGcm6L/ansible/lib/ansible
  executable location = /tmp/tmp.l5ZeiGcm6L/ansible/bin/ansible-playbook
  python version = 2.7.16 (default, Apr  6 2019, 01:42:57) [GCC 8.3.0]
No config file found; using defaults
 [WARNING]: No inventory was parsed, only implicit localhost is available

 [WARNING]: provided hosts list is empty, only localhost is available. Note that the implicit localhost does not match 'all'


PLAYBOOK: test.yaml **********************************************************************************************************
1 plays in test.yaml

PLAY [localhost] *************************************************************************************************************

TASK [Gathering Facts] *******************************************************************************************************
task path: /tmp/tmp.l5ZeiGcm6L/test.yaml:2
ok: [localhost]
META: ran handlers
META: 
META [check if auto_reboot_only_on_weekday matches] is ending play
META: ending play

PLAY RECAP *******************************************************************************************************************
localhost                  : ok=1    changed=0    unreachable=0    failed=0    skipped=0    rescued=0    ignored=0   

@ttyS4
Copy link
Author

ttyS4 commented Sep 4, 2019

you should be able to do the same with a callback and not change it for all users.

Maybe this should be changed for all users?
@bcoca after reading your question I realized that this could be considered as a bug and as such should be fixed.

@ttyS4
Copy link
Author

ttyS4 commented Sep 4, 2019

ready_for_review

@samdoran samdoran removed the needs_triage Needs a first human triage before being processed. label Sep 12, 2019
@sivel
Copy link
Member

sivel commented Sep 12, 2019

We have reviewed this and have decided to not accept this change. To get this information, you should be using a higher verbosity of at least -vv and you will already see end play right after the meta task that ended the play.

If you have further questions please stop by IRC or the mailing list:

@sivel sivel closed this Sep 12, 2019
@ttyS4
Copy link
Author

ttyS4 commented Sep 12, 2019

@sivel Right after opening the PR at #61796 (comment)
i tried to describe why higher verbosity does not help in this case.
Please check and you will see what I mean; to get to the point: currently you cannot know which task triggerred end_play.

@sivel
Copy link
Member

sivel commented Sep 12, 2019

Ok, I was under the impression that the task name of the meta task would still print.

In any case, this could be extended to the existing msg but not it's own display.

@sivel sivel reopened this Sep 12, 2019
lib/ansible/plugins/strategy/__init__.py Outdated Show resolved Hide resolved
Before the change no information was available why a playbook has ended.
By introducing this change information will be available which tells us which
task invoked meta end_play.
@ttyS4 ttyS4 requested a review from sivel September 12, 2019 22:31
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Sep 23, 2019
@ttyS4 ttyS4 closed this Oct 4, 2019
@ttyS4 ttyS4 reopened this Oct 4, 2019
@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Oct 4, 2019
@ansibot ansibot added stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. stale_review Updates were made after the last review and the last review is more than 7 days old. labels Oct 12, 2019
@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Mar 4, 2020
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed stale_review Updates were made after the last review and the last review is more than 7 days old. labels Mar 4, 2020
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. stale_review Updates were made after the last review and the last review is more than 7 days old. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Mar 28, 2020
@ansibot ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html and removed stale_review Updates were made after the last review and the last review is more than 7 days old. labels Aug 23, 2020
@ansibot ansibot added pre_azp This PR was last tested before migration to Azure Pipelines. and removed stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Dec 5, 2020
@bcoca bcoca self-assigned this Jan 12, 2022
@bcoca bcoca added the P3 Priority 3 - Approved, No Time Limitation label Jul 20, 2022
@bcoca
Copy link
Member

bcoca commented Jul 20, 2022

note: fix should be that 'explicit' meta tasks should output as normal tasks. we use many 'implicit meta tasks, that only show output under -v levels of verbosity, those should still stay hidden and we should ensure we make the distinction in the engine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.10 This issue/PR affects Ansible v2.10 feature This issue/PR relates to a feature request. needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. new_contributor This PR is the first contribution by a new community member. P3 Priority 3 - Approved, No Time Limitation pre_azp This PR was last tested before migration to Azure Pipelines. small_patch support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants