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

onfail_stop is not documented #52380

Open
gtmanfred opened this issue Apr 1, 2019 · 7 comments
Open

onfail_stop is not documented #52380

gtmanfred opened this issue Apr 1, 2019 · 7 comments
Labels
Confirmed Salt engineer has confirmed bug/feature - often including a MCVE Documentation Relates to Salt documentation Feature new functionality including changes to functionality and code refactors, etc. pending-changes The pull request needs additional changes before it can be merged severity-low 4th level, cosemtic problems, work around exists time-estimate-sprint
Milestone

Comments

@gtmanfred
Copy link
Contributor

Description of Issue/Question

the onfail_stop requisite is not documented.

It seems that it stops the states marked as onfail from being errored if an onfail_stop state succeeds.

@gtmanfred
Copy link
Contributor Author

Also, if that is how it works, then #16291 should be closed.

@frogunder frogunder added this to the Approved milestone Apr 2, 2019
@frogunder frogunder added Documentation Relates to Salt documentation Feature new functionality including changes to functionality and code refactors, etc. labels Apr 2, 2019
@stale
Copy link

stale bot commented Jan 8, 2020

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.

@stale stale bot added the stale label Jan 8, 2020
@max-arnold
Copy link
Contributor

Bump

@stale
Copy link

stale bot commented Jan 9, 2020

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

@stale stale bot removed the stale label Jan 9, 2020
@sagetherage sagetherage added the Confirmed Salt engineer has confirmed bug/feature - often including a MCVE label Mar 23, 2020
@OrangeDog
Copy link
Contributor

If that is what it does then it's a bad name. It sounds like it should stop execution in some way.

Before it's documented (and officially supported) change it to e.g. onfail_suppress, or a more composable way of doing it.

@barbaricyawps barbaricyawps added pending-changes The pull request needs additional changes before it can be merged severity-low 4th level, cosemtic problems, work around exists time-estimate-sprint labels May 16, 2022
@CrackerJackMack
Copy link

I agree with the shit naming. But this is precisely what I need for functionality. At least documenting it would help.

@max-arnold
Copy link
Contributor

I agree it is badly named, the usage is not intuitive, and it is not a requisite but a flag, and the flag is true by default:

Failing state:
  test.fail_without_changes: []

Failing state handler:
  test.succeed_with_changes:
    - onfail_stop: false
    - onfail:
      - test: Failing state
salt-call state.apply onfail_stop >/dev/null; echo $?
[ERROR   ] Failure!
0

What this does is ensures the resulting retcode is 0. See #39874

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Confirmed Salt engineer has confirmed bug/feature - often including a MCVE Documentation Relates to Salt documentation Feature new functionality including changes to functionality and code refactors, etc. pending-changes The pull request needs additional changes before it can be merged severity-low 4th level, cosemtic problems, work around exists time-estimate-sprint
Projects
None yet
Development

No branches or pull requests

7 participants