Salt exit codes are inconsistent and undocumented

[ConsoleCatzirl]

I would like to see salt give well-defined exit codes that are described in the man.1 page. Looking through the code base, it appears that sometimes salt exits with -1, 1, 2, or 42 -- and it's unclear when it exits with which. Other times salt exits by raising SaltSystemExit (on one occasion passing code=42). And sometimes an outputter will print an error but has no mechanism for passing an error state back the calling function, giving an error on screen but an exit code of 0.

I would like to see a clear reason for each exit code. I would also like to have a way for error information to propagate back through the call stack so that system exits can be consolidated to a single class.

[thatch45]

Yes, we might need to change a few of them as well...

[grantmcwilliams]

Indeed. I'm provisioning a large number of hosts via salt and the provisioning is automated. When I do salt 'CFS-CSE-013443.domain.com state.hightstate' and any errors occur I need the salt command to return a proper error code. Currently I get a 0.

[suprememoocow]

I'm using salt for continuous deployment from a Jenkins CI server. The problem is that if the deployment fails, the exit code is more often than not zero, so the Jenkins CI server sees the deployment as being successful even though it fails. I've bodged together a solution where I grep the output for "Failed" but it's a long way from being ideal.

[techdragon]

Being forced to hack around issue after issue in our CI & Deployment toolchain due to salt returning ... worse than useless... flat out misleading Exit Codes. Each hack takes me one step closer to being required to completely replace salt for every duty in our CI & Deployment chain... which kinda sucks.

[kiorky]

I also concur in the idea at least not to have an exit code of 0 if any of states run failed in case of a state run (sls, highstate, etc)

[kiorky]

As a workaround, we use a shell function to parse log to know if we failed or not : https://github.com/makinacorpus/makina-states/blob/master/_scripts/boot-salt.sh#L931

[kiorky]

@thedrow, you can grep with yaml outputter, it will work as we do.

[techdragon]

Ive taken some notes on the issue of the error pathways... but its 2am my time and I need some sleep before a 7am meeting. So ill have to post the notes tomorrow ... at the moment its about 200 lines long without much in the way of anything beyond being a list of all the places salt can call exit and produce a return code.

[techdragon]

I may make a pull request for this tomorrow after i document it all... would 1 big pull request be best or a few smaller ones... the changes are littered through the code. Then theres the issue of 'default behaviour' but to be fair I want salt to 'fail safe' by assuming an error far more frequently.

[techdragon]

Ok, to give an update on my work here. Ive been digging through source code, and since it looks like when I'm done, I'll be issuing a pull request containing anything from at least 25 through to over 200 changes, I'm writing up a nice long explanation of whats wrong before I post any kind of fix.

[techdragon]

Pull request!
This seems the best way to avoid requiring the modification of every states code to provide more args/kwargs back up to the calling code path so a more efficient check can be performed.

[techdragon]

Finally had time to sit down and redo this work. Now for

1. Sleep
2. Wake up
3. Clean up unnecessary debug lines
4. Submit new pull request

😄
... 😴

[JensRantil]

#11337 did not document the exit code. I created an issue for it here: #14155.