Lots of LVM unit tests + code formatting (#80).

2024-12-06 13:59:38 -08:00 · 2024-12-06 13:59:38 -08:00 · f2d93b85b4
commit f2d93b85b4
parent b999d2dc4d
12 changed files with 695 additions and 71 deletions
--- a/borgmatic/hooks/data_source/lvm.py
+++ b/borgmatic/hooks/data_source/lvm.py
@ -78,7 +78,7 @@ def snapshot_logical_volume(
    snapshot_name,
    logical_volume_device,
    snapshot_size,
-):  # pragma: no cover
+):
    '''
    Given an lvcreate command to run, a snapshot name, the path to the logical volume device to
    snapshot, and a snapshot size string, create a new LVM snapshot.
@ -221,10 +221,7 @@ def unmount_snapshot(umount_command, snapshot_mount_path):  # pragma: no cover
    Given a umount command to run and the mount path of a snapshot, unmount it.
    '''
    borgmatic.execute.execute_command(
-        tuple(umount_command.split(' '))
-        + (
-            snapshot_mount_path,
-        ),
+        tuple(umount_command.split(' ')) + (snapshot_mount_path,),
        output_log_level=logging.DEBUG,
    )

@ -279,6 +276,8 @@ def get_snapshots(lvs_command, snapshot_name=None):
            for snapshot in snapshot_info['report'][0]['lv']
            if snapshot_name is None or snapshot['lv_name'] == snapshot_name
        )
+    except IndexError:
+        raise ValueError(f'Invalid {lvs_command} output: Missing report data')
    except KeyError as error:
        raise ValueError(f'Invalid {lvs_command} output: Missing key "{error}"')

