add support for original-label and fix the tests from the previous changes.

borgmatic/actions/restore.py

@@ -22,8 +22,8 @@ UNSPECIFIED = object()
 
 Dump = collections.namedtuple(
     'Dump',
-    ('hook_name', 'data_source_name', 'hostname', 'port'),
-    defaults=('localhost', None),
+    ('hook_name', 'data_source_name', 'hostname', 'port', 'label'),
+    defaults=('localhost', None, None),
 )
 
 
@@ -57,11 +57,14 @@ def render_dump_metadata(dump):
     '''
     Given a Dump instance, make a display string describing it for use in log messages.
     '''
+    label = dump.label or UNSPECIFIED
     name = 'unspecified' if dump.data_source_name is UNSPECIFIED else dump.data_source_name
     hostname = dump.hostname or UNSPECIFIED
     port = None if dump.port is UNSPECIFIED else dump.port
 
-    if port:
+    if label is not UNSPECIFIED:
+        metadata = f'{name}@{label}'
+    elif 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}'
@@ -101,6 +104,7 @@ def get_configured_data_source(config, restore_dump):
                 hook_data_source.get('name'),
                 hook_data_source.get('hostname', 'localhost'),
                 hook_data_source.get('port'),
+                hook_data_source.get('label'),
             ),
             restore_dump,
             default_port,
@@ -172,7 +176,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'), data_source.get('label')),
     )
 
     logger.info(f'Restoring data source {dump_metadata}')
@@ -371,7 +375,7 @@ def collect_dumps_from_archive(
             except (ValueError, TypeError):
                 port = None
 
-            dumps_from_archive.add(Dump(hook_name, data_source_name, hostname, port))
+            dumps_from_archive.add(Dump(hook_name, data_source_name, hostname, port, host_and_port))
 
             # We've successfully parsed the dump path, so need to probe any further.
             break
@@ -408,6 +412,7 @@ def get_dumps_to_restore(restore_arguments, dumps_from_archive):
                 data_source_name=name,
                 hostname=restore_arguments.original_hostname or UNSPECIFIED,
                 port=restore_arguments.original_port,
+                label=restore_arguments.original_label,
             )
             for name in restore_arguments.data_sources or (UNSPECIFIED,)
         }
@@ -415,12 +420,14 @@ def get_dumps_to_restore(restore_arguments, dumps_from_archive):
         or restore_arguments.data_sources
         or restore_arguments.original_hostname
         or restore_arguments.original_port
+        or restore_arguments.original_label
         else {
             Dump(
                 hook_name=UNSPECIFIED,
                 data_source_name='all',
                 hostname=UNSPECIFIED,
                 port=UNSPECIFIED,
+                label=UNSPECIFIED,
             ),
         }
     )
