From cdd151d45ffb706dfc9d9fd7365573c2846170e1 Mon Sep 17 00:00:00 2001 From: Edwin Eefting Date: Tue, 4 Apr 2023 14:10:58 +0200 Subject: [PATCH] fix #190. --exclude-received now expects number of bytes instead that have to be changed for a dataset to not get excluded. (default 0) this also makes it so that it doesnt conflict with --allow-empty. added regression tests for exclude-unchanged as well. --- tests/test_zfsautobackup31.py | 22 ++++++++++++++++++++++ tests/test_zfsnode.py | 22 +++++++++++++++------- zfs_autobackup/CliBase.py | 2 +- zfs_autobackup/ZfsAuto.py | 4 ++-- zfs_autobackup/ZfsAutobackup.py | 11 +++++------ zfs_autobackup/ZfsAutoverify.py | 7 +++---- zfs_autobackup/ZfsDataset.py | 15 +++++++++------ zfs_autobackup/ZfsNode.py | 21 ++++++++++++++------- 8 files changed, 71 insertions(+), 33 deletions(-) diff --git a/tests/test_zfsautobackup31.py b/tests/test_zfsautobackup31.py index 1f89a07..7cd8727 100644 --- a/tests/test_zfsautobackup31.py +++ b/tests/test_zfsautobackup31.py @@ -95,3 +95,25 @@ test_target1/fs1@test-20101111000000 test_target1/fs1/sub@test-20101111000000 test_target1/fs2/sub@test-20101111000000 """) + + + def test_exclude_unchanged(self): + + shelltest("zfs snapshot -r test_source1@somesnapshot") + + with patch('time.strftime', return_value="test-20101111000000"): + self.assertFalse( + ZfsAutobackup( + "test test_target1 --verbose --allow-empty --exclude-unchanged=1".split(" ")).run()) + + #everything should be excluded, but should not return an error (see #190) + with patch('time.strftime', return_value="test-20101111000001"): + self.assertFalse( + ZfsAutobackup( + "test test_target1 --verbose --allow-empty --exclude-unchanged=1".split(" ")).run()) + + r = shelltest("zfs list -H -o name -r -t snapshot test_target1") + self.assertMultiLineEqual(r, """ +test_target1/test_source2/fs2/sub@test-20101111000000 +""") + diff --git a/tests/test_zfsnode.py b/tests/test_zfsnode.py index 893f549..4cc1baf 100644 --- a/tests/test_zfsnode.py +++ b/tests/test_zfsnode.py @@ -15,7 +15,9 @@ class TestZfsNode(unittest2.TestCase): node = ZfsNode(utc=False, snapshot_time_format="test-%Y%m%d%H%M%S", hold_name="zfs_autobackup:test", logger=logger, description=description) with self.subTest("first snapshot"): - node.consistent_snapshot(node.selected_datasets(property_name="autobackup:test",exclude_paths=[], exclude_received=False, exclude_unchanged=False, min_change=200000), "test-20101111000001", 100000) + (selected_datasets, excluded_datasets)=node.selected_datasets(property_name="autobackup:test", exclude_paths=[], exclude_received=False, + exclude_unchanged=0) + node.consistent_snapshot(selected_datasets, "test-20101111000001", 100000) r = shelltest("zfs list -H -o name -r -t all " + TEST_POOLS) self.assertEqual(r, """ test_source1 @@ -33,7 +35,9 @@ test_target1 """) with self.subTest("second snapshot, no changes, no snapshot"): - node.consistent_snapshot(node.selected_datasets(property_name="autobackup:test",exclude_paths=[], exclude_received=False, exclude_unchanged=False, min_change=200000), "test-20101111000002", 1) + (selected_datasets, excluded_datasets)=node.selected_datasets(property_name="autobackup:test", exclude_paths=[], exclude_received=False, + exclude_unchanged=0) + node.consistent_snapshot(selected_datasets, "test-20101111000002", 1) r = shelltest("zfs list -H -o name -r -t all " + TEST_POOLS) self.assertEqual(r, """ test_source1 @@ -51,7 +55,8 @@ test_target1 """) with self.subTest("second snapshot, no changes, empty snapshot"): - node.consistent_snapshot(node.selected_datasets(property_name="autobackup:test", exclude_paths=[], exclude_received=False, exclude_unchanged=False, min_change=200000), "test-20101111000002", 0) + (selected_datasets, excluded_datasets) =node.selected_datasets(property_name="autobackup:test", exclude_paths=[], exclude_received=False, exclude_unchanged=0) + node.consistent_snapshot(selected_datasets, "test-20101111000002", 0) r = shelltest("zfs list -H -o name -r -t all " + TEST_POOLS) self.assertEqual(r, """ test_source1 @@ -79,7 +84,8 @@ test_target1 with self.subTest("Test if all cmds are executed correctly (no failures)"): with OutputIO() as buf: with redirect_stdout(buf): - node.consistent_snapshot(node.selected_datasets(property_name="autobackup:test", exclude_paths=[], exclude_received=False, exclude_unchanged=False, min_change=1), "test-1", + (selected_datasets, excluded_datasets) =node.selected_datasets(property_name="autobackup:test", exclude_paths=[], exclude_received=False, exclude_unchanged=False, min_change=1) + node.consistent_snapshot(selected_datasets, "test-1", 0, pre_snapshot_cmds=["echo pre1", "echo pre2"], post_snapshot_cmds=["echo post1 >&2", "echo post2 >&2"] @@ -95,7 +101,8 @@ test_target1 with OutputIO() as buf: with redirect_stdout(buf): with self.assertRaises(ExecuteError): - node.consistent_snapshot(node.selected_datasets(property_name="autobackup:test", exclude_paths=[], exclude_received=False, exclude_unchanged=False, min_change=1), "test-1", + (selected_datasets, excluded_datasets) =node.selected_datasets(property_name="autobackup:test", exclude_paths=[], exclude_received=False, exclude_unchanged=False, min_change=1) + node.consistent_snapshot(selected_datasets, "test-1", 0, pre_snapshot_cmds=["echo pre1", "false", "echo pre2"], post_snapshot_cmds=["echo post1", "false", "echo post2"] @@ -112,7 +119,8 @@ test_target1 with redirect_stdout(buf): with self.assertRaises(ExecuteError): #same snapshot name as before so it fails - node.consistent_snapshot(node.selected_datasets(property_name="autobackup:test", exclude_paths=[], exclude_received=False, exclude_unchanged=False, min_change=1), "test-1", + (selected_datasets, excluded_datasets) =node.selected_datasets(property_name="autobackup:test", exclude_paths=[], exclude_received=False, exclude_unchanged=False, min_change=1) + node.consistent_snapshot(selected_datasets, "test-1", 0, pre_snapshot_cmds=["echo pre1", "echo pre2"], post_snapshot_cmds=["echo post1", "echo post2"] @@ -158,7 +166,7 @@ test_target1 logger = LogStub() description = "[Source]" node = ZfsNode(utc=False, snapshot_time_format="test-%Y%m%d%H%M%S", hold_name="zfs_autobackup:test", logger=logger, description=description) - s = pformat(node.selected_datasets(property_name="autobackup:test", exclude_paths=[], exclude_received=False, exclude_unchanged=True, min_change=1)) + s = pformat(node.selected_datasets(property_name="autobackup:test", exclude_paths=[], exclude_received=False, exclude_unchanged=1)) print(s) # basics diff --git a/zfs_autobackup/CliBase.py b/zfs_autobackup/CliBase.py index 518f032..f7ae15f 100644 --- a/zfs_autobackup/CliBase.py +++ b/zfs_autobackup/CliBase.py @@ -10,7 +10,7 @@ class CliBase(object): Overridden in subclasses that add stuff for the specific programs.""" # also used by setup.py - VERSION = "3.2-beta1" + VERSION = "3.2-beta2" HEADER = "{} v{} - (c)2022 E.H.Eefting (edwin@datux.nl)".format(os.path.basename(sys.argv[0]), VERSION) def __init__(self, argv, print_arguments=True): diff --git a/zfs_autobackup/ZfsAuto.py b/zfs_autobackup/ZfsAuto.py index 660e721..541a59d 100644 --- a/zfs_autobackup/ZfsAuto.py +++ b/zfs_autobackup/ZfsAuto.py @@ -98,8 +98,8 @@ class ZfsAuto(CliBase): group=parser.add_argument_group("Selection options") group.add_argument('--ignore-replicated', action='store_true', help=argparse.SUPPRESS) - group.add_argument('--exclude-unchanged', action='store_true', - help='Exclude datasets that have no changes since any last snapshot. (Useful in combination with proxmox HA replication)') + group.add_argument('--exclude-unchanged', metavar='BYTES', default=0, type=int, + help='Exclude datasets that have less than BYTES data changed since any last snapshot. (Use with proxmox HA replication)') group.add_argument('--exclude-received', action='store_true', help='Exclude datasets that have the origin of their autobackup: property as "received". ' 'This can avoid recursive replication between two backup partners.') diff --git a/zfs_autobackup/ZfsAutobackup.py b/zfs_autobackup/ZfsAutobackup.py index a1466d8..8b33466 100644 --- a/zfs_autobackup/ZfsAutobackup.py +++ b/zfs_autobackup/ZfsAutobackup.py @@ -67,7 +67,7 @@ class ZfsAutobackup(ZfsAuto): help='Only create snapshot if enough bytes are changed. (default %(' 'default)s)') group.add_argument('--allow-empty', action='store_true', - help='If nothing has changed, still create empty snapshots. (Faster. Same as --min-change=0)') + help='If nothing has changed, still create empty snapshots. (Same as --min-change=0)') group.add_argument('--other-snapshots', action='store_true', help='Send over other snapshots as well, not just the ones created by this tool.') group.add_argument('--set-snapshot-properties', metavar='PROPERTY=VALUE,...', type=str, @@ -110,7 +110,7 @@ class ZfsAutobackup(ZfsAuto): group.add_argument('--zfs-compressed', action='store_true', help='Transfer blocks that already have zfs-compression as-is.') - group = parser.add_argument_group("ZFS send/recv pipes") + group = parser.add_argument_group("Data transfer options") group.add_argument('--compress', metavar='TYPE', default=None, nargs='?', const='zstd-fast', choices=compressors.choices(), help='Use compression during transfer, defaults to zstd-fast if TYPE is not specified. ({})'.format( @@ -443,12 +443,11 @@ class ZfsAutobackup(ZfsAuto): ################# select source datasets self.set_title("Selecting") - source_datasets = source_node.selected_datasets(property_name=self.property_name, + ( source_datasets, excluded_datasets) = source_node.selected_datasets(property_name=self.property_name, exclude_received=self.args.exclude_received, exclude_paths=self.exclude_paths, - exclude_unchanged=self.args.exclude_unchanged, - min_change=self.args.min_change) - if not source_datasets: + exclude_unchanged=self.args.exclude_unchanged) + if not source_datasets and not excluded_datasets: self.print_error_sources() return 255 diff --git a/zfs_autobackup/ZfsAutoverify.py b/zfs_autobackup/ZfsAutoverify.py index 7568f8f..38d3f8c 100644 --- a/zfs_autobackup/ZfsAutoverify.py +++ b/zfs_autobackup/ZfsAutoverify.py @@ -239,12 +239,11 @@ class ZfsAutoverify(ZfsAuto): ################# select source datasets self.set_title("Selecting") - source_datasets = source_node.selected_datasets(property_name=self.property_name, + ( source_datasets, excluded_datasets) = source_node.selected_datasets(property_name=self.property_name, exclude_received=self.args.exclude_received, exclude_paths=self.exclude_paths, - exclude_unchanged=self.args.exclude_unchanged, - min_change=0) - if not source_datasets: + exclude_unchanged=self.args.exclude_unchanged) + if not source_datasets and not excluded_datasets: self.print_error_sources() return 255 diff --git a/zfs_autobackup/ZfsDataset.py b/zfs_autobackup/ZfsDataset.py index 8062f9e..06cf613 100644 --- a/zfs_autobackup/ZfsDataset.py +++ b/zfs_autobackup/ZfsDataset.py @@ -118,7 +118,7 @@ class ZfsDataset: """true if this dataset is a snapshot""" return self.name.find("@") != -1 - def is_selected(self, value, source, inherited, exclude_received, exclude_paths, exclude_unchanged, min_change): + def is_selected(self, value, source, inherited, exclude_received, exclude_paths, exclude_unchanged): """determine if dataset should be selected for backup (called from ZfsNode) @@ -128,12 +128,15 @@ class ZfsDataset: :type source: str :type inherited: bool :type exclude_received: bool - :type exclude_unchanged: bool - :type min_change: bool + :type exclude_unchanged: int :param value: Value of the zfs property ("false"/"true"/"child"/parent/"-") :param source: Source of the zfs property ("local"/"received", "-") :param inherited: True of the value/source was inherited from a higher dataset. + + Returns: True : Selected + False: Excluded + None: No property found """ # sanity checks @@ -149,7 +152,7 @@ class ZfsDataset: # non specified, ignore if value == "-": - return False + return None # only select childs of this dataset, ignore if value == "child" and not inherited: @@ -179,8 +182,8 @@ class ZfsDataset: self.verbose("Excluded (dataset already received)") return False - if exclude_unchanged and not self.is_changed(min_change): - self.verbose("Excluded (unchanged since last snapshot)") + if not self.is_changed(exclude_unchanged): + self.verbose("Excluded (by --exclude-unchanged)") return False self.verbose("Selected") diff --git a/zfs_autobackup/ZfsNode.py b/zfs_autobackup/ZfsNode.py index c635ac4..b87a118 100644 --- a/zfs_autobackup/ZfsNode.py +++ b/zfs_autobackup/ZfsNode.py @@ -235,10 +235,10 @@ class ZfsNode(ExecuteNode): except Exception as e: pass - def selected_datasets(self, property_name, exclude_received, exclude_paths, exclude_unchanged, min_change): + def selected_datasets(self, property_name, exclude_received, exclude_paths, exclude_unchanged): """determine filesystems that should be backed up by looking at the special autobackup-property, systemwide - returns: list of ZfsDataset + returns: ( list of selected ZfsDataset, list of excluded ZfsDataset) """ self.debug("Getting selected datasets") @@ -249,8 +249,10 @@ class ZfsNode(ExecuteNode): property_name ]) + # The returnlist of selected ZfsDataset's: selected_filesystems = [] + excluded_filesystems = [] # list of sources, used to resolve inherited sources sources = {} @@ -270,9 +272,14 @@ class ZfsNode(ExecuteNode): source = raw_source # determine it - if dataset.is_selected(value=value, source=source, inherited=inherited, exclude_received=exclude_received, - exclude_paths=exclude_paths, exclude_unchanged=exclude_unchanged, - min_change=min_change): - selected_filesystems.append(dataset) + selected=dataset.is_selected(value=value, source=source, inherited=inherited, exclude_received=exclude_received, + exclude_paths=exclude_paths, exclude_unchanged=exclude_unchanged) - return selected_filesystems + if selected==True: + selected_filesystems.append(dataset) + elif selected==False: + excluded_filesystems.append(dataset) + #returns None when no property is set. + + + return ( selected_filesystems, excluded_filesystems)