Return errors in salt-api calls; add API-centric support for new exit codes

[Grokzen]

I have setup salt-api (cherrypy) and a salt-master on the same machine and i have a few minions connected to the master.

If i then issue a command to be executed to the api that do not match any targets i get back from requests the following:

>>> r
<Response [200]>
>>> r.text
u'{"return": [{}]}'

But i get a string output on stdout from salt-api saying No minions matched the target. No command was sent, no jid was assigned. so clearly the error is recognized somewhere but the error is not handled by the root endpoint.

[jfindlay]

@Grokzen, thanks for reporting.

[whiteinge]

@Grokzen this behavior matches Salt's CLI and Python API (empty results); the CLI additionally checks that return is empty and reports that message on stderr if so. What would you prefer to see via the REST API?

[Grokzen]

Well i am not sure exactly what the best solution would be but i would say that it do not conform with error handling in the cli right now.

Look at this example:

salt-master:~ # salt foobar test.ping --out=json
No minions matched the target. No command was sent, no jid was assigned.

salt-master:~ # echo $?
2

salt-master:~ # salt '*' test.ping
salt-minion:
    True

salt-master:~ # echo $?
0

If i from the cli target a minion that do not exists i get back a empty response and that error on stdout that no minions matched the target. But when i look at the exit codes i still get 2 from the call that seemed to be ok.

If we translate this to how it works now in the api when my target matches no minions i get back OK and 200 code that would be like getting back 0 status code on the cli.

I have also looked at if a http error code would be good for this but i do not think so because i can't find any that makes sense in this case.

But it might be good enough at the same time as is and i would have to assume that if the return block is empty then no targets was matched even if that is a big assumption on what the problem really is.

[whiteinge]

Hm. Thanks for pointing that out. The retcode thing is fairly new. It looks like we started hiding the empty return as well. Though this is a separate issue, it's problematic at the Python API level because it's printing to stderr when it shouldn't:

>>> import salt.client
>>> client = salt.client.LocalClient()
>>> ret = client.cmd('stewart', 'test.ping')
No minions matched the target. No command was sent, no jid was assigned.
>>> print ret
{}

We can't approach this at the HTTP status code level because you can send more than one command in a request, each of which may have a different result:

% echo '[{client: local, tgt: jerry, fun: test.ping}, {client: local, tgt: stewart, fun: test.ping}]' | curl -sS localhost:8000 -H 'Content-type: application/x-yaml' -H 'Accept: application/x-yaml' -d@-

return:
- jerry: true
- {}

We could add an errors key to the return (same level as the return key):

return:
- jerry: true
- {}
errors:
- []
- ['No minions matched the target. No command was sent, no jid was assigned.']

[whiteinge]

Relevant CLI exit code commentary.

[Grokzen]

@whiteinge It sounds like a good improvment. It would make it easy to understand and track down what the cause of the problem is.

[whiteinge]

Cool. Classifying it as such. We'll try to make this change coincide with the proposal in #18510 too.

Thanks for filing this!

[inthecloud247]

+1

[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.

[whiteinge]

I think this can be closed now that #41356 is in. We can't change the HTTP status code for the root (/) endpoint since it can handle multiple function calls and is more RPC-like, but now the return code info can be returned from the API directly.

Edit: Oh, and feel free to comment, reopen, or file a follow-up issue for more specific things than just surfacing the retcode.