@@ -541,6 +548,7 @@ def run_restore(
 
         dumps_actually_restored = set()
         connection_params = {
+            'container': restore_arguments.container,
             'hostname': restore_arguments.hostname,
             'port': restore_arguments.port,
             'username': restore_arguments.username,
@@ -560,7 +568,7 @@ def run_restore(
             if not found_data_source:
                 found_data_source = get_configured_data_source(
                     config,
-                    Dump(restore_dump.hook_name, 'all', restore_dump.hostname, restore_dump.port),
+                    Dump(restore_dump.hook_name, 'all', restore_dump.hostname, restore_dump.port, restore_dump.label),
                 )
 
                 if not found_data_source:

borgmatic/commands/arguments.py

@@ -1491,6 +1491,10 @@ def make_parsers(schema, unparsed_arguments):  # noqa: PLR0915
         '--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-label',
+        help='The label where to dump to restore came from, only necessary if you need to disambiguate dumps',
+    )
     restore_group.add_argument(
         '--original-hostname',
         help='The hostname where the dump to restore came from, only necessary if you need to disambiguate dumps',

borgmatic/config/schema.yaml

@@ -1337,8 +1337,8 @@ properties:
                 restore_container:
                     type: string
                     description: |
-                        Container name/id to restore to. Defaults to the "container"
-                        option.
+                        Container name/id to restore to. Defaults to the
+                        "container" option.
                     example: restore_container
                 hostname:
                     type: string
@@ -1555,8 +1555,8 @@ properties:
                 restore_container:
                     type: string
                     description: |
-                        Container name/id to restore to. Defaults to the "container"
-                        option.
+                        Container name/id to restore to. Defaults to the
+                        "container" option.
                     example: restore_container
                 hostname:
                     type: string
@@ -1737,8 +1737,8 @@ properties:
                 restore_container:
                     type: string
                     description: |
-                        Container name/id to restore to. Defaults to the "container"
-                        option.
+                        Container name/id to restore to. Defaults to the
+                        "container" option.
                     example: restore_container
                 hostname:
                     type: string
@@ -1980,8 +1980,8 @@ properties:
                 restore_container:
                     type: string
                     description: |
-                        Container name/id to restore to. Defaults to the "container"
-                        option.
+                        Container name/id to restore to. Defaults to the
+                        "container" option.
                     example: restore_container
                 hostname:
                     type: string

borgmatic/hooks/data_source/utils.py

@@ -1,4 +1,5 @@
 import json
+import logging
 import shutil
 import subprocess
 
@@ -6,6 +7,7 @@ from borgmatic.execute import execute_command_and_capture_output
 
 IS_A_HOOK = False
 
+logger = logging.getLogger(__name__)
 
 def resolve_database_option(option, data_source, connection_params=None, restore=False):
     # Special case `hostname` since it overlaps with `container`
@@ -42,8 +44,9 @@ def get_ip_from_container(container):
     engines = [engine for engine in engines if engine]
 
     if not engines:
-        raise xxx  # TODO: What to raise here, tell the user to install docker/podman
+        raise ValueError("Neither 'docker' nor 'podman' could be found on the system")
 
+    last_error = None
     for engine in engines:
         try:
             output = execute_command_and_capture_output(
@@ -55,7 +58,9 @@ def get_ip_from_container(container):
                     container,
                 )
             )
-        except subprocess.CalledProcessError:
+        except subprocess.CalledProcessError as error:
+            last_error = error
+            logger.debug(f"Couldn't find container '{container}' with engine '{engine}'")
             continue  # Container does not exist
 
         network_data = json.loads(output.strip())
@@ -68,4 +73,7 @@ def get_ip_from_container(container):
             if ip:
                 return ip
 
-    raise xxx  # No container ip found, what to raise here
+    if last_error:
+        raise last_error
+
+    return None

tests/unit/actions/test_restore.py

@@ -172,6 +172,10 @@ def test_dumps_match_compares_two_dumps_while_respecting_unspecified_values(
             module.Dump('postgresql_databases', 'foo', 'host', module.UNSPECIFIED),
             'foo@host (postgresql_databases)',
         ),
+        (
+            module.Dump('postgresql_databases', 'foo', 'host', 1234, 'label'),
+            'foo@label (postgresql_databases)',
+        ),
         (
             module.Dump(
                 module.UNSPECIFIED,
@@ -580,9 +584,9 @@ def test_collect_dumps_from_archive_without_dumps_metadata_falls_back_to_parsing
     )
 
     assert archive_dumps == {
-        module.Dump('postgresql_databases', 'foo'),
-        module.Dump('postgresql_databases', 'bar', 'host', 1234),
-        module.Dump('mysql_databases', 'quux'),
+        module.Dump('postgresql_databases', 'foo', label='localhost'),
+        module.Dump('postgresql_databases', 'bar', 'host', 1234, 'host:1234'),
+        module.Dump('mysql_databases', 'quux', label='localhost'),
     }
 
 
@@ -623,10 +627,10 @@ def test_collect_dumps_from_archive_parses_archive_paths_with_different_base_dir
     )
 
     assert archive_dumps == {
-        module.Dump('postgresql_databases', 'foo'),
-        module.Dump('postgresql_databases', 'bar'),
-        module.Dump('postgresql_databases', 'baz'),
-        module.Dump('mysql_databases', 'quux'),
+        module.Dump('postgresql_databases', 'foo', label='localhost'),
+        module.Dump('postgresql_databases', 'bar', label='localhost'),
+        module.Dump('postgresql_databases', 'baz', label='localhost'),
+        module.Dump('mysql_databases', 'quux', label='localhost'),
     }
 
 
@@ -665,7 +669,7 @@ def test_collect_dumps_from_archive_parses_directory_format_archive_paths():
     )
 
     assert archive_dumps == {
-        module.Dump('postgresql_databases', 'foo'),
+        module.Dump('postgresql_databases', 'foo', label='localhost'),
     }
 
 
@@ -707,8 +711,8 @@ def test_collect_dumps_from_archive_skips_bad_archive_paths_or_bad_path_componen
     )
 
     assert archive_dumps == {
-        module.Dump('postgresql_databases', 'foo'),
-        module.Dump('postgresql_databases', 'bar'),
+        module.Dump('postgresql_databases', 'foo', label='localhost'),
+        module.Dump('postgresql_databases', 'bar', label='localhost:abcd'),
     }
 
 
@@ -734,6 +738,7 @@ def test_get_dumps_to_restore_gets_requested_dumps_found_in_archive():
             data_sources=['foo', 'bar'],
             original_hostname=None,
             original_port=None,
