Merge pull request #222 from pigmej/run-result-fix

SolarRunResult improved, fixes issues with puppet

solar/solar/core/handlers/puppet.py

@@ -49,7 +49,7 @@ class LibrarianPuppet(object):
             branch=git['branch']
         )
 
-        modules = puppetlabs.result.split('\n')
+        modules = puppetlabs.stdout.split('\n')
 
         # remove forge entry
         modules = [module for module in modules if not module.startswith('forge')]
@@ -117,10 +117,10 @@ class Puppet(TempFileHandler):
             warn_only=True
         )
         # 0 - no changes, 2 - successfull changes
-        if cmd.failed:
+        if cmd.return_code not in [0, 2]:
             raise errors.SolarError(
                 'Puppet for {} failed with {}'.format(
-                    resource.name, cmd.result))
+                    resource.name, cmd.return_code))
         return cmd
 
     def clone_manifests(self, resource):

solar/solar/core/transports/base.py

@@ -40,6 +40,41 @@ class Executor(object):
             self._executor(transport)
 
 
+class SolarRunResultWrp(object):
+
+    def __init__(self, name):
+        self.name = name
+
+    def __get__(self, obj, objtype):
+        res = obj._result
+        if isinstance(res, dict):
+            try:
+                return res[self.name]
+            except KeyError:
+                # Let's keep the same exceptions
+                raise AttributeError(self.name)
+        return getattr(obj._result, self.name)
+
+
+class SolarRunResult(object):
+
+    def __init__(self, result):
+        self._result = result
+
+    failed = SolarRunResultWrp('failed')
+    stdout = SolarRunResultWrp('stdout')
+    stderr = SolarRunResultWrp('stderr')
+    succeeded = SolarRunResultWrp('succeeded')
+    command = SolarRunResultWrp('command')
+    real_command = SolarRunResultWrp('real_command')
+    return_code = SolarRunResultWrp('return_code')
+
+    def __str__(self):
+        if self.failed:
+            return str(self.stderr)
+        return str(self.stdout)
+
+
 class SolarTransport(object):
 
     _mode = None
@@ -59,27 +94,6 @@ class SolarTransport(object):
         return transport
 
 
-
-class SolarRunResult(object):
-
-    def __init__(self, result, failed=True):
-        self._result = result
-        self._failed = failed
-
-    @property
-    def failed(self):
-        return self._failed
-
-    @property
-    def result(self):
-        return self._result
-
-    def __str__(self):
-        if self.failed:
-            return str(self.failed)
-        return str(self.result)
-
-
 class SyncTransport(SolarTransport):
     """
     Transport that is responsible for file / directory syncing.

solar/solar/core/transports/solard_transport.py

@@ -53,16 +53,12 @@ class SolardRunTransport(RunTransport, SolardTransport):
 
     preffered_transport_name = 'solard'
 
-    def get_result(self, result, failed=False):
+    def get_result(self, result):
-        return SolarRunResult(result, failed)
+        return SolarRunResult(result)
 
     def run(self, resource, *args, **kwargs):
         log.debug("Solard run: %s", args)
         client = self.get_client(resource)
-        try:
+        res = client.run(' '.join(args), **kwargs)
-            res = client.run(' '.join(args), **kwargs)
+        return self.get_result(res)
-            return self.get_result(res, failed=False)
-        except Exception as ex:
-            log.exception("Exception during solard run")
-            return self.get_result(ex, failed=True)
 

solar/solar/core/transports/ssh.py

@@ -91,10 +91,7 @@ class SSHRunTransport(RunTransport, _SSHTransport):
         """
         Needed for compatibility with other handlers / transports
         """
-        if output.failed:
+        return SolarRunResult(output)
-            return SolarRunResult(output, failed=True)
-        return SolarRunResult(output, failed=False)
-
 
     def run(self, resource, *args, **kwargs):
         log.debug('SSH: %s', args)

solard/solard/core.py

@@ -202,14 +202,17 @@ class SolardIface(object):
         if env:
             managers.append(fabric_api.shell_env(**kwargs['env']))
 
-        if kwargs.get('warn_only', False):
+        # we just warn, don't exit on solard
-            managers.append(fabric_api.warn_only())
+        # correct data is returned
+        managers.append(fabric_api.warn_only())
 
         with nested(*managers):
             out = executor(cmd, capture=True)
-            if out.failed:
+            result = {}
-                raise Exception("Remote failed")
+            for name in ('failed', 'return_code', 'stdout', 'stderr',
-            return out.stdout
+                         'succeeded', 'command', 'real_command'):
+                result[name] = getattr(out, name)
+            return result
 
     @staticmethod
     def copy_file(solard_context, stream_reader, path, size=None):