From ebb3bca4b316dbe398a4b9b52078c1fb22d7e698 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 12 Dec 2024 11:58:18 -0800 Subject: [PATCH] Fix findmnt command error in the Btrfs hook by switching to parsing JSON output (#954). --- NEWS | 3 ++ borgmatic/hooks/data_source/btrfs.py | 13 ++++++-- pyproject.toml | 2 +- tests/end-to-end/commands/fake_findmnt.py | 27 ++++++++++----- tests/unit/hooks/data_source/test_btrfs.py | 38 ++++++++++++++++++++-- 5 files changed, 69 insertions(+), 14 deletions(-) diff --git a/NEWS b/NEWS index ef7c6df0..61f9ac2e 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +1.9.5.dev0 + * #954: Fix findmnt command error in the Btrfs hook by switching to parsing JSON output. + 1.9.4 * #80 (beta): Add an LVM hook for snapshotting and backing up LVM logical volumes. See the documentation for more information: diff --git a/borgmatic/hooks/data_source/btrfs.py b/borgmatic/hooks/data_source/btrfs.py index 4ddc2ae8..8c7d24a9 100644 --- a/borgmatic/hooks/data_source/btrfs.py +++ b/borgmatic/hooks/data_source/btrfs.py @@ -1,5 +1,6 @@ import collections import glob +import json import logging import os import shutil @@ -26,13 +27,21 @@ def get_filesystem_mount_points(findmnt_command): findmnt_output = borgmatic.execute.execute_command_and_capture_output( tuple(findmnt_command.split(' ')) + ( - '-n', # No headings. '-t', # Filesystem type. 'btrfs', + '--json', + '--list', # Request a flat list instead of a nested subvolume hierarchy. ) ) - return tuple(line.rstrip().split(' ')[0] for line in findmnt_output.splitlines()) + try: + return tuple( + filesystem['target'] for filesystem in json.loads(findmnt_output)['filesystems'] + ) + except json.JSONDecodeError as error: + raise ValueError(f'Invalid {findmnt_command} JSON output: {error}') + except KeyError as error: + raise ValueError(f'Invalid {findmnt_command} output: Missing key "{error}"') def get_subvolumes_for_filesystem(btrfs_command, filesystem_mount_point): diff --git a/pyproject.toml b/pyproject.toml index 9ac3e839..4f5a540e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "borgmatic" -version = "1.9.4" +version = "1.9.5.dev0" authors = [ { name="Dan Helfman", email="witten@torsion.org" }, ] diff --git a/tests/end-to-end/commands/fake_findmnt.py b/tests/end-to-end/commands/fake_findmnt.py index d7393bb3..9f28e105 100644 --- a/tests/end-to-end/commands/fake_findmnt.py +++ b/tests/end-to-end/commands/fake_findmnt.py @@ -4,29 +4,38 @@ import sys def parse_arguments(*unparsed_arguments): parser = argparse.ArgumentParser(add_help=False) - parser.add_argument('-n', dest='headings', action='store_false', default=True) parser.add_argument('-t', dest='type') + parser.add_argument('--json', action='store_true') + parser.add_argument('--list', action='store_true') return parser.parse_args(unparsed_arguments) -BUILTIN_FILESYSTEM_MOUNT_LINES = ( - '/mnt/subvolume /dev/loop1 btrfs rw,relatime,ssd,space_cache=v2,subvolid=5,subvol=/', -) +BUILTIN_FILESYSTEM_MOUNT_OUTPUT = '''{ + "filesystems": [ + { + "target": "/mnt/subvolume", + "source": "/dev/loop0", + "fstype": "btrfs", + "options": "rw,relatime,ssd,space_cache=v2,subvolid=5,subvol=/" + } + ] + } + ''' -def print_filesystem_mounts(arguments): - for line in BUILTIN_FILESYSTEM_MOUNT_LINES: - print(line) +def print_filesystem_mounts(): + print(BUILTIN_FILESYSTEM_MOUNT_OUTPUT) def main(): arguments = parse_arguments(*sys.argv[1:]) - assert not arguments.headings assert arguments.type == 'btrfs' + assert arguments.json + assert arguments.list - print_filesystem_mounts(arguments) + print_filesystem_mounts() if __name__ == '__main__': diff --git a/tests/unit/hooks/data_source/test_btrfs.py b/tests/unit/hooks/data_source/test_btrfs.py index 00e4d186..02909c6a 100644 --- a/tests/unit/hooks/data_source/test_btrfs.py +++ b/tests/unit/hooks/data_source/test_btrfs.py @@ -1,3 +1,4 @@ +import pytest from flexmock import flexmock from borgmatic.hooks.data_source import btrfs as module @@ -7,13 +8,46 @@ def test_get_filesystem_mount_points_parses_findmnt_output(): flexmock(module.borgmatic.execute).should_receive( 'execute_command_and_capture_output' ).and_return( - '/mnt0 /dev/loop0 btrfs rw,relatime,ssd,space_cache=v2,subvolid=5,subvol=/\n' - '/mnt1 /dev/loop1 btrfs rw,relatime,ssd,space_cache=v2,subvolid=5,subvol=/\n' + '''{ + "filesystems": [ + { + "target": "/mnt0", + "source": "/dev/loop0", + "fstype": "btrfs", + "options": "rw,relatime,ssd,space_cache=v2,subvolid=5,subvol=/" + }, + { + "target": "/mnt1", + "source": "/dev/loop0", + "fstype": "btrfs", + "options": "rw,relatime,ssd,space_cache=v2,subvolid=5,subvol=/" + } + ] + } + ''' ) assert module.get_filesystem_mount_points('findmnt') == ('/mnt0', '/mnt1') +def test_get_filesystem_mount_points_with_invalid_findmnt_json_errors(): + flexmock(module.borgmatic.execute).should_receive( + 'execute_command_and_capture_output' + ).and_return('{') + + with pytest.raises(ValueError): + module.get_filesystem_mount_points('findmnt') + + +def test_get_filesystem_mount_points_with_findmnt_json_missing_filesystems_errors(): + flexmock(module.borgmatic.execute).should_receive( + 'execute_command_and_capture_output' + ).and_return('{"wtf": "something is wrong here"}') + + with pytest.raises(ValueError): + module.get_filesystem_mount_points('findmnt') + + def test_get_subvolumes_for_filesystem_parses_subvolume_list_output(): flexmock(module.borgmatic.execute).should_receive( 'execute_command_and_capture_output'