Don't allow mercurial states to return True with errors

Fixes #36553

The hg.present state was not checking for errors for any of the calls
to the hg.py execution module functions. All of the hg.py execution
module functions were using cmd.run, instead of cmd.run_all, which
allowed for hg.py execution module functions to log an error at the CLI
but the hg.present state would return True, even though there were
problems executing the module functions.

This change adds try/except blocks around the calls to the mercurial
execution module functions and retuns False when a CommandExecutionError
is raised by the module. The module has been changes to use cmd.run_all
instead of cmd.run in order to check for the retcode of the underlying
mercurial calls and raises a CommandExecutionError is the retcode != 0.

salt/modules/hg.py

@@ -2,11 +2,14 @@
 '''
 Support for the Mercurial SCM
 '''
+
+# Import Python libs
 from __future__ import absolute_import
 import logging
 
 # Import salt libs
-from salt import utils
+from salt.exceptions import CommandExecutionError
+import salt.utils
 
 log = logging.getLogger(__name__)
 
@@ -15,7 +18,7 @@ def __virtual__():
     '''
     Only load if hg is installed
     '''
-    if utils.which('hg') is None:
+    if salt.utils.which('hg') is None:
         return (False,
                 'The hg execution module cannot be loaded: hg unavailable.')
     else:
@@ -187,7 +190,14 @@ def pull(cwd, opts=None, user=None, identity=None, repository=None):
             cmd.append(opt)
     if repository is not None:
         cmd.append(repository)
-    return __salt__['cmd.run'](cmd, cwd=cwd, runas=user, python_shell=False)
+
+    ret = __salt__['cmd.run_all'](cmd, cwd=cwd, runas=user, python_shell=False)
+    if ret['retcode'] != 0:
+        raise CommandExecutionError(
+            'Hg command failed: {0}'.format(ret.get('stderr', ret['stdout']))
+        )
+
+    return ret['stdout']
 
 
 def update(cwd, rev, force=False, user=None):
@@ -215,7 +225,14 @@ def update(cwd, rev, force=False, user=None):
     cmd = ['hg', 'update', '{0}'.format(rev)]
     if force:
         cmd.append('-C')
-    return __salt__['cmd.run'](cmd, cwd=cwd, runas=user, python_shell=False)
+
+    ret = __salt__['cmd.run_all'](cmd, cwd=cwd, runas=user, python_shell=False)
+    if ret['retcode'] != 0:
+        raise CommandExecutionError(
+            'Hg command failed: {0}'.format(ret.get('stderr', ret['stdout']))
+        )
+
+    return ret['stdout']
 
 
 def clone(cwd, repository, opts=None, user=None, identity=None):
@@ -251,7 +268,14 @@ def clone(cwd, repository, opts=None, user=None, identity=None):
             cmd.append('{0}'.format(opt))
     if identity:
         cmd.extend(_ssh_flag(identity))
-    return __salt__['cmd.run'](cmd, runas=user, python_shell=False)
+
+    ret = __salt__['cmd.run_all'](cmd, runas=user, python_shell=False)
+    if ret['retcode'] != 0:
+        raise CommandExecutionError(
+            'Hg command failed: {0}'.format(ret.get('stderr', ret['stdout']))
+        )
+
+    return ret['stdout']
 
 
 def status(cwd, opts=None, user=None):
@@ -298,7 +322,7 @@ def status(cwd, opts=None, user=None):
             ret[t].append(f)
         return ret
 
-    if utils.is_iter(cwd):
+    if salt.utils.is_iter(cwd):
         return dict((cwd, _status(cwd)) for cwd in cwd)
     else:
         return _status(cwd)

salt/states/hg.py

@@ -13,15 +13,16 @@ in ~/.ssh/known_hosts, and the remote host has this host's public key.
           - rev: tip
           - target: /tmp/example_repo
 '''
-from __future__ import absolute_import
 
 # Import python libs
+from __future__ import absolute_import
 import logging
 import os
 import shutil
 
 # Import salt libs
 import salt.utils
+from salt.exceptions import CommandExecutionError
 from salt.states.git import _fail, _neutral_test
 
 log = logging.getLogger(__name__)
@@ -130,12 +131,27 @@ def _update_repo(ret, name, target, clean, user, identity, rev, opts):
                 ret,
                 test_result)
 
-    pull_out = __salt__['hg.pull'](target, user=user, identity=identity, opts=opts, repository=name)
+    try:
+        pull_out = __salt__['hg.pull'](target, user=user, identity=identity, opts=opts, repository=name)
+    except CommandExecutionError as err:
+        ret['result'] = False
+        ret['comment'] = err
+        return ret
 
     if rev:
-        __salt__['hg.update'](target, rev, force=clean, user=user)
+        try:
+            __salt__['hg.update'](target, rev, force=clean, user=user)
+        except CommandExecutionError as err:
+            ret['result'] = False
+            ret['comment'] = err
+            return ret
     else:
-        __salt__['hg.update'](target, 'tip', force=clean, user=user)
+        try:
+            __salt__['hg.update'](target, 'tip', force=clean, user=user)
+        except CommandExecutionError as err:
+            ret['result'] = False
+            ret['comment'] = err
+            return ret
 
     new_rev = __salt__['hg.revision'](cwd=target, user=user, rev='.')
 
@@ -172,13 +188,23 @@ def _handle_existing(ret, target, force):
 
 
 def _clone_repo(ret, target, name, user, identity, rev, opts):
-    result = __salt__['hg.clone'](target, name, user=user, identity=identity, opts=opts)
+    try:
+        result = __salt__['hg.clone'](target, name, user=user, identity=identity, opts=opts)
+    except CommandExecutionError as err:
+        ret['result'] = False
+        ret['comment'] = err
+        return ret
 
     if not os.path.isdir(target):
         return _fail(ret, result)
 
     if rev:
-        __salt__['hg.update'](target, rev, user=user)
+        try:
+            __salt__['hg.update'](target, rev, user=user)
+        except CommandExecutionError as err:
+            ret['result'] = False
+            ret['comment'] = err
+            return ret
 
     new_rev = __salt__['hg.revision'](cwd=target, user=user)
     message = 'Repository {0} cloned to {1}'.format(name, target)