Преглед на файлове

Fix PostgreSQL restore error on "all" database dump.

Dan Helfman преди 5 години
родител
ревизия
dce1928dc4
променени са 3 файла, в които са добавени 61 реда и са изтрити 18 реда
  1. 20 10
      borgmatic/hooks/postgresql.py
  2. 4 0
      tests/end-to-end/test_database.py
  3. 37 8
      tests/unit/hooks/test_postgresql.py

+ 20 - 10
borgmatic/hooks/postgresql.py

@@ -34,7 +34,12 @@ def dump_databases(databases, log_prefix, location_config, dry_run):
         )
         all_databases = bool(name == 'all')
         command = (
-            ('pg_dumpall' if all_databases else 'pg_dump', '--no-password', '--clean')
+            (
+                'pg_dumpall' if all_databases else 'pg_dump',
+                '--no-password',
+                '--clean',
+                '--if-exists',
+            )
             + ('--file', dump_filename)
             + (('--host', database['hostname']) if 'hostname' in database else ())
             + (('--port', str(database['port'])) if 'port' in database else ())
@@ -93,23 +98,28 @@ def restore_database_dumps(databases, log_prefix, location_config, dry_run):
         dump_filename = dump.make_database_dump_filename(
             make_dump_path(location_config), database['name'], database.get('hostname')
         )
-        restore_command = (
-            ('pg_restore', '--no-password', '--clean', '--if-exists', '--exit-on-error')
+        all_databases = bool(database['name'] == 'all')
+        analyze_command = (
+            ('psql', '--no-password', '--quiet')
             + (('--host', database['hostname']) if 'hostname' in database else ())
             + (('--port', str(database['port'])) if 'port' in database else ())
             + (('--username', database['username']) if 'username' in database else ())
-            + ('--dbname', database['name'])
-            + (dump_filename,)
+            + (('--dbname', database['name']) if not all_databases else ())
+            + ('--command', 'ANALYZE')
         )
-        extra_environment = {'PGPASSWORD': database['password']} if 'password' in database else None
-        analyze_command = (
-            ('psql', '--no-password', '--quiet')
+        restore_command = (
+            ('psql' if all_databases else 'pg_restore', '--no-password')
+            + (
+                ('--if-exists', '--exit-on-error', '--clean', '--dbname', database['name'])
+                if not all_databases
+                else ()
+            )
             + (('--host', database['hostname']) if 'hostname' in database else ())
             + (('--port', str(database['port'])) if 'port' in database else ())
             + (('--username', database['username']) if 'username' in database else ())
-            + ('--dbname', database['name'])
-            + ('--command', 'ANALYZE')
+            + (('-f', dump_filename) if all_databases else (dump_filename,))
         )
+        extra_environment = {'PGPASSWORD': database['password']} if 'password' in database else None
 
         logger.debug(
             '{}: Restoring PostgreSQL database {}{}'.format(

+ 4 - 0
tests/end-to-end/test_database.py

@@ -29,6 +29,10 @@ hooks:
           hostname: postgresql
           username: postgres
           password: test
+        - name: all
+          hostname: postgresql
+          username: postgres
+          password: test
     mysql_databases:
         - name: test
           hostname: mysql

+ 37 - 8
tests/unit/hooks/test_postgresql.py

@@ -17,6 +17,7 @@ def test_dump_databases_runs_pg_dump_for_each_database():
                 'pg_dump',
                 '--no-password',
                 '--clean',
+                '--if-exists',
                 '--file',
                 'databases/localhost/{}'.format(name),
                 '--format',
@@ -54,6 +55,7 @@ def test_dump_databases_runs_pg_dump_with_hostname_and_port():
             'pg_dump',
             '--no-password',
             '--clean',
+            '--if-exists',
             '--file',
             'databases/database.example.org/foo',
             '--host',
@@ -83,6 +85,7 @@ def test_dump_databases_runs_pg_dump_with_username_and_password():
             'pg_dump',
             '--no-password',
             '--clean',
+            '--if-exists',
             '--file',
             'databases/localhost/foo',
             '--username',
@@ -110,6 +113,7 @@ def test_dump_databases_runs_pg_dump_with_format():
             'pg_dump',
             '--no-password',
             '--clean',
+            '--if-exists',
             '--file',
             'databases/localhost/foo',
             '--format',
@@ -135,6 +139,7 @@ def test_dump_databases_runs_pg_dump_with_options():
             'pg_dump',
             '--no-password',
             '--clean',
+            '--if-exists',
             '--file',
             'databases/localhost/foo',
             '--format',
@@ -157,7 +162,14 @@ def test_dump_databases_runs_pg_dumpall_for_all_databases():
     flexmock(module.os).should_receive('makedirs')
 
     flexmock(module).should_receive('execute_command').with_args(
-        ('pg_dumpall', '--no-password', '--clean', '--file', 'databases/localhost/all'),
+        (
+            'pg_dumpall',
+            '--no-password',
+            '--clean',
+            '--if-exists',
+            '--file',
+            'databases/localhost/all',
+        ),
         extra_environment=None,
     ).once()
 
@@ -197,9 +209,9 @@ def test_restore_database_dumps_restores_each_database():
             (
                 'pg_restore',
                 '--no-password',
-                '--clean',
                 '--if-exists',
                 '--exit-on-error',
+                '--clean',
                 '--dbname',
                 name,
                 'databases/localhost/{}'.format(name),
@@ -225,15 +237,15 @@ def test_restore_database_dumps_runs_pg_restore_with_hostname_and_port():
         (
             'pg_restore',
             '--no-password',
-            '--clean',
             '--if-exists',
             '--exit-on-error',
+            '--clean',
+            '--dbname',
+            'foo',
             '--host',
             'database.example.org',
             '--port',
             '5433',
-            '--dbname',
-            'foo',
             'databases/localhost/foo',
         ),
         extra_environment=None,
@@ -269,13 +281,13 @@ def test_restore_database_dumps_runs_pg_restore_with_username_and_password():
         (
             'pg_restore',
             '--no-password',
-            '--clean',
             '--if-exists',
             '--exit-on-error',
-            '--username',
-            'postgres',
+            '--clean',
             '--dbname',
             'foo',
+            '--username',
+            'postgres',
             'databases/localhost/foo',
         ),
         extra_environment={'PGPASSWORD': 'trustsome1'},
@@ -296,3 +308,20 @@ def test_restore_database_dumps_runs_pg_restore_with_username_and_password():
     ).once()
 
     module.restore_database_dumps(databases, 'test.yaml', {}, dry_run=False)
+
+
+def test_restore_all_database_dump():
+    databases = [{'name': 'all'}]
+    flexmock(module).should_receive('make_dump_path').and_return('')
+    flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
+        'databases/localhost/all'
+    )
+
+    flexmock(module).should_receive('execute_command').with_args(
+        ('psql', '--no-password', '-f', 'databases/localhost/all'), extra_environment=None
+    ).once()
+    flexmock(module).should_receive('execute_command').with_args(
+        ('psql', '--no-password', '--quiet', '--command', 'ANALYZE'), extra_environment=None
+    ).once()
+
+    module.restore_database_dumps(databases, 'test.yaml', {}, dry_run=False)