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

Blocks: rescue or always not run when error comes from a run_once task #16937

Closed
Gugli opened this issue Aug 3, 2016 · 10 comments
Closed

Blocks: rescue or always not run when error comes from a run_once task #16937

Gugli opened this issue Aug 3, 2016 · 10 comments
Assignees
Labels
bug This issue/PR relates to a bug.
Milestone

Comments

@Gugli
Copy link

Gugli commented Aug 3, 2016

ISSUE TYPE
  • Bug Report
ANSIBLE VERSION
ansible 2.1.1.0
  config file = /etc/ansible/ansible.cfg
  configured module search path = Default w/o overrides
CONFIGURATION

Out of the box config on Ubuntu14

OS / ENVIRONMENT

Ansible runs on Ubuntu14, targeted machines on Ubuntu16.

SUMMARY

Using run_once on a task inside a block prevents rescue and always sections to be correctly triggered.

STEPS TO REPRODUCE

Run this playbook on any inventory file. This is basically the sample from the official documentation with just a run_once added

# Contents of bug_blocks.yml
- hosts: all
  gather_facts: false
  tasks:
  - block:
      - debug: msg='I execute normally'
      - command: /bin/false
        run_once: true
      - debug: msg='I never execute, cause ERROR!'
    rescue:
      - debug: msg='I caught an error'
      - command: /bin/false
      - debug: msg='I also never execute :-('
    always:
      - debug: msg="this always executes"
EXPECTED RESULTS

I was expecting

$ ansible-playbook bug_blocks.yml --inventory-file inventories/

PLAY [all] *********************************************************************

TASK [debug] *******************************************************************
ok: [localhost] => {
    "msg": "i execute normally"
}

TASK [command] *****************************************************************
fatal: [localhost]: FAILED! => {"changed": true, "cmd": ["/bin/false"], "delta": "0:00:00.002056", "end": "2016-08-03 17:04:07.156094", "failed": true, "rc": 1, "start": "2016-08-03 17:04:07.154038", "stderr": "", "stdout": "", "stdout_lines": [], "warnings": []}

TASK [debug] *******************************************************************
ok: [localhost] => {
    "msg": "I caught an error"
}

TASK [command] *****************************************************************
fatal: [localhost]: FAILED! => {"changed": true, "cmd": ["/bin/false"], "delta": "0:00:00.001853", "end": "2016-08-03 17:04:07.988953", "failed": true, "rc": 1, "start": "2016-08-03 17:04:07.987100", "stderr": "", "stdout": "", "stdout_lines": [], "warnings": []}

TASK [debug] *******************************************************************
ok: [localhost] => {
    "msg": "this always executes"
}

NO MORE HOSTS LEFT *************************************************************

PLAY RECAP *********************************************************************
localhost                  : ok=2    changed=0    unreachable=0    failed=1
TASK: command ----------------------------------------------------------- 0.79s
TASK: command ----------------------------------------------------------- 0.40s
TASK: debug ------------------------------------------------------------- 0.11s
TASK: debug ------------------------------------------------------------- 0.09s
TASK: debug ------------------------------------------------------------- 0.07s

Playbook finished: Wed Aug  3 17:04:08 2016, 5 total tasks.  0:00:01 elapsed.
ACTUAL RESULTS
$ ansible-playbook bug_blocks.yml --inventory-file inventories/

PLAY [all] *********************************************************************

TASK [debug] *******************************************************************
ok: [localhost] => {
    "msg": "i execute normally"
}

TASK [command] *****************************************************************
fatal: [localhost]: FAILED! => {"changed": true, "cmd": ["/bin/false"], "delta": "0:00:00.001284", "end": "2016-08-03 17:03:52.951110", "failed": true, "rc": 1, "start": "2016-08-03 17:03:52.949826", "stderr": "", "stdout": "", "stdout_lines": [], "warnings": []}

NO MORE HOSTS LEFT *************************************************************

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

TASK: command ----------------------------------------------------------- 0.71s
TASK: debug ------------------------------------------------------------- 0.09s

Playbook finished: Wed Aug  3 17:26:37 2016, 2 total tasks.  0:00:00 elapsed.
Related issues

This might be related to #16254, but the steps to reproduce are so different I though it best to make a new Issue report.

@Gugli
Copy link
Author

Gugli commented Aug 3, 2016

Might also be somewhat related to #16050.

@MadVikingGod
Copy link

