diff --git a/NEWS b/NEWS index 76292878..77390997 100644 --- a/NEWS +++ b/NEWS @@ -12,6 +12,8 @@ * #962: For the LVM hook, add support for nested logical volumes. * #965: Fix a borgmatic runtime directory error when running the "spot" check with a database hook enabled. + * #969: Fix the "restore" action to work on database dumps without a port when a default port is + present in configuration. * Fix the "spot" check to no longer consider pipe files within an archive for file comparisons. * Fix the "spot" check to have a nicer error when there are no source paths to compare. * Fix auto-excluding of special files (when databases are configured) to support relative source diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index b42dc364..05eeec58 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -27,15 +27,22 @@ Dump = collections.namedtuple( ) -def dumps_match(first, second): +def dumps_match(first, second, default_port=None): ''' Compare two Dump instances for equality while supporting a field value of UNSPECIFIED, which - indicates that the field should match any value. + indicates that the field should match any value. If a default port is given, then consider any + dump having that port to match with a dump having a None port. ''' for field_name in first._fields: first_value = getattr(first, field_name) second_value = getattr(second, field_name) + if default_port is not None and field_name == 'port': + if first_value == default_port and second_value is None: + continue + if second_value == default_port and first_value is None: + continue + if first_value == UNSPECIFIED or second_value == UNSPECIFIED: continue @@ -64,10 +71,10 @@ def render_dump_metadata(dump): return metadata -def get_configured_data_source(config, restore_dump): +def get_configured_data_source(config, restore_dump, log_prefix): ''' Search in the given configuration dict for dumps corresponding to the given dump to restore. If - there are multiple matches, error. + there are multiple matches, error. Log using the given log prefix. Return the found data source as a data source configuration dict or None if not found. ''' @@ -78,8 +85,16 @@ def get_configured_data_source(config, restore_dump): matching_dumps = tuple( hook_data_source - for (hook_name, hook) in hooks_to_search.items() - for hook_data_source in hook + for (hook_name, hook_config) in hooks_to_search.items() + for hook_data_source in hook_config + for default_port in ( + borgmatic.hooks.dispatch.call_hook( + function_name='get_default_port', + config=config, + log_prefix=log_prefix, + hook_name=hook_name, + ), + ) if dumps_match( Dump( hook_name, @@ -88,6 +103,7 @@ def get_configured_data_source(config, restore_dump): hook_data_source.get('port'), ), restore_dump, + default_port, ) ) @@ -478,6 +494,7 @@ def run_restore( found_data_source = get_configured_data_source( config, restore_dump, + log_prefix=repository['path'], ) # For a dump that wasn't found via an exact match in the configuration, try to fallback @@ -486,6 +503,7 @@ def run_restore( found_data_source = get_configured_data_source( config, Dump(restore_dump.hook_name, 'all', restore_dump.hostname, restore_dump.port), + log_prefix=repository['path'], ) if not found_data_source: diff --git a/borgmatic/hooks/data_source/mariadb.py b/borgmatic/hooks/data_source/mariadb.py index dfc6a3f1..aa84429e 100644 --- a/borgmatic/hooks/data_source/mariadb.py +++ b/borgmatic/hooks/data_source/mariadb.py @@ -118,6 +118,10 @@ def execute_dump_command( ) +def get_default_port(databases, config, log_prefix): # pragma: no cover + return 3306 + + def use_streaming(databases, config, log_prefix): ''' Given a sequence of MariaDB database configuration dicts, a configuration dict (ignored), and a diff --git a/borgmatic/hooks/data_source/mongodb.py b/borgmatic/hooks/data_source/mongodb.py index f63aefed..18ce6561 100644 --- a/borgmatic/hooks/data_source/mongodb.py +++ b/borgmatic/hooks/data_source/mongodb.py @@ -17,6 +17,10 @@ def make_dump_path(base_directory): # pragma: no cover return dump.make_data_source_dump_path(base_directory, 'mongodb_databases') +def get_default_port(databases, config, log_prefix): # pragma: no cover + return 27017 + + def use_streaming(databases, config, log_prefix): ''' Given a sequence of MongoDB database configuration dicts, a configuration dict (ignored), and a diff --git a/borgmatic/hooks/data_source/mysql.py b/borgmatic/hooks/data_source/mysql.py index 316a2797..febd2afc 100644 --- a/borgmatic/hooks/data_source/mysql.py +++ b/borgmatic/hooks/data_source/mysql.py @@ -117,6 +117,10 @@ def execute_dump_command( ) +def get_default_port(databases, config, log_prefix): # pragma: no cover + return 3306 + + def use_streaming(databases, config, log_prefix): ''' Given a sequence of MySQL database configuration dicts, a configuration dict (ignored), and a diff --git a/borgmatic/hooks/data_source/postgresql.py b/borgmatic/hooks/data_source/postgresql.py index ee470ef5..bb5e5a45 100644 --- a/borgmatic/hooks/data_source/postgresql.py +++ b/borgmatic/hooks/data_source/postgresql.py @@ -97,6 +97,10 @@ def database_names_to_dump(database, extra_environment, log_prefix, dry_run): ) +def get_default_port(databases, config, log_prefix): # pragma: no cover + return 5432 + + def use_streaming(databases, config, log_prefix): ''' Given a sequence of PostgreSQL database configuration dicts, a configuration dict (ignored), and diff --git a/borgmatic/hooks/data_source/sqlite.py b/borgmatic/hooks/data_source/sqlite.py index 15239e34..d901f3f7 100644 --- a/borgmatic/hooks/data_source/sqlite.py +++ b/borgmatic/hooks/data_source/sqlite.py @@ -17,6 +17,10 @@ def make_dump_path(base_directory): # pragma: no cover return dump.make_data_source_dump_path(base_directory, 'sqlite_databases') +def get_default_port(databases, config, log_prefix): # pragma: no cover + return None # SQLite doesn't use a port. + + def use_streaming(databases, config, log_prefix): ''' Given a sequence of SQLite database configuration dicts, a configuration dict (ignored), and a diff --git a/tests/unit/actions/test_restore.py b/tests/unit/actions/test_restore.py index e730ba43..9debd701 100644 --- a/tests/unit/actions/test_restore.py +++ b/tests/unit/actions/test_restore.py @@ -5,73 +5,96 @@ import borgmatic.actions.restore as module @pytest.mark.parametrize( - 'first_dump,second_dump,expected_result', + 'first_dump,second_dump,default_port,expected_result', ( ( module.Dump('postgresql_databases', 'foo'), module.Dump('postgresql_databases', 'foo'), + None, True, ), ( module.Dump('postgresql_databases', 'foo'), module.Dump('postgresql_databases', 'bar'), + None, False, ), ( module.Dump('postgresql_databases', 'foo'), module.Dump('mariadb_databases', 'foo'), + None, + False, + ), + ( + module.Dump('postgresql_databases', 'foo'), + module.Dump(module.UNSPECIFIED, 'foo'), + None, + True, + ), + ( + module.Dump('postgresql_databases', 'foo'), + module.Dump(module.UNSPECIFIED, 'bar'), + None, 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'), + None, True, ), ( module.Dump('postgresql_databases', module.UNSPECIFIED), module.Dump('mariadb_databases', 'foo'), + None, False, ), ( module.Dump('postgresql_databases', 'foo', 'myhost'), module.Dump('postgresql_databases', 'foo', 'myhost'), + None, True, ), ( module.Dump('postgresql_databases', 'foo', 'myhost'), module.Dump('postgresql_databases', 'foo', 'otherhost'), + None, False, ), ( module.Dump('postgresql_databases', 'foo', 'myhost'), module.Dump('postgresql_databases', 'foo', module.UNSPECIFIED), + None, True, ), ( module.Dump('postgresql_databases', 'foo', 'myhost'), module.Dump('postgresql_databases', 'bar', module.UNSPECIFIED), + None, False, ), ( module.Dump('postgresql_databases', 'foo', 'myhost', 1234), module.Dump('postgresql_databases', 'foo', 'myhost', 1234), + None, True, ), ( module.Dump('postgresql_databases', 'foo', 'myhost', 1234), module.Dump('postgresql_databases', 'foo', 'myhost', 4321), + None, False, ), ( module.Dump('postgresql_databases', 'foo', 'myhost', module.UNSPECIFIED), module.Dump('postgresql_databases', 'foo', 'myhost', 1234), + None, True, ), ( module.Dump('postgresql_databases', 'foo', 'myhost', module.UNSPECIFIED), module.Dump('postgresql_databases', 'foo', 'otherhost', 1234), + None, False, ), ( @@ -79,14 +102,33 @@ import borgmatic.actions.restore as module module.UNSPECIFIED, module.UNSPECIFIED, module.UNSPECIFIED, module.UNSPECIFIED ), module.Dump('postgresql_databases', 'foo', 'myhost', 1234), + None, True, ), + ( + module.Dump('postgresql_databases', 'foo', 'myhost', 5432), + module.Dump('postgresql_databases', 'foo', 'myhost', None), + 5432, + True, + ), + ( + module.Dump('postgresql_databases', 'foo', 'myhost', None), + module.Dump('postgresql_databases', 'foo', 'myhost', 5432), + 5432, + True, + ), + ( + module.Dump('postgresql_databases', 'foo', 'myhost', 5433), + module.Dump('postgresql_databases', 'foo', 'myhost', None), + 5432, + False, + ), ), ) def test_dumps_match_compares_two_dumps_while_respecting_unspecified_values( - first_dump, second_dump, expected_result + first_dump, second_dump, default_port, expected_result ): - assert module.dumps_match(first_dump, second_dump) == expected_result + assert module.dumps_match(first_dump, second_dump, default_port) == expected_result @pytest.mark.parametrize( @@ -137,10 +179,13 @@ def test_render_dump_metadata_renders_dump_values_into_string(dump, expected_res def test_get_configured_data_source_matches_data_source_with_restore_dump(): + default_port = flexmock() + flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(default_port) 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'), + default_port=default_port, ).and_return(True) assert module.get_configured_data_source( @@ -149,22 +194,26 @@ def test_get_configured_data_source_matches_data_source_with_restore_dump(): 'postgresql_databases': [{'name': 'foo'}, {'name': 'bar'}], }, restore_dump=module.Dump('postgresql_databases', 'bar'), + log_prefix='test', ) == {'name': 'bar'} def test_get_configured_data_source_matches_nothing_when_nothing_configured(): + flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(flexmock()) flexmock(module).should_receive('dumps_match').and_return(False) assert ( module.get_configured_data_source( config={}, restore_dump=module.Dump('postgresql_databases', 'quux'), + log_prefix='test', ) is None ) def test_get_configured_data_source_matches_nothing_when_restore_dump_does_not_match_configuration(): + flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(flexmock()) flexmock(module).should_receive('dumps_match').and_return(False) assert ( @@ -173,16 +222,20 @@ def test_get_configured_data_source_matches_nothing_when_restore_dump_does_not_m 'postgresql_databases': [{'name': 'foo'}], }, restore_dump=module.Dump('postgresql_databases', 'quux'), + log_prefix='test', ) is None ) def test_get_configured_data_source_with_multiple_matching_data_sources_errors(): + default_port = flexmock() + flexmock(module.borgmatic.hooks.dispatch).should_receive('call_hook').and_return(default_port) 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'), + default_port=default_port, ).and_return(True) flexmock(module).should_receive('render_dump_metadata').and_return('test') @@ -197,6 +250,7 @@ def test_get_configured_data_source_with_multiple_matching_data_sources_errors() ], }, restore_dump=module.Dump('postgresql_databases', 'bar'), + log_prefix='test', ) @@ -1010,14 +1064,17 @@ 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'), + log_prefix=object, ).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'), + log_prefix=object, ).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'), + log_prefix=object, ).and_return({'name': 'bar'}) flexmock(module).should_receive('restore_single_dump').with_args( repository=object, @@ -1091,14 +1148,17 @@ 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'), + log_prefix=object, ).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'), + log_prefix=object, ).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'), + log_prefix=object, ).and_return(None) flexmock(module).should_receive('restore_single_dump').with_args( repository=object, @@ -1172,10 +1232,12 @@ 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'), + log_prefix=object, ).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'), + log_prefix=object, ).and_return({'name': 'bar'}) flexmock(module).should_receive('restore_single_dump').with_args( repository=object,