states.hg.latest claims to succeed despite errors #36553

nilliams · 2016-09-24T09:51:00Z

Description of Issue/Question

Despite seeing errors from the hg command (which were my fault), the formatted salt report thing says that the state hg.latest succeeded.

Setup

# mercurial.sls
mercurial:
  hg.latest:
    - name: ssh://hg@bitbucket.org/nickwilliams/influx-salt
    - rev: tip
    - target: /srv
    - identity: /root/deploy_rsa

Steps to Reproduce Issue

In this example I've accidentally left an untracked file salt/mercurial.sls lying around in the checkout, which (correctly) fails the hg update with errors, however as you see the salt report claims it succeeded.

(ve-salt) vagrant@precise64:~$ sudo salt-call --local state.apply
[ERROR   ] Command '['hg', 'update', 'tip']' failed with return code: 255
[ERROR   ] output: abort: untracked file in working directory differs from file in requested revision: 'salt/mercurial.sls'
local:
----------
          ID: mercurial
    Function: hg.latest
        Name: ssh://hg@bitbucket.org/nickwilliams/influx-salt
      Result: True
     Comment:
     Started: 09:33:17.809941
    Duration: 2061.287 ms
     Changes:

Summary for local
------------
Succeeded: 1
Failed:    0

Versions Report

I'm running salt at this previous PR re: states.hg, but I did check the same thing happens with the latest release via salt-bootstrap.sh too (2016.3.3).

(ve-salt) vagrant@precise64:~$ sudo salt-call --versions-report
Salt Version:
           Salt: 2016.3.3-434-g9afe767

Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: Not Installed
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.8
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.8
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
         pygit2: Not Installed
         Python: 2.7.3 (default, Jun 22 2015, 19:33:41)
   python-gnupg: Not Installed
         PyYAML: 3.12
          PyZMQ: 15.4.0
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.4.1
            ZMQ: 4.1.5

System Versions:
           dist: Ubuntu 12.04 precise
        machine: x86_64
        release: 3.2.0-23-generic
         system: Linux
        version: Ubuntu 12.04 precise

The text was updated successfully, but these errors were encountered:

rallytime · 2016-09-26T22:28:04Z

@nilliams Thanks again for filing this. We're probably just not catching the error correctly and propagating it up to the state run to fail correctly. I can grab this one really quickly, too. I can grab it likely sometime tomorrow.

Fixes saltstack#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.

rallytime · 2016-09-27T23:37:03Z

@nilliams Can you give #36620 a try and please let me know how that goes?

nilliams · 2016-09-28T19:41:05Z

Yes that works for me, fails hard now, thank you @rallytime!

[ERROR   ] Command '['hg', 'update', 'tip']' failed with return code: 255
[ERROR   ] stderr: salt/mercurial.sls: untracked file differs
abort: untracked files in working directory differ from files in requested revision
[ERROR   ] retcode: 255
[ERROR   ] An exception occurred in this state: Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/salt/state.py", line 1733, in call
    **cdata['kwargs'])
  File "/usr/lib/python2.7/dist-packages/salt/loader.py", line 1652, in wrapper
    return f(*args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/salt/states/hg.py", line 91, in latest
    ret = _update_repo(ret, name, target, clean, user, identity, rev, opts)
  File "/usr/lib/python2.7/dist-packages/salt/states/hg.py", line 136, in _update_repo
    __salt__['hg.update'](target, rev, force=clean, user=user)
  File "/var/cache/salt/minion/extmods/modules/hg.py", line 233, in update
    'Hg command failed: {0}'.format(ret.get('stderr', ret['stdout']))
CommandExecutionError: Hg command failed: salt/mercurial.sls: untracked file differs
abort: untracked files in working directory differ from files in requested revision

local:
----------
          ID: mercurial
    Function: hg.latest
        Name: ssh://hg@bitbucket.org/nickwilliams/influx-salt
      Result: False
     Comment: An exception occurred in this state: Traceback (most recent call last):
                File "/usr/lib/python2.7/dist-packages/salt/state.py", line 1733, in call
                  **cdata['kwargs'])
                File "/usr/lib/python2.7/dist-packages/salt/loader.py", line 1652, in wrapper
                  return f(*args, **kwargs)
                File "/usr/lib/python2.7/dist-packages/salt/states/hg.py", line 91, in latest
                  ret = _update_repo(ret, name, target, clean, user, identity, rev, opts)
                File "/usr/lib/python2.7/dist-packages/salt/states/hg.py", line 136, in _update_repo
                  __salt__['hg.update'](target, rev, force=clean, user=user)
                File "/var/cache/salt/minion/extmods/modules/hg.py", line 233, in update
                  'Hg command failed: {0}'.format(ret.get('stderr', ret['stdout']))
              CommandExecutionError: Hg command failed: salt/mercurial.sls: untracked file differs
              abort: untracked files in working directory differ from files in requested revision
     Started: 19:39:26.399796
    Duration: 2848.899 ms
     Changes:

