promise: include promise output in PromiseError

This will improve the summary of problems at the end of slapos node
instance.

/reviewed-on nexedi/slapos.core!144

slapos/grid/promise/__init__.py

@@ -438,6 +438,8 @@ class PromiseLauncher(object):
 
       If the promise periodicity doesn't match, the previous promise result is
       checked.
+
+      Returns a TestResult or None if promise never ran.
     """
     try:
       promise_process = PromiseProcess(
@@ -469,8 +471,8 @@ class PromiseLauncher(object):
         if result is not None:
           if result.item.hasFailed():
             self.logger.error(result.item.message)
-            return True
-        return False
+            return result.item
+        return
       # we can do this because we run processes one by one
       # we cleanup queue in case previous result was written by a killed process
       self._emptyQueue()
@@ -479,7 +481,7 @@ class PromiseLauncher(object):
       # only print traceback to not prevent run other promises
       self.logger.error(traceback.format_exc())
       self.logger.warning("Promise %s skipped." % promise_name)
-      return True
+      return TestResult(problem=True, message=traceback.format_exc())
 
     self.logger.info("Checking promise %s..." % promise_name)
     queue_item = None
@@ -554,7 +556,7 @@ class PromiseLauncher(object):
       self.logger.debug("Finished promise %r in %s second(s)." % (
                        promise_name, execution_time))
 
-    return queue_item.item.hasFailed()
+    return queue_item.item
 
   def run(self):
     """
@@ -562,6 +564,7 @@ class PromiseLauncher(object):
     """
     promise_list = []
     failed_promise_name = ""
+    failed_promise_output = ""
     base_config = {
       'log-folder': self.log_folder,
       'partition-folder': self.partition_folder,
@@ -593,9 +596,10 @@ class PromiseLauncher(object):
         }
 
         config.update(base_config)
-        if self._launchPromise(promise_name, promise_path, config) and \
-            not failed_promise_name:
+        promise_result = self._launchPromise(promise_name, promise_path, config)
+        if promise_result and promise_result.hasFailed() and not failed_promise_name:
           failed_promise_name = promise_name
+          failed_promise_output = promise_result.message
 
     if not self.run_only_promise_list and os.path.exists(self.legacy_promise_folder) \
         and os.path.isdir(self.legacy_promise_folder):
@@ -613,16 +617,19 @@ class PromiseLauncher(object):
         }
         config.update(base_config)
         # We will use promise wrapper to run this
-        result_state = self._launchPromise(promise_name,
+        promise_result = self._launchPromise(promise_name,
                                            promise_path,
                                            config,
                                            wrap_process=True)
-        if result_state and not failed_promise_name:
+        if promise_result and promise_result.hasFailed() and not failed_promise_name:
           failed_promise_name = promise_name
+          failed_promise_output = promise_result.message
 
     self._updateFolderOwner(self.promise_output_dir)
     if self._skipped_amount > 0:
       self.logger.info("%s promises didn't need to be checked." % \
         self._skipped_amount)
     if failed_promise_name:
-      raise PromiseError("Promise %r failed." % failed_promise_name)
+      raise PromiseError("Promise %r failed with output: %s" % (
+          failed_promise_name,
+          failed_promise_output))

slapos/tests/test_promise.py

@@ -544,7 +544,7 @@ class RunPromise(GenericPromise):
     # run promise will fail when promise fail (usefull for slapgrid)
     with self.assertRaises(PromiseError) as exc:
       self.launcher.run()
-    self.assertEqual(str(exc.exception), 'Promise %r failed.' % second_promise)
+    self.assertEqual(str(exc.exception), 'Promise %r failed with output: failed' % second_promise)
 
     # force to reload the module without rerun python
     os.system('rm %s/*.pyc' % self.plugin_dir)
@@ -647,7 +647,7 @@ class RunPromise(GenericPromise):
 
     with self.assertRaises(PromiseError) as exc:
       self.launcher.run()
-    self.assertEqual(str(exc.exception), 'Promise %r failed.' % second_promise)
+    self.assertEqual(str(exc.exception), 'Promise %r failed with output: failed' % second_promise)
 
     self.assertTrue(os.path.exists(first_state_file))
     self.assertTrue(os.path.exists(second_state_file))
@@ -664,7 +664,7 @@ class RunPromise(GenericPromise):
     time.sleep(2)
     with self.assertRaises(PromiseError) as exc:
       self.launcher.run() # only my_first_promise will run but second_promise still failing
-    self.assertEqual(str(exc.exception), 'Promise %r failed.' % second_promise)
+    self.assertEqual(str(exc.exception), 'Promise %r failed with output: failed' % second_promise)
 
     with open(first_state_file) as f:
       first_result = json.load(f)
@@ -678,7 +678,7 @@ class RunPromise(GenericPromise):
     self.configureLauncher()
     with self.assertRaises(PromiseError) as exc:
       self.launcher.run()
-    self.assertEqual(str(exc.exception), 'Promise %r failed.' % second_promise)
+    self.assertEqual(str(exc.exception), 'Promise %r failed with output: failed' % second_promise)
 
     with open(first_state_file) as f:
       first_result = json.load(f)
@@ -703,7 +703,7 @@ class RunPromise(GenericPromise):
 
     with self.assertRaises(PromiseError) as exc:
       self.launcher.run()
-    self.assertEqual(str(exc.exception), 'Promise %r failed.' % second_promise)
+    self.assertEqual(str(exc.exception), 'Promise %r failed with output: failed' % second_promise)
 
     self.assertTrue(os.path.exists(first_state_file))
     self.assertTrue(os.path.exists(second_state_file))
@@ -720,7 +720,7 @@ class RunPromise(GenericPromise):
     time.sleep(2)
     with self.assertRaises(PromiseError) as exc:
       self.launcher.run() # only my_first_promise will run but second_promise still failing
-    self.assertEqual(str(exc.exception), 'Promise %r failed.' % second_promise)
+    self.assertEqual(str(exc.exception), 'Promise %r failed with output: failed' % second_promise)
 
     with open(first_state_file) as f:
       first_result = json.load(f)
@@ -832,7 +832,7 @@ exit 1
     state_file = os.path.join(self.partition_dir, PROMISE_STATE_FOLDER_NAME)
     with self.assertRaises(PromiseError) as exc:
       self.launcher.run()
-    self.assertEqual(str(exc.exception), 'Promise %r failed.' % promise_name)
+    self.assertEqual(str(exc.exception), 'Promise %r failed with output: This promise failed' % promise_name)
 
   def test_runpromise_wrapped_mixed(self):
     self.called = 0

slapos/tests/test_slapgrid.py

@@ -1721,7 +1721,7 @@ echo %s; echo %s; exit 42""" % (line1, line2))
         'Error with promises for the following partitions:')
     self.assertEqual(
         dummyLogger.mock_calls[-1][1][0] % dummyLogger.mock_calls[-1][1][1:],
-        "  2[(not ready)]: Promise 'failing_promise' failed.")
+        "  2[(not ready)]: Promise 'failing_promise' failed with output: fake promise error")
 
 
 class TestSlapgridUsageReport(MasterMixin, unittest.TestCase):