better handling of piped exit codes

tests/test_cmdpipe.py

@@ -9,26 +9,24 @@ class TestCmdPipe(unittest2.TestCase):
         p=CmdPipe(readonly=False, inp=None)
         err=[]
         out=[]
-        p.add(["ls", "-d", "/", "/", "/nonexistent"], stderr_handler=lambda line: err.append(line))
+        p.add(["ls", "-d", "/", "/", "/nonexistent"], stderr_handler=lambda line: err.append(line), exit_handler=lambda exit_code: self.assertEqual(exit_code,2))
         executed=p.execute(stdout_handler=lambda line: out.append(line))
 
         self.assertEqual(err, ["ls: cannot access '/nonexistent': No such file or directory"])
         self.assertEqual(out, ["/","/"])
         self.assertTrue(executed)
-        self.assertEqual(p.items[0]['process'].returncode,2)
 
     def test_input(self):
         """test stdinput"""
         p=CmdPipe(readonly=False, inp="test")
         err=[]
         out=[]
-        p.add(["echo", "test"], stderr_handler=lambda line: err.append(line))
+        p.add(["echo", "test"], stderr_handler=lambda line: err.append(line), exit_handler=lambda exit_code: self.assertEqual(exit_code,0))
         executed=p.execute(stdout_handler=lambda line: out.append(line))
 
         self.assertEqual(err, [])
         self.assertEqual(out, ["test"])
         self.assertTrue(executed)
-        self.assertEqual(p.items[0]['process'].returncode,0)
 
     def test_pipe(self):
         """test piped"""
@@ -37,9 +35,9 @@ class TestCmdPipe(unittest2.TestCase):
         err2=[]
         err3=[]
         out=[]
-        p.add(["echo", "test"], stderr_handler=lambda line: err1.append(line))
+        p.add(["echo", "test"], stderr_handler=lambda line: err1.append(line), exit_handler=lambda exit_code: self.assertEqual(exit_code,0))
-        p.add(["tr", "e", "E"], stderr_handler=lambda line: err2.append(line))
+        p.add(["tr", "e", "E"], stderr_handler=lambda line: err2.append(line), exit_handler=lambda exit_code: self.assertEqual(exit_code,0))
-        p.add(["tr", "t", "T"], stderr_handler=lambda line: err3.append(line))
+        p.add(["tr", "t", "T"], stderr_handler=lambda line: err3.append(line), exit_handler=lambda exit_code: self.assertEqual(exit_code,0))
         executed=p.execute(stdout_handler=lambda line: out.append(line))
 
         self.assertEqual(err1, [])
@@ -47,9 +45,6 @@ class TestCmdPipe(unittest2.TestCase):
         self.assertEqual(err3, [])
         self.assertEqual(out, ["TEsT"])
         self.assertTrue(executed)
-        self.assertEqual(p.items[0]['process'].returncode,0)
-        self.assertEqual(p.items[1]['process'].returncode,0)
-        self.assertEqual(p.items[2]['process'].returncode,0)
 
         #test str representation as well
         self.assertEqual(str(p), "(echo test) | (tr e E) | (tr t T)")
@@ -61,9 +56,9 @@ class TestCmdPipe(unittest2.TestCase):
         err2=[]
         err3=[]
         out=[]
-        p.add(["ls", "/nonexistent1"], stderr_handler=lambda line: err1.append(line))
+        p.add(["ls", "/nonexistent1"], stderr_handler=lambda line: err1.append(line), exit_handler=lambda exit_code: self.assertEqual(exit_code,2))
-        p.add(["ls", "/nonexistent2"], stderr_handler=lambda line: err2.append(line))
+        p.add(["ls", "/nonexistent2"], stderr_handler=lambda line: err2.append(line), exit_handler=lambda exit_code: self.assertEqual(exit_code,2))
-        p.add(["ls", "/nonexistent3"], stderr_handler=lambda line: err3.append(line))
+        p.add(["ls", "/nonexistent3"], stderr_handler=lambda line: err3.append(line), exit_handler=lambda exit_code: self.assertEqual(exit_code,2))
         executed=p.execute(stdout_handler=lambda line: out.append(line))
 
         self.assertEqual(err1, ["ls: cannot access '/nonexistent1': No such file or directory"])
@@ -71,9 +66,6 @@ class TestCmdPipe(unittest2.TestCase):
         self.assertEqual(err3, ["ls: cannot access '/nonexistent3': No such file or directory"])
         self.assertEqual(out, [])
         self.assertTrue(executed)
-        self.assertEqual(p.items[0]['process'].returncode,2)
-        self.assertEqual(p.items[1]['process'].returncode,2)
-        self.assertEqual(p.items[2]['process'].returncode,2)
 
     def test_exitcode(self):
         """test piped exitcodes """
