Salt: Controlled fail in state definitions

[cznewt]

Hello, puppet has mechanism to deliberately fail if some conditions are not met (i.e. os_family is not supported) like:

fail("No support for OSfamily \"${::osfamily}\"")

does salt have something similar, it would be very useful

[basepi]

Are you looking to fail a specific state or the entire state run?

[cznewt]

Is there any difference? More looking to fail entire state run.

[basepi]

Sure, there's a difference. If you failed a single state, then it would come back as unsuccessful but the state run would continue (anything that required that state would also fail, but unrelated states would run).

We don't have anything to give this behavior right now, but I think it would definitely be useful to add. Thanks for the request!

[kiorky]

You can already do that by making a conditionnal deliberate syntax error in your state file eg:

{% if not  grains['os'] in ['Ubuntu'] %}
THIS WILL FAILLLLL  HAAAAARDDDDD !!!!!!!!
{% endif %}

[basepi]

True. Hacky, but definitely works. =)

[whiteinge]

There was discussion on the IRCes last week requesting a Jinja filter that would throw a Python exception. It's an interesting idea with a potentially decent syntax. Something like:

{% set my_grain = grains['my_grain'] | raise("Some exception text") %}

[stephen-mw]

@whiteinge's jinja solution to me seems like a good idea. It's easy to get your sls to fail, but the missing sauce is a reasonable traceback that's useful for troubleshooting. I like the exception idea.

[erchn]

My $.02...

I think a Jinja-only solution would be useful, but not nearly as beneficial as an execution module that performed the same, for a few reasons.

• This would allow all renderers to throw exceptions
• This would allow state definitions to throw exceptions post-render time, as Jinja/Mako/etc. is only evaluated before any state execution.
• Combined with the recently committed postmortem/postfail, you could essentially define an "else" and raise an exception, instead of just performing cleanup.

Ideally there'd be a way of signalling that the exception could optionally include the failure details from the last failed individual state. Kind of akin to try: except: block, which would raise error detail.

As a state

a:
  some.module:
    ...
# if a fails
cleanup_a:
  module.run:
    - name: salt.raise
    - m_name: Error in module "a"
    - include_stack: True
    - postfail:
      - some: a

As Jinja

{% if salt['file.exists']('/path/that/must/exist') %}
...
{% else %}
{% if salt['salt.raise']("/path/that/must/exist does not exist!!") %}
{% endif %}
{% endif %}

Thoughts?

[stephen-mw]

+1 @erchn. That seems very reasonable to me. I would imagine a simple state file with exception handling looking like the following:

