Skip to content

Commit

Permalink
Fix PostgreSQL restore error on "all" database dump.
Browse files Browse the repository at this point in the history
  • Loading branch information
witten authored Apr 24, 2020
2 parents e511014 + 3c8dc49 commit dce1928
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 18 deletions.
30 changes: 20 additions & 10 deletions borgmatic/hooks/postgresql.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 ())
Expand Down Expand Up @@ -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(
Expand Down
4 changes: 4 additions & 0 deletions tests/end-to-end/test_database.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ def write_configuration(config_path, repository_path, borgmatic_source_directory
hostname: postgresql
username: postgres
password: test
- name: all
hostname: postgresql
username: postgres
password: test
mysql_databases:
- name: test
hostname: mysql
Expand Down
45 changes: 37 additions & 8 deletions tests/unit/hooks/test_postgresql.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand All @@ -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',
Expand All @@ -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()

Expand Down Expand Up @@ -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),
Expand All @@ -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,
Expand Down Expand Up @@ -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'},
Expand All @@ -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)

0 comments on commit dce1928

Please sign in to comment.