From fb0b6df1d0f4b5fd1bb8f97754a27954a49a054c Mon Sep 17 00:00:00 2001
From: Jedrzej Nowak <jnowak@mirantis.com>
Date: Fri, 2 Oct 2015 10:33:33 +0200
Subject: [PATCH] SolarRunResult improved, fixes issues with puppet

---
 solar/solar/core/handlers/puppet.py           |  6 +-
 solar/solar/core/transports/base.py           | 56 ++++++++++++-------
 .../solar/core/transports/solard_transport.py | 12 ++--
 solar/solar/core/transports/ssh.py            |  5 +-
 solard/solard/core.py                         | 13 +++--
 5 files changed, 51 insertions(+), 41 deletions(-)

diff --git a/solar/solar/core/handlers/puppet.py b/solar/solar/core/handlers/puppet.py
index b4d12a44..668d41d1 100644
--- a/solar/solar/core/handlers/puppet.py
+++ b/solar/solar/core/handlers/puppet.py
@@ -49,7 +49,7 @@ class LibrarianPuppet(object):
             branch=git['branch']
         )
 
-        modules = puppetlabs.result.split('\n')
+        modules = puppetlabs.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):
diff --git a/solar/solar/core/transports/base.py b/solar/solar/core/transports/base.py
index 241bae43..b5934007 100644
--- a/solar/solar/core/transports/base.py
+++ b/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')
+    return_code = SolarRunResultWrp('return_code')
+
+    def __str__(self):
+        if self.failed:
+            return str(self.stderr)
+        return str(self.stdout)
+
+    def split(self, *args, **kwargs):
+        return self.stdout.split(*args, **kwargs)
+
+
 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.
diff --git a/solar/solar/core/transports/solard_transport.py b/solar/solar/core/transports/solard_transport.py
index 3c0f7156..e1d76993 100644
--- a/solar/solar/core/transports/solard_transport.py
+++ b/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):
-        return SolarRunResult(result, failed)
+    def get_result(self, result):
+        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)
-            return self.get_result(res, failed=False)
-        except Exception as ex:
-            log.exception("Exception during solard run")
-            return self.get_result(ex, failed=True)
+        res = client.run(' '.join(args), **kwargs)
+        return self.get_result(res)
 
diff --git a/solar/solar/core/transports/ssh.py b/solar/solar/core/transports/ssh.py
index 6323037f..47b9e54a 100644
--- a/solar/solar/core/transports/ssh.py
+++ b/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, failed=True)
-        return SolarRunResult(output, failed=False)
-
+        return SolarRunResult(output)
 
     def run(self, resource, *args, **kwargs):
         log.debug('SSH: %s', args)
diff --git a/solard/solard/core.py b/solard/solard/core.py
index e000e3f6..cb515d21 100644
--- a/solard/solard/core.py
+++ b/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):
-            managers.append(fabric_api.warn_only())
+        # we just warn, don't exit on solard
+        # correct data is returned
+        managers.append(fabric_api.warn_only())
 
         with nested(*managers):
             out = executor(cmd, capture=True)
-            if out.failed:
-                raise Exception("Remote failed")
-            return out.stdout
+            result = {}
+            for name in ('failed', 'return_code', 'stdout', 'stderr',
+                         'succeeded', 'command', 'real_command'):
+                result[name] = getattr(out, name)
+            return result
 
     @staticmethod
     def copy_file(solard_context, stream_reader, path, size=None):