@@ -82,9 +74,9 @@ class TestCmdPipe(unittest2.TestCase):
         err2=[]
         err3=[]
         out=[]
-        p.add(["bash", "-c", "exit 1"], stderr_handler=lambda line: err1.append(line))
+        p.add(["bash", "-c", "exit 1"], stderr_handler=lambda line: err1.append(line), exit_handler=lambda exit_code: self.assertEqual(exit_code,1))
-        p.add(["bash", "-c", "exit 2"], stderr_handler=lambda line: err2.append(line))
+        p.add(["bash", "-c", "exit 2"], stderr_handler=lambda line: err2.append(line), exit_handler=lambda exit_code: self.assertEqual(exit_code,2))
-        p.add(["bash", "-c", "exit 3"], stderr_handler=lambda line: err3.append(line))
+        p.add(["bash", "-c", "exit 3"], stderr_handler=lambda line: err3.append(line), exit_handler=lambda exit_code: self.assertEqual(exit_code,3))
         executed=p.execute(stdout_handler=lambda line: out.append(line))
 
         self.assertEqual(err1, [])
@@ -92,9 +84,6 @@ class TestCmdPipe(unittest2.TestCase):
         self.assertEqual(err3, [])
         self.assertEqual(out, [])
         self.assertTrue(executed)
-        self.assertEqual(p.items[0]['process'].returncode,1)
-        self.assertEqual(p.items[1]['process'].returncode,2)
-        self.assertEqual(p.items[2]['process'].returncode,3)
 
     def test_readonly_execute(self):
         """everything readonly, just should execute"""

tests/test_executenode.py

@@ -1,5 +1,5 @@
 from basetest import *
-from zfs_autobackup.ExecuteNode import ExecuteNode
+from zfs_autobackup.ExecuteNode import *
 
 print("THIS TEST REQUIRES SSH TO LOCALHOST")
 
@@ -15,7 +15,7 @@ class TestExecuteNode(unittest2.TestCase):
             self.assertEqual(node.run(["echo","test"]), ["test"])
 
         with self.subTest("error exit code"):
-            with self.assertRaises(subprocess.CalledProcessError):
+            with self.assertRaises(ExecuteError):
                 node.run(["false"])
 
         #
@@ -81,29 +81,33 @@ class TestExecuteNode(unittest2.TestCase):
             nodeb.run(["true"], inp=output)
 
         with self.subTest("error on pipe input side"):
-            with self.assertRaises(subprocess.CalledProcessError):
+            with self.assertRaises(ExecuteError):
                 output=nodea.run(["false"], pipe=True)
                 nodeb.run(["true"], inp=output)
 
+        with self.subTest("error on both sides, ignore exit codes"):
+            output=nodea.run(["false"], pipe=True, valid_exitcodes=[])
+            nodeb.run(["false"], inp=output, valid_exitcodes=[])
+
         with self.subTest("error on pipe output side "):
-            with self.assertRaises(subprocess.CalledProcessError):
+            with self.assertRaises(ExecuteError):
                 output=nodea.run(["true"], pipe=True)
                 nodeb.run(["false"], inp=output)
 
         with self.subTest("error on both sides of pipe"):
-            with self.assertRaises(subprocess.CalledProcessError):
+            with self.assertRaises(ExecuteError):
                 output=nodea.run(["false"], pipe=True)
                 nodeb.run(["false"], inp=output)
 
         with self.subTest("check stderr on pipe output side"):
-            output=nodea.run(["true"], pipe=True)
+            output=nodea.run(["true"], pipe=True, valid_exitcodes=[0])
-            (stdout, stderr)=nodeb.run(["ls", "nonexistingfile"], inp=output, return_stderr=True, valid_exitcodes=[0,2])
+            (stdout, stderr)=nodeb.run(["ls", "nonexistingfile"], inp=output, return_stderr=True, valid_exitcodes=[2])
             self.assertEqual(stdout,[])
             self.assertRegex(stderr[0], "nonexistingfile" )
 
         with self.subTest("check stderr on pipe input side (should be only printed)"):
-            output=nodea.run(["ls", "nonexistingfile"], pipe=True)
+            output=nodea.run(["ls", "nonexistingfile"], pipe=True, valid_exitcodes=[2])
-            (stdout, stderr)=nodeb.run(["true"], inp=output, return_stderr=True, valid_exitcodes=[0,2])
+            (stdout, stderr)=nodeb.run(["true"], inp=output, return_stderr=True, valid_exitcodes=[0])
             self.assertEqual(stdout,[])
             self.assertEqual(stderr,[])
 