I can confirm this bug. It does not happen on v2.1.0.0-1, but everything later does not work correctly.

I have narrowed it down to fbec2d9 linear.py lines 364 and 365.

Reverting this part of that patch will fix this but, but I think will also undo the fix for #16937.

A real fix for both of these issues is to not treat run_once as any_error_fatal, and have a different logic path if it fails. This would mark all of the hosts in that play as failed, and then run the appropriate recovery logic.

@jctanner
Copy link
Contributor

jctanner commented Aug 5, 2016

cc @jimi-c

@jimi-c jimi-c added this to the stable-2.1 milestone Aug 5, 2016
jimi-c added a commit that referenced this issue Aug 5, 2016
Instead of immediately returning a failed code (indicating a break in
the play execution), we internally 'or' that failure code with the result
(now an integer flag instead of a boolean) so that we can properly handle
the rescue/always portions of blocks and still remember that the break
condition was hit.

Fixes #16937
@jimi-c
Copy link
Member

jimi-c commented Aug 5, 2016

@Gugli / @MadVikingGod the above branch is a first attempt at fixing this. Still needs some work but I think the basics are there. I still need to:

  • go through our other strategy plugins and base strategy to make sure at least in our code we're using integer results values now
  • my logic for the handler stuff (and vs. or) is probably not right, but haven't tested it yet (shippable is running on it)
  • only the first host in the list executes the rescue - no clue as to why yet because the rest are marked as failed and the PlayIterator should have them all at the same point too.

@jimi-c
Copy link
Member

jimi-c commented Aug 5, 2016

Hrm, and it looks like I've got some other bit of logic wrong.. this test succeeded but there was a non-zero exit code:

https://app.shippable.com/runs/57a4dfa5d640fc0d00f90f97/5/console

@jimi-c
Copy link
Member

jimi-c commented Aug 5, 2016

$ ansible -m ping localhost
localhost | SUCCESS => {
    "changed": false, 
    "ping": "pong"
}
$ echo $?
1

MadVikingGod pushed a commit to MadVikingGod/ansible that referenced this issue Aug 8, 2016
In the linear strategy the flag run_once was synomious with any_error_fatal for error checking.  This caused plays to abort before running any rescue or always blocks.  By remove this the play executes as expected, and the error handeling is handled by the executor. This is a fix for ansible#16937
jimi-c added a commit that referenced this issue Aug 12, 2016
Instead of immediately returning a failed code (indicating a break in
the play execution), we internally 'or' that failure code with the result
(now an integer flag instead of a boolean) so that we can properly handle
the rescue/always portions of blocks and still remember that the break
condition was hit.

Fixes #16937
@pporada-gl
Copy link

pporada-gl commented Aug 12, 2016

I hit this today on ansible 2.1.1.0 without the run_once.

jimi-c added a commit that referenced this issue Aug 12, 2016
Instead of immediately returning a failed code (indicating a break in
the play execution), we internally 'or' that failure code with the result
(now an integer flag instead of a boolean) so that we can properly handle
the rescue/always portions of blocks and still remember that the break
condition was hit.

Fixes #16937
@jimi-c
Copy link
Member

jimi-c commented Aug 12, 2016

The branch above is working and passing tests, if anyone else would like to give it a test before I merge it into devel.

@jimi-c jimi-c closed this as completed in c669a38 Aug 12, 2016
@jimi-c
Copy link
Member

jimi-c commented Aug 12, 2016

Closing This Ticket

Hi!

We believe the above commit should resolve this problem for you. This will also be included in the next release.

If you continue seeing any problems related to this issue, or if you have any further questions, please let us know by stopping by one of the two mailing lists, as appropriate:

Because this project is very active, we're unlikely to see comments made on closed tickets, but the mailing list is a great way to ask questions, or post if you don't think this particular issue is resolved.

Thank you!

1 similar comment
@jimi-c
Copy link
Member

jimi-c commented Aug 15, 2016

Closing This Ticket

Hi!

We believe the above commit should resolve this problem for you. This will also be included in the next release.

If you continue seeing any problems related to this issue, or if you have any further questions, please let us know by stopping by one of the two mailing lists, as appropriate:

Because this project is very active, we're unlikely to see comments made on closed tickets, but the mailing list is a great way to ask questions, or post if you don't think this particular issue is resolved.

Thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue/PR relates to a bug.
Projects
None yet
Development

No branches or pull requests

6 participants