Merge pull request #2418 from ffa/demo_module_reporting_standard

Demo of saltstack/salt#2417 module standards choice "1"

salt/modules/git.py

@@ -3,7 +3,24 @@ Support for the Git SCM
 '''
 
 import os
-from salt import utils
+from salt import utils, exceptions
+
+def _git_run(cmd, cwd=None, **kwargs):
+    '''
+    simple, throw an exception with the error message on an error return code.
+
+    this function may be moved to the command module, spliced with
+    'cmd.run_all', and used as an alternative to 'cmd.run_all'. Some
+    commands don't return proper retcodes, so this can't replace 'cmd.run_all'.
+    '''
+    result = __salt__['cmd.run_all'](cmd, cwd=cwd, **kwargs)
+
+    retcode = result['retcode']
+
+    if retcode == 0:
+        return result['stdout']
+    else:
+        raise exceptions.CommandExecutionError(result['stderr'])
 
 def _git_getdir(cwd, user=None):
     '''
@@ -45,12 +62,7 @@ def revision(cwd, rev='HEAD', short=False, user=None):
     _check_git()
 
     cmd = 'git rev-parse {0}{1}'.format('--short ' if short else '', rev)
-    result = __salt__['cmd.run_all'](cmd, cwd, runas=user)
+    return _git_run(cmd, cwd, runas=user)
-
-    if result['retcode'] == 0:
-        return result['stdout']
-    else:
-        return ''
 
 def clone(cwd, repository, opts=None, user=None):
     '''
@@ -82,7 +94,7 @@ def clone(cwd, repository, opts=None, user=None):
         opts = ''
     cmd = 'git clone {0} {1} {2}'.format(repository, cwd, opts)
 
-    return __salt__['cmd.run'](cmd, runas=user)
+    return _git_run(cmd, runas=user)
 
 def describe(cwd, rev='HEAD', user=None):
     '''
@@ -146,7 +158,7 @@ def archive(cwd, output, rev='HEAD', fmt=None, prefix=None, user=None):
         fmt = ' --format={0}'.format(fmt) if fmt else '',
         prefix = ' --prefix="{0}"'.format(prefix if prefix else basename))
 
-    return __salt__['cmd.run'](cmd, cwd=cwd, runas=user)
+    return _git_run(cmd, cwd=cwd, runas=user)
 
 def fetch(cwd, opts=None, user=None):
     '''
@@ -173,7 +185,7 @@ def fetch(cwd, opts=None, user=None):
         opts = ''
     cmd = 'git fetch {0}'.format(opts)
 
-    return __salt__['cmd.run'](cmd, cwd=cwd, runas=user)
+    return _git_run(cmd, cwd=cwd, runas=user)
 
 def pull(cwd, opts=None, user=None):
     '''
@@ -196,7 +208,7 @@ def pull(cwd, opts=None, user=None):
 
     if not opts:
         opts = ''
-    return __salt__['cmd.run']('git pull {0}'.format(opts), cwd=cwd, runas=user)
+    return _git_run('git pull {0}'.format(opts), cwd=cwd, runas=user)
 
 def rebase(cwd, rev='master', opts=None, user=None):
     '''
@@ -224,7 +236,7 @@ def rebase(cwd, rev='master', opts=None, user=None):
 
     if not opts:
         opts = ''
-    return __salt__['cmd.run']('git rebase {0}'.format(opts), cwd=cwd, runas=user)
+    return _git_run('git rebase {0}'.format(opts), cwd=cwd, runas=user)
 
 def checkout(cwd, rev, force=False, opts=None, user=None):
     '''
@@ -258,7 +270,7 @@ def checkout(cwd, rev, force=False, opts=None, user=None):
     if not opts:
         opts = ''
     cmd = 'git checkout {0} {1} {2}'.format(' -f' if force else '', rev, opts)
-    return __salt__['cmd.run'](cmd, cwd=cwd, runas=user)
+    return _git_run(cmd, cwd=cwd, runas=user)
 
 def merge(cwd, branch='@{upstream}', opts=None, user=None):
     '''
@@ -289,7 +301,7 @@ def merge(cwd, branch='@{upstream}', opts=None, user=None):
             branch,
             opts)
 
-    return __salt__['cmd.run'](cmd, cwd, runas=user)
+    return _git_run(cmd, cwd, runas=user)
 
 def init(cwd, opts=None, user=None):
     '''
@@ -311,7 +323,7 @@ def init(cwd, opts=None, user=None):
     _check_git()
 
     cmd = 'git init {0} {1}'.format(cwd, opts)
