cleanedup and improved select-code

tests/test_zfsautobackup.py

@@ -541,6 +541,30 @@ test_target1/test_source2/fs2/sub@test-20101111000000  canmount  -         -
             #should succeed by destroying incompatibles
             self.assertFalse(ZfsAutobackup("test test_target1 --verbose --allow-empty --destroy-incompatible".split(" ")).run())
 
+        r = shelltest("zfs list -H -o name -r -t all test_target1")
+        self.assertMultiLineEqual(r, """
+test_target1
+test_target1/test_source1
+test_target1/test_source1/fs1
+test_target1/test_source1/fs1@test-20101111000000
+test_target1/test_source1/fs1@compatible1
+test_target1/test_source1/fs1@compatible2
+test_target1/test_source1/fs1@test-20101111000001
+test_target1/test_source1/fs1@test-20101111000002
+test_target1/test_source1/fs1@test-20101111000003
+test_target1/test_source1/fs1/sub
+test_target1/test_source1/fs1/sub@test-20101111000000
+test_target1/test_source1/fs1/sub@test-20101111000001
+test_target1/test_source1/fs1/sub@test-20101111000002
+test_target1/test_source1/fs1/sub@test-20101111000003
+test_target1/test_source2
+test_target1/test_source2/fs2
+test_target1/test_source2/fs2/sub
+test_target1/test_source2/fs2/sub@test-20101111000000
+test_target1/test_source2/fs2/sub@test-20101111000001
+test_target1/test_source2/fs2/sub@test-20101111000002
+test_target1/test_source2/fs2/sub@test-20101111000003
+""")
 
 
 

zfs_autobackup/ExecuteNode.py

@@ -45,6 +45,36 @@ class ExecuteNode(LogStub):
         else:
             self.error("STDERR|> " + line.rstrip())
 