--- a/borgmatic/hooks/data_source/zfs.py
+++ b/borgmatic/hooks/data_source/zfs.py
@ -239,10 +239,7 @@ def unmount_snapshot(umount_command, snapshot_mount_path):  # pragma: no cover
    Given a umount command to run and the mount path of a snapshot, unmount it.
    '''
    borgmatic.execute.execute_command(
-        tuple(umount_command.split(' '))
-        + (
-            snapshot_mount_path,
-        ),
+        tuple(umount_command.split(' ')) + (snapshot_mount_path,),
        output_log_level=logging.DEBUG,
    )

--- a/tests/end-to-end/commands/fake_btrfs.py
+++ b/tests/end-to-end/commands/fake_btrfs.py
@ -53,7 +53,10 @@ def print_subvolume_list(arguments, snapshot_paths):
            print(line)

    for snapshot_path in snapshot_paths:
-        print(SUBVOLUME_LIST_LINE_PREFIX + snapshot_path[snapshot_path.index('.borgmatic-snapshot-'):])
+        print(
+            SUBVOLUME_LIST_LINE_PREFIX
+            + snapshot_path[snapshot_path.index('.borgmatic-snapshot-') :]
+        )


 def main():
@ -76,7 +79,8 @@ def main():
        shutil.rmtree(subdirectory)

        snapshot_paths = [
-            snapshot_path for snapshot_path in snapshot_paths
+            snapshot_path
+            for snapshot_path in snapshot_paths
            if snapshot_path.endswith('/' + arguments.snapshot_path)
        ]
        save_snapshots(snapshot_paths)
--- a/tests/end-to-end/commands/fake_lsblk.py
+++ b/tests/end-to-end/commands/fake_lsblk.py
@ -14,31 +14,22 @@ def parse_arguments(*unparsed_arguments):


 BUILTIN_BLOCK_DEVICES = {
-   'blockdevices': [
-      {
-         'name': 'loop0',
-         'path': '/dev/loop0',
-         'mountpoint': None,
-         'type': 'loop'
-      },
-      {
-         'name': 'cryptroot',
-         'path': '/dev/mapper/cryptroot',
-         'mountpoint': '/',
-         'type': 'crypt'
-      },{
-         'name': 'vgroup-lvolume',
-         'path': '/dev/mapper/vgroup-lvolume',
-         'mountpoint': '/mnt/lvolume',
-         'type': 'lvm'
-      },
-      {
-         'name': 'vgroup-lvolume-real',
-         'path': '/dev/mapper/vgroup-lvolume-real',
-         'mountpoint': None,
-         'type': 'lvm'
-      },
-   ]
+    'blockdevices': [
+        {'name': 'loop0', 'path': '/dev/loop0', 'mountpoint': None, 'type': 'loop'},
+        {'name': 'cryptroot', 'path': '/dev/mapper/cryptroot', 'mountpoint': '/', 'type': 'crypt'},
+        {
+            'name': 'vgroup-lvolume',
+            'path': '/dev/mapper/vgroup-lvolume',
+            'mountpoint': '/mnt/lvolume',
+            'type': 'lvm',
+        },
+        {
+            'name': 'vgroup-lvolume-real',
+            'path': '/dev/mapper/vgroup-lvolume-real',
+            'mountpoint': None,
+            'type': 'lvm',
+        },
+    ]
 }


@ -55,10 +46,10 @@ def print_logical_volumes_json(arguments, snapshots):
    for snapshot in snapshots:
        data['blockdevices'].extend(
            {
-               'name': snapshot['lv_name'],
-               'path': snapshot['lv_path'],
-               'mountpoint': None,
-               'type': 'lvm'
+                'name': snapshot['lv_name'],
+                'path': snapshot['lv_path'],
+                'mountpoint': None,
+                'type': 'lvm',
            }
            for snapshot in snapshots
        )
--- a/tests/end-to-end/commands/fake_lvremove.py
+++ b/tests/end-to-end/commands/fake_lvremove.py
@ -27,7 +27,8 @@ def main():
    arguments = parse_arguments(*sys.argv[1:])

    snapshots = [
-        snapshot for snapshot in load_snapshots()
+        snapshot
+        for snapshot in load_snapshots()
        if snapshot['lv_path'] != arguments.snapshot_device
    ]

--- a/tests/end-to-end/commands/fake_lvs.py
+++ b/tests/end-to-end/commands/fake_lvs.py
@ -32,10 +32,8 @@ def print_snapshots_json(arguments, snapshots):
                    {
                        'lv': snapshots,
                    }
-                ]
-                ,
-                'log': [
-                ]
+                ],
+                'log': [],
            }
        )
    )
--- a/tests/end-to-end/commands/fake_zfs.py
+++ b/tests/end-to-end/commands/fake_zfs.py
@ -39,9 +39,6 @@ BUILTIN_DATASETS = (
 )


-
-
-
 def load_snapshots():
    try:
        return json.load(open('/tmp/fake_zfs.json'))
@ -56,14 +53,9 @@ def save_snapshots(snapshots):
 def print_dataset_list(arguments, datasets, snapshots):
    properties = arguments.properties.split(',')
    data = (
-        (
-            tuple(property_name.upper() for property_name in properties),
-        )
-        if arguments.header else ()
+        (tuple(property_name.upper() for property_name in properties),) if arguments.header else ()
    ) + tuple(
-        tuple(
-            dataset.get(property_name, '-') for property_name in properties
-        )
+        tuple(dataset.get(property_name, '-') for property_name in properties)
        for dataset in (snapshots if arguments.type == 'snapshot' else datasets)
    )

@ -92,10 +84,7 @@ def main():
        )
        save_snapshots(snapshots)
    elif arguments.action == 'destroy':
-        snapshots = [
-            snapshot for snapshot in snapshots
-            if snapshot['name'] != arguments.name
-        ]
+        snapshots = [snapshot for snapshot in snapshots if snapshot['name'] != arguments.name]
        save_snapshots(snapshots)


--- a/tests/end-to-end/test_btrfs.py
+++ b/tests/end-to-end/test_btrfs.py
@ -55,7 +55,9 @@ def test_btrfs_create_and_list():

        # Assert that the snapshot has been deleted.
        assert not subprocess.check_output(
-            f'python3 /app/tests/end-to-end/commands/fake_btrfs.py subvolume list -s /mnt/subvolume'.split(' ')
+            f'python3 /app/tests/end-to-end/commands/fake_btrfs.py subvolume list -s /mnt/subvolume'.split(
+                ' '
+            )
        )
    finally:
        shutil.rmtree(temporary_directory)
--- a/tests/end-to-end/test_lvm.py
+++ b/tests/end-to-end/test_lvm.py
@ -59,8 +59,13 @@ def test_lvm_create_and_list():
        assert 'mnt/lvolume/subdir/file.txt' in output

        # Assert that the snapshot has been deleted.
-        assert not json.loads(subprocess.check_output(
-            'python3 /app/tests/end-to-end/commands/fake_lvs.py --report-format json --options lv_name,lv_path --select'.split(' ') + ['lv_attr =~ ^s']
-        ))['report'][0]['lv']
+        assert not json.loads(
+            subprocess.check_output(
+                'python3 /app/tests/end-to-end/commands/fake_lvs.py --report-format json --options lv_name,lv_path --select'.split(
+                    ' '
+                )
+                + ['lv_attr =~ ^s']
+            )
+        )['report'][0]['lv']
    finally:
        shutil.rmtree(temporary_directory)
--- a/tests/unit/hooks/data_source/test_btrfs.py
+++ b/tests/unit/hooks/data_source/test_btrfs.py
@ -21,7 +21,11 @@ def test_get_subvolumes_for_filesystem_parses_subvolume_list_output():
        'ID 270 gen 107 top level 5 path subvol1\nID 272 gen 74 top level 5 path subvol2\n'
    )

-    assert module.get_subvolumes_for_filesystem('btrfs', '/mnt') == ('/mnt', '/mnt/subvol1', '/mnt/subvol2')
+    assert module.get_subvolumes_for_filesystem('btrfs', '/mnt') == (
+        '/mnt',
+        '/mnt/subvol1',
+        '/mnt/subvol2',
+    )


 def test_get_subvolumes_for_filesystem_skips_empty_subvolume_paths():
@ -202,7 +206,9 @@ def test_dump_data_sources_uses_custom_findmnt_command_in_commands():
    config = {'btrfs': {'findmnt_command': '/usr/local/bin/findmnt'}}
    flexmock(module).should_receive('get_subvolumes').with_args(
        'btrfs', '/usr/local/bin/findmnt', source_directories
-    ).and_return((module.Subvolume('/mnt/subvol1', contained_source_directories=('/mnt/subvol1',)),)).once()
+    ).and_return(
+        (module.Subvolume('/mnt/subvol1', contained_source_directories=('/mnt/subvol1',)),)
+    ).once()
    flexmock(module).should_receive('make_snapshot_path').with_args('/mnt/subvol1').and_return(
        '/mnt/subvol1/.borgmatic-1234/mnt/subvol1'
    )
@ -301,7 +307,10 @@ def test_dump_data_sources_snapshots_adds_to_existing_exclude_patterns():
    source_directories = ['/foo', '/mnt/subvol1']
    config = {'btrfs': {}, 'exclude_patterns': ['/bar']}
    flexmock(module).should_receive('get_subvolumes').and_return(
-        (module.Subvolume('/mnt/subvol1', contained_source_directories=('/mnt/subvol1',)), module.Subvolume('/mnt/subvol2', contained_source_directories=('/mnt/subvol2',)))
+        (
+            module.Subvolume('/mnt/subvol1', contained_source_directories=('/mnt/subvol1',)),
+            module.Subvolume('/mnt/subvol2', contained_source_directories=('/mnt/subvol2',)),
+        )
    )
    flexmock(module).should_receive('make_snapshot_path').with_args('/mnt/subvol1').and_return(
        '/mnt/subvol1/.borgmatic-1234/mnt/subvol1'
@ -359,7 +368,10 @@ def test_dump_data_sources_snapshots_adds_to_existing_exclude_patterns():
 def test_remove_data_source_dumps_deletes_snapshots():
    config = {'btrfs': {}}
    flexmock(module).should_receive('get_subvolumes').and_return(
-        (module.Subvolume('/mnt/subvol1', contained_source_directories=('/mnt/subvol1',)), module.Subvolume('/mnt/subvol2', contained_source_directories=('/mnt/subvol2',)))
+        (
+            module.Subvolume('/mnt/subvol1', contained_source_directories=('/mnt/subvol1',)),
+            module.Subvolume('/mnt/subvol2', contained_source_directories=('/mnt/subvol2',)),
+        )
    )
    flexmock(module).should_receive('make_snapshot_path').with_args('/mnt/subvol1').and_return(
        '/mnt/subvol1/.borgmatic-1234/./mnt/subvol1'
@ -482,7 +494,10 @@ def test_remove_data_source_dumps_with_get_subvolumes_called_process_error_bails
 def test_remove_data_source_dumps_with_dry_run_skips_deletes():
    config = {'btrfs': {}}
    flexmock(module).should_receive('get_subvolumes').and_return(
-        (module.Subvolume('/mnt/subvol1', contained_source_directories=('/mnt/subvol1',)), module.Subvolume('/mnt/subvol2', contained_source_directories=('/mnt/subvol2',)))
+        (
+            module.Subvolume('/mnt/subvol1', contained_source_directories=('/mnt/subvol1',)),
+            module.Subvolume('/mnt/subvol2', contained_source_directories=('/mnt/subvol2',)),
+        )
    )
    flexmock(module).should_receive('make_snapshot_path').with_args('/mnt/subvol1').and_return(
        '/mnt/subvol1/.borgmatic-1234/./mnt/subvol1'
@ -562,7 +577,10 @@ def test_remove_data_source_dumps_without_subvolumes_skips_deletes():
 def test_remove_data_source_without_snapshots_skips_deletes():
    config = {'btrfs': {}}
    flexmock(module).should_receive('get_subvolumes').and_return(
-        (module.Subvolume('/mnt/subvol1', contained_source_directories=('/mnt/subvol1',)), module.Subvolume('/mnt/subvol2', contained_source_directories=('/mnt/subvol2',)))
+        (
+            module.Subvolume('/mnt/subvol1', contained_source_directories=('/mnt/subvol1',)),
+            module.Subvolume('/mnt/subvol2', contained_source_directories=('/mnt/subvol2',)),
+        )
    )
    flexmock(module).should_receive('make_snapshot_path').with_args('/mnt/subvol1').and_return(
        '/mnt/subvol1/.borgmatic-1234/./mnt/subvol1'
@ -603,7 +621,10 @@ def test_remove_data_source_without_snapshots_skips_deletes():
 def test_remove_data_source_dumps_with_delete_snapshot_file_not_found_error_bails():
    config = {'btrfs': {}}
    flexmock(module).should_receive('get_subvolumes').and_return(
-        (module.Subvolume('/mnt/subvol1', contained_source_directories=('/mnt/subvol1',)), module.Subvolume('/mnt/subvol2', contained_source_directories=('/mnt/subvol2',)))
+        (
+            module.Subvolume('/mnt/subvol1', contained_source_directories=('/mnt/subvol1',)),
+            module.Subvolume('/mnt/subvol2', contained_source_directories=('/mnt/subvol2',)),
+        )
    )
    flexmock(module).should_receive('make_snapshot_path').with_args('/mnt/subvol1').and_return(
        '/mnt/subvol1/.borgmatic-1234/./mnt/subvol1'
--- a/tests/unit/hooks/data_source/test_lvm.py
+++ b/tests/unit/hooks/data_source/test_lvm.py
@ -0,0 +1,617 @@
+from flexmock import flexmock
+import pytest
+
+from borgmatic.hooks.data_source import lvm as module
+
+
+def test_get_logical_volumes_filters_by_source_directories():
+    flexmock(module.borgmatic.execute).should_receive(
+        'execute_command_and_capture_output'
+    ).and_return(
+        '''
+        {
+            "blockdevices": [
+                {
+                   "name": "vgroup-notmounted",
+                   "path": "/dev/mapper/vgroup-notmounted",
+                   "mountpoint": null,
+                   "type": "lvm"
+                }, {
+                   "name": "vgroup-lvolume",
+                   "path": "/dev/mapper/vgroup-lvolume",
+                   "mountpoint": "/mnt/lvolume",
+                   "type": "lvm"
+                }, {
+                   "name": "vgroup-other",
+                   "path": "/dev/mapper/vgroup-other",
+                   "mountpoint": "/mnt/other",
+                   "type": "lvm"
+                }, {
+                   "name": "vgroup-notlvm",
+                   "path": "/dev/mapper/vgroup-notlvm",
+                   "mountpoint": "/mnt/notlvm",
+                   "type": "notlvm"
+                }
+            ]
+        }
+        '''
+    )
+    contained = {'/mnt/lvolume', '/mnt/lvolume/subdir'}
+    flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive(
+        'get_contained_directories'
+    ).with_args(None, contained).never()
+    flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive(
+        'get_contained_directories'
+    ).with_args('/mnt/lvolume', contained).and_return(('/mnt/lvolume', '/mnt/lvolume/subdir'))
+    flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive(
+        'get_contained_directories'
+    ).with_args('/mnt/other', contained).and_return(())
+    flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive(
+        'get_contained_directories'
+    ).with_args('/mnt/notlvm', contained).never()
+
+    assert module.get_logical_volumes(
+        'lsblk', source_directories=('/mnt/lvolume', '/mnt/lvolume/subdir')
+    ) == (
+        module.Logical_volume(
+            'vgroup-lvolume',
+            '/dev/mapper/vgroup-lvolume',
+            '/mnt/lvolume',
+            ('/mnt/lvolume', '/mnt/lvolume/subdir'),
+        ),
+    )
+
+
+def test_get_logical_volumes_with_invalid_lsblk_json_errors():
+    flexmock(module.borgmatic.execute).should_receive(
+        'execute_command_and_capture_output'
+    ).and_return('{')
+
+    contained = {'/mnt/lvolume', '/mnt/lvolume/subdir'}
+    flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive(
+        'get_contained_directories'
+    ).never()
+
+    with pytest.raises(ValueError):
+        module.get_logical_volumes(
+            'lsblk', source_directories=('/mnt/lvolume', '/mnt/lvolume/subdir')
+        )
+
+
+def test_get_logical_volumes_with_lsblk_json_missing_keys_errors():
+    flexmock(module.borgmatic.execute).should_receive(
+        'execute_command_and_capture_output'
+    ).and_return('{"block_devices": [{}]}')
+
+    contained = {'/mnt/lvolume', '/mnt/lvolume/subdir'}
+    flexmock(module.borgmatic.hooks.data_source.snapshot).should_receive(
+        'get_contained_directories'
+    ).never()
+
+    with pytest.raises(ValueError):
+        module.get_logical_volumes(
+            'lsblk', source_directories=('/mnt/lvolume', '/mnt/lvolume/subdir')
+        )
+
+
+def test_snapshot_logical_volume_with_percentage_snapshot_name_uses_lvcreate_extents_flag():
+    flexmock(module.borgmatic.execute).should_receive('execute_command').with_args(
+        (
+            'lvcreate',
+            '--snapshot',
+            '--extents',
+            '10%ORIGIN',
+            '--name',
+            'snap',
+            '/dev/snap',
+        ),
+        output_log_level=object,
+    )
+
+    module.snapshot_logical_volume('lvcreate', 'snap', '/dev/snap', '10%ORIGIN')
+
+
+def test_snapshot_logical_volume_with_non_percentage_snapshot_name_uses_lvcreate_size_flag():
+    flexmock(module.borgmatic.execute).should_receive('execute_command').with_args(
+        (
+            'lvcreate',
+            '--snapshot',
+            '--size',
+            '10TB',
+            '--name',
+            'snap',
+            '/dev/snap',
+        ),
+        output_log_level=object,
+    )
+
+    module.snapshot_logical_volume('lvcreate', 'snap', '/dev/snap', '10TB')
+
+
+def test_dump_data_sources_snapshots_and_mounts_and_updates_source_directories():
+    config = {'lvm': {}}
+    source_directories = ['/mnt/lvolume1/subdir', '/mnt/lvolume2']
+    flexmock(module).should_receive('get_logical_volumes').and_return(
+        (
+            module.Logical_volume(
+                name='lvolume1',
+                device_path='/dev/lvolume1',
+                mount_point='/mnt/lvolume1',
+                contained_source_directories=('/mnt/lvolume1/subdir',),
+            ),
+            module.Logical_volume(
+                name='lvolume2',
+                device_path='/dev/lvolume2',
+                mount_point='/mnt/lvolume2',
+                contained_source_directories=('/mnt/lvolume2',),
+            ),
+        )
+    )
+    flexmock(module.os).should_receive('getpid').and_return(1234)
+    flexmock(module).should_receive('snapshot_logical_volume').with_args(
+        'lvcreate', 'lvolume1_borgmatic-1234', '/dev/lvolume1', module.DEFAULT_SNAPSHOT_SIZE
+    ).once()
+    flexmock(module).should_receive('snapshot_logical_volume').with_args(
+        'lvcreate', 'lvolume2_borgmatic-1234', '/dev/lvolume2', module.DEFAULT_SNAPSHOT_SIZE
+    ).once()
+    flexmock(module).should_receive('get_snapshots').with_args(
+        'lvs', snapshot_name='lvolume1_borgmatic-1234'
+    ).and_return(
+        (module.Snapshot(name='lvolume1_borgmatic-1234', device_path='/dev/lvolume1_snap'),)
+    )
+    flexmock(module).should_receive('get_snapshots').with_args(
+        'lvs', snapshot_name='lvolume2_borgmatic-1234'
+    ).and_return(
+        (module.Snapshot(name='lvolume2_borgmatic-1234', device_path='/dev/lvolume2_snap'),)
+    )
+    flexmock(module).should_receive('mount_snapshot').with_args(
+        'mount', '/dev/lvolume1_snap', '/run/borgmatic/lvm_snapshots/mnt/lvolume1'
+    ).once()
+    flexmock(module).should_receive('mount_snapshot').with_args(
+        'mount', '/dev/lvolume2_snap', '/run/borgmatic/lvm_snapshots/mnt/lvolume2'
+    ).once()
+
+    assert (
+        module.dump_data_sources(
+            hook_config=config['lvm'],
+            config=config,
+            log_prefix='test',
+            config_paths=('test.yaml',),
+            borgmatic_runtime_directory='/run/borgmatic',
+            source_directories=source_directories,
+            dry_run=False,
+        )
+        == []
+    )
+
+    assert source_directories == [
+        '/run/borgmatic/lvm_snapshots/./mnt/lvolume1/subdir',
+        '/run/borgmatic/lvm_snapshots/./mnt/lvolume2',
+    ]
+
+
+def test_dump_data_sources_with_no_logical_volumes_skips_snapshots():
+    config = {'lvm': {}}
+    source_directories = ['/mnt/lvolume1/subdir', '/mnt/lvolume2']
+    flexmock(module).should_receive('get_logical_volumes').and_return(())
+    flexmock(module).should_receive('snapshot_logical_volume').never()
+    flexmock(module).should_receive('mount_snapshot').never()
+
+    assert (
+        module.dump_data_sources(
+            hook_config=config['lvm'],
+            config=config,
+            log_prefix='test',
+            config_paths=('test.yaml',),
+            borgmatic_runtime_directory='/run/borgmatic',
+            source_directories=source_directories,
+            dry_run=False,
+        )
+        == []
+    )
+
+    assert source_directories == ['/mnt/lvolume1/subdir', '/mnt/lvolume2']
+
+
+def test_dump_data_sources_uses_snapshot_size_for_snapshot():
+    config = {'lvm': {'snapshot_size': '1000PB'}}
+    source_directories = ['/mnt/lvolume1/subdir', '/mnt/lvolume2']
+    flexmock(module).should_receive('get_logical_volumes').and_return(
+        (
+            module.Logical_volume(
+                name='lvolume1',
+                device_path='/dev/lvolume1',
+                mount_point='/mnt/lvolume1',
+                contained_source_directories=('/mnt/lvolume1/subdir',),
+            ),
+            module.Logical_volume(
+                name='lvolume2',
+                device_path='/dev/lvolume2',
+                mount_point='/mnt/lvolume2',
+                contained_source_directories=('/mnt/lvolume2',),
+            ),
+        )
+    )
+    flexmock(module.os).should_receive('getpid').and_return(1234)
+    flexmock(module).should_receive('snapshot_logical_volume').with_args(
+        'lvcreate',
+        'lvolume1_borgmatic-1234',
+        '/dev/lvolume1',
+        '1000PB',
+    ).once()
+    flexmock(module).should_receive('snapshot_logical_volume').with_args(
+        'lvcreate',
+        'lvolume2_borgmatic-1234',
+        '/dev/lvolume2',
+        '1000PB',
+    ).once()
+    flexmock(module).should_receive('get_snapshots').with_args(
+        'lvs', snapshot_name='lvolume1_borgmatic-1234'
+    ).and_return(
+        (module.Snapshot(name='lvolume1_borgmatic-1234', device_path='/dev/lvolume1_snap'),)
+    )
+    flexmock(module).should_receive('get_snapshots').with_args(
+        'lvs', snapshot_name='lvolume2_borgmatic-1234'
+    ).and_return(
+        (module.Snapshot(name='lvolume2_borgmatic-1234', device_path='/dev/lvolume2_snap'),)
+    )
+    flexmock(module).should_receive('mount_snapshot').with_args(
+        'mount', '/dev/lvolume1_snap', '/run/borgmatic/lvm_snapshots/mnt/lvolume1'
+    ).once()
+    flexmock(module).should_receive('mount_snapshot').with_args(
+        'mount', '/dev/lvolume2_snap', '/run/borgmatic/lvm_snapshots/mnt/lvolume2'
+    ).once()
+
+    assert (
+        module.dump_data_sources(
+            hook_config=config['lvm'],
+            config=config,
+            log_prefix='test',
+            config_paths=('test.yaml',),
+            borgmatic_runtime_directory='/run/borgmatic',
+            source_directories=source_directories,
+            dry_run=False,
+        )
+        == []
+    )
+
+    assert source_directories == [
+        '/run/borgmatic/lvm_snapshots/./mnt/lvolume1/subdir',
+        '/run/borgmatic/lvm_snapshots/./mnt/lvolume2',
+    ]
+
+
+def test_dump_data_sources_uses_custom_commands():
+    config = {
+        'lvm': {
+            'lsblk_command': '/usr/local/bin/lsblk',
+            'lvcreate_command': '/usr/local/bin/lvcreate',
+            'lvs_command': '/usr/local/bin/lvs',
+            'mount_command': '/usr/local/bin/mount',
+        },
+    }
+    source_directories = ['/mnt/lvolume1/subdir', '/mnt/lvolume2']
+    flexmock(module).should_receive('get_logical_volumes').and_return(
+        (
+            module.Logical_volume(
+                name='lvolume1',
+                device_path='/dev/lvolume1',
+                mount_point='/mnt/lvolume1',
+                contained_source_directories=('/mnt/lvolume1/subdir',),
+            ),
+            module.Logical_volume(
+                name='lvolume2',
+                device_path='/dev/lvolume2',
+                mount_point='/mnt/lvolume2',
+                contained_source_directories=('/mnt/lvolume2',),
+            ),
+        )
+    )
+    flexmock(module.os).should_receive('getpid').and_return(1234)
+    flexmock(module).should_receive('snapshot_logical_volume').with_args(
+        '/usr/local/bin/lvcreate',
+        'lvolume1_borgmatic-1234',
+        '/dev/lvolume1',
+        module.DEFAULT_SNAPSHOT_SIZE,
+    ).once()
+    flexmock(module).should_receive('snapshot_logical_volume').with_args(
+        '/usr/local/bin/lvcreate',
+        'lvolume2_borgmatic-1234',
+        '/dev/lvolume2',
+        module.DEFAULT_SNAPSHOT_SIZE,
+    ).once()
+    flexmock(module).should_receive('get_snapshots').with_args(
+        '/usr/local/bin/lvs', snapshot_name='lvolume1_borgmatic-1234'
+    ).and_return(
+        (module.Snapshot(name='lvolume1_borgmatic-1234', device_path='/dev/lvolume1_snap'),)
+    )
+    flexmock(module).should_receive('get_snapshots').with_args(
+        '/usr/local/bin/lvs', snapshot_name='lvolume2_borgmatic-1234'
+    ).and_return(
+        (module.Snapshot(name='lvolume2_borgmatic-1234', device_path='/dev/lvolume2_snap'),)
+    )
+    flexmock(module).should_receive('mount_snapshot').with_args(
+        '/usr/local/bin/mount', '/dev/lvolume1_snap', '/run/borgmatic/lvm_snapshots/mnt/lvolume1'
+    ).once()
+    flexmock(module).should_receive('mount_snapshot').with_args(
+        '/usr/local/bin/mount', '/dev/lvolume2_snap', '/run/borgmatic/lvm_snapshots/mnt/lvolume2'
+    ).once()
+
+    assert (
+        module.dump_data_sources(
+            hook_config=config['lvm'],
+            config=config,
+            log_prefix='test',
+            config_paths=('test.yaml',),
+            borgmatic_runtime_directory='/run/borgmatic',
+            source_directories=source_directories,
+            dry_run=False,
+        )
+        == []
+    )
+
+    assert source_directories == [
+        '/run/borgmatic/lvm_snapshots/./mnt/lvolume1/subdir',
+        '/run/borgmatic/lvm_snapshots/./mnt/lvolume2',
+    ]
+
+
+def test_dump_data_sources_with_dry_run_skips_snapshots_and_does_not_touch_source_directories():
+    config = {'lvm': {}}
+    source_directories = ['/mnt/lvolume1/subdir', '/mnt/lvolume2']
+    flexmock(module).should_receive('get_logical_volumes').and_return(
+        (
+            module.Logical_volume(
+                name='lvolume1',
+                device_path='/dev/lvolume1',
+                mount_point='/mnt/lvolume1',
+                contained_source_directories=('/mnt/lvolume1/subdir',),
+            ),
+            module.Logical_volume(
+                name='lvolume2',
+                device_path='/dev/lvolume2',
+                mount_point='/mnt/lvolume2',
+                contained_source_directories=('/mnt/lvolume2',),
+            ),
+        )
+    )
+    flexmock(module.os).should_receive('getpid').and_return(1234)
+    flexmock(module).should_receive('snapshot_logical_volume').never()
+    flexmock(module).should_receive('get_snapshots').with_args(
+        'lvs', snapshot_name='lvolume1_borgmatic-1234'
+    ).and_return(
+        (module.Snapshot(name='lvolume1_borgmatic-1234', device_path='/dev/lvolume1_snap'),)
+    )
+    flexmock(module).should_receive('get_snapshots').with_args(
+        'lvs', snapshot_name='lvolume2_borgmatic-1234'
+    ).and_return(
+        (module.Snapshot(name='lvolume2_borgmatic-1234', device_path='/dev/lvolume2_snap'),)
+    )
+    flexmock(module).should_receive('mount_snapshot').never()
+
+    assert (
+        module.dump_data_sources(
+            hook_config=config['lvm'],
+            config=config,
+            log_prefix='test',
+            config_paths=('test.yaml',),
+            borgmatic_runtime_directory='/run/borgmatic',
+            source_directories=source_directories,
+            dry_run=True,
+        )
+        == []
+    )
+
+    assert source_directories == [
+        '/mnt/lvolume1/subdir',
+        '/mnt/lvolume2',
+    ]
+
+
+def test_dump_data_sources_ignores_mismatch_between_source_directories_and_contained_source_directories():
+    config = {'lvm': {}}
+    source_directories = ['/hmm']
+    flexmock(module).should_receive('get_logical_volumes').and_return(
+        (
+            module.Logical_volume(
+                name='lvolume1',
+                device_path='/dev/lvolume1',
+                mount_point='/mnt/lvolume1',
+                contained_source_directories=('/mnt/lvolume1/subdir',),
+            ),
+            module.Logical_volume(
+                name='lvolume2',
+                device_path='/dev/lvolume2',
+                mount_point='/mnt/lvolume2',
+                contained_source_directories=('/mnt/lvolume2',),
+            ),
+        )
+    )
+    flexmock(module.os).should_receive('getpid').and_return(1234)
+    flexmock(module).should_receive('snapshot_logical_volume').with_args(
+        'lvcreate', 'lvolume1_borgmatic-1234', '/dev/lvolume1', module.DEFAULT_SNAPSHOT_SIZE
+    ).once()
+    flexmock(module).should_receive('snapshot_logical_volume').with_args(
+        'lvcreate', 'lvolume2_borgmatic-1234', '/dev/lvolume2', module.DEFAULT_SNAPSHOT_SIZE
+    ).once()
+    flexmock(module).should_receive('get_snapshots').with_args(
+        'lvs', snapshot_name='lvolume1_borgmatic-1234'
+    ).and_return(
+        (module.Snapshot(name='lvolume1_borgmatic-1234', device_path='/dev/lvolume1_snap'),)
+    )
+    flexmock(module).should_receive('get_snapshots').with_args(
+        'lvs', snapshot_name='lvolume2_borgmatic-1234'
+    ).and_return(
+        (module.Snapshot(name='lvolume2_borgmatic-1234', device_path='/dev/lvolume2_snap'),)
+    )
+    flexmock(module).should_receive('mount_snapshot').with_args(
+        'mount', '/dev/lvolume1_snap', '/run/borgmatic/lvm_snapshots/mnt/lvolume1'
+    ).once()
+    flexmock(module).should_receive('mount_snapshot').with_args(
+        'mount', '/dev/lvolume2_snap', '/run/borgmatic/lvm_snapshots/mnt/lvolume2'
+    ).once()
+
+    assert (
+        module.dump_data_sources(
+            hook_config=config['lvm'],
+            config=config,
+            log_prefix='test',
+            config_paths=('test.yaml',),
+            borgmatic_runtime_directory='/run/borgmatic',
+            source_directories=source_directories,
+            dry_run=False,
+        )
+        == []
+    )
+
+    assert source_directories == [
+        '/hmm',
+        '/run/borgmatic/lvm_snapshots/./mnt/lvolume1/subdir',
+        '/run/borgmatic/lvm_snapshots/./mnt/lvolume2',
+    ]
+
+
+def test_dump_data_sources_with_missing_snapshot_errors():
+    config = {'lvm': {}}
+    source_directories = ['/mnt/lvolume1/subdir', '/mnt/lvolume2']
+    flexmock(module).should_receive('get_logical_volumes').and_return(
+        (
+            module.Logical_volume(
+                name='lvolume1',
+                device_path='/dev/lvolume1',
+                mount_point='/mnt/lvolume1',
+                contained_source_directories=('/mnt/lvolume1/subdir',),
+            ),
+            module.Logical_volume(
+                name='lvolume2',
+                device_path='/dev/lvolume2',
+                mount_point='/mnt/lvolume2',
+                contained_source_directories=('/mnt/lvolume2',),
+            ),
+        )
+    )
+    flexmock(module.os).should_receive('getpid').and_return(1234)
+    flexmock(module).should_receive('snapshot_logical_volume').with_args(
+        'lvcreate', 'lvolume1_borgmatic-1234', '/dev/lvolume1', module.DEFAULT_SNAPSHOT_SIZE
+    ).once()
+    flexmock(module).should_receive('snapshot_logical_volume').with_args(
+        'lvcreate', 'lvolume2_borgmatic-1234', '/dev/lvolume2', module.DEFAULT_SNAPSHOT_SIZE
+    ).never()
+    flexmock(module).should_receive('get_snapshots').with_args(
+        'lvs', snapshot_name='lvolume1_borgmatic-1234'
+    ).and_return(())
+    flexmock(module).should_receive('get_snapshots').with_args(
+        'lvs', snapshot_name='lvolume2_borgmatic-1234'
+    ).never()
+    flexmock(module).should_receive('mount_snapshot').never()
+
+    with pytest.raises(ValueError):
+        module.dump_data_sources(
+            hook_config=config['lvm'],
+            config=config,
+            log_prefix='test',
+            config_paths=('test.yaml',),
+            borgmatic_runtime_directory='/run/borgmatic',
+            source_directories=source_directories,
+            dry_run=False,
+        )
+
+
+def test_get_snapshots_lists_all_snapshots():
+    flexmock(module.borgmatic.execute).should_receive(
+        'execute_command_and_capture_output'
+    ).and_return(
+        '''
+          {
+              "report": [
+                  {
+                      "lv": [
+                          {"lv_name": "snap1", "lv_path": "/dev/snap1"},
+                          {"lv_name": "snap2", "lv_path": "/dev/snap2"}
+                      ]
+                  }
+              ],
+              "log": [
+              ]
+          }
+        '''
+    )
+
+    assert module.get_snapshots('lvs') == (
+        module.Snapshot('snap1', '/dev/snap1'),
+        module.Snapshot('snap2', '/dev/snap2'),
+    )
+
+
+def test_get_snapshots_with_snapshot_name_lists_just_that_snapshot():
+    flexmock(module.borgmatic.execute).should_receive(
+        'execute_command_and_capture_output'
+    ).and_return(
+        '''
+          {
+              "report": [
+                  {
+                      "lv": [
+                          {"lv_name": "snap1", "lv_path": "/dev/snap1"},
+                          {"lv_name": "snap2", "lv_path": "/dev/snap2"}
+                      ]
+                  }
+              ],
+              "log": [
+              ]
+          }
+        '''
+    )
+
+    assert module.get_snapshots('lvs', snapshot_name='snap2') == (
+        module.Snapshot('snap2', '/dev/snap2'),
+    )
+
+
+def test_get_snapshots_with_invalid_lvs_json_errors():
+    flexmock(module.borgmatic.execute).should_receive(
+        'execute_command_and_capture_output'
+    ).and_return('{')
+
+    with pytest.raises(ValueError):
+        assert module.get_snapshots('lvs')
+
+
+def test_get_snapshots_with_lvs_json_missing_report_errors():
+    flexmock(module.borgmatic.execute).should_receive(
+        'execute_command_and_capture_output'
+    ).and_return(
+        '''
+          {
+              "report": [],
+              "log": [
+              ]
+          }
+        '''
+    )
+
+    with pytest.raises(ValueError):
+        assert module.get_snapshots('lvs')
+
+
+def test_get_snapshots_with_lvs_json_missing_keys_errors():
+    flexmock(module.borgmatic.execute).should_receive(
+        'execute_command_and_capture_output'
+    ).and_return(
+        '''
+          {
+              "report": [
+                  {
+                      "lv": [
+                          {}
+                      ]
+                  }
+              ],
+              "log": [
+              ]
+          }
+        '''
+    )
+
+    with pytest.raises(ValueError):
+        assert module.get_snapshots('lvs')
--- a/tests/unit/hooks/data_source/test_zfs.py
+++ b/tests/unit/hooks/data_source/test_zfs.py
@ -121,7 +121,7 @@ def test_dump_data_sources_snapshots_and_mounts_and_updates_source_directories()
    assert source_directories == [os.path.join(snapshot_mount_path, 'subdir')]


-def test_dump_data_sources_snapshots_with_no_datasets_skips_snapshots():
+def test_dump_data_sources_with_no_datasets_skips_snapshots():
    flexmock(module).should_receive('get_datasets_to_backup').and_return(())
    flexmock(module.os).should_receive('getpid').and_return(1234)
    flexmock(module).should_receive('snapshot_dataset').never()