zfs_autobackup/CmdPipe.py

@@ -17,12 +17,13 @@ class CmdPipe:
         self.readonly = readonly
         self._should_execute = True
 
-    def add(self, cmd, readonly=False, stderr_handler=None):
+    def add(self, cmd, readonly=False, stderr_handler=None, exit_handler=None):
         """adds a command to pipe"""
 
         self.items.append({
             'cmd': cmd,
-            'stderr_handler': stderr_handler
+            'stderr_handler': stderr_handler,
+            'exit_handler': exit_handler
         })
 
         if not readonly and self.readonly:
@@ -117,10 +118,15 @@ class CmdPipe:
             if eof_count == len(selectors) and done_count == len(self.items):
                 break
 
-        # ret = []
+        #close filehandles
         last_stdout.close()
         for item in self.items:
             item['process'].stderr.close()
-            # ret.append(item['process'].returncode)
+
+        #call exit handlers
+        for item in self.items:
+            if item['exit_handler'] is not None:
+                item['exit_handler'](item['process'].returncode)
+
 
         return True

zfs_autobackup/ExecuteNode.py

@@ -5,6 +5,8 @@ import subprocess
 from zfs_autobackup.CmdPipe import CmdPipe
 from zfs_autobackup.LogStub import LogStub
 
+class ExecuteError(Exception):
+    pass
 
 class ExecuteNode(LogStub):
     """an endpoint to execute local or remote commands via ssh"""
@@ -108,9 +110,20 @@ class ExecuteNode(LogStub):
                 error_lines.append(line.rstrip())
             self._parse_stderr(line, hide_errors)
 
+        # exit code hanlder
+        if valid_exitcodes is None:
+            valid_exitcodes = [0]
+
+        def exit_handler(exit_code):
+            if self.debug_output:
+                self.debug("EXIT   > {}".format(exit_code))
+
+            if (valid_exitcodes != []) and (exit_code not in valid_exitcodes):
+             raise (ExecuteError("Command '{}' return exit code '{}' (valid codes: {})".format(" ".join(cmd), exit_code, valid_exitcodes)))
+
         # add command to pipe
         encoded_cmd = self._remote_cmd(cmd)
-        p.add(cmd=encoded_cmd, readonly=readonly, stderr_handler=stderr_handler)
+        p.add(cmd=encoded_cmd, readonly=readonly, stderr_handler=stderr_handler, exit_handler=exit_handler)
 
         # return pipe instead of executing?
         if pipe:
@@ -130,21 +143,8 @@ class ExecuteNode(LogStub):
         else:
             self.debug("CMDSKIP> {}".format(p))
 
-        # execute and verify exit codes
+        # execute and calls handlers in CmdPipe
-        if p.execute(stdout_handler=stdout_handler) and valid_exitcodes is not []:
+        p.execute(stdout_handler=stdout_handler)
-            if valid_exitcodes is None:
-                valid_exitcodes = [0]
-
-            item_nr=1
-            for item in p.items:
-                exit_code=item['process'].returncode
-
-                if self.debug_output:
-                    self.debug("EXIT{}  > {}".format(item_nr, exit_code))
-
-                if exit_code not in valid_exitcodes:
-                    raise (subprocess.CalledProcessError(exit_code, " ".join(item['cmd'])))
-                item_nr=item_nr+1
 
         if return_stderr:
             return output_lines, error_lines

zfs_autobackup/ZfsDataset.py

@@ -3,6 +3,7 @@ import subprocess
 import time
 
 from zfs_autobackup.CachedProperty import CachedProperty
+from zfs_autobackup.ExecuteNode import ExecuteError
 
 
 class ZfsDataset:
@@ -250,7 +251,7 @@ class ZfsDataset:
             self.invalidate()
             self.force_exists = False
             return True
-        except subprocess.CalledProcessError:
+        except ExecuteError:
             if not fail_exception:
                 return False
             else:

zfs_autobackup/ZfsNode.py

@@ -10,6 +10,7 @@ from zfs_autobackup.Thinner import Thinner
 from zfs_autobackup.CachedProperty import CachedProperty
 from zfs_autobackup.ZfsPool import ZfsPool
 from zfs_autobackup.ZfsDataset import ZfsDataset
+from zfs_autobackup.ExecuteNode import ExecuteError
 
 
 class ZfsNode(ExecuteNode):
@@ -81,7 +82,7 @@ class ZfsNode(ExecuteNode):
 
         try:
             self.run(cmd, hide_errors=True, valid_exitcodes=[0, 1])
-        except subprocess.CalledProcessError:
+        except ExecuteError:
             return False
 
         return True