-    return __salt__['cmd.run'](cmd, runas=user)
+    return _git_run(cmd, runas=user)
 
 def submodule(cwd, init=True, opts=None, user=None):
     '''
@@ -334,4 +346,4 @@ def submodule(cwd, init=True, opts=None, user=None):
     if not opts:
         opts = ''
     cmd = 'git submodule update {0} {1}'.format('--init' if init else '', opts)
-    return __salt__['cmd.run'](cmd, cwd=cwd, runas=user)
+    return _git_run(cmd, cwd=cwd, runas=user)

salt/states/git.py

@@ -63,11 +63,12 @@ def latest(name,
                 'target {0} is found, "git pull" is probably required'.format(
                     target)
                 )
+        try:
             current_rev = __salt__['git.revision'](target, user=runas)
-        if not current_rev:
+        except Exception, exception:
             return _fail(
                     ret,
-                    'Seems that {0} is not a valid git repo'.format(target))
+                    str(exception))
         if __opts__['test']:
             return _neutral_test(
                     ret,
@@ -76,6 +77,7 @@ def latest(name,
 
         pull_out = __salt__['git.pull'](target, user=runas)
 
+        try:
             if rev:
                 __salt__['git.checkout'](target, rev, user=runas)
 
@@ -83,6 +85,12 @@ def latest(name,
                 __salt__['git.submodule'](target, user=runas, opts='--recursive')
 
             new_rev = __salt__['git.revision'](cwd=target, user=runas)
+
+        except Exception, exception:
+            return _fail(
+                    ret,
+                    str(exception))
+
         if current_rev != new_rev:
             log.info('Repository {0} updated: {1} => {2}'.format(target,
                                                                  current_rev,
@@ -90,13 +98,6 @@ def latest(name,
             ret['comment'] = 'Repository {0} updated'.format(target)
             ret['changes']['revision'] = '{0} => {1}'.format(
                     current_rev, new_rev)
-        else:
-            # Check that there was no error preventing the revision update
-            if 'error:' in pull_out:
-                return _fail(
-                    ret,
-                    'An error was thrown by git:\n{0}'.format(pull_out)
-                    )
     else:
         if os.path.isdir(target):
             # git clone is required, but target exists -- however it is empty
@@ -125,10 +126,9 @@ def latest(name,
                     ret,
                     'Repository {0} is about to be cloned to {1}'.format(
                         name, target))
+        try:
             # make the clone
             result = __salt__['git.clone'](target, name, user=runas)
-        if not os.path.isdir(target):
-            return _fail(ret, result)
 
             if rev:
                 __salt__['git.checkout'](target, rev, user=runas)
@@ -138,6 +138,11 @@ def latest(name,
 
             new_rev = __salt__['git.revision'](cwd=target, user=runas)
 
+        except Exception, exception:
+            return _fail(
+                    ret,
+                    str(exception))
+
         message = 'Repository {0} cloned to {1}'.format(name, target)
         log.info(message)
         ret['comment'] = message

tests/integration/states/git.py

@@ -0,0 +1,85 @@
+'''
+Tests for the Git state
+'''
+import os
+import shutil
+import integration
+
+
+class GitTest(integration.ModuleCase):
+    '''
+    Validate the git state
+    '''
+
+    def test_latest(self):
+        '''
+        git.latest
+        '''
+        name = os.path.join(integration.TMP, 'salt_repo')
+        ret = self.run_state(
+            'git.latest',
+            name='https://github.com/saltstack/salt.git',
+            rev='develop',
+            target=name,
+            submodules=True
+        )
+        self.assertTrue(os.path.isdir(os.path.join(name, '.git')))
+        result = self.state_result(ret)
+        self.assertTrue(result)
+        shutil.rmtree(name, ignore_errors=True)
+
+    def test_latest_failure(self):
+        '''
+        git.latest
+        '''
+        name = os.path.join(integration.TMP, 'salt_repo')
+        ret = self.run_state(
+            'git.latest',
+            name='https://youSpelledGithubWrong.com/saltstack/salt.git',
+            rev='develop',
+            target=name,
+            submodules=True
+        )
+        self.assertFalse(os.path.isdir(os.path.join(name, '.git')))
+        result = self.state_result(ret)
+        self.assertFalse(result)
+        shutil.rmtree(name, ignore_errors=True)
+
+    def test_present(self):
+        '''
+        git.present
+        '''
+        name = os.path.join(integration.TMP, 'salt_repo')
+        ret = self.run_state(
+            'git.present',
+            name=name,
+            bare=True
+        )
+        self.assertTrue(os.path.isfile(os.path.join(name, 'HEAD')))
+        result = self.state_result(ret)
+        self.assertTrue(result)
+        shutil.rmtree(name, ignore_errors=True)
+
+    def test_present_failure(self):
+        '''
+        git.present
+        '''
+        name = os.path.join(integration.TMP, 'salt_repo')
+        os.mkdir(name)
+        fname = os.path.join(name, 'stoptheprocess')
+        with file(fname, 'a'):
+            pass
+        ret = self.run_state(
+            'git.present',
+            name=name,
+            bare=True
+        )
+        self.assertFalse(os.path.isfile(os.path.join(name, 'HEAD')))
+        result = self.state_result(ret)
+        self.assertFalse(result)
+        shutil.rmtree(name, ignore_errors=True)
+
+
+if __name__ == '__main__':
+    from integration import run_tests
+    run_tests(GitTest)