create_lots_of_files:
    cmd.run:
        - name: for i in {1..99999}; do touch mydirectory/$i ;done
        - except: # non-zero exit status from subprocess
            - raise: "Unable to create tmp files. Check disk space?"
            - cmd.run: rm mydirectory/*

[erchn]

I like that option as well, though the postfail requisite would make it easy to define a single state that can raise an exception from multiple failed states, but having both would be awesome!

[whiteinge]

Recent addition to recover from a failed state here in a7cb1af which achieves half of this goal. There's also this addition in #11023 but I think that still needs a little polish before this full controlled fail workflow is complete.

[zerowolfgang]

+1 on the module/jinja feature. At the moment I'm trying to gracefully exit a state that requires params to be passed on the command line (pillar={dict}) when running a highstate and am having to resort to using cmd.echo "nope!" to return meaningful info about the fail, whereas a raise would be much cleaner.

[powellchristoph]

+1 I would love to have this functionality.

[whiteinge]

I think we have answers to all the asks in this thread now with the upcoming 2015.8 release of Salt. Is there anything anyone would still like to see or can we close this issue out?

We should add a "Throwing and Handling Errors" section to the states/formulas docs that lists each approach. Adding the "Documentation" label.

Reacting to, or recovering from, a failed state:

create_lots_of_files:
    cmd.run:
        - name: for i in {1..99999}; do touch mydirectory/$i ;done

"Unable to create tmp files. Check disk space?":
  cmd.run:
    - name: rm mydirectory/*
    - onfail:
      - cmd: create_lots_of_files

Throwing an exception during the Pillar compilation phase:

{% if salt['file.exists']('/path/that/must/exist') %}
...
{% else %}
{{ salt.test.exception("/path/that/must/exist does not exist!!") }}
{% endif %}

Requiring certain Pillar values (or Pillar types) with the test.check_pillar state function:

is-pillar-foo-present-and-bar-is-int:
  test.check_pillar:
    - present:
        - foo
    - integer:
        - bar

[powellchristoph]

@whiteinge Thank you for the update. The test.check_pillar is very nice!

[mitar]

{{ salt.test.exception("/path/that/must/exist does not exist!!") }} to me does not work. It seems like it returns the value with _error set to "Failed to return clean data" but it does not fail the compilation of the whole state.

[whiteinge]

@mitar Sorry for the bad wording above. Throwing an exception will stop the parsing of only the .sls file containing that error when generating Pillar. It will stop the full execution of a state run, however.

[mitar]

So maybe I am not testing this correctly. But i have a state test.sls like:

group-users:
  group.present:
    - gid: |
        {{ salt.test.exception("/path/that/must/exist does not exist!!") }}

I call:

salt-ssh '*' state.show_sls test

And I get:

testserver:
    ----------
    group-users:
        ----------
        __env__:
            base
        __sls__:
            test
        group:
            |_
              ----------
              gid:
                  {'retcode': 0, '_error': 'Failed to return clean data', 'stderr': 'Traceback (most recent call last):\n  File "/var/tmp/.mitar_cb9e2b_salt/salt-call", line 15, in <module>\n    salt_call()\n  File "/var/tmp/.mitar_cb9e2b_salt/py2/salt/scripts.py", line 379, in salt_call\n    client.run()\n  File "/var/tmp/.mitar_cb9e2b_salt/py2/salt/cli/call.py", line 58, in run\n    caller.run()\n  File "/var/tmp/.mitar_cb9e2b_salt/py2/salt/cli/caller.py", line 134, in run\n    ret = self.call()\n  File "/var/tmp/.mitar_cb9e2b_salt/py2/salt/cli/caller.py", line 197, in call\n    ret[\'return\'] = func(*args, **kwargs)\n  File "/var/tmp/.mitar_cb9e2b_salt/py2/salt/modules/test.py", line 512, in exception\n    raise Exception(message)\nException: /path/that/must/exist does not exist!!', 'stdout': ''}
            - present
            |_
              ----------
              order:
                  10000

So it did not raise the exception, but convert it to an object with retcode, _error and some other fields.

Using salt 2016.11.1.

[whiteinge]

I see. This looks like a consistency issue with salt-ssh. 😦

Regardless of which behavior we finalize on, salt-ssh and salt should decidedly produce the same outcome. Will you please file a separate issue for that and tag me on it?

[mitar]

Done: #42065.

[stale[bot]]

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.

[cebe]

old issue but still not implemented or documented as far as I see. Should stay open.

[stale[bot]]

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

[basepi]

@saltstack/team-triage

[bwesemann-cxp]

@cebe There is a "test" salt module for states documented here that can be used as a controlled fail. How I've used it in relation to the original post:

{% if grains['os'] in ['Ubuntu', 'CentOS'] %}
new_repo:
  pkgrepo.managed:
    - blah
{% else %}
failure:
  test.fail_without_changes:
    - name: "OS not supported!"
    - failhard: True
{% endif %}

output looks like this

server1:
    ----------
    test_|-failure_|-OS not supported!_|-fail_without_changes:
        ----------
        __id__:
            failure
        __run_num__:
            0
        __sls__:
            wazuh
        changes:
            ----------
        comment:
            Failure!
        duration:
            1.224
        name:
            OS not supported!
        result:
            False
        start_time:
            23:53:14.395904

[stale[bot]]

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]

this is still "unfixed"

[stale[bot]]

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

[sagetherage]

the documentation looks to be added - is there more to do here @disaster123?

[mitar]

I think the problem is that it does not work over salt SSH: #42065

[sagetherage]

ok closing this issue in favor of #42065 docs looks to be added in all branches, please mention me if this is incorrect.

[ScriptAutomate]

https://github.com/saltstack/salt/pull/56913 (merged to master in Sodium)