--failhard option not working as expected

[danlsgiga]

The --failhard parameter for the salt command is supposed to stop batch execution upon first "bad" return. This is not happening even when failhard = True in the master config.
Quick example:
salt -b 1 --failhard 'nginx*' cmd.run 'systemctl restart nginx'
is executing in all targetted systems regardless of the error return code. It should stop the batch execution at the first failed execution.

[justinta]

@danlsgiga Can you provide the output from salt --versions-report? Thanks!

[danlsgiga]

Sure! Here it goes:
salt --versions-report
Salt: 2015.5.2
Python: 2.7.5 (default, Jun 24 2015, 00:41:19)
Jinja2: 2.7.2
M2Crypto: 0.21.1
msgpack-python: 0.4.6
msgpack-pure: Not Installed
pycrypto: 2.6.1
libnacl: Not Installed
PyYAML: 3.10
ioflo: Not Installed
PyZMQ: 14.3.1
RAET: Not Installed
ZMQ: 3.2.5
Mako: Not Installed

[danlsgiga]

Just as an update, I made some tests using the batch mode and --failhard with modules and it is working fine, the issue is happening only when using the cmd.run module. So, for example:
salt -b 1 --failhard '' service.restart httpd -> Works
salt -b 1 --failhard '' cmd.run 'systemctl restart httpd' -> Doesn't work even when ret code != 0

[danlsgiga]

I was reading the salt -d documentation and tried the cmd.run_all module with the -b1 --failhard parameters. So, it turned out that I must use cmd.run_all instead of cmd.run to have the --failhard parameter to be effectively working. Is that on purpose or is it a bug?

[danlsgiga]

Here are the results for the cmd.run module:

salt -b1 --failhard 'minion*' cmd.run 'systemctl restart httpd'

Executing run on ['minion3.local.dom']

minion3.local.dom:
/bin/sh: systemctl: command not found
retcode:
127

Executing run on ['minion1.local.dom']

minion1.local.dom:
Failed to issue method call: Unit httpd.service failed to load: No such file or directory.
retcode:
6

Executing run on ['minion2.local.dom']

minion2.local.dom:
Failed to issue method call: Unit httpd.service failed to load: No such file or directory.
retcode:
6

[danlsgiga]

Here are the results for the cmd.run module:

salt -b1 --failhard 'minion*' cmd.run_all 'systemctl restart httpd'

Executing run on ['minion3.local.dom']

[justinta]

@danlsgiga I'm so sorry for the delay on this, I seem to have not gotten a notification from your updates until today... Let me look into --failhard a little more. I'm leaning towards this being a bug, but I may be missing something about it being expected behavior.

[danlsgiga]

@jtand No worries... The behaviour I expected on cmd.run using the batch mode and --failhard is working great if I use cmd.run_all. But yeah, it's a good idea to check if cmd.run must behave like that or if it is a bug. I'm also leaning towards this being a bug, because the only specification difference between cmd.run and cmd.run_all should be cmd.run returns data as string and cmd.run_all as a dict. Thanks!! ;)

[rallytime]

@danlsgiga The fix in #33048 should button this up for you. I showed an example of the previous behavior and the new behavior in the pull request. Please give that a try and let me know how it goes.

Note that the command doesn't currently give any output about the failure as it stands in that fix. I'm going to address that in a new PR to fix #32452, which is linked above.

[danlsgiga]

Hey @rallytime, awesome news! Thanks for working on this!
So, yeah, I've just changed my batch.py code to the proposed commit, restarted salt-master and it works like a charm!!!
Awesome!!!!

[rallytime]

@danlsgiga Sweeeet! Glad to hear it! :D