+    def _encode_cmd(self, cmd):
+        """returns cmd in encoded and escaped form that can be used with popen."""
+
+        encoded_cmd=[]
+
+        # make sure the command gets all the data in utf8 format:
+        # (this is necessary if LC_ALL=en_US.utf8 is not set in the environment)
+
+        # use ssh?
+        if self.ssh_to is not None:
+            encoded_cmd.append("ssh".encode('utf-8'))
+
+            if self.ssh_config is not None:
+                encoded_cmd.extend(["-F".encode('utf-8'), self.ssh_config.encode('utf-8')])
+
+            encoded_cmd.append(self.ssh_to.encode('utf-8'))
+
+            for arg in cmd:
+                # add single quotes for remote commands to support spaces and other weird stuff (remote commands are
+                # executed in a shell) and escape existing single quotes (bash needs ' to end the quoted string,
+                # then a \' for the actual quote and then another ' to start a new quoted string) (and then python
+                # needs the double \ to get a single \)
+                encoded_cmd.append(("'" + arg.replace("'", "'\\''") + "'").encode('utf-8'))
+
+        else:
+            for arg in cmd:
+                encoded_cmd.append(arg.encode('utf-8'))
+
+        return encoded_cmd
+
     def run(self, cmd, inp=None, tab_split=False, valid_exitcodes=None, readonly=False, hide_errors=False, pipe=False,
             return_stderr=False):
         """run a command on the node.
@@ -66,29 +96,7 @@ class ExecuteNode(LogStub):
         if valid_exitcodes is None:
             valid_exitcodes = [0]
 
-        encoded_cmd = []
+        encoded_cmd = self._encode_cmd(cmd)
-
-        # use ssh?
-        if self.ssh_to is not None:
-            encoded_cmd.append("ssh".encode('utf-8'))
-
-            if self.ssh_config is not None:
-                encoded_cmd.extend(["-F".encode('utf-8'), self.ssh_config.encode('utf-8')])
-
-            encoded_cmd.append(self.ssh_to.encode('utf-8'))
-
-            # make sure the command gets all the data in utf8 format:
-            # (this is necessary if LC_ALL=en_US.utf8 is not set in the environment)
-            for arg in cmd:
-                # add single quotes for remote commands to support spaces and other weird stuff (remote commands are
-                # executed in a shell) and escape existing single quotes (bash needs ' to end the quoted string,
-                # then a \' for the actual quote and then another ' to start a new quoted string) (and then python
-                # needs the double \ to get a single \)
-                encoded_cmd.append(("'" + arg.replace("'", "'\\''") + "'").encode('utf-8'))
-
-        else:
-            for arg in cmd:
-                encoded_cmd.append(arg.encode('utf-8'))
 
         # debug and test stuff
         debug_txt = ""

zfs_autobackup/ZfsAutobackup.py

@@ -12,7 +12,7 @@ from zfs_autobackup.ThinnerRule import ThinnerRule
 class ZfsAutobackup:
     """main class"""
 
-    VERSION = "3.1-beta1"
+    VERSION = "3.1-beta2"
     HEADER = "zfs-autobackup v{} - Copyright 2020 E.H.Eefting (edwin@datux.nl)".format(VERSION)
 
     def __init__(self, argv, print_arguments=True):
@@ -24,14 +24,14 @@ class ZfsAutobackup:
         parser = argparse.ArgumentParser(
             description=self.HEADER,
             epilog='Full manual at: https://github.com/psy0rz/zfs_autobackup')
-        parser.add_argument('--ssh-config', default=None, help='Custom ssh client config')
+        parser.add_argument('--ssh-config', metavar='CONFIG-FILE', default=None, help='Custom ssh client config')
-        parser.add_argument('--ssh-source', default=None,
+        parser.add_argument('--ssh-source', metavar='USER@HOST', default=None,
-                            help='Source host to get backup from. (user@hostname) Default %(default)s.')
+                            help='Source host to get backup from.')
-        parser.add_argument('--ssh-target', default=None,
+        parser.add_argument('--ssh-target', metavar='USER@HOST', default=None,
-                            help='Target host to push backup to. (user@hostname) Default  %(default)s.')
+                            help='Target host to push backup to.')
-        parser.add_argument('--keep-source', type=str, default="10,1d1w,1w1m,1m1y",
+        parser.add_argument('--keep-source', metavar='SCHEDULE', type=str, default="10,1d1w,1w1m,1m1y",
                             help='Thinning schedule for old source snapshots. Default: %(default)s')
-        parser.add_argument('--keep-target', type=str, default="10,1d1w,1w1m,1m1y",
+        parser.add_argument('--keep-target', metavar='SCHEDULE', type=str, default="10,1d1w,1w1m,1m1y",
                             help='Thinning schedule for old target snapshots. Default: %(default)s')
 
         parser.add_argument('backup_name', metavar='backup-name',
@@ -50,7 +50,7 @@ class ZfsAutobackup:
         parser.add_argument('--no-thinning', action='store_true', help="Do not destroy any snapshots.")
         parser.add_argument('--no-holds', action='store_true',
                             help='Don\'t hold snapshots. (Faster. Allows you to destroy common snapshot.)')
-        parser.add_argument('--min-change', type=int, default=1,
+        parser.add_argument('--min-change', metavar='BYTES', type=int, default=1,
                             help='Number of bytes written after which we consider a dataset changed (default %('
                                  'default)s)')
         parser.add_argument('--allow-empty', action='store_true',
@@ -60,7 +60,7 @@ class ZfsAutobackup:
                                  'lastest snapshot. Useful for proxmox HA replication)')
 
         parser.add_argument('--resume', action='store_true', help=argparse.SUPPRESS)
-        parser.add_argument('--strip-path', default=0, type=int,
+        parser.add_argument('--strip-path', metavar='N', default=0, type=int,
                             help='Number of directories to strip from target path (use 1 when cloning zones between 2 '
                                  'SmartOS machines)')
         # parser.add_argument('--buffer', default="",  help='Use mbuffer with specified size to speedup zfs transfer.
@@ -72,10 +72,10 @@ class ZfsAutobackup:
         parser.add_argument('--clear-mountpoint', action='store_true',
                             help='Set property canmount=noauto for new datasets. (recommended, prevents mount '
                                  'conflicts. same as --set-properties canmount=noauto)')
-        parser.add_argument('--filter-properties', type=str,
+        parser.add_argument('--filter-properties', metavar='PROPERY,...', type=str,
                             help='List of properties to "filter" when receiving filesystems. (you can still restore '
                                  'them with zfs inherit -S)')
-        parser.add_argument('--set-properties', type=str,
+        parser.add_argument('--set-properties', metavar='PROPERTY=VALUE,...', type=str,
                             help='List of propererties to override when receiving filesystems. (you can still restore '
                                  'them with zfs inherit -S)')
         parser.add_argument('--rollback', action='store_true',
@@ -83,7 +83,7 @@ class ZfsAutobackup:
                                  'prevent changes by setting the readonly property on the target_path to on)')
         parser.add_argument('--destroy-incompatible', action='store_true',
                             help='Destroy incompatible snapshots on target. Use with care! (implies --rollback)')
-        parser.add_argument('--destroy-missing', type=str, default=None,
+        parser.add_argument('--destroy-missing', metavar="SCHEDULE", type=str, default=None,
                             help='Destroy datasets on target that are missing on the source. Specify the time since '
                                  'the last snapshot, e.g: --destroy-missing 30d')
         parser.add_argument('--ignore-transfer-errors', action='store_true',
@@ -104,6 +104,12 @@ class ZfsAutobackup:
                             help='show zfs progress output. Enabled automaticly on ttys. (use --no-progress to disable)')
         parser.add_argument('--no-progress', action='store_true', help=argparse.SUPPRESS) # needed to workaround a zfs recv -v bug
 
+        # parser.add_argument('--output-pipe', metavar="COMMAND", default=[], action='append',
+        #                     help='add zfs send output pipe command')
+        #
+        # parser.add_argument('--input-pipe', metavar="COMMAND", default=[], action='append',
+        #                     help='add zfs recv input pipe command')
+
 
         # note args is the only global variable we use, since its a global readonly setting anyway
         args = parser.parse_args(argv)

zfs_autobackup/ZfsDataset.py

@@ -90,6 +90,35 @@ class ZfsDataset:
         """true if this dataset is a snapshot"""
         return self.name.find("@") != -1
 
+    def is_selected(self, value, source, inherited, ignore_received):
+        """determine if dataset should be selected for backup (called from ZfsNode)"""
+
+        # sanity checks
+        if source not in ["local", "received", "-"]:
+            # probably a program error in zfs-autobackup or new feature in zfs
+            raise (Exception(
+                "{} autobackup-property has illegal source: '{}' (possible BUG)".format(self.name, source)))
+        if value not in ["false", "true", "child", "-"]:
+            # user error
+            raise (Exception(
+                "{} autobackup-property has illegal value: '{}'".format(self.name, value)))
+
+        # now determine if its actually selected
+        if value == "false":
+            self.verbose("Ignored (disabled)")
+            return False
+        elif value == "true" or (value == "child" and inherited):
+            if source == "local":
+                self.verbose("Selected")
+                return True
+            elif source == "received":
+                if ignore_received:
+                    self.verbose("Ignored (local backup)")
+                    return False
+                else:
+                    self.verbose("Selected")
+                    return True
+
     @CachedProperty
     def parent(self):
         """get zfs-parent of this dataset. for snapshots this means it will get the filesystem/volume that it belongs
