Add an option for the onfail requisite to not report a failure

[whiteinge]

If an onfail requisite provides some mitigation for the failure we should have an option to not count the original failure as a fail.

[cachedout]

Oh, so you can have "clean" output due to the fact that another state handled the failure condition of the first? Interesting. Naming this thing could be a challenge. :]

[whiteinge]

Exactly. Perhaps something along the lines of the 'reload' option to the
service mod_watch function to modify the default behavior.

[dstokes]

just ran into this. so confuse ;)

[arthurzenika]

@whiteinge @cachedout this feature would be really nice. Indeed naming is a difficult part of the job.

@whiteinge I'm not sure what you mean about the reload in mod_watch.

What about, for onfail_in :

onfail_in_but_report_ok_if : 
    - plan_b_that_succeeds

or for onfail :

onfail_but_fails_reports_as_ok:
   - plan_a_that_fails

[whiteinge]

The service mod_watch function accepts arguments. E.g.,

restart_stuff:
  service.restart:
    - watch: another_state
    - reload: True  # graceful reload instead of a restart

An onfail equivalent could be:

recover_after_failure:
  some.recovery-action:
    - onfail: another_state
    - catch_error: True  # set result to True on the watched state because we recovered

[vernondcole]

Me, too! I was about to write this up as a bug, rather than an enhancement request. Recovered failures are not failures. For example, attempting to stop a service which has never been installed. I wrote several scripts like:...

provision_dynamux_a:
  service.dead:
    - name: rabbitmq-server
    - onfail:
      - p_d_a_ignore_failures
p_d_a_ignore_failures:
  test.nop

but, of course, that did not work as I had hoped.
For my purposes, something like the following would work, and be simpler to use and to implement:...

provision_dynamux_a:
  service.dead:
    - name: rabbitmq-server
    - ignore_failures: true

[disaster123]

ran again into this one? Is there any workaround available, if a state is expected to fail and can be recovered by another state?

[whiteinge]

I vaguely recall this was addressed...in #39874 perhaps? Looks like it wasn't documented/announced, if so. I don't have an env handy to verify -- do you mind trying the following syntax?

some_id:
  some.state:
    - onfail_stop: False
    - onfail:
      - other_id

[disaster123]

May be i'm too stupid but i created this example:

id exit 1 state:
  cmd.run:
    - name: echo error; exit 1

id echo state:
  cmd.run:
    - name: echo recovered
    - onfail_stop: False
    - onfail:
      - cmd: id exit 1 state

But the output is still:

[ERROR   ] Command 'echo error; exit 1' failed with return code: 1
[ERROR   ] stdout: error
[ERROR   ] retcode: 1
[ERROR   ] {'pid': 14488, 'retcode': 1, 'stderr': '', 'stdout': 'error'}
...
Succeeded: 1 (changed=2)
Failed:    1

But i would expect that this state can't fail as it has recovered.

[whiteinge]

Pinging @kiorky. Am I reading that PR correctly?

[BlaineAtAffirm]

Should this be re-labeled bug since the feature is there, but has never worked ?

confirmed that the PR in the linked issue didn't work. I am seeing the same behaviour with or without the flag present. When present, set to true or false makes no difference either

$ salt test-minion state.sls onfail; echo retcode=$?
test-minion:
----------
          ID: fail
    Function: test.fail_without_changes
      Result: False
     Comment: Failure!
     Started: 00:24:56.127803
    Duration: 10.895 ms
     Changes:
----------
          ID: pass
    Function: test.succeed_without_changes
      Result: True
     Comment: Success!
     Started: 00:24:56.139146
    Duration: 0.652 ms
     Changes:

Summary for test-minion
------------
Succeeded: 1
Failed:    1
------------
Total states run:     2
Total run time:  11.547 ms
ERROR: Minions returned with non-zero exit code
retcode=1
.
.
.

$ cat onfail.sls ;echo
fail:
  test.fail_without_changes: []

pass:
  test.succeed_without_changes:
    - onfail_stop: True
    - onfail:
      - id: fail
.
.
.
$ salt test-minion state.sls onfail-take-2; echo retcode=$?
test-minion:
----------
          ID: fail
    Function: test.fail_without_changes
      Result: False
     Comment: Failure!
     Started: 00:24:56.127803
    Duration: 10.895 ms
     Changes:
----------
          ID: pass
    Function: test.succeed_without_changes
      Result: True
     Comment: Success!
     Started: 00:24:56.139146
    Duration: 0.652 ms
     Changes:

Summary for test-minion
------------
Succeeded: 1
Failed:    1
------------
Total states run:     2
Total run time:  11.547 ms
ERROR: Minions returned with non-zero exit code
retcode=1
.
.
.

$ cat onfail-take-2.sls ;echo
fail:
  test.fail_without_changes: []

pass:
  test.succeed_without_changes:
    - onfail_stop: False
    - onfail:
      - id: fail

This is the latest release version of salt

$ salt --versions
Salt Version:
           Salt: 2019.2.0

Dependency Versions:
           cffi: 1.11.5
       cherrypy: Not Installed
       dateutil: 2.7.2
      docker-py: 3.2.1
          gitdb: 0.6.4
      gitpython: 1.0.1
          ioflo: Not Installed
         Jinja2: 2.10
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: 1.0.3
   msgpack-pure: Not Installed
 msgpack-python: 0.4.6
   mysql-python: Not Installed
      pycparser: 2.18
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.12 (default, Nov 12 2018, 14:36:49)
   python-gnupg: 0.4.3
         PyYAML: 3.12
          PyZMQ: 15.2.0
           RAET: Not Installed
          smmap: 0.9.0
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.1.4

System Versions:
           dist: Ubuntu 16.04 xenial
         locale: UTF-8
        machine: x86_64
        release: 4.4.0-1075-aws
         system: Linux
        version: Ubuntu 16.04 xenial

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[disaster123]

still valid

[stale]

Thank you for updating this issue. It is no longer marked as stale.

[kiorky]

@whiteinge you are right, it was on that purpose.
sry, issue lost in my github noise ...
See https://github.com/makinacorpus/makina-states/blob/v2/salt/makina-states/_macros/h.jinja#L121 for a real life example.

[dmurphy18]

@whiteinge Closing this due to age, the old version of Salt and Python 2.
Can you retest this with the latest release of Salt 3006.2 and if still an issue, please open a new issue, which will have metrics in tracking issues.

Understanding that you probably add this to track the issue back when on the core team in 2014.