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

salt exit codes #18510

Closed
Deshke opened this issue Nov 26, 2014 · 60 comments
Closed

salt exit codes #18510

Deshke opened this issue Nov 26, 2014 · 60 comments
Assignees
Labels
Bug broken, incorrect, or confusing behavior Core relates to code central or existential to Salt P1 Priority 1 severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Milestone

Comments

@Deshke
Copy link

Deshke commented Nov 26, 2014

is there a reason why test.* returns False and exits with bash exit code 0 instead of 1 ?

~ # salt some-minion file.access  /var/run/reboot-required f; echo $?
<>:
    False
0

~ #  salt some-other-minion  file.access  /var/run/reboot-required f; echo $?
<>:
    True
0
@jfindlay jfindlay added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Nov 26, 2014
@jfindlay jfindlay added this to the Blocked milestone Nov 26, 2014
@jfindlay jfindlay added the info-needed waiting for more info label Nov 26, 2014
@jfindlay
Copy link
Contributor

Thanks for bringing this up, I believe the behavior you are seeing is the expected behavior. The command salt <minion> file.access is executed successfully in both cases, so the return value in both cases is 0, regardless of the result of the file.access function itself.

@Deshke
Copy link
Author

Deshke commented Nov 26, 2014

hey, sorry. i would expect a different behavior, like if my test goes wrong, the return code should be 1 (http://tldp.org/LDP/abs/html/exitcodes.html) regardless if my salt command runs successfully.

salt <minion> file.file_exists /some/file/that/doesn't/exists ; echo $?
   False
0

salt <minion> file.file_exists /some/file/that/exists ; echo $?
   True
0

@jfindlay jfindlay modified the milestones: Approved, Blocked Dec 1, 2014
@jfindlay jfindlay removed the info-needed waiting for more info label Dec 1, 2014
@jfindlay
Copy link
Contributor

jfindlay commented Dec 1, 2014

My reason for thinking a return of 0 was appropriate in this circumstance is that I am thinking of a return code as local to a system. Since salt runs successfully as intended on the master, it's return code would not be affected by the state of the remote minion. I admit that my reasoning on this is not very solid and am inclined to rather support your arguments.

A further consultation of the applicable standards as you have suggested is needed. I believe there is an issue somewhere about generally refactoring return statuses/return codes. I can't find it at the moment, but that ought to be referenced here.

@jfindlay jfindlay added Bug broken, incorrect, or confusing behavior Low Severity labels Dec 1, 2014
@jfindlay
Copy link
Contributor

jfindlay commented Dec 2, 2014

Here it is: #7013.

@tjyang
Copy link
Contributor

tjyang commented Dec 3, 2014

@jfindlay , thanks for the info on #7013. This bug should be classified above "Low Severity", IMO.

@jfindlay jfindlay added severity-low 4th level, cosemtic problems, work around exists and removed Low Severity labels Dec 3, 2014
@jfindlay
Copy link
Contributor

jfindlay commented Dec 3, 2014

@tjyang, you're right. It seems that not all the return code issues were not resolved with #11337.

@jfindlay jfindlay removed the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Dec 5, 2014
@jfindlay
Copy link
Contributor

jfindlay commented Dec 8, 2014

This is the plan for unifying and standardizing salt exit codes.

  • salt beryllium:
    • Introduce a standard exit code enum-like data structure, probably somewhere in exitcodes.py such that integers and enums could be used interchangeably, but with enums being preferred within the source code.
    • Add a configuration option to enable the use of this new exit code standard by automatically evaluating the truthiness of the return value of the execution module functions and translating it into exit codes.
  • salt boron: Ensure that all salt execution modules explicitly comply with
    the new salt exit code standard.

@thatch45, @cachedout, @basepi, or @UtahDave may have further commentary on this issue.

@inthecloud247
Copy link
Contributor

+1

1 similar comment
@simple10
Copy link

+1

@Oloremo
Copy link
Contributor

Oloremo commented Nov 3, 2018

Wait, what is the current status of retcodes for salt operations? It's been 4 years since the issue was created...

@terminalmage
Copy link
Contributor

Exit codes have been normalized in #48361 for the upcoming Fluorine release. The following conditions will set a nonzero exit code:

  • An exception is raised
  • The code being run sets __context__['retcode'] to a nonzero value
  • The return data for a given function is a dictionary with either a result or success key in it, which resolves as False

file.file_exists would not trigger any of those conditions, however. The problem here is if we use something like __context__['retcode'] in file.file_exists to denote failure, then we've marked the run as failed, so if any state invokesfile.file_exists, this would make salt reutrn a nonzero exit code even if the file not existing was expected (and therefore not an error).

@oliver-dungey
Copy link

I'm a bit confused about the status - I've just picked up salt 2018.3.4 which has the latest fixes in for return codes for salt-call. If I pass the argument --retcode-passthrough to salt-call I now get non-zero exit codes for failures but without it I always get 0 - is this the expected functionality at the current time? Anybody know when this functionality becomes the default?

@adubkov
Copy link
Contributor

adubkov commented Mar 4, 2019

I think this is expected, as far as I know. To get status code from operation executed by salt-call you have to use --retcode-passthrough if nothing was changed.

@terminalmage
Copy link
Contributor

@oliver-dungey As explained above, the retcode changes are in the Fluorine release (2019.2.0).

@terminalmage
Copy link
Contributor

@saltstack/team-core Since Fluorine was released, this can be closed.

@waynew waynew closed this as completed Mar 26, 2019
@dz-pyps
Copy link

dz-pyps commented Jun 13, 2019

Using salt version below, the issue with the incorrect exit code is still present when you add --batch-size and --batch-wait parameters

Salt Version:
           Salt: 2019.2.0
 
Dependency Versions:
           cffi: 1.6.0
       cherrypy: unknown
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: 0.6.4
      gitpython: 1.0.1
          ioflo: Not Installed
         Jinja2: 2.7.2
        libgit2: 0.26.3
        libnacl: Not Installed
       M2Crypto: 0.31.0
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: 2.14
       pycrypto: 2.6.1
   pycryptodome: 3.7.3
         pygit2: 0.26.4
         Python: 2.7.5 (default, Apr  9 2019, 14:30:50)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 15.3.0
           RAET: Not Installed
          smmap: 0.9.0
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.1.4

System Versions:
           dist: centos 7.6.1810 Core
         locale: UTF-8
        machine: x86_64
        release: 3.10.0-957.5.1.el7.x86_64
         system: Linux
        version: CentOS Linux 7.6.1810 Core

sudo salt -v -L minion-1,minion-2 --batch-size 50% --batch-wait 1 state.apply queue=True test-state

Even though there is an error:

jid:
    20190613124535690469
retcode:
    1
gc-euw1-salt-1:
    Data failed to compile:
----------
    Rendering SLS 'base:test-state' failed: Jinja syntax error: expected token 'end of statement block', got 'string'; line 1

salt exit status is 0.

echo $?
0

@terminalmage
Copy link
Contributor

Please open a new issue. Commenting on a closed issue doesn't do much good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior Core relates to code central or existential to Salt P1 Priority 1 severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around
Projects
None yet
Development

No branches or pull requests