+            original_label=None,
         ),
         dumps_from_archive=dumps_from_archive,
     ) == {
@@ -756,6 +761,7 @@ def test_get_dumps_to_restore_raises_for_requested_dumps_missing_from_archive():
                 data_sources=['foo', 'bar'],
                 original_hostname=None,
                 original_port=None,
+                original_label=None,
             ),
             dumps_from_archive=dumps_from_archive,
         )
@@ -775,6 +781,7 @@ def test_get_dumps_to_restore_without_requested_dumps_finds_all_archive_dumps():
                 data_sources=[],
                 original_hostname=None,
                 original_port=None,
+                original_label=None,
             ),
             dumps_from_archive=dumps_from_archive,
         )
@@ -804,6 +811,7 @@ def test_get_dumps_to_restore_with_all_in_requested_dumps_finds_all_archive_dump
                 data_sources=['all'],
                 original_hostname=None,
                 original_port=None,
+                original_label=None,
             ),
             dumps_from_archive=dumps_from_archive,
         )
@@ -833,6 +841,7 @@ def test_get_dumps_to_restore_with_all_in_requested_dumps_plus_additional_reques
                 data_sources=['all', 'foo', 'bar'],
                 original_hostname=None,
                 original_port=None,
+                original_label=None,
             ),
             dumps_from_archive=dumps_from_archive,
         )
@@ -859,6 +868,7 @@ def test_get_dumps_to_restore_raises_for_multiple_matching_dumps_in_archive():
                 data_sources=['foo'],
                 original_hostname=None,
                 original_port=None,
+                original_label=None,
             ),
             dumps_from_archive={
                 module.Dump('postgresql_databases', 'foo'),
@@ -882,6 +892,7 @@ def test_get_dumps_to_restore_raises_for_all_in_requested_dumps_and_requested_du
                 data_sources=['all', 'foo', 'bar'],
                 original_hostname=None,
                 original_port=None,
+                original_label=None,
             ),
             dumps_from_archive={module.Dump('postresql_databases', 'foo')},
         )
@@ -905,6 +916,7 @@ def test_get_dumps_to_restore_with_requested_hook_name_filters_dumps_found_in_ar
             data_sources=['foo'],
             original_hostname=None,
             original_port=None,
+            original_label=None,
         ),
         dumps_from_archive=dumps_from_archive,
     ) == {
@@ -930,6 +942,7 @@ def test_get_dumps_to_restore_with_requested_shortened_hook_name_filters_dumps_f
             data_sources=['foo'],
             original_hostname=None,
             original_port=None,
+            original_label=None,
         ),
         dumps_from_archive=dumps_from_archive,
     ) == {
@@ -955,6 +968,7 @@ def test_get_dumps_to_restore_with_requested_hostname_filters_dumps_found_in_arc
             data_sources=['foo'],
             original_hostname='host',
             original_port=None,
+            original_label=None,
         ),
         dumps_from_archive=dumps_from_archive,
     ) == {
@@ -980,6 +994,7 @@ def test_get_dumps_to_restore_with_requested_port_filters_dumps_found_in_archive
             data_sources=['foo'],
             original_hostname='host',
             original_port=1234,
+            original_label=None,
         ),
         dumps_from_archive=dumps_from_archive,
     ) == {
@@ -1087,6 +1102,7 @@ def test_run_restore_restores_each_data_source():
             username=None,
             password=None,
             restore_path=None,
+            container=None
         ),
         global_arguments=flexmock(dry_run=False),
         local_path=flexmock(),
@@ -1171,6 +1187,7 @@ def test_run_restore_restores_data_source_by_falling_back_to_all_name():
             username=None,
             password=None,
             restore_path=None,
+            container=None,
         ),
         global_arguments=flexmock(dry_run=False),
         local_path=flexmock(),
@@ -1252,6 +1269,7 @@ def test_run_restore_restores_data_source_configured_with_all_name():
             username=None,
             password=None,
             restore_path=None,
+            container=None,
         ),
         global_arguments=flexmock(dry_run=False),
         local_path=flexmock(),
@@ -1333,6 +1351,7 @@ def test_run_restore_skips_missing_data_source():
             username=None,
             password=None,
             restore_path=None,
+            container=None,
         ),
         global_arguments=flexmock(dry_run=False),
         local_path=flexmock(),
@@ -1410,6 +1429,7 @@ def test_run_restore_restores_data_sources_from_different_hooks():
             username=None,
             password=None,
             restore_path=None,
+            container=None,
         ),
         global_arguments=flexmock(dry_run=False),
         local_path=flexmock(),