@@ -413,7 +442,7 @@ class ZfsDataset:
 
         # progress output
         if show_progress:
-            # cmd.append("-v")
+            cmd.append("-v")
             cmd.append("-P")
 
         # resume a previous send? (don't need more parameters in that case)

zfs_autobackup/ZfsNode.py

@@ -194,7 +194,7 @@ class ZfsNode(ExecuteNode):
             self.run(cmd, readonly=False)
 
     @CachedProperty
-    def selected_datasets(self):
+    def selected_datasets(self, ignore_received=True):
         """determine filesystems that should be backupped by looking at the special autobackup-property, systemwide
 
            returns: list of ZfsDataset
@@ -204,35 +204,32 @@ class ZfsNode(ExecuteNode):
 
         # get all source filesystems that have the backup property
         lines = self.run(tab_split=True, readonly=True, cmd=[
-            "zfs", "get", "-t", "volume,filesystem", "-o", "name,value,source", "-s", "local,inherited", "-H",
+            "zfs", "get", "-t", "volume,filesystem", "-o", "name,value,source", "-H",
             "autobackup:" + self.backup_name
         ])
 
-        # determine filesystems that should be actually backupped
+        # The returnlist of selected ZfsDataset's:
         selected_filesystems = []
-        direct_filesystems = []
+
+        # list of sources, used to resolve inherited sources
+        sources = {}
+
         for line in lines:
-            (name, value, source) = line
+            (name, value, raw_source) = line
             dataset = ZfsDataset(self, name)
 
-            if value == "false":
+            # "resolve" inherited sources
-                dataset.verbose("Ignored (disabled)")
+            sources[name] = raw_source
+            if raw_source.find("inherited from ") == 0:
+                inherited = True
+                inherited_from = re.sub("^inherited from ", "", raw_source)
+                source = sources[inherited_from]
+            else:
+                inherited = False
+                source = raw_source
 
-            else:
+            # determine it
-                if source == "local" and (value == "true" or value == "child"):
+            if dataset.is_selected(value=value, source=source, inherited=inherited, ignore_received=ignore_received):
-                    direct_filesystems.append(name)
-
-                if source == "local" and value == "true":
-                    dataset.verbose("Selected (direct selection)")
                 selected_filesystems.append(dataset)
-                elif source.find("inherited from ") == 0 and (value == "true" or value == "child"):
-                    inherited_from = re.sub("^inherited from ", "", source)
-                    if inherited_from in direct_filesystems:
-                        selected_filesystems.append(dataset)
-                        dataset.verbose("Selected (inherited selection)")
-                    else:
-                        dataset.debug("Ignored (already a backup)")
-                else:
-                    dataset.verbose("Ignored (only childs)")
 
         return selected_filesystems