Summary for local
------------
Succeeded: 0
Failed:    1
------------
Total states run:     1
Total run time:   2.849 s

rallytime · 2016-09-28T19:48:48Z

Hrm...I could probably clean up stack trace in your state output. That's not very pretty...

nilliams · 2016-09-28T20:28:46Z

Hehe I guess, I'm new to Salt so wasn't really sure what to expect there. Feel free to shout me to try out changes.

rallytime · 2016-09-29T20:45:21Z

@nilliams Can you tell me how you updated to test this? It seems to me like you have updated the execution module, but not the hg.py state. Updating the hg.py state as well as the execution module should clean up those stacktraces in your state return.

nilliams · 2016-09-30T09:21:07Z

Sorry, my bad! Great diagnosis of my dumbness!

Yes it works if I actually update properly and use the state too:

[ERROR   ] Command '['hg', 'pull', '--ssh', 'ssh -i /root/deploy_rsa', 'ssh://hg@bitbucket.org/nickwilliams/influx-salt']' failed with return code: 255
[ERROR   ] stdout: pulling from ssh://hg@bitbucket.org/nickwilliams/influx-salt
remote: Warning: Permanently added the RSA host key for IP address '104.192.143.3' to the list of known hosts.
remote: conq: repository does not exist.
[ERROR   ] stderr: abort: no suitable response from remote hg!
[ERROR   ] retcode: 255
[ERROR   ] Hg command failed: abort: no suitable response from remote hg!
local:
----------
          ID: mercurial
    Function: hg.latest
        Name: ssh://hg@bitbucket.org/nickwilliams/influx-salt
      Result: False
     Comment: Hg command failed: abort: no suitable response from remote hg!
     Started: 09:06:14.242413
    Duration: 6320.05 ms
     Changes:

Summary for local
------------
Succeeded: 0
Failed:    1
------------

EDIT that was actually an example of a diff error, failing on the pull, not the update. But can confirm the update failure case from before is also cleaner too:

[ERROR   ] Command '['hg', 'update', 'tip']' failed with return code: 255
[ERROR   ] stderr: salt/mercurial.sls: untracked file differs
abort: untracked files in working directory differ from files in requested revision
[ERROR   ] retcode: 255
[ERROR   ] Hg command failed: salt/mercurial.sls: untracked file differs
abort: untracked files in working directory differ from files in requested revision
local:
----------
          ID: mercurial
    Function: hg.latest
        Name: ssh://hg@bitbucket.org/tipandrews/influx-salt
      Result: False
     Comment: Hg command failed: salt/mercurial.sls: untracked file differs
              abort: untracked files in working directory differ from files in requested revision
     Started: 09:22:18.443135
    Duration: 2347.395 ms
     Changes:

Summary for local
------------
Succeeded: 0
Failed:    1
------------

rallytime · 2016-09-30T14:15:40Z

Excellent! Thanks for confirming! I'll close this up.

nilliams mentioned this issue Sep 24, 2016

salt.stages.hg errors when -identity option is used #36514

Closed

nilliams changed the title ~~salt.states.hg claims to succeed despite errors~~ states.hg.latest claims to succeed despite errors Sep 25, 2016

rallytime added Bug broken, incorrect, or confusing behavior severity-medium 3rd level, incorrect or bad functionality, confusing and lacks a work around P2 Priority 2 Platform Relates to OS, containers, platform-based utilities like FS, system based apps TEAM Core labels Sep 26, 2016

rallytime added this to the C 3 milestone Sep 26, 2016

rallytime self-assigned this Sep 26, 2016

rallytime mentioned this issue Sep 27, 2016

Don't allow mercurial states to return True with errors #36620

Merged

rallytime added the fixed-pls-verify fix is linked, bug author to confirm fix label Sep 27, 2016

rallytime closed this as completed Sep 30, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

states.hg.latest claims to succeed despite errors #36553

states.hg.latest claims to succeed despite errors #36553

nilliams commented Sep 24, 2016 •

edited

rallytime commented Sep 26, 2016

rallytime commented Sep 27, 2016

nilliams commented Sep 28, 2016

rallytime commented Sep 28, 2016 •

edited

nilliams commented Sep 28, 2016

rallytime commented Sep 29, 2016

nilliams commented Sep 30, 2016 •

edited

rallytime commented Sep 30, 2016

states.hg.latest claims to succeed despite errors #36553

states.hg.latest claims to succeed despite errors #36553

Comments

nilliams commented Sep 24, 2016 • edited

Description of Issue/Question

Setup

Steps to Reproduce Issue

Versions Report

rallytime commented Sep 26, 2016

rallytime commented Sep 27, 2016

nilliams commented Sep 28, 2016

rallytime commented Sep 28, 2016 • edited

nilliams commented Sep 28, 2016

rallytime commented Sep 29, 2016

nilliams commented Sep 30, 2016 • edited

rallytime commented Sep 30, 2016

nilliams commented Sep 24, 2016 •

edited

rallytime commented Sep 28, 2016 •

edited

nilliams commented Sep 30, 2016 •

edited