batch mode with --failhard doesn't work

[michalchmielewski]

Hi,

in salt 2017.7.2 in batch mode the --failhard option doesn't work. Execution is not stopped when retcode from server is non zero:

# salt -b 1 --failhard "app*" cmd.run "ls -l /tmp/test"; echo $?

Executing run on ['app1']

app1:
    ls: cannot access /tmp/test: No such file or directory
jid:
    20171024123336239687
retcode:
    2

Executing run on ['app2']

app2:
    total 0
jid:
    20171024123336390559
retcode:
    0

Executing run on ['app3']

app3:
    total 0
jid:
    20171024123336544721
retcode:
    0
0

It works when I use cmd.run_all instead of cmd.run.
I have found that the issue was resolved once, by applying the following changes: https://github.com/saltstack/salt/pull/33048/files
With the patch the command works perfectly.
Could you verify that and apply the patch to next salt release version ?

Salt version report from test environment:

Salt Version:
           Salt: 2017.7.2

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
      docker-py: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.8.1
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.6
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.13 (default, Mar 27 2017, 16:47:34)
   python-gnupg: Not Installed
         PyYAML: 3.11
          PyZMQ: 14.5.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.2.1
            ZMQ: 4.0.5

[Ch3LL]

That patch was included in releases a long time ago. It was removed here with this PR: #37625 to resolve a different issue related to return codes. So i think we need to find a balance of ensuring proper retcodes are resolved and adding the failhard option.
ping @cachedout or @rallytime any ideas here?

[michalchmielewski]

Looks like functionality introduced in #37625 doesn't work as well (even without --failhard option):

# salt -b 1 "app*" cmd.run "ls -l /tmp/test"; echo $?

Executing run on ['app1']

app1:
    total 0
jid:
    20171106115747708533
retcode:
    0

Executing run on ['app2']

app2:
    total 0
jid:
    20171106115747890474
retcode:
    0

Executing run on ['app3']

app3:
    ls: cannot access /tmp/test: No such file or directory
jid:
    20171106115748075488
retcode:
    2
0

Salt exits with return code 0 instead of 2 which is the highest in this example.

[michalchmielewski]

However the highest return code is returned when cmd.run_all is used.

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