From b1e343f15c074427c534dd110970147beeefb2fc Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Mon, 9 Dec 2024 08:48:34 -0800 Subject: [PATCH 01/14] Initial work on supporting same-named database with different ports, hosts, or hooks (#418). --- NEWS | 4 + borgmatic/actions/restore.py | 362 +++++++++++++--------- borgmatic/commands/arguments.py | 15 +- borgmatic/hooks/data_source/dump.py | 10 +- borgmatic/hooks/data_source/mariadb.py | 5 +- borgmatic/hooks/data_source/mongodb.py | 5 +- borgmatic/hooks/data_source/mysql.py | 5 +- borgmatic/hooks/data_source/postgresql.py | 5 +- pyproject.toml | 2 +- 9 files changed, 250 insertions(+), 163 deletions(-) diff --git a/NEWS b/NEWS index ef7c6df0..1b505b65 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,7 @@ +1.9.5.dev0 + * #418: Backup and restore databases that have the same name but with different ports, hostnames, + or hooks. + 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/actions/restore.py b/borgmatic/actions/restore.py index 109ac1e7..667c26c9 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -1,3 +1,4 @@ +import collections import copy import logging import os @@ -17,30 +18,75 @@ import borgmatic.hooks.dispatch logger = logging.getLogger(__name__) -UNSPECIFIED_HOOK = object() +UNSPECIFIED = object() -def get_configured_data_source( - config, - archive_data_source_names, - hook_name, - data_source_name, - configuration_data_source_name=None, +class Dump( + collections.namedtuple( + 'Dump', + ('hook_name', 'data_source_name', 'hostname', 'port'), + defaults=('localhost', None), + ) ): + def __eq__(self, other): + ''' + Compare two namedtuples for equality while supporting a field value of UNSPECIFIED, which + indicates that the field should match any value. + ''' + for field_name in self._fields: + self_value = getattr(self, field_name) + other_value = getattr(other, field_name) + + if self_value == UNSPECIFIED or other_value == UNSPECIFIED: + continue + + if self_value != other_value: + return False + + return True + + def __ne__(self, other): + return not self == other + + def __lt__(self, other): + return self.data_source_name < other.data_source_name + + def __gt__(self, other): + return self.data_source_name > other.data_source_name + + def __hash__(self): + return hash(tuple(self)) + + +def render_dump_metadata(dump): ''' - Find the first data source with the given hook name and data source name in the configuration - dict and the given archive data source names dict (from hook name to data source names contained - in a particular backup archive). If UNSPECIFIED_HOOK is given as the hook name, search all data - source hooks for the named data source. If a configuration data source name is given, use that - instead of the data source name to lookup the data source in the given hooks configuration. + Given a Dump instance, make a display string describing it for use in log messges. + ''' + name = dump.data_source_name if dump.data_source_name != UNSPECIFIED else 'unspecified' + hostname = dump.hostname or 'localhost' + port = dump.port if dump.port != UNSPECIFIED else None + + if port: + metadata = f'{name}@:{port}' if hostname is UNSPECIFIED else f'{name}@{hostname}:{port}' + else: + metadata = f'{name}' if hostname is UNSPECIFIED else f'{name}@{hostname}' + + if dump.hook_name not in (None, UNSPECIFIED): + return f'{metadata} ({dump.hook_name})' + + return metadata + + +def get_configured_data_source(config, restore_dump): + ''' + Search in the given configuration dict for dumps corresponding to the given dump to restore. If + there are multiple matches, error. If UNSPECIFIED is given as any field in the restore dump, + then that can match any valid value. Return the found data source as a tuple of (found hook name, data source configuration dict) or (None, None) if not found. ''' - if not configuration_data_source_name: - configuration_data_source_name = data_source_name - - if hook_name == UNSPECIFIED_HOOK: + if restore_dump.hook_name == UNSPECIFIED: hooks_to_search = { hook_name: value for (hook_name, value) in config.items() @@ -49,21 +95,33 @@ def get_configured_data_source( } else: try: - hooks_to_search = {hook_name: config[hook_name]} + hooks_to_search = {restore_dump.hook_name: config[restore_dump.hook_name]} except KeyError: return (None, None) - return next( - ( - (name, hook_data_source) - for (name, hook) in hooks_to_search.items() - for hook_data_source in hook - if hook_data_source['name'] == configuration_data_source_name - and data_source_name in archive_data_source_names.get(name, []) - ), - (None, None), + matching_dumps = tuple( + (hook_name, hook_data_source) + for (hook_name, hook) in hooks_to_search.items() + for hook_data_source in hook + if Dump( + hook_name, + hook_data_source.get('name'), + hook_data_source.get('hostname'), + hook_data_source.get('port'), + ) + == restore_dump ) + if not matching_dumps: + return (None, None) + + if len(matching_dumps) > 1: + raise ValueError( + f'Cannot restore data source {render_dump_metadata(restore_dump)} because there are multiple matching dumps in the archive. Try adding additional flags to disambiguate.' + ) + + return matching_dumps[0] + def strip_path_prefix_from_extracted_dump_destination( destination_path, borgmatic_runtime_directory @@ -98,7 +156,7 @@ def strip_path_prefix_from_extracted_dump_destination( break -def restore_single_data_source( +def restore_single_dump( repository, config, local_borg_version, @@ -116,8 +174,12 @@ def restore_single_data_source( username/password as connection params, and a configured data source configuration dict, restore that data source from the archive. ''' + dump_metadata = render_dump_metadata( + Dump(hook_name, data_source["name"], data_source.get("hostname"), data_source.get("port")) + ) + logger.info( - f'{repository.get("label", repository["path"])}: Restoring data source {data_source["name"]}' + f'{repository.get("label", repository["path"])}: Restoring data source {dump_metadata}' ) dump_patterns = borgmatic.hooks.dispatch.call_hooks( @@ -180,7 +242,7 @@ def restore_single_data_source( ) -def collect_archive_data_source_names( +def collect_dumps_from_archive( repository, archive, config, @@ -192,17 +254,17 @@ def collect_archive_data_source_names( ): ''' Given a local or remote repository path, a resolved archive name, a configuration dict, the - local Borg version, global_arguments an argparse.Namespace, local and remote Borg paths, and the - borgmatic runtime directory, query the archive for the names of data sources it contains as - dumps and return them as a dict from hook name to a sequence of data source names. + local Borg version, global arguments an argparse.Namespace, local and remote Borg paths, and the + borgmatic runtime directory, query the archive for the names of data sources dumps it contains + and return them as a set of Dump instances. ''' borgmatic_source_directory = str( pathlib.Path(borgmatic.config.paths.get_borgmatic_source_directory(config)) ) # Probe for the data source dumps in multiple locations, as the default location has moved to - # the borgmatic runtime directory (which get stored as just "/borgmatic" with Borg 1.4+). But we - # still want to support reading dumps from previously created archives as well. + # the borgmatic runtime directory (which gets stored as just "/borgmatic" with Borg 1.4+). But + # we still want to support reading dumps from previously created archives as well. dump_paths = borgmatic.borg.list.capture_archive_listing( repository, archive, @@ -224,9 +286,8 @@ def collect_archive_data_source_names( remote_path=remote_path, ) - # Determine the data source names corresponding to the dumps found in the archive and - # add them to restore_names. - archive_data_source_names = {} + # Parse out the details for the dumps found in the archive. + dumps_from_archive = set() for dump_path in dump_paths: if not dump_path: @@ -238,96 +299,112 @@ def collect_archive_data_source_names( borgmatic_source_directory, ): try: - (hook_name, _, data_source_name) = dump_path.split(base_directory + os.path.sep, 1)[ - 1 - ].split(os.path.sep)[0:3] + (hook_name, host_and_port, data_source_name) = dump_path.split( + base_directory + os.path.sep, 1 + )[1].split(os.path.sep)[0:3] except (ValueError, IndexError): - pass - else: - if data_source_name not in archive_data_source_names.get(hook_name, []): - archive_data_source_names.setdefault(hook_name, []).extend([data_source_name]) - break + continue + + parts = host_and_port.split(':', 1) + + if len(parts) == 1: + parts += (None,) + + (hostname, port) = parts + + try: + port = int(port) + except (ValueError, TypeError): + port = None + + dumps_from_archive.add(Dump(hook_name, data_source_name, hostname, port)) + + break else: logger.warning( f'{repository}: Ignoring invalid data source dump path "{dump_path}" in archive {archive}' ) - return archive_data_source_names + return dumps_from_archive -def find_data_sources_to_restore(requested_data_source_names, archive_data_source_names): +def get_dumps_to_restore(restore_arguments, dumps_from_archive): ''' - Given a sequence of requested data source names to restore and a dict of hook name to the names - of data sources found in an archive, return an expanded sequence of data source names to - restore, replacing "all" with actual data source names as appropriate. + Given restore arguments as an argparse.Namespace instance indicating which dumps to restore and + a set of Dump instances representing the dumps found in an archive, return a set of Dump + instances to restore. As part of this, replace any Dump having a data source name of "all" with + multiple named Dump instances as appropriate. Raise ValueError if any of the requested data source names cannot be found in the archive. ''' - # A map from data source hook name to the data source names to restore for that hook. - restore_names = ( - {UNSPECIFIED_HOOK: requested_data_source_names} - if requested_data_source_names - else {UNSPECIFIED_HOOK: ['all']} + # A map from data source hook name to the dumps to restore for that hook. + dumps_to_restore = ( + { + Dump( + hook_name=(restore_arguments.hook if restore_arguments.hook.endswith('_databases') else f'{restore_arguments.hook}_databases') if restore_arguments.hook else UNSPECIFIED, + data_source_name=name, + hostname=restore_arguments.original_hostname or 'localhost', + port=restore_arguments.original_port + ) + for name in restore_arguments.data_sources + } + if restore_arguments.data_sources + else { + Dump( + hook_name=UNSPECIFIED, + data_source_name='all', + hostname=UNSPECIFIED, + port=UNSPECIFIED, + ) + } ) - # If "all" is in restore_names, then replace it with the names of dumps found within the - # archive. - if 'all' in restore_names[UNSPECIFIED_HOOK]: - restore_names[UNSPECIFIED_HOOK].remove('all') + # If "all" is in dumps_to_restore, then replace it with named dumps found within the archive. + try: + all_dump = next(dump for dump in dumps_to_restore if dump.data_source_name == 'all') + except StopIteration: + pass + else: + dumps_to_restore.remove(all_dump) - for hook_name, data_source_names in archive_data_source_names.items(): - restore_names.setdefault(hook_name, []).extend(data_source_names) + for dump in dumps_from_archive: + if all_dump.hook_name == UNSPECIFIED or dump.hook_name == all_dump.hook_name: + dumps_to_restore.add(dump) - # If a data source is to be restored as part of "all", then remove it from restore names - # so it doesn't get restored twice. - for data_source_name in data_source_names: - if data_source_name in restore_names[UNSPECIFIED_HOOK]: - restore_names[UNSPECIFIED_HOOK].remove(data_source_name) + missing_dumps = { + restore_dump + for restore_dump in dumps_to_restore + if all(restore_dump != archive_dump for archive_dump in dumps_from_archive) + } - if not restore_names[UNSPECIFIED_HOOK]: - restore_names.pop(UNSPECIFIED_HOOK) + if missing_dumps: + rendered_dumps = ', '.join(f'{render_dump_metadata(dump)}' for dump in sorted(missing_dumps)) - combined_restore_names = set( - name for data_source_names in restore_names.values() for name in data_source_names - ) - combined_archive_data_source_names = set( - name - for data_source_names in archive_data_source_names.values() - for name in data_source_names - ) - - missing_names = sorted(set(combined_restore_names) - combined_archive_data_source_names) - if missing_names: - joined_names = ', '.join(f'"{name}"' for name in missing_names) raise ValueError( - f"Cannot restore data source{'s' if len(missing_names) > 1 else ''} {joined_names} missing from archive" + f"Cannot restore data source{'s' if len(missing_dumps) > 1 else ''} {rendered_dumps} missing from archive" ) - return restore_names + return dumps_to_restore -def ensure_data_sources_found(restore_names, remaining_restore_names, found_names): +def ensure_requested_dumps_restored(dumps_to_restore, dumps_actually_restored): ''' - Given a dict from hook name to data source names to restore, a dict from hook name to remaining - data source names to restore, and a sequence of found (actually restored) data source names, - raise ValueError if requested data source to restore were missing from the archive and/or - configuration. + Given a set of requested dumps to restore and a set of dumps actually restored, raise ValueError + if any requested dumps to restore weren't restored, indicating that they were missing from the + archive and/or configuration. ''' - combined_restore_names = set( - name - for data_source_names in tuple(restore_names.values()) - + tuple(remaining_restore_names.values()) - for name in data_source_names - ) - - if not combined_restore_names and not found_names: + if not dumps_actually_restored: raise ValueError('No data source dumps were found to restore') - missing_names = sorted(set(combined_restore_names) - set(found_names)) - if missing_names: - joined_names = ', '.join(f'"{name}"' for name in missing_names) + missing_dumps = sorted( + dumps_to_restore - dumps_actually_restored, key=lambda dump: dump.data_source_name + ) + + if missing_dumps: + rendered_dumps = ', '.join(f'{render_dump_metadata(dump)}' for dump in missing_dumps) + raise ValueError( - f"Cannot restore data source{'s' if len(missing_names) > 1 else ''} {joined_names} missing from borgmatic's configuration" + f"Cannot restore data source{'s' if len(missing_dumps) > 1 else ''} {rendered_dumps} missing from borgmatic's configuration" ) @@ -375,7 +452,7 @@ def run_restore( local_path, remote_path, ) - archive_data_source_names = collect_archive_data_source_names( + dumps_from_archive = collect_dumps_from_archive( repository['path'], archive_name, config, @@ -385,11 +462,9 @@ def run_restore( remote_path, borgmatic_runtime_directory, ) - restore_names = find_data_sources_to_restore( - restore_arguments.data_sources, archive_data_source_names - ) - found_names = set() - remaining_restore_names = {} + dumps_to_restore = get_dumps_to_restore(restore_arguments, dumps_from_archive) + + dumps_actually_restored = set() connection_params = { 'hostname': restore_arguments.hostname, 'port': restore_arguments.port, @@ -398,61 +473,42 @@ def run_restore( 'restore_path': restore_arguments.restore_path, } - for hook_name, data_source_names in restore_names.items(): - for data_source_name in data_source_names: + # Restore each dump. + for restore_dump in dumps_to_restore: + found_hook_name, found_data_source = get_configured_data_source( + config, + restore_dump, + ) + + # For any data sources that weren't found via exact matches in the configuration, try to + # fallback to "all" entries. + if not found_data_source: found_hook_name, found_data_source = get_configured_data_source( - config, archive_data_source_names, hook_name, data_source_name - ) - - if not found_data_source: - remaining_restore_names.setdefault(found_hook_name or hook_name, []).append( - data_source_name - ) - continue - - found_names.add(data_source_name) - restore_single_data_source( - repository, config, - local_borg_version, - global_arguments, - local_path, - remote_path, - archive_name, - found_hook_name or hook_name, - dict(found_data_source, **{'schemas': restore_arguments.schemas}), - connection_params, - borgmatic_runtime_directory, - ) - - # For any data sources that weren't found via exact matches in the configuration, try to - # fallback to "all" entries. - for hook_name, data_source_names in remaining_restore_names.items(): - for data_source_name in data_source_names: - found_hook_name, found_data_source = get_configured_data_source( - config, archive_data_source_names, hook_name, data_source_name, 'all' + Dump(restore_dump.hook_name, 'all', restore_dump.hostname, restore_dump.port), ) if not found_data_source: continue - found_names.add(data_source_name) - data_source = copy.copy(found_data_source) - data_source['name'] = data_source_name + found_data_source = dict(found_data_source) + found_data_source['name'] = restore_dump.data_source_name - restore_single_data_source( - repository, - config, - local_borg_version, - global_arguments, - local_path, - remote_path, - archive_name, - found_hook_name or hook_name, - dict(data_source, **{'schemas': restore_arguments.schemas}), - connection_params, - borgmatic_runtime_directory, - ) + dumps_actually_restored.add(restore_dump) + + restore_single_dump( + repository, + config, + local_borg_version, + global_arguments, + local_path, + remote_path, + archive_name, + found_hook_name or restore_dump.hook_name, + dict(found_data_source, **{'schemas': restore_arguments.schemas}), + connection_params, + borgmatic_runtime_directory, + ) borgmatic.hooks.dispatch.call_hooks_even_if_unconfigured( 'remove_data_source_dumps', @@ -463,4 +519,4 @@ def run_restore( global_arguments.dry_run, ) - ensure_data_sources_found(restore_names, remaining_restore_names, found_names) + ensure_requested_dumps_restored(dumps_to_restore, dumps_actually_restored) diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index ac407ee4..7e08b726 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -1153,7 +1153,7 @@ def make_parsers(): metavar='NAME', dest='data_sources', action='append', - help="Name of data source (e.g. database) to restore from archive, must be defined in borgmatic's configuration, can specify flag multiple times, defaults to all data sources in the archive", + help="Name of data source (e.g. database) to restore from the archive, must be defined in borgmatic's configuration, can specify the flag multiple times, defaults to all data sources in the archive", ) restore_group.add_argument( '--schema', @@ -1182,6 +1182,19 @@ def make_parsers(): '--restore-path', help='Path to restore SQLite database dumps to. Defaults to the "restore_path" option in borgmatic\'s configuration', ) + restore_group.add_argument( + '--original-hostname', + help="The hostname where the dump to restore came from, only necessary if you need to disambiguate dumps", + ) + restore_group.add_argument( + '--original-port', + type=int, + help="The port where the dump to restore came from, only necessary if you need to disambiguate dumps", + ) + restore_group.add_argument( + '--hook', + help="The name of the data source hook for the dump to restore, only necessary if you need to disambiguate dumps", + ) restore_group.add_argument( '-h', '--help', action='help', help='Show this help message and exit' ) diff --git a/borgmatic/hooks/data_source/dump.py b/borgmatic/hooks/data_source/dump.py index 1fe0c7b1..c602875f 100644 --- a/borgmatic/hooks/data_source/dump.py +++ b/borgmatic/hooks/data_source/dump.py @@ -16,17 +16,19 @@ def make_data_source_dump_path(borgmatic_runtime_directory, data_source_hook_nam return os.path.join(borgmatic_runtime_directory, data_source_hook_name) -def make_data_source_dump_filename(dump_path, name, hostname=None): +def make_data_source_dump_filename(dump_path, name, hostname=None, port=None): ''' - Based on the given dump directory path, data source name, and hostname, return a filename to use - for the data source dump. The hostname defaults to localhost. + Based on the given dump directory path, data source name, hostname, and port, return a filename + to use for the data source dump. The hostname defaults to localhost. Raise ValueError if the data source name is invalid. ''' if os.path.sep in name: raise ValueError(f'Invalid data source name {name}') - return os.path.join(dump_path, hostname or 'localhost', name) + return os.path.join( + dump_path, (hostname or 'localhost') + (f':{port}' if port is not None else ''), name + ) def create_parent_directory_for_dump(dump_path): diff --git a/borgmatic/hooks/data_source/mariadb.py b/borgmatic/hooks/data_source/mariadb.py index d9235d03..8ef93ffe 100644 --- a/borgmatic/hooks/data_source/mariadb.py +++ b/borgmatic/hooks/data_source/mariadb.py @@ -73,7 +73,10 @@ def execute_dump_command( ''' database_name = database['name'] dump_filename = dump.make_data_source_dump_filename( - dump_path, database['name'], database.get('hostname') + dump_path, + database['name'], + database.get('hostname'), + database.get('port'), ) if os.path.exists(dump_filename): diff --git a/borgmatic/hooks/data_source/mongodb.py b/borgmatic/hooks/data_source/mongodb.py index e8b87988..3287d36e 100644 --- a/borgmatic/hooks/data_source/mongodb.py +++ b/borgmatic/hooks/data_source/mongodb.py @@ -51,7 +51,10 @@ def dump_data_sources( for database in databases: name = database['name'] dump_filename = dump.make_data_source_dump_filename( - make_dump_path(borgmatic_runtime_directory), name, database.get('hostname') + make_dump_path(borgmatic_runtime_directory), + name, + database.get('hostname'), + database.get('port'), ) dump_format = database.get('format', 'archive') diff --git a/borgmatic/hooks/data_source/mysql.py b/borgmatic/hooks/data_source/mysql.py index 8ec6ad7a..1e55b4da 100644 --- a/borgmatic/hooks/data_source/mysql.py +++ b/borgmatic/hooks/data_source/mysql.py @@ -73,7 +73,10 @@ def execute_dump_command( ''' database_name = database['name'] dump_filename = dump.make_data_source_dump_filename( - dump_path, database['name'], database.get('hostname') + dump_path, + database['name'], + database.get('hostname'), + database.get('port'), ) if os.path.exists(dump_filename): diff --git a/borgmatic/hooks/data_source/postgresql.py b/borgmatic/hooks/data_source/postgresql.py index 263bf2e4..d576142f 100644 --- a/borgmatic/hooks/data_source/postgresql.py +++ b/borgmatic/hooks/data_source/postgresql.py @@ -151,7 +151,10 @@ def dump_data_sources( for part in shlex.split(database.get('pg_dump_command') or default_dump_command) ) dump_filename = dump.make_data_source_dump_filename( - dump_path, database_name, database.get('hostname') + dump_path, + database_name, + database.get('hostname'), + database.get('port'), ) if os.path.exists(dump_filename): logger.warning( 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" }, ] From 36bcbd05927f2156caa2fbe579792bdd976e696f Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 17 Dec 2024 08:51:04 -0800 Subject: [PATCH 02/14] Documentation about restoring datebases with the same name (#418). --- docs/how-to/backup-your-databases.md | 137 +++++++++++++++++---------- 1 file changed, 85 insertions(+), 52 deletions(-) diff --git a/docs/how-to/backup-your-databases.md b/docs/how-to/backup-your-databases.md index 76b8e4f7..f908a10a 100644 --- a/docs/how-to/backup-your-databases.md +++ b/docs/how-to/backup-your-databases.md @@ -391,7 +391,7 @@ with the repository's path or its label as configured in your borgmatic configuration file. ```bash -borgmatic restore --repository repo.borg --archive host-2023-... +borgmatic restore --repository repo.borg --archive latest ``` ### Restore particular databases @@ -401,7 +401,7 @@ restore one of them, use the `--database` flag to select one or more databases. For instance: ```bash -borgmatic restore --archive host-2023-... --database users --database orders +borgmatic restore --archive latest --database users --database orders ``` New in version 1.7.6 You can @@ -409,29 +409,63 @@ also restore individual databases even if you dumped them as "all"—as long as you dumped them into separate files via use of the "format" option. See above for more information. +### Restore databases sharing a name + +New in version 1.9.5 If you've +backed up multiple databases that happen to share the same name (e.g. from +different hostnames or ports), you can include additional flags to disambiguate +which database you'd like to restore. For instance: + +```bash +borgmatic restore --archive latest --database users --original-hostname myhost.org +``` + +This selects a `users` database to restore, but only if it originally came from +the host `myhost.org`—and doesn't restore `users` databases from any other +hosts. + +Here's another example: + +```bash +borgmatic restore --archive latest --database users --original-port 5433 +``` + +That restores a `users` database only if it originally came from port 5433. + +Finally: + +```bash +borgmatic restore --archive latest --database users --hook postgresql +``` + +That restores a `users` database only if it was dumped using the +`postgresql_databases:` database hook—and doesn't restore `users` databases that +were dumped using other database hooks. + +Note that these flags don't change the hostname or port where the database is +restored. For that, see below about restoring to an alternate host. + ### Restore all databases To restore all databases: ```bash -borgmatic restore --archive host-2023-... --database all +borgmatic restore --archive latest --database all ``` Or omit the `--database` flag entirely: ```bash -borgmatic restore --archive host-2023-... +borgmatic restore --archive latest ``` -Prior to borgmatic version 1.7.6, this restores a combined "all" database -dump from the archive. - New in version 1.7.6 Restoring "all" databases restores each database found in the selected archive. That includes any combined dump file named "all" and any other individual database -dumps found in the archive. +dumps found in the archive. Prior to borgmatic version 1.7.6, restoring "all" +only restored a combined "all" database dump from the archive. ### Restore particular schemas @@ -466,50 +500,6 @@ postgresql_databases: restore_password: trustsome1 ``` - -### Limitations - -There are a few important limitations with borgmatic's current database -restoration feature that you should know about: - -1. borgmatic does not currently support backing up or restoring multiple -databases that share the exact same name on different hosts or with different -ports. -2. When database hooks are enabled, borgmatic instructs Borg to consume -special files (via `--read-special`) to support database dump -streaming—regardless of the value of your `read_special` configuration option. -And because this can cause Borg to hang, borgmatic also automatically excludes -special files (and symlinks to them) that Borg may get stuck on. Even so, -there are still potential edge cases in which applications on your system -create new special files *after* borgmatic constructs its exclude list, -resulting in Borg hangs. If that occurs, you can resort to manually excluding -those files. And if you explicitly set the `read_special` option to `true`, -borgmatic will opt you out of the auto-exclude feature entirely, but will -still instruct Borg to consume special files—and you will be on your own to -exclude them. Prior to version -1.7.3Special files were not auto-excluded, and you were responsible for -excluding them yourself. Common directories to exclude are `/dev` and `/run`, -but that may not be exhaustive. -3. Prior to version 1.9.0 -Database hooks also implicitly enabled the `one_file_system` option, which -meant Borg wouldn't cross filesystem boundaries when looking for files to -backup. When borgmatic was running in a container, this often required a -work-around to explicitly add each mounted backup volume to -`source_directories` instead of relying on Borg to include them implicitly via -a parent directory. But as of borgmatic 1.9.0, `one_file_system` is no longer -auto-enabled and such work-arounds aren't necessary. -4. Prior to version 1.9.0 You -must restore as the same Unix user that created the archive containing the -database dump. That's because the user's home directory path is encoded into -the path of the database dump within the archive. -5. Prior to version 1.7.15 As -mentioned above, borgmatic can only restore a database that's defined in -borgmatic's own configuration file. So include your configuration files in -backups to avoid getting caught without a way to restore a database. But -starting from version 1.7.15, borgmatic includes your configuration files -automatically. - - ### Manual restoration If you prefer to restore a database without the help of borgmatic, first @@ -533,6 +523,49 @@ Also see the documentation on [listing database dumps](https://torsion.org/borgmatic/docs/how-to/inspect-your-backups/#listing-database-dumps). +## Limitations + +There are a few important limitations with borgmatic's current database +hooks that you should know about: + +1. When database hooks are enabled, borgmatic instructs Borg to consume +special files (via `--read-special`) to support database dump +streaming—regardless of the value of your `read_special` configuration option. +And because this can cause Borg to hang, borgmatic also automatically excludes +special files (and symlinks to them) that Borg may get stuck on. Even so, +there are still potential edge cases in which applications on your system +create new special files *after* borgmatic constructs its exclude list, +resulting in Borg hangs. If that occurs, you can resort to manually excluding +those files. And if you explicitly set the `read_special` option to `true`, +borgmatic will opt you out of the auto-exclude feature entirely, but will +still instruct Borg to consume special files—and you will be on your own to +exclude them. Prior to version +1.7.3Special files were not auto-excluded, and you were responsible for +excluding them yourself. Common directories to exclude are `/dev` and `/run`, +but that may not be exhaustive. +2. Prior to version 1.9.5 +borgmatic did not support backing up or restoring multiple databases that +shared the exact same name on different hosts or with different ports. +3. Prior to version 1.9.0 +Database hooks also implicitly enabled the `one_file_system` option, which +meant Borg wouldn't cross filesystem boundaries when looking for files to +backup. When borgmatic was running in a container, this often required a +work-around to explicitly add each mounted backup volume to +`source_directories` instead of relying on Borg to include them implicitly via +a parent directory. But as of borgmatic 1.9.0, `one_file_system` is no longer +auto-enabled and such work-arounds aren't necessary. +4. Prior to version 1.9.0 You +must restore as the same Unix user that created the archive containing the +database dump. That's because the user's home directory path is encoded into +the path of the database dump within the archive. +5. Prior to version 1.7.15 As +mentioned above, borgmatic can only restore a database that's defined in +borgmatic's own configuration file. So include your configuration files in +backups to avoid getting caught without a way to restore a database. But +starting from version 1.7.15, borgmatic includes your configuration files +automatically. + + ## Preparation and cleanup hooks If this database integration is too limited for needs, borgmatic also supports From 8696cbfa223d8688758066db7b8e3779ab6aebb0 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 17 Dec 2024 12:02:31 -0800 Subject: [PATCH 03/14] Clarify some comments (#418). --- borgmatic/actions/restore.py | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index 667c26c9..a65db5f5 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -81,7 +81,7 @@ def get_configured_data_source(config, restore_dump): ''' Search in the given configuration dict for dumps corresponding to the given dump to restore. If there are multiple matches, error. If UNSPECIFIED is given as any field in the restore dump, - then that can match any valid value. + then that can match any value. Return the found data source as a tuple of (found hook name, data source configuration dict) or (None, None) if not found. @@ -175,7 +175,7 @@ def restore_single_dump( that data source from the archive. ''' dump_metadata = render_dump_metadata( - Dump(hook_name, data_source["name"], data_source.get("hostname"), data_source.get("port")) + Dump(hook_name, data_source['name'], data_source.get('hostname'), data_source.get('port')) ) logger.info( @@ -337,14 +337,21 @@ def get_dumps_to_restore(restore_arguments, dumps_from_archive): Raise ValueError if any of the requested data source names cannot be found in the archive. ''' - # A map from data source hook name to the dumps to restore for that hook. dumps_to_restore = ( { Dump( - hook_name=(restore_arguments.hook if restore_arguments.hook.endswith('_databases') else f'{restore_arguments.hook}_databases') if restore_arguments.hook else UNSPECIFIED, + hook_name=( + ( + restore_arguments.hook + if restore_arguments.hook.endswith('_databases') + else f'{restore_arguments.hook}_databases' + ) + if restore_arguments.hook + else UNSPECIFIED + ), data_source_name=name, hostname=restore_arguments.original_hostname or 'localhost', - port=restore_arguments.original_port + port=restore_arguments.original_port, ) for name in restore_arguments.data_sources } @@ -378,7 +385,9 @@ def get_dumps_to_restore(restore_arguments, dumps_from_archive): } if missing_dumps: - rendered_dumps = ', '.join(f'{render_dump_metadata(dump)}' for dump in sorted(missing_dumps)) + rendered_dumps = ', '.join( + f'{render_dump_metadata(dump)}' for dump in sorted(missing_dumps) + ) raise ValueError( f"Cannot restore data source{'s' if len(missing_dumps) > 1 else ''} {rendered_dumps} missing from archive" @@ -391,7 +400,7 @@ def ensure_requested_dumps_restored(dumps_to_restore, dumps_actually_restored): ''' Given a set of requested dumps to restore and a set of dumps actually restored, raise ValueError if any requested dumps to restore weren't restored, indicating that they were missing from the - archive and/or configuration. + configuration. ''' if not dumps_actually_restored: raise ValueError('No data source dumps were found to restore') From d53ea09adb41c9ec5feb46a7ed5365391d82ca5c Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 17 Dec 2024 16:28:59 -0800 Subject: [PATCH 04/14] In tests, account for some function renames (#418). --- tests/unit/actions/test_restore.py | 110 ++++++++++++++--------------- 1 file changed, 55 insertions(+), 55 deletions(-) diff --git a/tests/unit/actions/test_restore.py b/tests/unit/actions/test_restore.py index 9753acf0..27322ff4 100644 --- a/tests/unit/actions/test_restore.py +++ b/tests/unit/actions/test_restore.py @@ -85,7 +85,7 @@ def test_strip_path_prefix_from_extracted_dump_destination_renames_first_matchin module.strip_path_prefix_from_extracted_dump_destination('/foo', '/run/user/0/borgmatic') -def test_restore_single_data_source_extracts_and_restores_single_file_dump(): +def test_restore_single_dump_extracts_and_restores_single_file_dump(): flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hooks').with_args( 'make_data_source_dump_patterns', object, object, object, object, object ).and_return({'postgresql': flexmock()}) @@ -110,7 +110,7 @@ def test_restore_single_data_source_extracts_and_restores_single_file_dump(): borgmatic_runtime_directory=object, ).once() - module.restore_single_data_source( + module.restore_single_dump( repository={'path': 'test.borg'}, config=flexmock(), local_borg_version=flexmock(), @@ -125,7 +125,7 @@ def test_restore_single_data_source_extracts_and_restores_single_file_dump(): ) -def test_restore_single_data_source_extracts_and_restores_directory_dump(): +def test_restore_single_dump_extracts_and_restores_directory_dump(): flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hooks').with_args( 'make_data_source_dump_patterns', object, object, object, object, object ).and_return({'postgresql': flexmock()}) @@ -152,7 +152,7 @@ def test_restore_single_data_source_extracts_and_restores_directory_dump(): borgmatic_runtime_directory='/run/borgmatic', ).once() - module.restore_single_data_source( + module.restore_single_dump( repository={'path': 'test.borg'}, config=flexmock(), local_borg_version=flexmock(), @@ -167,7 +167,7 @@ def test_restore_single_data_source_extracts_and_restores_directory_dump(): ) -def test_restore_single_data_source_with_directory_dump_error_cleans_up_temporary_directory(): +def test_restore_single_dump_with_directory_dump_error_cleans_up_temporary_directory(): flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hooks').with_args( 'make_data_source_dump_patterns', object, object, object, object, object ).and_return({'postgresql': flexmock()}) @@ -195,7 +195,7 @@ def test_restore_single_data_source_with_directory_dump_error_cleans_up_temporar ).never() with pytest.raises(ValueError): - module.restore_single_data_source( + module.restore_single_dump( repository={'path': 'test.borg'}, config=flexmock(), local_borg_version=flexmock(), @@ -210,7 +210,7 @@ def test_restore_single_data_source_with_directory_dump_error_cleans_up_temporar ) -def test_restore_single_data_source_with_directory_dump_and_dry_run_skips_directory_move_and_cleanup(): +def test_restore_single_dump_with_directory_dump_and_dry_run_skips_directory_move_and_cleanup(): flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hooks').with_args( 'make_data_source_dump_patterns', object, object, object, object, object ).and_return({'postgresql': flexmock()}) @@ -235,7 +235,7 @@ def test_restore_single_data_source_with_directory_dump_and_dry_run_skips_direct borgmatic_runtime_directory='/run/borgmatic', ).once() - module.restore_single_data_source( + module.restore_single_dump( repository={'path': 'test.borg'}, config=flexmock(), local_borg_version=flexmock(), @@ -250,7 +250,7 @@ def test_restore_single_data_source_with_directory_dump_and_dry_run_skips_direct ) -def test_collect_archive_data_source_names_parses_archive_paths(): +def test_collect_dumps_from_archive_parses_archive_paths(): flexmock(module.borgmatic.config.paths).should_receive( 'get_borgmatic_source_directory' ).and_return('/root/.borgmatic') @@ -265,7 +265,7 @@ def test_collect_archive_data_source_names_parses_archive_paths(): ] ) - archive_data_source_names = module.collect_archive_data_source_names( + archive_data_source_names = module.collect_dumps_from_archive( repository={'path': 'repo'}, archive='archive', config={}, @@ -282,7 +282,7 @@ def test_collect_archive_data_source_names_parses_archive_paths(): } -def test_collect_archive_data_source_names_parses_archive_paths_with_different_base_directories(): +def test_collect_dumps_from_archive_parses_archive_paths_with_different_base_directories(): flexmock(module.borgmatic.config.paths).should_receive( 'get_borgmatic_source_directory' ).and_return('/root/.borgmatic') @@ -298,7 +298,7 @@ def test_collect_archive_data_source_names_parses_archive_paths_with_different_b ] ) - archive_data_source_names = module.collect_archive_data_source_names( + archive_data_source_names = module.collect_dumps_from_archive( repository={'path': 'repo'}, archive='archive', config={}, @@ -315,7 +315,7 @@ def test_collect_archive_data_source_names_parses_archive_paths_with_different_b } -def test_collect_archive_data_source_names_parses_directory_format_archive_paths(): +def test_collect_dumps_from_archive_parses_directory_format_archive_paths(): flexmock(module.borgmatic.config.paths).should_receive( 'get_borgmatic_source_directory' ).and_return('/root/.borgmatic') @@ -329,7 +329,7 @@ def test_collect_archive_data_source_names_parses_directory_format_archive_paths ] ) - archive_data_source_names = module.collect_archive_data_source_names( + archive_data_source_names = module.collect_dumps_from_archive( repository={'path': 'repo'}, archive='archive', config={}, @@ -345,7 +345,7 @@ def test_collect_archive_data_source_names_parses_directory_format_archive_paths } -def test_collect_archive_data_source_names_skips_bad_archive_paths(): +def test_collect_dumps_from_archive_skips_bad_archive_paths(): flexmock(module.borgmatic.config.paths).should_receive( 'get_borgmatic_source_directory' ).and_return('/root/.borgmatic') @@ -361,7 +361,7 @@ def test_collect_archive_data_source_names_skips_bad_archive_paths(): ] ) - archive_data_source_names = module.collect_archive_data_source_names( + archive_data_source_names = module.collect_dumps_from_archive( repository={'path': 'repo'}, archive='archive', config={}, @@ -377,8 +377,8 @@ def test_collect_archive_data_source_names_skips_bad_archive_paths(): } -def test_find_data_sources_to_restore_passes_through_requested_names_found_in_archive(): - restore_names = module.find_data_sources_to_restore( +def test_get_dumps_to_restore_passes_through_requested_names_found_in_archive(): + restore_names = module.get_dumps_to_restore( requested_data_source_names=['foo', 'bar'], archive_data_source_names={'postresql_databases': ['foo', 'bar', 'baz']}, ) @@ -386,18 +386,18 @@ def test_find_data_sources_to_restore_passes_through_requested_names_found_in_ar assert restore_names == {module.UNSPECIFIED_HOOK: ['foo', 'bar']} -def test_find_data_sources_to_restore_raises_for_requested_names_missing_from_archive(): +def test_get_dumps_to_restore_raises_for_requested_names_missing_from_archive(): with pytest.raises(ValueError): - module.find_data_sources_to_restore( + module.get_dumps_to_restore( requested_data_source_names=['foo', 'bar'], archive_data_source_names={'postresql_databases': ['foo']}, ) -def test_find_data_sources_to_restore_without_requested_names_finds_all_archive_data_sources(): +def test_get_dumps_to_restore_without_requested_names_finds_all_archive_data_sources(): archive_data_source_names = {'postresql_databases': ['foo', 'bar']} - restore_names = module.find_data_sources_to_restore( + restore_names = module.get_dumps_to_restore( requested_data_source_names=[], archive_data_source_names=archive_data_source_names, ) @@ -405,10 +405,10 @@ def test_find_data_sources_to_restore_without_requested_names_finds_all_archive_ assert restore_names == archive_data_source_names -def test_find_data_sources_to_restore_with_all_in_requested_names_finds_all_archive_data_sources(): +def test_get_dumps_to_restore_with_all_in_requested_names_finds_all_archive_data_sources(): archive_data_source_names = {'postresql_databases': ['foo', 'bar']} - restore_names = module.find_data_sources_to_restore( + restore_names = module.get_dumps_to_restore( requested_data_source_names=['all'], archive_data_source_names=archive_data_source_names, ) @@ -416,10 +416,10 @@ def test_find_data_sources_to_restore_with_all_in_requested_names_finds_all_arch assert restore_names == archive_data_source_names -def test_find_data_sources_to_restore_with_all_in_requested_names_plus_additional_requested_names_omits_duplicates(): +def test_get_dumps_to_restore_with_all_in_requested_names_plus_additional_requested_names_omits_duplicates(): archive_data_source_names = {'postresql_databases': ['foo', 'bar']} - restore_names = module.find_data_sources_to_restore( + restore_names = module.get_dumps_to_restore( requested_data_source_names=['all', 'foo', 'bar'], archive_data_source_names=archive_data_source_names, ) @@ -427,34 +427,34 @@ def test_find_data_sources_to_restore_with_all_in_requested_names_plus_additiona assert restore_names == archive_data_source_names -def test_find_data_sources_to_restore_raises_for_all_in_requested_names_and_requested_named_missing_from_archives(): +def test_get_dumps_to_restore_raises_for_all_in_requested_names_and_requested_named_missing_from_archives(): with pytest.raises(ValueError): - module.find_data_sources_to_restore( + module.get_dumps_to_restore( requested_data_source_names=['all', 'foo', 'bar'], archive_data_source_names={'postresql_databases': ['foo']}, ) -def test_ensure_data_sources_found_with_all_data_sources_found_does_not_raise(): - module.ensure_data_sources_found( +def test_ensure_requested_dumps_restored_with_all_data_sources_found_does_not_raise(): + module.ensure_requested_dumps_restored( restore_names={'postgresql_databases': ['foo']}, remaining_restore_names={'postgresql_databases': ['bar']}, found_names=['foo', 'bar'], ) -def test_ensure_data_sources_found_with_no_data_sources_raises(): +def test_ensure_requested_dumps_restored_with_no_data_sources_raises(): with pytest.raises(ValueError): - module.ensure_data_sources_found( + module.ensure_requested_dumps_restored( restore_names={'postgresql_databases': []}, remaining_restore_names={}, found_names=[], ) -def test_ensure_data_sources_found_with_missing_data_sources_raises(): +def test_ensure_requested_dumps_restored_with_missing_data_sources_raises(): with pytest.raises(ValueError): - module.ensure_data_sources_found( + module.ensure_requested_dumps_restored( restore_names={'postgresql_databases': ['foo']}, remaining_restore_names={'postgresql_databases': ['bar']}, found_names=['foo'], @@ -478,12 +478,12 @@ def test_run_restore_restores_each_data_source(): flexmock(module.borgmatic.borg.repo_list).should_receive('resolve_archive_name').and_return( flexmock() ) - flexmock(module).should_receive('collect_archive_data_source_names').and_return(flexmock()) - flexmock(module).should_receive('find_data_sources_to_restore').and_return(restore_names) + flexmock(module).should_receive('collect_dumps_from_archive').and_return(flexmock()) + flexmock(module).should_receive('get_dumps_to_restore').and_return(restore_names) flexmock(module).should_receive('get_configured_data_source').and_return( ('postgresql_databases', {'name': 'foo'}) ).and_return(('postgresql_databases', {'name': 'bar'})) - flexmock(module).should_receive('restore_single_data_source').with_args( + flexmock(module).should_receive('restore_single_dump').with_args( repository=object, config=object, local_borg_version=object, @@ -496,7 +496,7 @@ def test_run_restore_restores_each_data_source(): connection_params=object, borgmatic_runtime_directory=borgmatic_runtime_directory, ).once() - flexmock(module).should_receive('restore_single_data_source').with_args( + flexmock(module).should_receive('restore_single_dump').with_args( repository=object, config=object, local_borg_version=object, @@ -509,7 +509,7 @@ def test_run_restore_restores_each_data_source(): connection_params=object, borgmatic_runtime_directory=borgmatic_runtime_directory, ).once() - flexmock(module).should_receive('ensure_data_sources_found') + flexmock(module).should_receive('ensure_requested_dumps_restored') module.run_restore( repository={'path': 'repo'}, @@ -545,7 +545,7 @@ def test_run_restore_bails_for_non_matching_repository(): flexmock(module.borgmatic.hooks.dispatch).should_receive( 'call_hooks_even_if_unconfigured' ).never() - flexmock(module).should_receive('restore_single_data_source').never() + flexmock(module).should_receive('restore_single_dump').never() module.run_restore( repository={'path': 'repo'}, @@ -575,8 +575,8 @@ def test_run_restore_restores_data_source_configured_with_all_name(): flexmock(module.borgmatic.borg.repo_list).should_receive('resolve_archive_name').and_return( flexmock() ) - flexmock(module).should_receive('collect_archive_data_source_names').and_return(flexmock()) - flexmock(module).should_receive('find_data_sources_to_restore').and_return(restore_names) + flexmock(module).should_receive('collect_dumps_from_archive').and_return(flexmock()) + flexmock(module).should_receive('get_dumps_to_restore').and_return(restore_names) flexmock(module).should_receive('get_configured_data_source').with_args( config=object, archive_data_source_names=object, @@ -596,7 +596,7 @@ def test_run_restore_restores_data_source_configured_with_all_name(): data_source_name='bar', configuration_data_source_name='all', ).and_return(('postgresql_databases', {'name': 'bar'})) - flexmock(module).should_receive('restore_single_data_source').with_args( + flexmock(module).should_receive('restore_single_dump').with_args( repository=object, config=object, local_borg_version=object, @@ -609,7 +609,7 @@ def test_run_restore_restores_data_source_configured_with_all_name(): connection_params=object, borgmatic_runtime_directory=borgmatic_runtime_directory, ).once() - flexmock(module).should_receive('restore_single_data_source').with_args( + flexmock(module).should_receive('restore_single_dump').with_args( repository=object, config=object, local_borg_version=object, @@ -622,7 +622,7 @@ def test_run_restore_restores_data_source_configured_with_all_name(): connection_params=object, borgmatic_runtime_directory=borgmatic_runtime_directory, ).once() - flexmock(module).should_receive('ensure_data_sources_found') + flexmock(module).should_receive('ensure_requested_dumps_restored') module.run_restore( repository={'path': 'repo'}, @@ -662,8 +662,8 @@ def test_run_restore_skips_missing_data_source(): flexmock(module.borgmatic.borg.repo_list).should_receive('resolve_archive_name').and_return( flexmock() ) - flexmock(module).should_receive('collect_archive_data_source_names').and_return(flexmock()) - flexmock(module).should_receive('find_data_sources_to_restore').and_return(restore_names) + flexmock(module).should_receive('collect_dumps_from_archive').and_return(flexmock()) + flexmock(module).should_receive('get_dumps_to_restore').and_return(restore_names) flexmock(module).should_receive('get_configured_data_source').with_args( config=object, archive_data_source_names=object, @@ -683,7 +683,7 @@ def test_run_restore_skips_missing_data_source(): data_source_name='bar', configuration_data_source_name='all', ).and_return((None, None)) - flexmock(module).should_receive('restore_single_data_source').with_args( + flexmock(module).should_receive('restore_single_dump').with_args( repository=object, config=object, local_borg_version=object, @@ -696,7 +696,7 @@ def test_run_restore_skips_missing_data_source(): connection_params=object, borgmatic_runtime_directory=borgmatic_runtime_directory, ).once() - flexmock(module).should_receive('restore_single_data_source').with_args( + flexmock(module).should_receive('restore_single_dump').with_args( repository=object, config=object, local_borg_version=object, @@ -709,7 +709,7 @@ def test_run_restore_skips_missing_data_source(): connection_params=object, borgmatic_runtime_directory=borgmatic_runtime_directory, ).never() - flexmock(module).should_receive('ensure_data_sources_found') + flexmock(module).should_receive('ensure_requested_dumps_restored') module.run_restore( repository={'path': 'repo'}, @@ -750,8 +750,8 @@ def test_run_restore_restores_data_sources_from_different_hooks(): flexmock(module.borgmatic.borg.repo_list).should_receive('resolve_archive_name').and_return( flexmock() ) - flexmock(module).should_receive('collect_archive_data_source_names').and_return(flexmock()) - flexmock(module).should_receive('find_data_sources_to_restore').and_return(restore_names) + flexmock(module).should_receive('collect_dumps_from_archive').and_return(flexmock()) + flexmock(module).should_receive('get_dumps_to_restore').and_return(restore_names) flexmock(module).should_receive('get_configured_data_source').with_args( config=object, archive_data_source_names=object, @@ -764,7 +764,7 @@ def test_run_restore_restores_data_sources_from_different_hooks(): hook_name='mysql_databases', data_source_name='bar', ).and_return(('mysql_databases', {'name': 'bar'})) - flexmock(module).should_receive('restore_single_data_source').with_args( + flexmock(module).should_receive('restore_single_dump').with_args( repository=object, config=object, local_borg_version=object, @@ -777,7 +777,7 @@ def test_run_restore_restores_data_sources_from_different_hooks(): connection_params=object, borgmatic_runtime_directory=borgmatic_runtime_directory, ).once() - flexmock(module).should_receive('restore_single_data_source').with_args( + flexmock(module).should_receive('restore_single_dump').with_args( repository=object, config=object, local_borg_version=object, @@ -790,7 +790,7 @@ def test_run_restore_restores_data_sources_from_different_hooks(): connection_params=object, borgmatic_runtime_directory=borgmatic_runtime_directory, ).once() - flexmock(module).should_receive('ensure_data_sources_found') + flexmock(module).should_receive('ensure_requested_dumps_restored') module.run_restore( repository={'path': 'repo'}, From 3db79b435265a623e57859c4ed6bc9f9fc9243c4 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Fri, 20 Dec 2024 22:40:20 -0800 Subject: [PATCH 05/14] Simplified dump metadata comparison logic and got a few tests passing (#418). --- NEWS | 2 +- borgmatic/actions/restore.py | 102 +++++++++++------------ tests/unit/actions/test_restore.py | 126 +++++++++++++++-------------- 3 files changed, 113 insertions(+), 117 deletions(-) diff --git a/NEWS b/NEWS index d3cd1602..53d2272f 100644 --- a/NEWS +++ b/NEWS @@ -3,7 +3,7 @@ or hooks. * #947: To avoid a hang in the database hooks, error and exit when the borgmatic runtime directory overlaps with the configured excludes. - * #954: Fix findmnt command error in the Btrfs hook by switching to parsing JSON output. + * #954: Fix a findmnt command error in the Btrfs hook by switching to parsing JSON output. * When the ZFS, Btrfs, or LVM hooks aren't configured, don't try to cleanup snapshots for them. 1.9.4 diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index a65db5f5..d11f90ad 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -21,41 +21,29 @@ logger = logging.getLogger(__name__) UNSPECIFIED = object() -class Dump( - collections.namedtuple( - 'Dump', - ('hook_name', 'data_source_name', 'hostname', 'port'), - defaults=('localhost', None), - ) -): - def __eq__(self, other): - ''' - Compare two namedtuples for equality while supporting a field value of UNSPECIFIED, which - indicates that the field should match any value. - ''' - for field_name in self._fields: - self_value = getattr(self, field_name) - other_value = getattr(other, field_name) +Dump = collections.namedtuple( + 'Dump', + ('hook_name', 'data_source_name', 'hostname', 'port'), + defaults=('localhost', None), +) - if self_value == UNSPECIFIED or other_value == UNSPECIFIED: - continue - if self_value != other_value: - return False +def dumps_match(first, second): + ''' + Compare two Dump instances for equality while supporting a field value of UNSPECIFIED, which + indicates that the field should match any value. + ''' + for field_name in first._fields: + first_value = getattr(first, field_name) + second_value = getattr(second, field_name) - return True + if first_value == UNSPECIFIED or second_value == UNSPECIFIED: + continue - def __ne__(self, other): - return not self == other + if first_value != second_value: + return False - def __lt__(self, other): - return self.data_source_name < other.data_source_name - - def __gt__(self, other): - return self.data_source_name > other.data_source_name - - def __hash__(self): - return hash(tuple(self)) + return True def render_dump_metadata(dump): @@ -103,13 +91,15 @@ def get_configured_data_source(config, restore_dump): (hook_name, hook_data_source) for (hook_name, hook) in hooks_to_search.items() for hook_data_source in hook - if Dump( - hook_name, - hook_data_source.get('name'), - hook_data_source.get('hostname'), - hook_data_source.get('port'), + if dumps_match( + Dump( + hook_name, + hook_data_source.get('name'), + hook_data_source.get('hostname'), + hook_data_source.get('port'), + ), + restore_dump, ) - == restore_dump ) if not matching_dumps: @@ -331,13 +321,13 @@ def collect_dumps_from_archive( def get_dumps_to_restore(restore_arguments, dumps_from_archive): ''' Given restore arguments as an argparse.Namespace instance indicating which dumps to restore and - a set of Dump instances representing the dumps found in an archive, return a set of Dump - instances to restore. As part of this, replace any Dump having a data source name of "all" with - multiple named Dump instances as appropriate. + a set of Dump instances representing the dumps found in an archive, return a set of specific + Dump instances from the archive to restore. As part of this, replace any Dump having a data + source name of "all" with multiple named Dump instances as appropriate. Raise ValueError if any of the requested data source names cannot be found in the archive. ''' - dumps_to_restore = ( + requested_dumps = ( { Dump( hook_name=( @@ -365,24 +355,24 @@ def get_dumps_to_restore(restore_arguments, dumps_from_archive): ) } ) + missing_dumps = set() + dumps_to_restore = set() - # If "all" is in dumps_to_restore, then replace it with named dumps found within the archive. - try: - all_dump = next(dump for dump in dumps_to_restore if dump.data_source_name == 'all') - except StopIteration: - pass - else: - dumps_to_restore.remove(all_dump) + # If there's a requested "all" dump, add every dump from the archive to the dumps to restore. + if any(dump for dump in requested_dumps if dump.data_source_name == 'all'): + dumps_to_restore.update(dumps_from_archive) - for dump in dumps_from_archive: - if all_dump.hook_name == UNSPECIFIED or dump.hook_name == all_dump.hook_name: - dumps_to_restore.add(dump) + # Put any archive dump matching a requested dump in the dumps to restore. + for requested_dump in requested_dumps: + if requested_dump.data_source_name == 'all': + continue - missing_dumps = { - restore_dump - for restore_dump in dumps_to_restore - if all(restore_dump != archive_dump for archive_dump in dumps_from_archive) - } + for archive_dump in dumps_from_archive: + if dumps_match(requested_dump, archive_dump): + dumps_to_restore.add(archive_dump) + break + else: + missing_dumps.add(requested_dump) if missing_dumps: rendered_dumps = ', '.join( @@ -390,7 +380,7 @@ def get_dumps_to_restore(restore_arguments, dumps_from_archive): ) raise ValueError( - f"Cannot restore data source{'s' if len(missing_dumps) > 1 else ''} {rendered_dumps} missing from archive" + f"Cannot restore data source dump{'s' if len(missing_dumps) > 1 else ''} {rendered_dumps} missing from archive" ) return dumps_to_restore diff --git a/tests/unit/actions/test_restore.py b/tests/unit/actions/test_restore.py index 27322ff4..17a9439a 100644 --- a/tests/unit/actions/test_restore.py +++ b/tests/unit/actions/test_restore.py @@ -1,3 +1,5 @@ +import collections + import pytest from flexmock import flexmock @@ -417,53 +419,73 @@ def test_get_dumps_to_restore_with_all_in_requested_names_finds_all_archive_data def test_get_dumps_to_restore_with_all_in_requested_names_plus_additional_requested_names_omits_duplicates(): - archive_data_source_names = {'postresql_databases': ['foo', 'bar']} + dumps_from_archive = { + module.Dump('postresql_databases', 'foo'), + module.Dump('postresql_databases', 'bar'), + } - restore_names = module.get_dumps_to_restore( - requested_data_source_names=['all', 'foo', 'bar'], - archive_data_source_names=archive_data_source_names, + assert ( + module.get_dumps_to_restore( + restore_arguments=flexmock( + hook=None, + data_sources=['all', 'foo', 'bar'], + original_hostname=None, + original_port=None, + ), + dumps_from_archive=dumps_from_archive, + ) + == dumps_from_archive ) - assert restore_names == archive_data_source_names - -def test_get_dumps_to_restore_raises_for_all_in_requested_names_and_requested_named_missing_from_archives(): +def test_get_dumps_to_restore_raises_for_all_in_requested_names_and_requested_names_missing_from_archives(): with pytest.raises(ValueError): module.get_dumps_to_restore( - requested_data_source_names=['all', 'foo', 'bar'], - archive_data_source_names={'postresql_databases': ['foo']}, + restore_arguments=flexmock( + hook=None, + data_sources=['all', 'foo', 'bar'], + original_hostname=None, + original_port=None, + ), + dumps_from_archive={module.Dump('postresql_databases', 'foo')}, ) -def test_ensure_requested_dumps_restored_with_all_data_sources_found_does_not_raise(): +def test_ensure_requested_dumps_restored_with_all_dumps_restored_does_not_raise(): module.ensure_requested_dumps_restored( - restore_names={'postgresql_databases': ['foo']}, - remaining_restore_names={'postgresql_databases': ['bar']}, - found_names=['foo', 'bar'], + dumps_to_restore={ + module.Dump(hook_name='postgresql_databases', data_source_name='foo'), + module.Dump(hook_name='postgresql_databases', data_source_name='bar'), + }, + dumps_actually_restored={ + module.Dump(hook_name='postgresql_databases', data_source_name='foo'), + module.Dump(hook_name='postgresql_databases', data_source_name='bar'), + }, ) -def test_ensure_requested_dumps_restored_with_no_data_sources_raises(): +def test_ensure_requested_dumps_restored_with_no_dumps_raises(): with pytest.raises(ValueError): module.ensure_requested_dumps_restored( - restore_names={'postgresql_databases': []}, - remaining_restore_names={}, - found_names=[], + dumps_to_restore={}, + dumps_actually_restored={}, ) -def test_ensure_requested_dumps_restored_with_missing_data_sources_raises(): +def test_ensure_requested_dumps_restored_with_missing_dumps_raises(): with pytest.raises(ValueError): module.ensure_requested_dumps_restored( - restore_names={'postgresql_databases': ['foo']}, - remaining_restore_names={'postgresql_databases': ['bar']}, - found_names=['foo'], + dumps_to_restore={module.Dump(hook_name='postgresql_databases', data_source_name='foo')}, + dumps_actually_restored={ + module.Dump(hook_name='postgresql_databases', data_source_name='bar') + }, ) def test_run_restore_restores_each_data_source(): - restore_names = { - 'postgresql_databases': ['foo', 'bar'], + dumps_to_restore = { + module.Dump(hook_name='postgresql_databases', data_source_name='foo'), + module.Dump(hook_name='postgresql_databases', data_source_name='bar'), } flexmock(module.borgmatic.config.validate).should_receive('repositories_match').and_return(True) @@ -479,7 +501,7 @@ def test_run_restore_restores_each_data_source(): flexmock() ) flexmock(module).should_receive('collect_dumps_from_archive').and_return(flexmock()) - flexmock(module).should_receive('get_dumps_to_restore').and_return(restore_names) + flexmock(module).should_receive('get_dumps_to_restore').and_return(dumps_to_restore) flexmock(module).should_receive('get_configured_data_source').and_return( ('postgresql_databases', {'name': 'foo'}) ).and_return(('postgresql_databases', {'name': 'bar'})) @@ -559,8 +581,9 @@ def test_run_restore_bails_for_non_matching_repository(): def test_run_restore_restores_data_source_configured_with_all_name(): - restore_names = { - 'postgresql_databases': ['foo', 'bar'], + dumps_to_restore = { + module.Dump(hook_name='postgresql_databases', data_source_name='foo'), + module.Dump(hook_name='postgresql_databases', data_source_name='bar'), } flexmock(module.borgmatic.config.validate).should_receive('repositories_match').and_return(True) @@ -576,25 +599,18 @@ def test_run_restore_restores_data_source_configured_with_all_name(): flexmock() ) flexmock(module).should_receive('collect_dumps_from_archive').and_return(flexmock()) - flexmock(module).should_receive('get_dumps_to_restore').and_return(restore_names) + flexmock(module).should_receive('get_dumps_to_restore').and_return(dumps_to_restore) flexmock(module).should_receive('get_configured_data_source').with_args( config=object, - archive_data_source_names=object, - hook_name='postgresql_databases', - data_source_name='foo', + restore_dump=module.Dump(hook_name='postgresql_databases', data_source_name='foo'), ).and_return(('postgresql_databases', {'name': 'foo'})) flexmock(module).should_receive('get_configured_data_source').with_args( config=object, - archive_data_source_names=object, - hook_name='postgresql_databases', - data_source_name='bar', + restore_dump=module.Dump(hook_name='postgresql_databases', data_source_name='bar'), ).and_return((None, None)) flexmock(module).should_receive('get_configured_data_source').with_args( config=object, - archive_data_source_names=object, - hook_name='postgresql_databases', - data_source_name='bar', - configuration_data_source_name='all', + restore_dump=module.Dump(hook_name='postgresql_databases', data_source_name='all'), ).and_return(('postgresql_databases', {'name': 'bar'})) flexmock(module).should_receive('restore_single_dump').with_args( repository=object, @@ -646,8 +662,9 @@ def test_run_restore_restores_data_source_configured_with_all_name(): def test_run_restore_skips_missing_data_source(): - restore_names = { - 'postgresql_databases': ['foo', 'bar'], + dumps_to_restore = { + module.Dump(hook_name='postgresql_databases', data_source_name='foo'), + module.Dump(hook_name='postgresql_databases', data_source_name='bar'), } flexmock(module.borgmatic.config.validate).should_receive('repositories_match').and_return(True) @@ -663,25 +680,18 @@ def test_run_restore_skips_missing_data_source(): flexmock() ) flexmock(module).should_receive('collect_dumps_from_archive').and_return(flexmock()) - flexmock(module).should_receive('get_dumps_to_restore').and_return(restore_names) + flexmock(module).should_receive('get_dumps_to_restore').and_return(dumps_to_restore) flexmock(module).should_receive('get_configured_data_source').with_args( config=object, - archive_data_source_names=object, - hook_name='postgresql_databases', - data_source_name='foo', + restore_dump=module.Dump(hook_name='postgresql_databases', data_source_name='foo'), ).and_return(('postgresql_databases', {'name': 'foo'})) flexmock(module).should_receive('get_configured_data_source').with_args( config=object, - archive_data_source_names=object, - hook_name='postgresql_databases', - data_source_name='bar', + restore_dump=module.Dump(hook_name='postgresql_databases', data_source_name='bar'), ).and_return((None, None)) flexmock(module).should_receive('get_configured_data_source').with_args( config=object, - archive_data_source_names=object, - hook_name='postgresql_databases', - data_source_name='bar', - configuration_data_source_name='all', + restore_dump=module.Dump(hook_name='postgresql_databases', data_source_name='all'), ).and_return((None, None)) flexmock(module).should_receive('restore_single_dump').with_args( repository=object, @@ -733,9 +743,9 @@ def test_run_restore_skips_missing_data_source(): def test_run_restore_restores_data_sources_from_different_hooks(): - restore_names = { - 'postgresql_databases': ['foo'], - 'mysql_databases': ['bar'], + dumps_to_restore = { + module.Dump(hook_name='postgresql_databases', data_source_name='foo'), + module.Dump(hook_name='mysql_databases', data_source_name='foo'), } flexmock(module.borgmatic.config.validate).should_receive('repositories_match').and_return(True) @@ -751,18 +761,14 @@ def test_run_restore_restores_data_sources_from_different_hooks(): flexmock() ) flexmock(module).should_receive('collect_dumps_from_archive').and_return(flexmock()) - flexmock(module).should_receive('get_dumps_to_restore').and_return(restore_names) + flexmock(module).should_receive('get_dumps_to_restore').and_return(dumps_to_restore) flexmock(module).should_receive('get_configured_data_source').with_args( config=object, - archive_data_source_names=object, - hook_name='postgresql_databases', - data_source_name='foo', + restore_dump=module.Dump(hook_name='postgresql_databases', data_source_name='foo'), ).and_return(('postgresql_databases', {'name': 'foo'})) flexmock(module).should_receive('get_configured_data_source').with_args( config=object, - archive_data_source_names=object, - hook_name='mysql_databases', - data_source_name='bar', + restore_dump=module.Dump(hook_name='mysql_databases', data_source_name='foo'), ).and_return(('mysql_databases', {'name': 'bar'})) flexmock(module).should_receive('restore_single_dump').with_args( repository=object, From 5174a781099a4ca817f84a817181fecb80ba9315 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sat, 21 Dec 2024 13:35:00 -0800 Subject: [PATCH 06/14] Get existing tests passing (#418). --- borgmatic/actions/restore.py | 4 +- tests/unit/actions/test_restore.py | 224 ++++++++++++++++++----------- 2 files changed, 145 insertions(+), 83 deletions(-) diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index d11f90ad..83f22068 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -95,7 +95,7 @@ def get_configured_data_source(config, restore_dump): Dump( hook_name, hook_data_source.get('name'), - hook_data_source.get('hostname'), + hook_data_source.get('hostname', 'localhost'), hook_data_source.get('port'), ), restore_dump, @@ -107,7 +107,7 @@ def get_configured_data_source(config, restore_dump): if len(matching_dumps) > 1: raise ValueError( - f'Cannot restore data source {render_dump_metadata(restore_dump)} because there are multiple matching dumps in the archive. Try adding additional flags to disambiguate.' + f'Cannot restore data source {render_dump_metadata(restore_dump)} because there are multiple matching configured data sources. Try adding additional flags to disambiguate.' ) return matching_dumps[0] diff --git a/tests/unit/actions/test_restore.py b/tests/unit/actions/test_restore.py index 17a9439a..59d64421 100644 --- a/tests/unit/actions/test_restore.py +++ b/tests/unit/actions/test_restore.py @@ -7,63 +7,54 @@ import borgmatic.actions.restore as module def test_get_configured_data_source_matches_data_source_by_name(): + flexmock(module).should_receive('dumps_match').and_return(False) + flexmock(module).should_receive('dumps_match').with_args( + module.Dump('postgresql_databases', 'bar'), + module.Dump('postgresql_databases', 'bar'), + ).and_return(True) + assert module.get_configured_data_source( config={ 'other_databases': [{'name': 'other'}], 'postgresql_databases': [{'name': 'foo'}, {'name': 'bar'}], }, - archive_data_source_names={'postgresql_databases': ['other', 'foo', 'bar']}, - hook_name='postgresql_databases', - data_source_name='bar', + restore_dump=module.Dump('postgresql_databases', 'bar'), ) == ('postgresql_databases', {'name': 'bar'}) def test_get_configured_data_source_matches_nothing_when_nothing_configured(): + flexmock(module).should_receive('dumps_match').and_return(False) + assert module.get_configured_data_source( config={}, - archive_data_source_names={'postgresql_databases': ['foo']}, - hook_name='postgresql_databases', - data_source_name='quux', + restore_dump=module.Dump('postgresql_databases', 'quux'), ) == (None, None) def test_get_configured_data_source_matches_nothing_when_data_source_name_not_configured(): + flexmock(module).should_receive('dumps_match').and_return(False) + assert module.get_configured_data_source( - config={'postgresql_databases': [{'name': 'foo'}, {'name': 'bar'}]}, - archive_data_source_names={'postgresql_databases': ['foo']}, - hook_name='postgresql_databases', - data_source_name='quux', + config={ + 'postgresql_databases': [{'name': 'foo'}], + }, + restore_dump=module.Dump('postgresql_databases', 'quux'), ) == (None, None) -def test_get_configured_data_source_matches_nothing_when_data_source_name_not_in_archive(): - assert module.get_configured_data_source( - config={'postgresql_databases': [{'name': 'foo'}, {'name': 'bar'}]}, - archive_data_source_names={'postgresql_databases': ['bar']}, - hook_name='postgresql_databases', - data_source_name='foo', - ) == (None, None) - - -def test_get_configured_data_source_matches_data_source_by_configuration_data_source_name(): - assert module.get_configured_data_source( - config={'postgresql_databases': [{'name': 'all'}, {'name': 'bar'}]}, - archive_data_source_names={'postgresql_databases': ['foo']}, - hook_name='postgresql_databases', - data_source_name='foo', - configuration_data_source_name='all', - ) == ('postgresql_databases', {'name': 'all'}) - - def test_get_configured_data_source_with_unspecified_hook_matches_data_source_by_name(): + flexmock(module).should_receive('dumps_match').and_return(False) + flexmock(module).should_receive('dumps_match').with_args( + module.Dump('postgresql_databases', 'bar'), + module.Dump(module.UNSPECIFIED, 'bar'), + ).and_return(True) + assert module.get_configured_data_source( config={ 'other_databases': [{'name': 'other'}], 'postgresql_databases': [{'name': 'foo'}, {'name': 'bar'}], }, - archive_data_source_names={'postgresql_databases': ['other', 'foo', 'bar']}, - hook_name=module.UNSPECIFIED_HOOK, - data_source_name='bar', + restore_dump=module.Dump(module.UNSPECIFIED, 'bar'), ) == ('postgresql_databases', {'name': 'bar'}) @@ -267,7 +258,7 @@ def test_collect_dumps_from_archive_parses_archive_paths(): ] ) - archive_data_source_names = module.collect_dumps_from_archive( + archive_dumps = module.collect_dumps_from_archive( repository={'path': 'repo'}, archive='archive', config={}, @@ -278,9 +269,10 @@ def test_collect_dumps_from_archive_parses_archive_paths(): borgmatic_runtime_directory='/run/borgmatic', ) - assert archive_data_source_names == { - 'postgresql_databases': ['foo', 'bar'], - 'mysql_databases': ['quux'], + assert archive_dumps == { + module.Dump('postgresql_databases', 'foo'), + module.Dump('postgresql_databases', 'bar'), + module.Dump('mysql_databases', 'quux'), } @@ -300,7 +292,7 @@ def test_collect_dumps_from_archive_parses_archive_paths_with_different_base_dir ] ) - archive_data_source_names = module.collect_dumps_from_archive( + archive_dumps = module.collect_dumps_from_archive( repository={'path': 'repo'}, archive='archive', config={}, @@ -311,9 +303,11 @@ def test_collect_dumps_from_archive_parses_archive_paths_with_different_base_dir borgmatic_runtime_directory='/run/borgmatic', ) - assert archive_data_source_names == { - 'postgresql_databases': ['foo', 'bar', 'baz'], - 'mysql_databases': ['quux'], + assert archive_dumps == { + module.Dump('postgresql_databases', 'foo'), + module.Dump('postgresql_databases', 'bar'), + module.Dump('postgresql_databases', 'baz'), + module.Dump('mysql_databases', 'quux'), } @@ -331,7 +325,7 @@ def test_collect_dumps_from_archive_parses_directory_format_archive_paths(): ] ) - archive_data_source_names = module.collect_dumps_from_archive( + archive_dumps = module.collect_dumps_from_archive( repository={'path': 'repo'}, archive='archive', config={}, @@ -342,8 +336,8 @@ def test_collect_dumps_from_archive_parses_directory_format_archive_paths(): borgmatic_runtime_directory='/run/borgmatic', ) - assert archive_data_source_names == { - 'postgresql_databases': ['foo'], + assert archive_dumps == { + module.Dump('postgresql_databases', 'foo'), } @@ -363,7 +357,7 @@ def test_collect_dumps_from_archive_skips_bad_archive_paths(): ] ) - archive_data_source_names = module.collect_dumps_from_archive( + archive_dumps = module.collect_dumps_from_archive( repository={'path': 'repo'}, archive='archive', config={}, @@ -374,55 +368,117 @@ def test_collect_dumps_from_archive_skips_bad_archive_paths(): borgmatic_runtime_directory='/run/borgmatic', ) - assert archive_data_source_names == { - 'postgresql_databases': ['foo'], + assert archive_dumps == { + module.Dump('postgresql_databases', 'foo'), } -def test_get_dumps_to_restore_passes_through_requested_names_found_in_archive(): - restore_names = module.get_dumps_to_restore( - requested_data_source_names=['foo', 'bar'], - archive_data_source_names={'postresql_databases': ['foo', 'bar', 'baz']}, - ) +def test_get_dumps_to_restore_gets_requested_dumps_found_in_archive(): + dumps_from_archive = { + module.Dump('postgresql_databases', 'foo'), + module.Dump('postgresql_databases', 'bar'), + module.Dump('postgresql_databases', 'baz'), + } + flexmock(module).should_receive('dumps_match').and_return(False) + flexmock(module).should_receive('dumps_match').with_args( + module.Dump(module.UNSPECIFIED, 'foo'), + module.Dump('postgresql_databases', 'foo'), + ).and_return(True) + flexmock(module).should_receive('dumps_match').with_args( + module.Dump(module.UNSPECIFIED, 'bar'), + module.Dump('postgresql_databases', 'bar'), + ).and_return(True) - assert restore_names == {module.UNSPECIFIED_HOOK: ['foo', 'bar']} + assert module.get_dumps_to_restore( + restore_arguments=flexmock( + hook=None, + data_sources=['foo', 'bar'], + original_hostname=None, + original_port=None, + ), + dumps_from_archive=dumps_from_archive, + ) == { + module.Dump('postgresql_databases', 'foo'), + module.Dump('postgresql_databases', 'bar'), + } -def test_get_dumps_to_restore_raises_for_requested_names_missing_from_archive(): +def test_get_dumps_to_restore_raises_for_requested_dumps_missing_from_archive(): + dumps_from_archive = { + module.Dump('postgresql_databases', 'foo'), + } + flexmock(module).should_receive('dumps_match').and_return(False) + with pytest.raises(ValueError): module.get_dumps_to_restore( - requested_data_source_names=['foo', 'bar'], - archive_data_source_names={'postresql_databases': ['foo']}, + restore_arguments=flexmock( + hook=None, + data_sources=['foo', 'bar'], + original_hostname=None, + original_port=None, + ), + dumps_from_archive=dumps_from_archive, ) -def test_get_dumps_to_restore_without_requested_names_finds_all_archive_data_sources(): - archive_data_source_names = {'postresql_databases': ['foo', 'bar']} - - restore_names = module.get_dumps_to_restore( - requested_data_source_names=[], - archive_data_source_names=archive_data_source_names, - ) - - assert restore_names == archive_data_source_names - - -def test_get_dumps_to_restore_with_all_in_requested_names_finds_all_archive_data_sources(): - archive_data_source_names = {'postresql_databases': ['foo', 'bar']} - - restore_names = module.get_dumps_to_restore( - requested_data_source_names=['all'], - archive_data_source_names=archive_data_source_names, - ) - - assert restore_names == archive_data_source_names - - -def test_get_dumps_to_restore_with_all_in_requested_names_plus_additional_requested_names_omits_duplicates(): +def test_get_dumps_to_restore_without_requested_dumps_finds_all_archive_dumps(): dumps_from_archive = { - module.Dump('postresql_databases', 'foo'), - module.Dump('postresql_databases', 'bar'), + module.Dump('postgresql_databases', 'foo'), + module.Dump('postgresql_databases', 'bar'), } + flexmock(module).should_receive('dumps_match').and_return(False) + + assert module.get_dumps_to_restore( + restore_arguments=flexmock( + hook=None, + data_sources=[], + original_hostname=None, + original_port=None, + ), + dumps_from_archive=dumps_from_archive, + ) == dumps_from_archive + + +def test_get_dumps_to_restore_with_all_in_requested_dumps_finds_all_archive_dumps(): + dumps_from_archive = { + module.Dump('postgresql_databases', 'foo'), + module.Dump('postgresql_databases', 'bar'), + } + flexmock(module).should_receive('dumps_match').and_return(False) + flexmock(module).should_receive('dumps_match').with_args( + module.Dump(module.UNSPECIFIED, 'foo'), + module.Dump('postgresql_databases', 'foo'), + ).and_return(True) + flexmock(module).should_receive('dumps_match').with_args( + module.Dump(module.UNSPECIFIED, 'bar'), + module.Dump('postgresql_databases', 'bar'), + ).and_return(True) + + assert module.get_dumps_to_restore( + restore_arguments=flexmock( + hook=None, + data_sources=['all'], + original_hostname=None, + original_port=None, + ), + dumps_from_archive=dumps_from_archive, + ) == dumps_from_archive + + +def test_get_dumps_to_restore_with_all_in_requested_dumps_plus_additional_requested_dumps_omits_duplicates(): + dumps_from_archive = { + module.Dump('postgresql_databases', 'foo'), + module.Dump('postgresql_databases', 'bar'), + } + flexmock(module).should_receive('dumps_match').and_return(False) + flexmock(module).should_receive('dumps_match').with_args( + module.Dump(module.UNSPECIFIED, 'foo'), + module.Dump('postgresql_databases', 'foo'), + ).and_return(True) + flexmock(module).should_receive('dumps_match').with_args( + module.Dump(module.UNSPECIFIED, 'bar'), + module.Dump('postgresql_databases', 'bar'), + ).and_return(True) assert ( module.get_dumps_to_restore( @@ -438,7 +494,13 @@ def test_get_dumps_to_restore_with_all_in_requested_names_plus_additional_reques ) -def test_get_dumps_to_restore_raises_for_all_in_requested_names_and_requested_names_missing_from_archives(): +def test_get_dumps_to_restore_raises_for_all_in_requested_dumps_and_requested_dumps_missing_from_archives(): + flexmock(module).should_receive('dumps_match').and_return(False) + flexmock(module).should_receive('dumps_match').with_args( + module.Dump(module.UNSPECIFIED, 'foo'), + module.Dump('postgresql_databases', 'foo'), + ).and_return(True) + with pytest.raises(ValueError): module.get_dumps_to_restore( restore_arguments=flexmock( From d4705602fa262b6f9994915746338f617ef3e041 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Sun, 22 Dec 2024 22:02:53 -0800 Subject: [PATCH 07/14] Handle more edge cases by erroring (#418). --- borgmatic/actions/restore.py | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index 83f22068..0b012a2b 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -325,7 +325,8 @@ def get_dumps_to_restore(restore_arguments, dumps_from_archive): Dump instances from the archive to restore. As part of this, replace any Dump having a data source name of "all" with multiple named Dump instances as appropriate. - Raise ValueError if any of the requested data source names cannot be found in the archive. + Raise ValueError if any of the requested data source names cannot be found in the archive or if + there are multiple archive dump matches for a given requested dump. ''' requested_dumps = ( { @@ -367,12 +368,20 @@ def get_dumps_to_restore(restore_arguments, dumps_from_archive): if requested_dump.data_source_name == 'all': continue - for archive_dump in dumps_from_archive: - if dumps_match(requested_dump, archive_dump): - dumps_to_restore.add(archive_dump) - break - else: + matching_dumps = ( + archive_dump + for archive_dump in dumps_from_archive + if dumps_match(requested_dump, archive_dump) + ) + + if len(matching_dumps) == 0: missing_dumps.add(requested_dump) + elif len(matching_dumps) == 1: + dumps_to_restore.add(archive_dump) + else: + raise ValueError( + f'Cannot restore data source {render_dump_metadata(requested_dump)} because there are multiple matching dumps in the archive. Try adding additional flags to disambiguate.' + ) if missing_dumps: rendered_dumps = ', '.join( From d9d6d3f7f283bc0b7448824507c5648c99efc516 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Mon, 23 Dec 2024 22:12:47 -0800 Subject: [PATCH 08/14] Simplify logic to get configured data sources during restoration (#418). --- borgmatic/actions/restore.py | 40 +++++++++++----------------- tests/unit/actions/test_restore.py | 42 +++++++++--------------------- 2 files changed, 28 insertions(+), 54 deletions(-) diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index 0b012a2b..ba1d9664 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -68,27 +68,17 @@ def render_dump_metadata(dump): def get_configured_data_source(config, restore_dump): ''' Search in the given configuration dict for dumps corresponding to the given dump to restore. If - there are multiple matches, error. If UNSPECIFIED is given as any field in the restore dump, - then that can match any value. + there are multiple matches, error. - Return the found data source as a tuple of (found hook name, data source configuration dict) or - (None, None) if not found. + Return the found data source as a data source configuration dict or None if not found. ''' - if restore_dump.hook_name == UNSPECIFIED: - hooks_to_search = { - hook_name: value - for (hook_name, value) in config.items() - if hook_name.split('_databases')[0] - in borgmatic.hooks.dispatch.get_submodule_names(borgmatic.hooks.data_source) - } - else: - try: - hooks_to_search = {restore_dump.hook_name: config[restore_dump.hook_name]} - except KeyError: - return (None, None) + try: + hooks_to_search = {restore_dump.hook_name: config[restore_dump.hook_name]} + except KeyError: + return None matching_dumps = tuple( - (hook_name, hook_data_source) + hook_data_source for (hook_name, hook) in hooks_to_search.items() for hook_data_source in hook if dumps_match( @@ -103,11 +93,11 @@ def get_configured_data_source(config, restore_dump): ) if not matching_dumps: - return (None, None) + return None if len(matching_dumps) > 1: raise ValueError( - f'Cannot restore data source {render_dump_metadata(restore_dump)} because there are multiple matching configured data sources. Try adding additional flags to disambiguate.' + f'Cannot restore data source {render_dump_metadata(restore_dump)} because there are multiple matching configured data sources.' ) return matching_dumps[0] @@ -368,7 +358,7 @@ def get_dumps_to_restore(restore_arguments, dumps_from_archive): if requested_dump.data_source_name == 'all': continue - matching_dumps = ( + matching_dumps = tuple( archive_dump for archive_dump in dumps_from_archive if dumps_match(requested_dump, archive_dump) @@ -377,10 +367,10 @@ def get_dumps_to_restore(restore_arguments, dumps_from_archive): if len(matching_dumps) == 0: missing_dumps.add(requested_dump) elif len(matching_dumps) == 1: - dumps_to_restore.add(archive_dump) + dumps_to_restore.add(matching_dumps[0]) else: raise ValueError( - f'Cannot restore data source {render_dump_metadata(requested_dump)} because there are multiple matching dumps in the archive. Try adding additional flags to disambiguate.' + f'Cannot restore data source {render_dump_metadata(requested_dump)} because there are multiple matching dumps in the archive. Try adding flags to disambiguate.' ) if missing_dumps: @@ -483,7 +473,7 @@ def run_restore( # Restore each dump. for restore_dump in dumps_to_restore: - found_hook_name, found_data_source = get_configured_data_source( + found_data_source = get_configured_data_source( config, restore_dump, ) @@ -491,7 +481,7 @@ def run_restore( # For any data sources that weren't found via exact matches in the configuration, try to # fallback to "all" entries. if not found_data_source: - found_hook_name, found_data_source = get_configured_data_source( + found_data_source = get_configured_data_source( config, Dump(restore_dump.hook_name, 'all', restore_dump.hostname, restore_dump.port), ) @@ -512,7 +502,7 @@ def run_restore( local_path, remote_path, archive_name, - found_hook_name or restore_dump.hook_name, + restore_dump.hook_name, dict(found_data_source, **{'schemas': restore_arguments.schemas}), connection_params, borgmatic_runtime_directory, diff --git a/tests/unit/actions/test_restore.py b/tests/unit/actions/test_restore.py index 59d64421..cd064688 100644 --- a/tests/unit/actions/test_restore.py +++ b/tests/unit/actions/test_restore.py @@ -19,7 +19,7 @@ def test_get_configured_data_source_matches_data_source_by_name(): 'postgresql_databases': [{'name': 'foo'}, {'name': 'bar'}], }, restore_dump=module.Dump('postgresql_databases', 'bar'), - ) == ('postgresql_databases', {'name': 'bar'}) + ) == {'name': 'bar'} def test_get_configured_data_source_matches_nothing_when_nothing_configured(): @@ -28,7 +28,7 @@ def test_get_configured_data_source_matches_nothing_when_nothing_configured(): assert module.get_configured_data_source( config={}, restore_dump=module.Dump('postgresql_databases', 'quux'), - ) == (None, None) + ) is None def test_get_configured_data_source_matches_nothing_when_data_source_name_not_configured(): @@ -39,23 +39,7 @@ def test_get_configured_data_source_matches_nothing_when_data_source_name_not_co 'postgresql_databases': [{'name': 'foo'}], }, restore_dump=module.Dump('postgresql_databases', 'quux'), - ) == (None, None) - - -def test_get_configured_data_source_with_unspecified_hook_matches_data_source_by_name(): - flexmock(module).should_receive('dumps_match').and_return(False) - flexmock(module).should_receive('dumps_match').with_args( - module.Dump('postgresql_databases', 'bar'), - module.Dump(module.UNSPECIFIED, 'bar'), - ).and_return(True) - - assert module.get_configured_data_source( - config={ - 'other_databases': [{'name': 'other'}], - 'postgresql_databases': [{'name': 'foo'}, {'name': 'bar'}], - }, - restore_dump=module.Dump(module.UNSPECIFIED, 'bar'), - ) == ('postgresql_databases', {'name': 'bar'}) + ) is None def test_strip_path_prefix_from_extracted_dump_destination_renames_first_matching_databases_subdirectory(): @@ -565,8 +549,8 @@ def test_run_restore_restores_each_data_source(): flexmock(module).should_receive('collect_dumps_from_archive').and_return(flexmock()) flexmock(module).should_receive('get_dumps_to_restore').and_return(dumps_to_restore) flexmock(module).should_receive('get_configured_data_source').and_return( - ('postgresql_databases', {'name': 'foo'}) - ).and_return(('postgresql_databases', {'name': 'bar'})) + {'name': 'foo'} + ).and_return({'name': 'bar'}) flexmock(module).should_receive('restore_single_dump').with_args( repository=object, config=object, @@ -665,15 +649,15 @@ def test_run_restore_restores_data_source_configured_with_all_name(): flexmock(module).should_receive('get_configured_data_source').with_args( config=object, restore_dump=module.Dump(hook_name='postgresql_databases', data_source_name='foo'), - ).and_return(('postgresql_databases', {'name': 'foo'})) + ).and_return({'name': 'foo'}) flexmock(module).should_receive('get_configured_data_source').with_args( config=object, restore_dump=module.Dump(hook_name='postgresql_databases', data_source_name='bar'), - ).and_return((None, None)) + ).and_return(None) flexmock(module).should_receive('get_configured_data_source').with_args( config=object, restore_dump=module.Dump(hook_name='postgresql_databases', data_source_name='all'), - ).and_return(('postgresql_databases', {'name': 'bar'})) + ).and_return({'name': 'bar'}) flexmock(module).should_receive('restore_single_dump').with_args( repository=object, config=object, @@ -746,15 +730,15 @@ def test_run_restore_skips_missing_data_source(): flexmock(module).should_receive('get_configured_data_source').with_args( config=object, restore_dump=module.Dump(hook_name='postgresql_databases', data_source_name='foo'), - ).and_return(('postgresql_databases', {'name': 'foo'})) + ).and_return({'name': 'foo'}) flexmock(module).should_receive('get_configured_data_source').with_args( config=object, restore_dump=module.Dump(hook_name='postgresql_databases', data_source_name='bar'), - ).and_return((None, None)) + ).and_return(None) flexmock(module).should_receive('get_configured_data_source').with_args( config=object, restore_dump=module.Dump(hook_name='postgresql_databases', data_source_name='all'), - ).and_return((None, None)) + ).and_return(None) flexmock(module).should_receive('restore_single_dump').with_args( repository=object, config=object, @@ -827,11 +811,11 @@ def test_run_restore_restores_data_sources_from_different_hooks(): flexmock(module).should_receive('get_configured_data_source').with_args( config=object, restore_dump=module.Dump(hook_name='postgresql_databases', data_source_name='foo'), - ).and_return(('postgresql_databases', {'name': 'foo'})) + ).and_return({'name': 'foo'}) flexmock(module).should_receive('get_configured_data_source').with_args( config=object, restore_dump=module.Dump(hook_name='mysql_databases', data_source_name='foo'), - ).and_return(('mysql_databases', {'name': 'bar'})) + ).and_return({'name': 'bar'}) flexmock(module).should_receive('restore_single_dump').with_args( repository=object, config=object, From b8041f5c39cbd55fa36551857fd154289795a585 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 24 Dec 2024 09:13:42 -0800 Subject: [PATCH 09/14] Fix end-to-end tests broken by new database config checks during restore (#418). --- tests/end-to-end/test_database.py | 32 ++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/tests/end-to-end/test_database.py b/tests/end-to-end/test_database.py index 826b8a3a..7dba4fd6 100644 --- a/tests/end-to-end/test_database.py +++ b/tests/end-to-end/test_database.py @@ -16,6 +16,8 @@ def write_configuration( repository_path, user_runtime_directory, postgresql_dump_format='custom', + postgresql_all_dump_format=None, + mariadb_mysql_all_dump_format=None, mongodb_dump_format='archive', ): ''' @@ -23,6 +25,9 @@ def write_configuration( for testing. This includes injecting the given repository path, borgmatic source directory for storing database dumps, dump format (for PostgreSQL), and encryption passphrase. ''' + postgresql_all_format_option = f'format: {postgresql_all_dump_format}' if postgresql_all_dump_format else '' + mariadb_mysql_dump_format_option = f'format: {mariadb_mysql_all_dump_format}' if mariadb_mysql_all_dump_format else '' + config_yaml = f''' source_directories: - {source_directory} @@ -39,11 +44,7 @@ postgresql_databases: password: test format: {postgresql_dump_format} - name: all - hostname: postgresql - username: postgres - password: test - - name: all - format: custom + {postgresql_all_format_option} hostname: postgresql username: postgres password: test @@ -53,11 +54,7 @@ mariadb_databases: username: root password: test - name: all - hostname: mariadb - username: root - password: test - - name: all - format: sql + {mariadb_mysql_dump_format_option} hostname: mariadb username: root password: test @@ -67,11 +64,7 @@ mysql_databases: username: root password: test - name: all - hostname: not-actually-mysql - username: root - password: test - - name: all - format: sql + {mariadb_mysql_dump_format_option} hostname: not-actually-mysql username: root password: test @@ -97,12 +90,21 @@ sqlite_databases: return ruamel.yaml.YAML(typ='safe').load(config_yaml) +@pytest.mark.parametrize( + 'postgresql_all_dump_format,mariadb_mysql_all_dump_format', + ( + (None, None), + ('custom', 'sql'), + ) +) def write_custom_restore_configuration( source_directory, config_path, repository_path, user_runtime_directory, postgresql_dump_format='custom', + postgresql_all_dump_format=None, + mariadb_mysql_all_dump_format=None, mongodb_dump_format='archive', ): ''' From 9b85c5bc61410eca8a013547535119f0779c7c42 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 24 Dec 2024 15:24:09 -0800 Subject: [PATCH 10/14] Add missing restore test coverage (#418). --- borgmatic/actions/restore.py | 7 +- borgmatic/commands/arguments.py | 6 +- tests/end-to-end/test_database.py | 10 +- tests/unit/actions/test_restore.py | 269 +++++++++++++++++++++++++---- 4 files changed, 248 insertions(+), 44 deletions(-) diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index ba1d9664..9323588f 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -1,5 +1,4 @@ import collections -import copy import logging import os import pathlib @@ -48,7 +47,7 @@ def dumps_match(first, second): def render_dump_metadata(dump): ''' - Given a Dump instance, make a display string describing it for use in log messges. + Given a Dump instance, make a display string describing it for use in log messages. ''' name = dump.data_source_name if dump.data_source_name != UNSPECIFIED else 'unspecified' hostname = dump.hostname or 'localhost' @@ -97,7 +96,7 @@ def get_configured_data_source(config, restore_dump): if len(matching_dumps) > 1: raise ValueError( - f'Cannot restore data source {render_dump_metadata(restore_dump)} because there are multiple matching configured data sources.' + f'Cannot restore data source {render_dump_metadata(restore_dump)} because there are multiple matching configured data sources' ) return matching_dumps[0] @@ -353,7 +352,7 @@ def get_dumps_to_restore(restore_arguments, dumps_from_archive): if any(dump for dump in requested_dumps if dump.data_source_name == 'all'): dumps_to_restore.update(dumps_from_archive) - # Put any archive dump matching a requested dump in the dumps to restore. + # If any archive dump matches a requested dump, add the archive dump to the dumps to restore. for requested_dump in requested_dumps: if requested_dump.data_source_name == 'all': continue diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index 7e08b726..7bd1d74e 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -1184,16 +1184,16 @@ def make_parsers(): ) restore_group.add_argument( '--original-hostname', - help="The hostname where the dump to restore came from, only necessary if you need to disambiguate dumps", + help='The hostname where the dump to restore came from, only necessary if you need to disambiguate dumps', ) restore_group.add_argument( '--original-port', type=int, - help="The port where the dump to restore came from, only necessary if you need to disambiguate dumps", + help='The port where the dump to restore came from, only necessary if you need to disambiguate dumps', ) restore_group.add_argument( '--hook', - help="The name of the data source hook for the dump to restore, only necessary if you need to disambiguate dumps", + help='The name of the data source hook for the dump to restore, only necessary if you need to disambiguate dumps', ) restore_group.add_argument( '-h', '--help', action='help', help='Show this help message and exit' diff --git a/tests/end-to-end/test_database.py b/tests/end-to-end/test_database.py index 7dba4fd6..a0b9e898 100644 --- a/tests/end-to-end/test_database.py +++ b/tests/end-to-end/test_database.py @@ -25,8 +25,12 @@ def write_configuration( for testing. This includes injecting the given repository path, borgmatic source directory for storing database dumps, dump format (for PostgreSQL), and encryption passphrase. ''' - postgresql_all_format_option = f'format: {postgresql_all_dump_format}' if postgresql_all_dump_format else '' - mariadb_mysql_dump_format_option = f'format: {mariadb_mysql_all_dump_format}' if mariadb_mysql_all_dump_format else '' + postgresql_all_format_option = ( + f'format: {postgresql_all_dump_format}' if postgresql_all_dump_format else '' + ) + mariadb_mysql_dump_format_option = ( + f'format: {mariadb_mysql_all_dump_format}' if mariadb_mysql_all_dump_format else '' + ) config_yaml = f''' source_directories: @@ -95,7 +99,7 @@ sqlite_databases: ( (None, None), ('custom', 'sql'), - ) + ), ) def write_custom_restore_configuration( source_directory, diff --git a/tests/unit/actions/test_restore.py b/tests/unit/actions/test_restore.py index cd064688..0f254780 100644 --- a/tests/unit/actions/test_restore.py +++ b/tests/unit/actions/test_restore.py @@ -1,12 +1,142 @@ -import collections - import pytest from flexmock import flexmock import borgmatic.actions.restore as module -def test_get_configured_data_source_matches_data_source_by_name(): +@pytest.mark.parametrize( + 'first_dump,second_dump,expected_result', + ( + ( + module.Dump('postgresql_databases', 'foo'), + module.Dump('postgresql_databases', 'foo'), + True, + ), + ( + module.Dump('postgresql_databases', 'foo'), + module.Dump('postgresql_databases', 'bar'), + False, + ), + ( + module.Dump('postgresql_databases', 'foo'), + module.Dump('mariadb_databases', 'foo'), + False, + ), + (module.Dump('postgresql_databases', 'foo'), module.Dump(module.UNSPECIFIED, 'foo'), True), + (module.Dump('postgresql_databases', 'foo'), module.Dump(module.UNSPECIFIED, 'bar'), False), + ( + module.Dump('postgresql_databases', module.UNSPECIFIED), + module.Dump('postgresql_databases', 'foo'), + True, + ), + ( + module.Dump('postgresql_databases', module.UNSPECIFIED), + module.Dump('mariadb_databases', 'foo'), + False, + ), + ( + module.Dump('postgresql_databases', 'foo', 'myhost'), + module.Dump('postgresql_databases', 'foo', 'myhost'), + True, + ), + ( + module.Dump('postgresql_databases', 'foo', 'myhost'), + module.Dump('postgresql_databases', 'foo', 'otherhost'), + False, + ), + ( + module.Dump('postgresql_databases', 'foo', 'myhost'), + module.Dump('postgresql_databases', 'foo', module.UNSPECIFIED), + True, + ), + ( + module.Dump('postgresql_databases', 'foo', 'myhost'), + module.Dump('postgresql_databases', 'bar', module.UNSPECIFIED), + False, + ), + ( + module.Dump('postgresql_databases', 'foo', 'myhost', 1234), + module.Dump('postgresql_databases', 'foo', 'myhost', 1234), + True, + ), + ( + module.Dump('postgresql_databases', 'foo', 'myhost', 1234), + module.Dump('postgresql_databases', 'foo', 'myhost', 4321), + False, + ), + ( + module.Dump('postgresql_databases', 'foo', 'myhost', module.UNSPECIFIED), + module.Dump('postgresql_databases', 'foo', 'myhost', 1234), + True, + ), + ( + module.Dump('postgresql_databases', 'foo', 'myhost', module.UNSPECIFIED), + module.Dump('postgresql_databases', 'foo', 'otherhost', 1234), + False, + ), + ( + module.Dump( + module.UNSPECIFIED, module.UNSPECIFIED, module.UNSPECIFIED, module.UNSPECIFIED + ), + module.Dump('postgresql_databases', 'foo', 'myhost', 1234), + True, + ), + ), +) +def test_dumps_match_compares_two_dumps_while_respecting_unspecified_values( + first_dump, second_dump, expected_result +): + assert module.dumps_match(first_dump, second_dump) == expected_result + + +@pytest.mark.parametrize( + 'dump,expected_result', + ( + ( + module.Dump('postgresql_databases', 'foo'), + 'foo@localhost (postgresql_databases)', + ), + ( + module.Dump(module.UNSPECIFIED, 'foo'), + 'foo@localhost', + ), + ( + module.Dump('postgresql_databases', module.UNSPECIFIED), + 'unspecified@localhost (postgresql_databases)', + ), + ( + module.Dump('postgresql_databases', 'foo', 'host'), + 'foo@host (postgresql_databases)', + ), + ( + module.Dump('postgresql_databases', 'foo', module.UNSPECIFIED), + 'foo (postgresql_databases)', + ), + ( + module.Dump('postgresql_databases', 'foo', 'host', 1234), + 'foo@host:1234 (postgresql_databases)', + ), + ( + module.Dump('postgresql_databases', 'foo', module.UNSPECIFIED, 1234), + 'foo@:1234 (postgresql_databases)', + ), + ( + module.Dump('postgresql_databases', 'foo', 'host', module.UNSPECIFIED), + 'foo@host (postgresql_databases)', + ), + ( + module.Dump( + module.UNSPECIFIED, module.UNSPECIFIED, module.UNSPECIFIED, module.UNSPECIFIED + ), + 'unspecified', + ), + ), +) +def test_render_dump_metadata_renders_dump_values_into_string(dump, expected_result): + assert module.render_dump_metadata(dump) == expected_result + + +def test_get_configured_data_source_matches_data_source_with_restore_dump(): flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( module.Dump('postgresql_databases', 'bar'), @@ -25,21 +155,49 @@ def test_get_configured_data_source_matches_data_source_by_name(): def test_get_configured_data_source_matches_nothing_when_nothing_configured(): flexmock(module).should_receive('dumps_match').and_return(False) - assert module.get_configured_data_source( - config={}, - restore_dump=module.Dump('postgresql_databases', 'quux'), - ) is None + assert ( + module.get_configured_data_source( + config={}, + restore_dump=module.Dump('postgresql_databases', 'quux'), + ) + is None + ) -def test_get_configured_data_source_matches_nothing_when_data_source_name_not_configured(): +def test_get_configured_data_source_matches_nothing_when_restore_dump_does_not_match_configuration(): flexmock(module).should_receive('dumps_match').and_return(False) - assert module.get_configured_data_source( - config={ - 'postgresql_databases': [{'name': 'foo'}], - }, - restore_dump=module.Dump('postgresql_databases', 'quux'), - ) is None + assert ( + module.get_configured_data_source( + config={ + 'postgresql_databases': [{'name': 'foo'}], + }, + restore_dump=module.Dump('postgresql_databases', 'quux'), + ) + is None + ) + + +def test_get_configured_data_source_with_multiple_matching_data_sources_errors(): + flexmock(module).should_receive('dumps_match').and_return(False) + flexmock(module).should_receive('dumps_match').with_args( + module.Dump('postgresql_databases', 'bar'), + module.Dump('postgresql_databases', 'bar'), + ).and_return(True) + flexmock(module).should_receive('render_dump_metadata').and_return('test') + + with pytest.raises(ValueError): + module.get_configured_data_source( + config={ + 'other_databases': [{'name': 'other'}], + 'postgresql_databases': [ + {'name': 'foo'}, + {'name': 'bar'}, + {'name': 'bar', 'format': 'directory'}, + ], + }, + restore_dump=module.Dump('postgresql_databases', 'bar'), + ) def test_strip_path_prefix_from_extracted_dump_destination_renames_first_matching_databases_subdirectory(): @@ -63,6 +221,7 @@ def test_strip_path_prefix_from_extracted_dump_destination_renames_first_matchin def test_restore_single_dump_extracts_and_restores_single_file_dump(): + flexmock(module).should_receive('render_dump_metadata').and_return('test') flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hooks').with_args( 'make_data_source_dump_patterns', object, object, object, object, object ).and_return({'postgresql': flexmock()}) @@ -103,6 +262,7 @@ def test_restore_single_dump_extracts_and_restores_single_file_dump(): def test_restore_single_dump_extracts_and_restores_directory_dump(): + flexmock(module).should_receive('render_dump_metadata').and_return('test') flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hooks').with_args( 'make_data_source_dump_patterns', object, object, object, object, object ).and_return({'postgresql': flexmock()}) @@ -145,6 +305,7 @@ def test_restore_single_dump_extracts_and_restores_directory_dump(): def test_restore_single_dump_with_directory_dump_error_cleans_up_temporary_directory(): + flexmock(module).should_receive('render_dump_metadata').and_return('test') flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hooks').with_args( 'make_data_source_dump_patterns', object, object, object, object, object ).and_return({'postgresql': flexmock()}) @@ -188,6 +349,7 @@ def test_restore_single_dump_with_directory_dump_error_cleans_up_temporary_direc def test_restore_single_dump_with_directory_dump_and_dry_run_skips_directory_move_and_cleanup(): + flexmock(module).should_receive('render_dump_metadata').and_return('test') flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hooks').with_args( 'make_data_source_dump_patterns', object, object, object, object, object ).and_return({'postgresql': flexmock()}) @@ -392,6 +554,7 @@ def test_get_dumps_to_restore_raises_for_requested_dumps_missing_from_archive(): module.Dump('postgresql_databases', 'foo'), } flexmock(module).should_receive('dumps_match').and_return(False) + flexmock(module).should_receive('render_dump_metadata').and_return('test') with pytest.raises(ValueError): module.get_dumps_to_restore( @@ -412,15 +575,18 @@ def test_get_dumps_to_restore_without_requested_dumps_finds_all_archive_dumps(): } flexmock(module).should_receive('dumps_match').and_return(False) - assert module.get_dumps_to_restore( - restore_arguments=flexmock( - hook=None, - data_sources=[], - original_hostname=None, - original_port=None, - ), - dumps_from_archive=dumps_from_archive, - ) == dumps_from_archive + assert ( + module.get_dumps_to_restore( + restore_arguments=flexmock( + hook=None, + data_sources=[], + original_hostname=None, + original_port=None, + ), + dumps_from_archive=dumps_from_archive, + ) + == dumps_from_archive + ) def test_get_dumps_to_restore_with_all_in_requested_dumps_finds_all_archive_dumps(): @@ -438,15 +604,18 @@ def test_get_dumps_to_restore_with_all_in_requested_dumps_finds_all_archive_dump module.Dump('postgresql_databases', 'bar'), ).and_return(True) - assert module.get_dumps_to_restore( - restore_arguments=flexmock( - hook=None, - data_sources=['all'], - original_hostname=None, - original_port=None, - ), - dumps_from_archive=dumps_from_archive, - ) == dumps_from_archive + assert ( + module.get_dumps_to_restore( + restore_arguments=flexmock( + hook=None, + data_sources=['all'], + original_hostname=None, + original_port=None, + ), + dumps_from_archive=dumps_from_archive, + ) + == dumps_from_archive + ) def test_get_dumps_to_restore_with_all_in_requested_dumps_plus_additional_requested_dumps_omits_duplicates(): @@ -478,12 +647,40 @@ def test_get_dumps_to_restore_with_all_in_requested_dumps_plus_additional_reques ) -def test_get_dumps_to_restore_raises_for_all_in_requested_dumps_and_requested_dumps_missing_from_archives(): +def test_get_dumps_to_restore_raises_for_multiple_matching_dumps_in_archive(): flexmock(module).should_receive('dumps_match').and_return(False) flexmock(module).should_receive('dumps_match').with_args( module.Dump(module.UNSPECIFIED, 'foo'), module.Dump('postgresql_databases', 'foo'), ).and_return(True) + flexmock(module).should_receive('dumps_match').with_args( + module.Dump(module.UNSPECIFIED, 'foo'), + module.Dump('mariadb_databases', 'foo'), + ).and_return(True) + flexmock(module).should_receive('render_dump_metadata').and_return('test') + + with pytest.raises(ValueError): + module.get_dumps_to_restore( + restore_arguments=flexmock( + hook=None, + data_sources=['foo'], + original_hostname=None, + original_port=None, + ), + dumps_from_archive={ + module.Dump('postgresql_databases', 'foo'), + module.Dump('mariadb_databases', 'foo'), + }, + ) + + +def test_get_dumps_to_restore_raises_for_all_in_requested_dumps_and_requested_dumps_missing_from_archive(): + flexmock(module).should_receive('dumps_match').and_return(False) + flexmock(module).should_receive('dumps_match').with_args( + module.Dump(module.UNSPECIFIED, 'foo'), + module.Dump('postgresql_databases', 'foo'), + ).and_return(True) + flexmock(module).should_receive('render_dump_metadata').and_return('test') with pytest.raises(ValueError): module.get_dumps_to_restore( @@ -519,9 +716,13 @@ def test_ensure_requested_dumps_restored_with_no_dumps_raises(): def test_ensure_requested_dumps_restored_with_missing_dumps_raises(): + flexmock(module).should_receive('render_dump_metadata').and_return('test') + with pytest.raises(ValueError): module.ensure_requested_dumps_restored( - dumps_to_restore={module.Dump(hook_name='postgresql_databases', data_source_name='foo')}, + dumps_to_restore={ + module.Dump(hook_name='postgresql_databases', data_source_name='foo') + }, dumps_actually_restored={ module.Dump(hook_name='postgresql_databases', data_source_name='bar') }, From be6b865a8171531d717e9e43f60f5ea406e64cc6 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 24 Dec 2024 23:09:44 -0800 Subject: [PATCH 11/14] Add even more missing test coverage (#418). --- borgmatic/actions/restore.py | 2 +- tests/unit/actions/test_restore.py | 166 +++++++++++++++++++++- tests/unit/hooks/data_source/test_dump.py | 7 + 3 files changed, 171 insertions(+), 4 deletions(-) diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index 9323588f..658c8073 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -96,7 +96,7 @@ def get_configured_data_source(config, restore_dump): if len(matching_dumps) > 1: raise ValueError( - f'Cannot restore data source {render_dump_metadata(restore_dump)} because there are multiple matching configured data sources' + f'Cannot restore data source {render_dump_metadata(restore_dump)} because there are multiple matching data sources configured' ) return matching_dumps[0] diff --git a/tests/unit/actions/test_restore.py b/tests/unit/actions/test_restore.py index 0f254780..e730ba43 100644 --- a/tests/unit/actions/test_restore.py +++ b/tests/unit/actions/test_restore.py @@ -399,7 +399,7 @@ def test_collect_dumps_from_archive_parses_archive_paths(): flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return( [ 'borgmatic/postgresql_databases/localhost/foo', - 'borgmatic/postgresql_databases/localhost/bar', + 'borgmatic/postgresql_databases/host:1234/bar', 'borgmatic/mysql_databases/localhost/quux', ] ) @@ -417,7 +417,7 @@ def test_collect_dumps_from_archive_parses_archive_paths(): assert archive_dumps == { module.Dump('postgresql_databases', 'foo'), - module.Dump('postgresql_databases', 'bar'), + module.Dump('postgresql_databases', 'bar', 'host', 1234), module.Dump('mysql_databases', 'quux'), } @@ -487,7 +487,7 @@ def test_collect_dumps_from_archive_parses_directory_format_archive_paths(): } -def test_collect_dumps_from_archive_skips_bad_archive_paths(): +def test_collect_dumps_from_archive_skips_bad_archive_paths_or_bad_path_components(): flexmock(module.borgmatic.config.paths).should_receive( 'get_borgmatic_source_directory' ).and_return('/root/.borgmatic') @@ -497,6 +497,7 @@ def test_collect_dumps_from_archive_skips_bad_archive_paths(): flexmock(module.borgmatic.borg.list).should_receive('capture_archive_listing').and_return( [ 'borgmatic/postgresql_databases/localhost/foo', + 'borgmatic/postgresql_databases/localhost:abcd/bar', 'borgmatic/invalid', 'invalid/as/well', '', @@ -516,6 +517,7 @@ def test_collect_dumps_from_archive_skips_bad_archive_paths(): assert archive_dumps == { module.Dump('postgresql_databases', 'foo'), + module.Dump('postgresql_databases', 'bar'), } @@ -694,6 +696,106 @@ def test_get_dumps_to_restore_raises_for_all_in_requested_dumps_and_requested_du ) +def test_get_dumps_to_restore_with_requested_hook_name_filters_dumps_found_in_archive(): + dumps_from_archive = { + module.Dump('mariadb_databases', 'foo'), + module.Dump('postgresql_databases', 'foo'), + module.Dump('sqlite_databases', 'bar'), + } + flexmock(module).should_receive('dumps_match').and_return(False) + flexmock(module).should_receive('dumps_match').with_args( + module.Dump('postgresql_databases', 'foo'), + module.Dump('postgresql_databases', 'foo'), + ).and_return(True) + + assert module.get_dumps_to_restore( + restore_arguments=flexmock( + hook='postgresql_databases', + data_sources=['foo'], + original_hostname=None, + original_port=None, + ), + dumps_from_archive=dumps_from_archive, + ) == { + module.Dump('postgresql_databases', 'foo'), + } + + +def test_get_dumps_to_restore_with_requested_shortened_hook_name_filters_dumps_found_in_archive(): + dumps_from_archive = { + module.Dump('mariadb_databases', 'foo'), + module.Dump('postgresql_databases', 'foo'), + module.Dump('sqlite_databases', 'bar'), + } + flexmock(module).should_receive('dumps_match').and_return(False) + flexmock(module).should_receive('dumps_match').with_args( + module.Dump('postgresql_databases', 'foo'), + module.Dump('postgresql_databases', 'foo'), + ).and_return(True) + + assert module.get_dumps_to_restore( + restore_arguments=flexmock( + hook='postgresql', + data_sources=['foo'], + original_hostname=None, + original_port=None, + ), + dumps_from_archive=dumps_from_archive, + ) == { + module.Dump('postgresql_databases', 'foo'), + } + + +def test_get_dumps_to_restore_with_requested_hostname_filters_dumps_found_in_archive(): + dumps_from_archive = { + module.Dump('postgresql_databases', 'foo'), + module.Dump('postgresql_databases', 'foo', 'host'), + module.Dump('postgresql_databases', 'bar'), + } + flexmock(module).should_receive('dumps_match').and_return(False) + flexmock(module).should_receive('dumps_match').with_args( + module.Dump('postgresql_databases', 'foo', 'host'), + module.Dump('postgresql_databases', 'foo', 'host'), + ).and_return(True) + + assert module.get_dumps_to_restore( + restore_arguments=flexmock( + hook='postgresql_databases', + data_sources=['foo'], + original_hostname='host', + original_port=None, + ), + dumps_from_archive=dumps_from_archive, + ) == { + module.Dump('postgresql_databases', 'foo', 'host'), + } + + +def test_get_dumps_to_restore_with_requested_port_filters_dumps_found_in_archive(): + dumps_from_archive = { + module.Dump('postgresql_databases', 'foo', 'host'), + module.Dump('postgresql_databases', 'foo', 'host', 1234), + module.Dump('postgresql_databases', 'bar'), + } + flexmock(module).should_receive('dumps_match').and_return(False) + flexmock(module).should_receive('dumps_match').with_args( + module.Dump('postgresql_databases', 'foo', 'host', 1234), + module.Dump('postgresql_databases', 'foo', 'host', 1234), + ).and_return(True) + + assert module.get_dumps_to_restore( + restore_arguments=flexmock( + hook='postgresql_databases', + data_sources=['foo'], + original_hostname='host', + original_port=1234, + ), + dumps_from_archive=dumps_from_archive, + ) == { + module.Dump('postgresql_databases', 'foo', 'host', 1234), + } + + def test_ensure_requested_dumps_restored_with_all_dumps_restored_does_not_raise(): module.ensure_requested_dumps_restored( dumps_to_restore={ @@ -827,6 +929,64 @@ def test_run_restore_bails_for_non_matching_repository(): ) +def test_run_restore_restores_data_source_by_falling_back_to_all_name(): + dumps_to_restore = { + module.Dump(hook_name='postgresql_databases', data_source_name='foo'), + } + + flexmock(module.borgmatic.config.validate).should_receive('repositories_match').and_return(True) + borgmatic_runtime_directory = flexmock() + flexmock(module.borgmatic.config.paths).should_receive('Runtime_directory').and_return( + borgmatic_runtime_directory + ) + flexmock(module.borgmatic.config.paths).should_receive( + 'make_runtime_directory_glob' + ).replace_with(lambda path: path) + flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hooks_even_if_unconfigured') + flexmock(module.borgmatic.borg.repo_list).should_receive('resolve_archive_name').and_return( + flexmock() + ) + flexmock(module).should_receive('collect_dumps_from_archive').and_return(flexmock()) + flexmock(module).should_receive('get_dumps_to_restore').and_return(dumps_to_restore) + flexmock(module).should_receive('get_configured_data_source').and_return( + {'name': 'foo'} + ).and_return({'name': 'all'}) + flexmock(module).should_receive('restore_single_dump').with_args( + repository=object, + config=object, + local_borg_version=object, + global_arguments=object, + local_path=object, + remote_path=object, + archive_name=object, + hook_name='postgresql_databases', + data_source={'name': 'foo', 'schemas': None}, + connection_params=object, + borgmatic_runtime_directory=borgmatic_runtime_directory, + ).once() + flexmock(module).should_receive('ensure_requested_dumps_restored') + + module.run_restore( + repository={'path': 'repo'}, + config=flexmock(), + local_borg_version=flexmock(), + restore_arguments=flexmock( + repository='repo', + archive='archive', + data_sources=flexmock(), + schemas=None, + hostname=None, + port=None, + username=None, + password=None, + restore_path=None, + ), + global_arguments=flexmock(dry_run=False), + local_path=flexmock(), + remote_path=flexmock(), + ) + + def test_run_restore_restores_data_source_configured_with_all_name(): dumps_to_restore = { module.Dump(hook_name='postgresql_databases', data_source_name='foo'), diff --git a/tests/unit/hooks/data_source/test_dump.py b/tests/unit/hooks/data_source/test_dump.py index 01e00e17..ffc7819c 100644 --- a/tests/unit/hooks/data_source/test_dump.py +++ b/tests/unit/hooks/data_source/test_dump.py @@ -15,6 +15,13 @@ def test_make_data_source_dump_filename_uses_name_and_hostname(): ) +def test_make_data_source_dump_filename_uses_name_and_hostname_and_port(): + assert ( + module.make_data_source_dump_filename('databases', 'test', 'hostname', 1234) + == 'databases/hostname:1234/test' + ) + + def test_make_data_source_dump_filename_without_hostname_defaults_to_localhost(): assert module.make_data_source_dump_filename('databases', 'test') == 'databases/localhost/test' From 69476a4fab55801950f8d84d8e8a0912e47e2c1b Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Tue, 24 Dec 2024 23:25:26 -0800 Subject: [PATCH 12/14] Documentation clarifications (#418). --- docs/how-to/backup-your-databases.md | 65 +++++++++++++++++++++------- 1 file changed, 50 insertions(+), 15 deletions(-) diff --git a/docs/how-to/backup-your-databases.md b/docs/how-to/backup-your-databases.md index f908a10a..c1e0576b 100644 --- a/docs/how-to/backup-your-databases.md +++ b/docs/how-to/backup-your-databases.md @@ -412,38 +412,73 @@ for more information. ### Restore databases sharing a name New in version 1.9.5 If you've -backed up multiple databases that happen to share the same name (e.g. from -different hostnames or ports), you can include additional flags to disambiguate -which database you'd like to restore. For instance: +backed up multiple databases that happen to share the same name but different +hostnames, ports, or hooks, you can include additional flags to disambiguate +which database you'd like to restore. For instance, let's say you've backed up +the following configured databases: -```bash -borgmatic restore --archive latest --database users --original-hostname myhost.org +```yaml +postgresql_databases: + - name: users + hostname: host1.example.org + - name: users + hostname: host2.example.org ``` -This selects a `users` database to restore, but only if it originally came from -the host `myhost.org`—and doesn't restore `users` databases from any other -hosts. +... then you can run the following command to restore only one of them: -Here's another example: +```bash +borgmatic restore --archive latest --database users --original-hostname host1.example.org +``` + +This selects a `users` database to restore, but only if it originally came +from the host `host1.example.org`. And this command won't restore `users` +databases from any other hosts. + +Here's another example configuration: + +```yaml +postgresql_databases: + - name: users + hostname: example.org + port: 5433 + - name: users + hostname: example.org + port: 5434 +``` + +And a command to restore just one of the databases: ```bash borgmatic restore --archive latest --database users --original-port 5433 ``` -That restores a `users` database only if it originally came from port 5433. +That restores a `users` database only if it originally came from port `5433`. -Finally: +Finally, check out this configuration: + +```yaml +postgresql_databases: + - name: users + hostname: example.org +mariadb_databases: + - name: users + hostname: example.org +``` + +And to select just one of the databases to restore: ```bash borgmatic restore --archive latest --database users --hook postgresql ``` That restores a `users` database only if it was dumped using the -`postgresql_databases:` database hook—and doesn't restore `users` databases that -were dumped using other database hooks. +`postgresql_databases:` data source hook. And this command won't restore +`users` databases that were dumped using other hooks. -Note that these flags don't change the hostname or port where the database is -restored. For that, see below about restoring to an alternate host. +Note that these flags don't change the hostname or port to which the database +is actually restored. For that, see below about restoring to an alternate +host. ### Restore all databases From 77df425bd1eb74b7f51bc66b6f17632d5404a763 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Wed, 25 Dec 2024 21:59:10 -0800 Subject: [PATCH 13/14] Minor edits and clarifying comments (#418). --- borgmatic/actions/restore.py | 17 ++++++++++------- borgmatic/hooks/data_source/dump.py | 2 +- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index 658c8073..b42dc364 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -49,9 +49,9 @@ def render_dump_metadata(dump): ''' Given a Dump instance, make a display string describing it for use in log messages. ''' - name = dump.data_source_name if dump.data_source_name != UNSPECIFIED else 'unspecified' + name = 'unspecified' if dump.data_source_name is UNSPECIFIED else dump.data_source_name hostname = dump.hostname or 'localhost' - port = dump.port if dump.port != UNSPECIFIED else None + port = None if dump.port is UNSPECIFIED else dump.port if port: metadata = f'{name}@:{port}' if hostname is UNSPECIFIED else f'{name}@{hostname}:{port}' @@ -168,7 +168,7 @@ def restore_single_dump( borgmatic.hooks.dispatch.Hook_type.DATA_SOURCE, borgmatic_runtime_directory, data_source['name'], - )[hook_name.split('_databases')[0]] + )[hook_name.split('_databases', 1)[0]] destination_path = ( tempfile.mkdtemp(dir=borgmatic_runtime_directory) @@ -265,13 +265,14 @@ def collect_dumps_from_archive( remote_path=remote_path, ) - # Parse out the details for the dumps found in the archive. + # Parse the paths of dumps found in the archive to get their respective dump metadata. dumps_from_archive = set() for dump_path in dump_paths: if not dump_path: continue + # Probe to find the base directory that's at the start of the dump path. for base_directory in ( 'borgmatic', borgmatic_runtime_directory, @@ -298,6 +299,7 @@ def collect_dumps_from_archive( dumps_from_archive.add(Dump(hook_name, data_source_name, hostname, port)) + # We've successfully parsed the dump path, so need to probe any further. break else: logger.warning( @@ -418,7 +420,8 @@ def run_restore( Run the "restore" action for the given repository, but only if the repository matches the requested repository in restore arguments. - Raise ValueError if a configured data source could not be found to restore. + Raise ValueError if a configured data source could not be found to restore or there's no + matching dump in the archive. ''' if restore_arguments.repository and not borgmatic.config.validate.repositories_match( repository, restore_arguments.repository @@ -477,8 +480,8 @@ def run_restore( restore_dump, ) - # For any data sources that weren't found via exact matches in the configuration, try to - # fallback to "all" entries. + # For a dump that wasn't found via an exact match in the configuration, try to fallback + # to an "all" data source. if not found_data_source: found_data_source = get_configured_data_source( config, diff --git a/borgmatic/hooks/data_source/dump.py b/borgmatic/hooks/data_source/dump.py index c602875f..ea3e6a13 100644 --- a/borgmatic/hooks/data_source/dump.py +++ b/borgmatic/hooks/data_source/dump.py @@ -27,7 +27,7 @@ def make_data_source_dump_filename(dump_path, name, hostname=None, port=None): raise ValueError(f'Invalid data source name {name}') return os.path.join( - dump_path, (hostname or 'localhost') + (f':{port}' if port is not None else ''), name + dump_path, (hostname or 'localhost') + ('' if port is None else f':{port}'), name ) From a99c48c1154670f802f7b8c6bd367014df7de7d8 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 26 Dec 2024 15:16:29 -0800 Subject: [PATCH 14/14] Documentation / CLI help clarifications around "--original-port" (#418). --- borgmatic/commands/arguments.py | 2 +- docs/how-to/backup-your-databases.md | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index 7bd1d74e..1252270b 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -1189,7 +1189,7 @@ def make_parsers(): restore_group.add_argument( '--original-port', type=int, - help='The port where the dump to restore came from, only necessary if you need to disambiguate dumps', + help="The port where the dump to restore came from (if that port is in borgmatic's configuration), only necessary if you need to disambiguate dumps", ) restore_group.add_argument( '--hook', diff --git a/docs/how-to/backup-your-databases.md b/docs/how-to/backup-your-databases.md index c1e0576b..71ce70ff 100644 --- a/docs/how-to/backup-your-databases.md +++ b/docs/how-to/backup-your-databases.md @@ -432,7 +432,7 @@ borgmatic restore --archive latest --database users --original-hostname host1.ex ``` This selects a `users` database to restore, but only if it originally came -from the host `host1.example.org`. And this command won't restore `users` +from the host `host1.example.org`. This command won't restore `users` databases from any other hosts. Here's another example configuration: @@ -453,7 +453,8 @@ And a command to restore just one of the databases: borgmatic restore --archive latest --database users --original-port 5433 ``` -That restores a `users` database only if it originally came from port `5433`. +That restores a `users` database only if it originally came from port `5433` +*and* if that port is in borgmatic's configuration, e.g. `port: 5433`. Finally, check out this configuration: @@ -473,8 +474,8 @@ borgmatic restore --archive latest --database users --hook postgresql ``` That restores a `users` database only if it was dumped using the -`postgresql_databases:` data source hook. And this command won't restore -`users` databases that were dumped using other hooks. +`postgresql_databases:` data source hook. This command won't restore `users` +databases that were dumped using other hooks. Note that these flags don't change the hostname or port to which the database is actually restored. For that, see below about restoring to an alternate