From cd51e9c1ea85712ddaffb4e92473bb69dc28cd5c Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Mon, 14 Aug 2023 12:43:21 -0700 Subject: [PATCH] Fix for database "restore" action not actually restore anything (#738). --- .drone.yml | 13 +- NEWS | 3 + borgmatic/actions/restore.py | 32 ++-- borgmatic/hooks/mariadb.py | 24 +-- borgmatic/hooks/mongodb.py | 28 +--- borgmatic/hooks/mysql.py | 24 +-- borgmatic/hooks/postgresql.py | 25 +-- borgmatic/hooks/sqlite.py | 28 +--- scripts/run-full-tests | 2 +- tests/end-to-end/docker-compose.yaml | 12 +- tests/end-to-end/test_database.py | 222 ++++++++++++++++++++++++--- tests/unit/actions/test_restore.py | 9 ++ tests/unit/hooks/test_mariadb.py | 65 +++----- tests/unit/hooks/test_mongodb.py | 112 +++++--------- tests/unit/hooks/test_mysql.py | 65 +++----- tests/unit/hooks/test_postgresql.py | 96 +++++------- tests/unit/hooks/test_sqlite.py | 41 ++--- 17 files changed, 421 insertions(+), 380 deletions(-) diff --git a/.drone.yml b/.drone.yml index ed1762ca..526ba44a 100644 --- a/.drone.yml +++ b/.drone.yml @@ -13,7 +13,6 @@ services: environment: POSTGRES_PASSWORD: test2 POSTGRES_DB: test - POSTGRES_USER: postgres2 commands: - docker-entrypoint.sh -p 5433 - name: mariadb @@ -28,6 +27,18 @@ services: MARIADB_DATABASE: test commands: - docker-entrypoint.sh --port=3307 + - name: not-actually-mysql + image: docker.io/mariadb:10.11.4 + environment: + MARIADB_ROOT_PASSWORD: test + MARIADB_DATABASE: test + - name: not-actually-mysql2 + image: docker.io/mariadb:10.11.4 + environment: + MARIADB_ROOT_PASSWORD: test2 + MARIADB_DATABASE: test + commands: + - docker-entrypoint.sh --port=3307 - name: mongodb image: docker.io/mongo:5.0.5 environment: diff --git a/NEWS b/NEWS index 065859f6..7b9b5c57 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,9 @@ * #727: Add a MariaDB database hook that uses native MariaDB commands instead of the deprecated MySQL ones. Be aware though that any existing backups made with the "mysql_databases:" hook are only restorable with a "mysql_databases:" configuration. + * #738: Fix for potential data loss (data not getting restored) in which the database "restore" + action didn't actually restore anything and indicated success anyway. + * Remove the deprecated use of the MongoDB hook's "--db" flag for database restoration. * Add source code reference documentation for getting oriented with the borgmatic code as a developer: https://torsion.org/borgmatic/docs/reference/source-code/ diff --git a/borgmatic/actions/restore.py b/borgmatic/actions/restore.py index 06fd1b87..6b41c2f5 100644 --- a/borgmatic/actions/restore.py +++ b/borgmatic/actions/restore.py @@ -27,7 +27,8 @@ def get_configured_database( hooks for the named database. If a configuration database name is given, use that instead of the database name to lookup the database in the given hooks configuration. - Return the found database as a tuple of (found hook name, database configuration dict). + Return the found database as a tuple of (found hook name, database configuration dict) or (None, + None) if not found. ''' if not configuration_database_name: configuration_database_name = database_name @@ -39,7 +40,10 @@ def get_configured_database( if hook_name in borgmatic.hooks.dump.DATABASE_HOOK_NAMES } else: - hooks_to_search = {hook_name: config[hook_name]} + try: + hooks_to_search = {hook_name: config[hook_name]} + except KeyError: + return (None, None) return next( ( @@ -73,9 +77,9 @@ def restore_single_database( connection_params, ): # pragma: no cover ''' - Given (among other things) an archive name, a database hook name, the hostname, - port, username and password as connection params, and a configured database - configuration dict, restore that database from the archive. + Given (among other things) an archive name, a database hook name, the hostname, port, + username/password as connection params, and a configured database configuration dict, restore + that database from the archive. ''' logger.info( f'{repository.get("label", repository["path"])}: Restoring database {database["name"]}' @@ -108,14 +112,14 @@ def restore_single_database( # Run a single database restore, consuming the extract stdout (if any). borgmatic.hooks.dispatch.call_hooks( - 'restore_database_dump', - config, - repository['path'], - database['name'], - borgmatic.hooks.dump.DATABASE_HOOK_NAMES, - global_arguments.dry_run, - extract_process, - connection_params, + function_name='restore_database_dump', + config=config, + log_prefix=repository['path'], + hook_names=[hook_name], + database=database, + dry_run=global_arguments.dry_run, + extract_process=extract_process, + connection_params=connection_params, ) @@ -333,7 +337,7 @@ def run_restore( connection_params, ) - # For any database that weren't found via exact matches in the configuration, try to fallback + # For any databases that weren't found via exact matches in the configuration, try to fallback # to "all" entries. for hook_name, database_names in remaining_restore_names.items(): for database_name in database_names: diff --git a/borgmatic/hooks/mariadb.py b/borgmatic/hooks/mariadb.py index 13246287..32569e84 100644 --- a/borgmatic/hooks/mariadb.py +++ b/borgmatic/hooks/mariadb.py @@ -184,28 +184,16 @@ def make_database_dump_pattern(databases, config, log_prefix, name=None): # pra def restore_database_dump( - databases_config, config, log_prefix, database_name, dry_run, extract_process, connection_params + hook_config, config, log_prefix, database, dry_run, extract_process, connection_params ): ''' - Restore the given MariaDB database from an extract stream. The databases are supplied as a - sequence containing one dict describing each database (as per the configuration schema), but - only the database corresponding to the given database name is restored. Use the given log prefix - in any log entries. If this is a dry run, then don't actually restore anything. Trigger the - given active extract process (an instance of subprocess.Popen) to produce output to consume. + Restore a database from the given extract stream. The database is supplied as a configuration + dict, but the given hook configuration is ignored. The given configuration dict is used to + construct the destination path, and the given log prefix is used for any log entries. If this is + a dry run, then don't actually restore anything. Trigger the given active extract process (an + instance of subprocess.Popen) to produce output to consume. ''' dry_run_label = ' (dry run; not actually restoring anything)' if dry_run else '' - - try: - database = next( - database_config - for database_config in databases_config - if database_config.get('name') == database_name - ) - except StopIteration: - raise ValueError( - f'A database named "{database_name}" could not be found in the configuration' - ) - hostname = connection_params['hostname'] or database.get( 'restore_hostname', database.get('hostname') ) diff --git a/borgmatic/hooks/mongodb.py b/borgmatic/hooks/mongodb.py index 0f8cc2c6..3c4bb093 100644 --- a/borgmatic/hooks/mongodb.py +++ b/borgmatic/hooks/mongodb.py @@ -97,32 +97,19 @@ def make_database_dump_pattern(databases, config, log_prefix, name=None): # pra def restore_database_dump( - databases_config, config, log_prefix, database_name, dry_run, extract_process, connection_params + hook_config, config, log_prefix, database, dry_run, extract_process, connection_params ): ''' - Restore the given MongoDB database from an extract stream. The databases are supplied as a - sequence containing one dict describing each database (as per the configuration schema), but - only the database corresponding to the given database name is restored. Use the configuration - dict to construct the destination path and the given log prefix in any log entries. If this is a - dry run, then don't actually restore anything. Trigger the given active extract process (an + Restore a database from the given extract stream. The database is supplied as a configuration + dict, but the given hook configuration is ignored. The given configuration dict is used to + construct the destination path, and the given log prefix is used for any log entries. If this is + a dry run, then don't actually restore anything. Trigger the given active extract process (an instance of subprocess.Popen) to produce output to consume. If the extract process is None, then restore the dump from the filesystem rather than from an extract stream. ''' dry_run_label = ' (dry run; not actually restoring anything)' if dry_run else '' - - try: - database = next( - database_config - for database_config in databases_config - if database_config.get('name') == database_name - ) - except StopIteration: - raise ValueError( - f'A database named "{database_name}" could not be found in the configuration' - ) - dump_filename = dump.make_database_dump_filename( make_dump_path(config), database['name'], database.get('hostname') ) @@ -165,7 +152,7 @@ def build_restore_command(extract_process, database, dump_filename, connection_p else: command.extend(('--dir', dump_filename)) if database['name'] != 'all': - command.extend(('--drop', '--db', database['name'])) + command.extend(('--drop',)) if hostname: command.extend(('--host', hostname)) if port: @@ -178,7 +165,8 @@ def build_restore_command(extract_process, database, dump_filename, connection_p command.extend(('--authenticationDatabase', database['authentication_database'])) if 'restore_options' in database: command.extend(database['restore_options'].split(' ')) - if database['schemas']: + if database.get('schemas'): for schema in database['schemas']: command.extend(('--nsInclude', schema)) + return command diff --git a/borgmatic/hooks/mysql.py b/borgmatic/hooks/mysql.py index a3b34f15..434fdbd0 100644 --- a/borgmatic/hooks/mysql.py +++ b/borgmatic/hooks/mysql.py @@ -181,28 +181,16 @@ def make_database_dump_pattern(databases, config, log_prefix, name=None): # pra def restore_database_dump( - databases_config, config, log_prefix, database_name, dry_run, extract_process, connection_params + hook_config, config, log_prefix, database, dry_run, extract_process, connection_params ): ''' - Restore the given MySQL/MariaDB database from an extract stream. The databases are supplied as a - sequence containing one dict describing each database (as per the configuration schema), but - only the database corresponding to the given database name is restored. Use the given log - prefix in any log entries. If this is a dry run, then don't actually restore anything. Trigger - the given active extract process (an instance of subprocess.Popen) to produce output to consume. + Restore a database from the given extract stream. The database is supplied as a configuration + dict, but the given hook configuration is ignored. The given configuration dict is used to + construct the destination path, and the given log prefix is used for any log entries. If this is + a dry run, then don't actually restore anything. Trigger the given active extract process (an + instance of subprocess.Popen) to produce output to consume. ''' dry_run_label = ' (dry run; not actually restoring anything)' if dry_run else '' - - try: - database = next( - database_config - for database_config in databases_config - if database_config.get('name') == database_name - ) - except StopIteration: - raise ValueError( - f'A database named "{database_name}" could not be found in the configuration' - ) - hostname = connection_params['hostname'] or database.get( 'restore_hostname', database.get('hostname') ) diff --git a/borgmatic/hooks/postgresql.py b/borgmatic/hooks/postgresql.py index 598b878c..6cfb79e3 100644 --- a/borgmatic/hooks/postgresql.py +++ b/borgmatic/hooks/postgresql.py @@ -202,15 +202,14 @@ def make_database_dump_pattern(databases, config, log_prefix, name=None): # pra def restore_database_dump( - databases_config, config, log_prefix, database_name, dry_run, extract_process, connection_params + hook_config, config, log_prefix, database, dry_run, extract_process, connection_params ): ''' - Restore the given PostgreSQL database from an extract stream. The databases are supplied as a - sequence containing one dict describing each database (as per the configuration schema), but - only the database corresponding to the given database name is restored. Use the given - configuration dict to construct the destination path and the given log prefix in any log - entries. If this is a dry run, then don't actually restore anything. Trigger the given active - extract process (an instance of subprocess.Popen) to produce output to consume. + Restore a database from the given extract stream. The database is supplied as a configuration + dict, but the given hook configuration is ignored. The given configuration dict is used to + construct the destination path, and the given log prefix is used for any log entries. If this is + a dry run, then don't actually restore anything. Trigger the given active extract process (an + instance of subprocess.Popen) to produce output to consume. If the extract process is None, then restore the dump from the filesystem rather than from an extract stream. @@ -219,18 +218,6 @@ def restore_database_dump( hostname, port, username, and password. ''' dry_run_label = ' (dry run; not actually restoring anything)' if dry_run else '' - - try: - database = next( - database_config - for database_config in databases_config - if database_config.get('name') == database_name - ) - except StopIteration: - raise ValueError( - f'A database named "{database_name}" could not be found in the configuration' - ) - hostname = connection_params['hostname'] or database.get( 'restore_hostname', database.get('hostname') ) diff --git a/borgmatic/hooks/sqlite.py b/borgmatic/hooks/sqlite.py index 524318bc..3b2bae50 100644 --- a/borgmatic/hooks/sqlite.py +++ b/borgmatic/hooks/sqlite.py @@ -32,10 +32,10 @@ def dump_databases(databases, config, log_prefix, dry_run): database_path = database['path'] if database['name'] == 'all': - logger.warning('The "all" database name has no meaning for SQLite3 databases') + logger.warning('The "all" database name has no meaning for SQLite databases') if not os.path.exists(database_path): logger.warning( - f'{log_prefix}: No SQLite database at {database_path}; An empty database will be created and dumped' + f'{log_prefix}: No SQLite database at {database_path}; an empty database will be created and dumped' ) dump_path = make_dump_path(config) @@ -84,28 +84,16 @@ def make_database_dump_pattern(databases, config, log_prefix, name=None): # pra def restore_database_dump( - databases_config, config, log_prefix, database_name, dry_run, extract_process, connection_params + hook_config, config, log_prefix, database, dry_run, extract_process, connection_params ): ''' - Restore the given SQLite3 database from an extract stream. The databases are supplied as a - sequence containing one dict describing each database (as per the configuration schema), but - only the database corresponding to the given database name is restored. Use the given log prefix - in any log entries. If this is a dry run, then don't actually restore anything. Trigger the - given active extract process (an instance of subprocess.Popen) to produce output to consume. + Restore a database from the given extract stream. The database is supplied as a configuration + dict, but the given hook configuration is ignored. The given configuration dict is used to + construct the destination path, and the given log prefix is used for any log entries. If this is + a dry run, then don't actually restore anything. Trigger the given active extract process (an + instance of subprocess.Popen) to produce output to consume. ''' dry_run_label = ' (dry run; not actually restoring anything)' if dry_run else '' - - try: - database = next( - database_config - for database_config in databases_config - if database_config.get('name') == database_name - ) - except StopIteration: - raise ValueError( - f'A database named "{database_name}" could not be found in the configuration' - ) - database_path = connection_params['restore_path'] or database.get( 'restore_path', database.get('path') ) diff --git a/scripts/run-full-tests b/scripts/run-full-tests index a7a49a2a..b75cb399 100755 --- a/scripts/run-full-tests +++ b/scripts/run-full-tests @@ -21,7 +21,7 @@ apk add --no-cache python3 py3-pip borgbackup postgresql-client mariadb-client m py3-ruamel.yaml py3-ruamel.yaml.clib bash sqlite fish # If certain dependencies of black are available in this version of Alpine, install them. apk add --no-cache py3-typed-ast py3-regex || true -python3 -m pip install --no-cache --upgrade pip==22.2.2 setuptools==64.0.1 +python3 -m pip install --no-cache --upgrade pip==22.2.2 setuptools==64.0.1 pymongo==4.4.1 pip3 install --ignore-installed tox==3.25.1 export COVERAGE_FILE=/tmp/.coverage diff --git a/tests/end-to-end/docker-compose.yaml b/tests/end-to-end/docker-compose.yaml index f87beb59..3fe3afd0 100644 --- a/tests/end-to-end/docker-compose.yaml +++ b/tests/end-to-end/docker-compose.yaml @@ -10,7 +10,6 @@ services: environment: POSTGRES_PASSWORD: test2 POSTGRES_DB: test - POSTGRES_USER: postgres2 command: docker-entrypoint.sh -p 5433 mariadb: image: docker.io/mariadb:10.11.4 @@ -23,6 +22,17 @@ services: MARIADB_ROOT_PASSWORD: test2 MARIADB_DATABASE: test command: docker-entrypoint.sh --port=3307 + not-actually-mysql: + image: docker.io/mariadb:10.11.4 + environment: + MARIADB_ROOT_PASSWORD: test + MARIADB_DATABASE: test + not-actually-mysql2: + image: docker.io/mariadb:10.11.4 + environment: + MARIADB_ROOT_PASSWORD: test2 + MARIADB_DATABASE: test + command: docker-entrypoint.sh --port=3307 mongodb: image: docker.io/mongo:5.0.5 environment: diff --git a/tests/end-to-end/test_database.py b/tests/end-to-end/test_database.py index 1ab02701..a307a9a1 100644 --- a/tests/end-to-end/test_database.py +++ b/tests/end-to-end/test_database.py @@ -5,7 +5,9 @@ import subprocess import sys import tempfile +import pymongo import pytest +import ruamel.yaml def write_configuration( @@ -21,7 +23,7 @@ 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. ''' - config = f''' + config_yaml = f''' source_directories: - {source_directory} repositories: @@ -61,16 +63,16 @@ mariadb_databases: password: test mysql_databases: - name: test - hostname: mariadb + hostname: not-actually-mysql username: root password: test - name: all - hostname: mariadb + hostname: not-actually-mysql username: root password: test - name: all format: sql - hostname: mariadb + hostname: not-actually-mysql username: root password: test mongodb_databases: @@ -90,7 +92,9 @@ sqlite_databases: ''' with open(config_path, 'w') as config_file: - config_file.write(config) + config_file.write(config_yaml) + + return ruamel.yaml.YAML(typ='safe').load(config_yaml) def write_custom_restore_configuration( @@ -106,7 +110,7 @@ def write_custom_restore_configuration( for testing with custom restore options. This includes a custom restore_hostname, restore_port, restore_username, restore_password and restore_path. ''' - config = f''' + config_yaml = f''' source_directories: - {source_directory} repositories: @@ -123,7 +127,6 @@ postgresql_databases: format: {postgresql_dump_format} restore_hostname: postgresql2 restore_port: 5433 - restore_username: postgres2 restore_password: test2 mariadb_databases: - name: test @@ -136,10 +139,10 @@ mariadb_databases: restore_password: test2 mysql_databases: - name: test - hostname: mariadb + hostname: not-actually-mysql username: root password: test - restore_hostname: mariadb2 + restore_hostname: not-actually-mysql2 restore_port: 3307 restore_username: root restore_password: test2 @@ -161,7 +164,9 @@ sqlite_databases: ''' with open(config_path, 'w') as config_file: - config_file.write(config) + config_file.write(config_yaml) + + return ruamel.yaml.YAML(typ='safe').load(config_yaml) def write_simple_custom_restore_configuration( @@ -177,7 +182,7 @@ def write_simple_custom_restore_configuration( custom restore_hostname, restore_port, restore_username and restore_password as we only test these options for PostgreSQL. ''' - config = f''' + config_yaml = f''' source_directories: - {source_directory} repositories: @@ -195,7 +200,147 @@ postgresql_databases: ''' with open(config_path, 'w') as config_file: - config_file.write(config) + config_file.write(config_yaml) + + return ruamel.yaml.YAML(typ='safe').load(config_yaml) + + +def get_connection_params(database, use_restore_options=False): + hostname = (database.get('restore_hostname') if use_restore_options else None) or database.get( + 'hostname' + ) + port = (database.get('restore_port') if use_restore_options else None) or database.get('port') + username = (database.get('restore_username') if use_restore_options else None) or database.get( + 'username' + ) + password = (database.get('restore_password') if use_restore_options else None) or database.get( + 'password' + ) + + return (hostname, port, username, password) + + +def run_postgresql_command(command, config, use_restore_options=False): + (hostname, port, username, password) = get_connection_params( + config['postgresql_databases'][0], use_restore_options + ) + + subprocess.check_call( + [ + '/usr/bin/psql', + f'--host={hostname}', + f'--port={port or 5432}', + f"--username={username or 'root'}", + f'--command={command}', + 'test', + ], + env={'PGPASSWORD': password}, + ) + + +def run_mariadb_command(command, config, use_restore_options=False, binary_name='mariadb'): + (hostname, port, username, password) = get_connection_params( + config[f'{binary_name}_databases'][0], use_restore_options + ) + + subprocess.check_call( + [ + f'/usr/bin/{binary_name}', + f'--host={hostname}', + f'--port={port or 3306}', + f'--user={username}', + f'--execute={command}', + 'test', + ], + env={'MYSQL_PWD': password}, + ) + + +def get_mongodb_database_client(config, use_restore_options=False): + (hostname, port, username, password) = get_connection_params( + config['mongodb_databases'][0], use_restore_options + ) + + return pymongo.MongoClient(f'mongodb://{username}:{password}@{hostname}:{port or 27017}').test + + +def run_sqlite_command(command, config, use_restore_options=False): + database = config['sqlite_databases'][0] + path = (database.get('restore_path') if use_restore_options else None) or database.get('path') + + subprocess.check_call( + [ + '/usr/bin/sqlite3', + path, + command, + '.exit', + ], + ) + + +DEFAULT_HOOK_NAMES = {'postgresql', 'mariadb', 'mysql', 'mongodb', 'sqlite'} + + +def create_test_tables(config, use_restore_options=False): + ''' + Create test tables for borgmatic to dump and backup. + ''' + command = 'create table test{id} (thing int); insert into test{id} values (1);' + + if 'postgresql_databases' in config: + run_postgresql_command(command.format(id=1), config, use_restore_options) + if 'mariadb_databases' in config: + run_mariadb_command(command.format(id=2), config, use_restore_options) + if 'mysql_databases' in config: + run_mariadb_command(command.format(id=3), config, use_restore_options, binary_name='mysql') + if 'mongodb_databases' in config: + get_mongodb_database_client(config, use_restore_options)['test4'].insert_one({'thing': 1}) + if 'sqlite_databases' in config: + run_sqlite_command(command.format(id=5), config, use_restore_options) + + +def drop_test_tables(config, use_restore_options=False): + ''' + Drop the test tables in preparation for borgmatic restoring them. + ''' + command = 'drop table if exists test{id};' + + if 'postgresql_databases' in config: + run_postgresql_command(command.format(id=1), config, use_restore_options) + if 'mariadb_databases' in config: + run_mariadb_command(command.format(id=2), config, use_restore_options) + if 'mysql_databases' in config: + run_mariadb_command(command.format(id=3), config, use_restore_options, binary_name='mysql') + if 'mongodb_databases' in config: + get_mongodb_database_client(config, use_restore_options)['test4'].drop() + if 'sqlite_databases' in config: + run_sqlite_command(command.format(id=5), config, use_restore_options) + + +def select_test_tables(config, use_restore_options=False): + ''' + Select the test tables to make sure they exist. + + Raise if the expected tables cannot be selected, for instance if a restore hasn't worked as + expected. + ''' + command = 'select count(*) from test{id};' + + if 'postgresql_databases' in config: + run_postgresql_command(command.format(id=1), config, use_restore_options) + if 'mariadb_databases' in config: + run_mariadb_command(command.format(id=2), config, use_restore_options) + if 'mysql_databases' in config: + run_mariadb_command(command.format(id=3), config, use_restore_options, binary_name='mysql') + if 'mongodb_databases' in config: + assert ( + get_mongodb_database_client(config, use_restore_options)['test4'].count_documents( + filter={} + ) + > 0 + ) + if 'sqlite_databases' in config: + run_sqlite_command(command.format(id=5), config, use_restore_options) def test_database_dump_and_restore(): @@ -211,15 +356,17 @@ def test_database_dump_and_restore(): try: config_path = os.path.join(temporary_directory, 'test.yaml') - write_configuration( + config = write_configuration( temporary_directory, config_path, repository_path, borgmatic_source_directory ) + create_test_tables(config) + select_test_tables(config) subprocess.check_call( ['borgmatic', '-v', '2', '--config', config_path, 'rcreate', '--encryption', 'repokey'] ) - # Run borgmatic to generate a backup archive including a database dump. + # Run borgmatic to generate a backup archive including database dumps. subprocess.check_call(['borgmatic', 'create', '--config', config_path, '-v', '2']) # Get the created archive name. @@ -232,16 +379,21 @@ def test_database_dump_and_restore(): assert len(parsed_output[0]['archives']) == 1 archive_name = parsed_output[0]['archives'][0]['archive'] - # Restore the database from the archive. + # Restore the databases from the archive. + drop_test_tables(config) subprocess.check_call( ['borgmatic', '-v', '2', '--config', config_path, 'restore', '--archive', archive_name] ) + + # Ensure the test tables have actually been restored. + select_test_tables(config) finally: os.chdir(original_working_directory) shutil.rmtree(temporary_directory) + drop_test_tables(config) -def test_database_dump_and_restore_with_restore_cli_arguments(): +def test_database_dump_and_restore_with_restore_cli_flags(): # Create a Borg repository. temporary_directory = tempfile.mkdtemp() repository_path = os.path.join(temporary_directory, 'test.borg') @@ -251,9 +403,11 @@ def test_database_dump_and_restore_with_restore_cli_arguments(): try: config_path = os.path.join(temporary_directory, 'test.yaml') - write_simple_custom_restore_configuration( + config = write_simple_custom_restore_configuration( temporary_directory, config_path, repository_path, borgmatic_source_directory ) + create_test_tables(config) + select_test_tables(config) subprocess.check_call( ['borgmatic', '-v', '2', '--config', config_path, 'rcreate', '--encryption', 'repokey'] @@ -273,6 +427,7 @@ def test_database_dump_and_restore_with_restore_cli_arguments(): archive_name = parsed_output[0]['archives'][0]['archive'] # Restore the database from the archive. + drop_test_tables(config) subprocess.check_call( [ 'borgmatic', @@ -287,15 +442,25 @@ def test_database_dump_and_restore_with_restore_cli_arguments(): 'postgresql2', '--port', '5433', - '--username', - 'postgres2', '--password', 'test2', ] ) + + # Ensure the test tables have actually been restored. But first modify the config to contain + # the altered restore values from the borgmatic command above. This ensures that the test + # tables are selected from the correct database. + database = config['postgresql_databases'][0] + database['restore_hostname'] = 'postgresql2' + database['restore_port'] = '5433' + database['restore_password'] = 'test2' + + select_test_tables(config, use_restore_options=True) finally: os.chdir(original_working_directory) shutil.rmtree(temporary_directory) + drop_test_tables(config) + drop_test_tables(config, use_restore_options=True) def test_database_dump_and_restore_with_restore_configuration_options(): @@ -308,9 +473,11 @@ def test_database_dump_and_restore_with_restore_configuration_options(): try: config_path = os.path.join(temporary_directory, 'test.yaml') - write_custom_restore_configuration( + config = write_custom_restore_configuration( temporary_directory, config_path, repository_path, borgmatic_source_directory ) + create_test_tables(config) + select_test_tables(config) subprocess.check_call( ['borgmatic', '-v', '2', '--config', config_path, 'rcreate', '--encryption', 'repokey'] @@ -330,12 +497,18 @@ def test_database_dump_and_restore_with_restore_configuration_options(): archive_name = parsed_output[0]['archives'][0]['archive'] # Restore the database from the archive. + drop_test_tables(config) subprocess.check_call( ['borgmatic', '-v', '2', '--config', config_path, 'restore', '--archive', archive_name] ) + + # Ensure the test tables have actually been restored. + select_test_tables(config, use_restore_options=True) finally: os.chdir(original_working_directory) shutil.rmtree(temporary_directory) + drop_test_tables(config) + drop_test_tables(config, use_restore_options=True) def test_database_dump_and_restore_with_directory_format(): @@ -348,7 +521,7 @@ def test_database_dump_and_restore_with_directory_format(): try: config_path = os.path.join(temporary_directory, 'test.yaml') - write_configuration( + config = write_configuration( temporary_directory, config_path, repository_path, @@ -356,6 +529,8 @@ def test_database_dump_and_restore_with_directory_format(): postgresql_dump_format='directory', mongodb_dump_format='directory', ) + create_test_tables(config) + select_test_tables(config) subprocess.check_call( ['borgmatic', '-v', '2', '--config', config_path, 'rcreate', '--encryption', 'repokey'] @@ -365,12 +540,17 @@ def test_database_dump_and_restore_with_directory_format(): subprocess.check_call(['borgmatic', 'create', '--config', config_path, '-v', '2']) # Restore the database from the archive. + drop_test_tables(config) subprocess.check_call( ['borgmatic', '--config', config_path, 'restore', '--archive', 'latest'] ) + + # Ensure the test tables have actually been restored. + select_test_tables(config) finally: os.chdir(original_working_directory) shutil.rmtree(temporary_directory) + drop_test_tables(config) def test_database_dump_with_error_causes_borgmatic_to_exit(): diff --git a/tests/unit/actions/test_restore.py b/tests/unit/actions/test_restore.py index 35488808..ac4ba581 100644 --- a/tests/unit/actions/test_restore.py +++ b/tests/unit/actions/test_restore.py @@ -16,6 +16,15 @@ def test_get_configured_database_matches_database_by_name(): ) == ('postgresql_databases', {'name': 'bar'}) +def test_get_configured_database_matches_nothing_when_nothing_configured(): + assert module.get_configured_database( + config={}, + archive_database_names={'postgresql_databases': ['foo']}, + hook_name='postgresql_databases', + database_name='quux', + ) == (None, None) + + def test_get_configured_database_matches_nothing_when_database_name_not_configured(): assert module.get_configured_database( config={'postgresql_databases': [{'name': 'foo'}, {'name': 'bar'}]}, diff --git a/tests/unit/hooks/test_mariadb.py b/tests/unit/hooks/test_mariadb.py index cc7f3fd6..c17ff46a 100644 --- a/tests/unit/hooks/test_mariadb.py +++ b/tests/unit/hooks/test_mariadb.py @@ -380,7 +380,7 @@ def test_dump_databases_does_not_error_for_missing_all_databases_with_dry_run(): def test_restore_database_dump_runs_mariadb_to_restore(): - databases_config = [{'name': 'foo'}, {'name': 'bar'}] + hook_config = [{'name': 'foo'}, {'name': 'bar'}] extract_process = flexmock(stdout=flexmock()) flexmock(module).should_receive('execute_command_with_processes').with_args( @@ -392,10 +392,10 @@ def test_restore_database_dump_runs_mariadb_to_restore(): ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database={'name': 'foo'}, dry_run=False, extract_process=extract_process, connection_params={ @@ -407,31 +407,8 @@ def test_restore_database_dump_runs_mariadb_to_restore(): ) -def test_restore_database_dump_errors_when_database_missing_from_configuration(): - databases_config = [{'name': 'foo'}, {'name': 'bar'}] - extract_process = flexmock(stdout=flexmock()) - - flexmock(module).should_receive('execute_command_with_processes').never() - - with pytest.raises(ValueError): - module.restore_database_dump( - databases_config, - {}, - 'test.yaml', - database_name='other', - dry_run=False, - extract_process=extract_process, - connection_params={ - 'hostname': None, - 'port': None, - 'username': None, - 'password': None, - }, - ) - - def test_restore_database_dump_runs_mariadb_with_options(): - databases_config = [{'name': 'foo', 'restore_options': '--harder'}] + hook_config = [{'name': 'foo', 'restore_options': '--harder'}] extract_process = flexmock(stdout=flexmock()) flexmock(module).should_receive('execute_command_with_processes').with_args( @@ -443,10 +420,10 @@ def test_restore_database_dump_runs_mariadb_with_options(): ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database=hook_config[0], dry_run=False, extract_process=extract_process, connection_params={ @@ -459,7 +436,7 @@ def test_restore_database_dump_runs_mariadb_with_options(): def test_restore_database_dump_runs_mariadb_with_hostname_and_port(): - databases_config = [{'name': 'foo', 'hostname': 'database.example.org', 'port': 5433}] + hook_config = [{'name': 'foo', 'hostname': 'database.example.org', 'port': 5433}] extract_process = flexmock(stdout=flexmock()) flexmock(module).should_receive('execute_command_with_processes').with_args( @@ -480,10 +457,10 @@ def test_restore_database_dump_runs_mariadb_with_hostname_and_port(): ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database=hook_config[0], dry_run=False, extract_process=extract_process, connection_params={ @@ -496,7 +473,7 @@ def test_restore_database_dump_runs_mariadb_with_hostname_and_port(): def test_restore_database_dump_runs_mariadb_with_username_and_password(): - databases_config = [{'name': 'foo', 'username': 'root', 'password': 'trustsome1'}] + hook_config = [{'name': 'foo', 'username': 'root', 'password': 'trustsome1'}] extract_process = flexmock(stdout=flexmock()) flexmock(module).should_receive('execute_command_with_processes').with_args( @@ -508,10 +485,10 @@ def test_restore_database_dump_runs_mariadb_with_username_and_password(): ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database=hook_config[0], dry_run=False, extract_process=extract_process, connection_params={ @@ -524,7 +501,7 @@ def test_restore_database_dump_runs_mariadb_with_username_and_password(): def test_restore_database_dump_with_connection_params_uses_connection_params_for_restore(): - databases_config = [ + hook_config = [ { 'name': 'foo', 'username': 'root', @@ -557,10 +534,10 @@ def test_restore_database_dump_with_connection_params_uses_connection_params_for ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database=hook_config[0], dry_run=False, extract_process=extract_process, connection_params={ @@ -573,7 +550,7 @@ def test_restore_database_dump_with_connection_params_uses_connection_params_for def test_restore_database_dump_without_connection_params_uses_restore_params_in_config_for_restore(): - databases_config = [ + hook_config = [ { 'name': 'foo', 'username': 'root', @@ -608,10 +585,10 @@ def test_restore_database_dump_without_connection_params_uses_restore_params_in_ ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database=hook_config[0], dry_run=False, extract_process=extract_process, connection_params={ @@ -624,15 +601,15 @@ def test_restore_database_dump_without_connection_params_uses_restore_params_in_ def test_restore_database_dump_with_dry_run_skips_restore(): - databases_config = [{'name': 'foo'}] + hook_config = [{'name': 'foo'}] flexmock(module).should_receive('execute_command_with_processes').never() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database={'name': 'foo'}, dry_run=True, extract_process=flexmock(), connection_params={ diff --git a/tests/unit/hooks/test_mongodb.py b/tests/unit/hooks/test_mongodb.py index 83d75d34..580d7654 100644 --- a/tests/unit/hooks/test_mongodb.py +++ b/tests/unit/hooks/test_mongodb.py @@ -1,6 +1,5 @@ import logging -import pytest from flexmock import flexmock from borgmatic.hooks import mongodb as module @@ -131,7 +130,15 @@ def test_dump_databases_runs_mongodump_with_options(): flexmock(module.dump).should_receive('create_named_pipe_for_dump') flexmock(module).should_receive('execute_command').with_args( - ('mongodump', '--db', 'foo', '--stuff=such', '--archive', '>', 'databases/localhost/foo'), + ( + 'mongodump', + '--db', + 'foo', + '--stuff=such', + '--archive', + '>', + 'databases/localhost/foo', + ), shell=True, run_to_completion=False, ).and_return(process).once() @@ -158,23 +165,23 @@ def test_dump_databases_runs_mongodumpall_for_all_databases(): def test_restore_database_dump_runs_mongorestore(): - databases_config = [{'name': 'foo', 'schemas': None}, {'name': 'bar'}] + hook_config = [{'name': 'foo', 'schemas': None}, {'name': 'bar'}] extract_process = flexmock(stdout=flexmock()) flexmock(module).should_receive('make_dump_path') flexmock(module.dump).should_receive('make_database_dump_filename') flexmock(module).should_receive('execute_command_with_processes').with_args( - ['mongorestore', '--archive', '--drop', '--db', 'foo'], + ['mongorestore', '--archive', '--drop'], processes=[extract_process], output_log_level=logging.DEBUG, input_file=extract_process.stdout, ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database={'name': 'foo'}, dry_run=False, extract_process=extract_process, connection_params={ @@ -186,33 +193,8 @@ def test_restore_database_dump_runs_mongorestore(): ) -def test_restore_database_dump_errors_on_empty_databases_config(): - databases_config = [] - - flexmock(module).should_receive('make_dump_path') - flexmock(module.dump).should_receive('make_database_dump_filename') - flexmock(module).should_receive('execute_command_with_processes').never() - flexmock(module).should_receive('execute_command').never() - - with pytest.raises(ValueError): - module.restore_database_dump( - databases_config, - {}, - 'test.yaml', - database_name='foo', - dry_run=False, - extract_process=flexmock(), - connection_params={ - 'hostname': None, - 'port': None, - 'username': None, - 'password': None, - }, - ) - - def test_restore_database_dump_runs_mongorestore_with_hostname_and_port(): - databases_config = [ + hook_config = [ {'name': 'foo', 'hostname': 'database.example.org', 'port': 5433, 'schemas': None} ] extract_process = flexmock(stdout=flexmock()) @@ -224,8 +206,6 @@ def test_restore_database_dump_runs_mongorestore_with_hostname_and_port(): 'mongorestore', '--archive', '--drop', - '--db', - 'foo', '--host', 'database.example.org', '--port', @@ -237,10 +217,10 @@ def test_restore_database_dump_runs_mongorestore_with_hostname_and_port(): ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database=hook_config[0], dry_run=False, extract_process=extract_process, connection_params={ @@ -253,7 +233,7 @@ def test_restore_database_dump_runs_mongorestore_with_hostname_and_port(): def test_restore_database_dump_runs_mongorestore_with_username_and_password(): - databases_config = [ + hook_config = [ { 'name': 'foo', 'username': 'mongo', @@ -271,8 +251,6 @@ def test_restore_database_dump_runs_mongorestore_with_username_and_password(): 'mongorestore', '--archive', '--drop', - '--db', - 'foo', '--username', 'mongo', '--password', @@ -286,10 +264,10 @@ def test_restore_database_dump_runs_mongorestore_with_username_and_password(): ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database=hook_config[0], dry_run=False, extract_process=extract_process, connection_params={ @@ -302,7 +280,7 @@ def test_restore_database_dump_runs_mongorestore_with_username_and_password(): def test_restore_database_dump_with_connection_params_uses_connection_params_for_restore(): - databases_config = [ + hook_config = [ { 'name': 'foo', 'username': 'mongo', @@ -324,8 +302,6 @@ def test_restore_database_dump_with_connection_params_uses_connection_params_for 'mongorestore', '--archive', '--drop', - '--db', - 'foo', '--host', 'clihost', '--port', @@ -343,10 +319,10 @@ def test_restore_database_dump_with_connection_params_uses_connection_params_for ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database=hook_config[0], dry_run=False, extract_process=extract_process, connection_params={ @@ -359,7 +335,7 @@ def test_restore_database_dump_with_connection_params_uses_connection_params_for def test_restore_database_dump_without_connection_params_uses_restore_params_in_config_for_restore(): - databases_config = [ + hook_config = [ { 'name': 'foo', 'username': 'mongo', @@ -381,8 +357,6 @@ def test_restore_database_dump_without_connection_params_uses_restore_params_in_ 'mongorestore', '--archive', '--drop', - '--db', - 'foo', '--host', 'restorehost', '--port', @@ -400,10 +374,10 @@ def test_restore_database_dump_without_connection_params_uses_restore_params_in_ ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database=hook_config[0], dry_run=False, extract_process=extract_process, connection_params={ @@ -416,23 +390,23 @@ def test_restore_database_dump_without_connection_params_uses_restore_params_in_ def test_restore_database_dump_runs_mongorestore_with_options(): - databases_config = [{'name': 'foo', 'restore_options': '--harder', 'schemas': None}] + hook_config = [{'name': 'foo', 'restore_options': '--harder', 'schemas': None}] extract_process = flexmock(stdout=flexmock()) flexmock(module).should_receive('make_dump_path') flexmock(module.dump).should_receive('make_database_dump_filename') flexmock(module).should_receive('execute_command_with_processes').with_args( - ['mongorestore', '--archive', '--drop', '--db', 'foo', '--harder'], + ['mongorestore', '--archive', '--drop', '--harder'], processes=[extract_process], output_log_level=logging.DEBUG, input_file=extract_process.stdout, ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database=hook_config[0], dry_run=False, extract_process=extract_process, connection_params={ @@ -445,7 +419,7 @@ def test_restore_database_dump_runs_mongorestore_with_options(): def test_restore_databases_dump_runs_mongorestore_with_schemas(): - databases_config = [{'name': 'foo', 'schemas': ['bar', 'baz']}] + hook_config = [{'name': 'foo', 'schemas': ['bar', 'baz']}] extract_process = flexmock(stdout=flexmock()) flexmock(module).should_receive('make_dump_path') @@ -455,8 +429,6 @@ def test_restore_databases_dump_runs_mongorestore_with_schemas(): 'mongorestore', '--archive', '--drop', - '--db', - 'foo', '--nsInclude', 'bar', '--nsInclude', @@ -468,10 +440,10 @@ def test_restore_databases_dump_runs_mongorestore_with_schemas(): ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database=hook_config[0], dry_run=False, extract_process=extract_process, connection_params={ @@ -484,7 +456,7 @@ def test_restore_databases_dump_runs_mongorestore_with_schemas(): def test_restore_database_dump_runs_psql_for_all_database_dump(): - databases_config = [{'name': 'all', 'schemas': None}] + hook_config = [{'name': 'all', 'schemas': None}] extract_process = flexmock(stdout=flexmock()) flexmock(module).should_receive('make_dump_path') @@ -497,10 +469,10 @@ def test_restore_database_dump_runs_psql_for_all_database_dump(): ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='all', + database=hook_config[0], dry_run=False, extract_process=extract_process, connection_params={ @@ -513,17 +485,17 @@ def test_restore_database_dump_runs_psql_for_all_database_dump(): def test_restore_database_dump_with_dry_run_skips_restore(): - databases_config = [{'name': 'foo', 'schemas': None}] + hook_config = [{'name': 'foo', 'schemas': None}] flexmock(module).should_receive('make_dump_path') flexmock(module.dump).should_receive('make_database_dump_filename') flexmock(module).should_receive('execute_command_with_processes').never() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database={'name': 'foo'}, dry_run=True, extract_process=flexmock(), connection_params={ @@ -536,22 +508,22 @@ def test_restore_database_dump_with_dry_run_skips_restore(): def test_restore_database_dump_without_extract_process_restores_from_disk(): - databases_config = [{'name': 'foo', 'format': 'directory', 'schemas': None}] + hook_config = [{'name': 'foo', 'format': 'directory', 'schemas': None}] flexmock(module).should_receive('make_dump_path') flexmock(module.dump).should_receive('make_database_dump_filename').and_return('/dump/path') flexmock(module).should_receive('execute_command_with_processes').with_args( - ['mongorestore', '--dir', '/dump/path', '--drop', '--db', 'foo'], + ['mongorestore', '--dir', '/dump/path', '--drop'], processes=[], output_log_level=logging.DEBUG, input_file=None, ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database={'name': 'foo'}, dry_run=False, extract_process=None, connection_params={ diff --git a/tests/unit/hooks/test_mysql.py b/tests/unit/hooks/test_mysql.py index 55abebb1..c20107d1 100644 --- a/tests/unit/hooks/test_mysql.py +++ b/tests/unit/hooks/test_mysql.py @@ -380,7 +380,7 @@ def test_dump_databases_does_not_error_for_missing_all_databases_with_dry_run(): def test_restore_database_dump_runs_mysql_to_restore(): - databases_config = [{'name': 'foo'}, {'name': 'bar'}] + hook_config = [{'name': 'foo'}, {'name': 'bar'}] extract_process = flexmock(stdout=flexmock()) flexmock(module).should_receive('execute_command_with_processes').with_args( @@ -392,10 +392,10 @@ def test_restore_database_dump_runs_mysql_to_restore(): ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database={'name': 'foo'}, dry_run=False, extract_process=extract_process, connection_params={ @@ -407,31 +407,8 @@ def test_restore_database_dump_runs_mysql_to_restore(): ) -def test_restore_database_dump_errors_when_database_missing_from_configuration(): - databases_config = [{'name': 'foo'}, {'name': 'bar'}] - extract_process = flexmock(stdout=flexmock()) - - flexmock(module).should_receive('execute_command_with_processes').never() - - with pytest.raises(ValueError): - module.restore_database_dump( - databases_config, - {}, - 'test.yaml', - database_name='other', - dry_run=False, - extract_process=extract_process, - connection_params={ - 'hostname': None, - 'port': None, - 'username': None, - 'password': None, - }, - ) - - def test_restore_database_dump_runs_mysql_with_options(): - databases_config = [{'name': 'foo', 'restore_options': '--harder'}] + hook_config = [{'name': 'foo', 'restore_options': '--harder'}] extract_process = flexmock(stdout=flexmock()) flexmock(module).should_receive('execute_command_with_processes').with_args( @@ -443,10 +420,10 @@ def test_restore_database_dump_runs_mysql_with_options(): ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database=hook_config[0], dry_run=False, extract_process=extract_process, connection_params={ @@ -459,7 +436,7 @@ def test_restore_database_dump_runs_mysql_with_options(): def test_restore_database_dump_runs_mysql_with_hostname_and_port(): - databases_config = [{'name': 'foo', 'hostname': 'database.example.org', 'port': 5433}] + hook_config = [{'name': 'foo', 'hostname': 'database.example.org', 'port': 5433}] extract_process = flexmock(stdout=flexmock()) flexmock(module).should_receive('execute_command_with_processes').with_args( @@ -480,10 +457,10 @@ def test_restore_database_dump_runs_mysql_with_hostname_and_port(): ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database=hook_config[0], dry_run=False, extract_process=extract_process, connection_params={ @@ -496,7 +473,7 @@ def test_restore_database_dump_runs_mysql_with_hostname_and_port(): def test_restore_database_dump_runs_mysql_with_username_and_password(): - databases_config = [{'name': 'foo', 'username': 'root', 'password': 'trustsome1'}] + hook_config = [{'name': 'foo', 'username': 'root', 'password': 'trustsome1'}] extract_process = flexmock(stdout=flexmock()) flexmock(module).should_receive('execute_command_with_processes').with_args( @@ -508,10 +485,10 @@ def test_restore_database_dump_runs_mysql_with_username_and_password(): ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database=hook_config[0], dry_run=False, extract_process=extract_process, connection_params={ @@ -524,7 +501,7 @@ def test_restore_database_dump_runs_mysql_with_username_and_password(): def test_restore_database_dump_with_connection_params_uses_connection_params_for_restore(): - databases_config = [ + hook_config = [ { 'name': 'foo', 'username': 'root', @@ -557,10 +534,10 @@ def test_restore_database_dump_with_connection_params_uses_connection_params_for ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database={'name': 'foo'}, dry_run=False, extract_process=extract_process, connection_params={ @@ -573,7 +550,7 @@ def test_restore_database_dump_with_connection_params_uses_connection_params_for def test_restore_database_dump_without_connection_params_uses_restore_params_in_config_for_restore(): - databases_config = [ + hook_config = [ { 'name': 'foo', 'username': 'root', @@ -608,10 +585,10 @@ def test_restore_database_dump_without_connection_params_uses_restore_params_in_ ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database=hook_config[0], dry_run=False, extract_process=extract_process, connection_params={ @@ -624,15 +601,15 @@ def test_restore_database_dump_without_connection_params_uses_restore_params_in_ def test_restore_database_dump_with_dry_run_skips_restore(): - databases_config = [{'name': 'foo'}] + hook_config = [{'name': 'foo'}] flexmock(module).should_receive('execute_command_with_processes').never() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database={'name': 'foo'}, dry_run=True, extract_process=flexmock(), connection_params={ diff --git a/tests/unit/hooks/test_postgresql.py b/tests/unit/hooks/test_postgresql.py index 73d67dad..619c68d6 100644 --- a/tests/unit/hooks/test_postgresql.py +++ b/tests/unit/hooks/test_postgresql.py @@ -464,7 +464,7 @@ def test_dump_databases_runs_non_default_pg_dump(): def test_restore_database_dump_runs_pg_restore(): - databases_config = [{'name': 'foo', 'schemas': None}, {'name': 'bar'}] + hook_config = [{'name': 'foo', 'schemas': None}, {'name': 'bar'}] extract_process = flexmock(stdout=flexmock()) flexmock(module).should_receive('make_extra_environment').and_return({'PGSSLMODE': 'disable'}) @@ -500,10 +500,10 @@ def test_restore_database_dump_runs_pg_restore(): ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database={'name': 'foo'}, dry_run=False, extract_process=extract_process, connection_params={ @@ -515,32 +515,8 @@ def test_restore_database_dump_runs_pg_restore(): ) -def test_restore_database_dump_errors_when_database_missing_from_configuration(): - databases_config = [{'name': 'foo', 'schemas': None}, {'name': 'bar'}] - extract_process = flexmock(stdout=flexmock()) - - flexmock(module).should_receive('execute_command_with_processes').never() - flexmock(module).should_receive('execute_command').never() - - with pytest.raises(ValueError): - module.restore_database_dump( - databases_config, - {}, - 'test.yaml', - database_name='other', - dry_run=False, - extract_process=extract_process, - connection_params={ - 'hostname': None, - 'port': None, - 'username': None, - 'password': None, - }, - ) - - def test_restore_database_dump_runs_pg_restore_with_hostname_and_port(): - databases_config = [ + hook_config = [ {'name': 'foo', 'hostname': 'database.example.org', 'port': 5433, 'schemas': None} ] extract_process = flexmock(stdout=flexmock()) @@ -586,10 +562,10 @@ def test_restore_database_dump_runs_pg_restore_with_hostname_and_port(): ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database=hook_config[0], dry_run=False, extract_process=extract_process, connection_params={ @@ -602,7 +578,7 @@ def test_restore_database_dump_runs_pg_restore_with_hostname_and_port(): def test_restore_database_dump_runs_pg_restore_with_username_and_password(): - databases_config = [ + hook_config = [ {'name': 'foo', 'username': 'postgres', 'password': 'trustsome1', 'schemas': None} ] extract_process = flexmock(stdout=flexmock()) @@ -646,10 +622,10 @@ def test_restore_database_dump_runs_pg_restore_with_username_and_password(): ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database=hook_config[0], dry_run=False, extract_process=extract_process, connection_params={ @@ -662,7 +638,7 @@ def test_restore_database_dump_runs_pg_restore_with_username_and_password(): def test_restore_database_dump_with_connection_params_uses_connection_params_for_restore(): - databases_config = [ + hook_config = [ { 'name': 'foo', 'hostname': 'database.example.org', @@ -725,10 +701,10 @@ def test_restore_database_dump_with_connection_params_uses_connection_params_for ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database={'name': 'foo'}, dry_run=False, extract_process=extract_process, connection_params={ @@ -741,7 +717,7 @@ def test_restore_database_dump_with_connection_params_uses_connection_params_for def test_restore_database_dump_without_connection_params_uses_restore_params_in_config_for_restore(): - databases_config = [ + hook_config = [ { 'name': 'foo', 'hostname': 'database.example.org', @@ -804,10 +780,10 @@ def test_restore_database_dump_without_connection_params_uses_restore_params_in_ ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database=hook_config[0], dry_run=False, extract_process=extract_process, connection_params={ @@ -820,7 +796,7 @@ def test_restore_database_dump_without_connection_params_uses_restore_params_in_ def test_restore_database_dump_runs_pg_restore_with_options(): - databases_config = [ + hook_config = [ { 'name': 'foo', 'restore_options': '--harder', @@ -865,10 +841,10 @@ def test_restore_database_dump_runs_pg_restore_with_options(): ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database=hook_config[0], dry_run=False, extract_process=extract_process, connection_params={ @@ -881,7 +857,7 @@ def test_restore_database_dump_runs_pg_restore_with_options(): def test_restore_database_dump_runs_psql_for_all_database_dump(): - databases_config = [{'name': 'all', 'schemas': None}] + hook_config = [{'name': 'all', 'schemas': None}] extract_process = flexmock(stdout=flexmock()) flexmock(module).should_receive('make_extra_environment').and_return({'PGSSLMODE': 'disable'}) @@ -904,10 +880,10 @@ def test_restore_database_dump_runs_psql_for_all_database_dump(): ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='all', + database={'name': 'all'}, dry_run=False, extract_process=extract_process, connection_params={ @@ -920,7 +896,7 @@ def test_restore_database_dump_runs_psql_for_all_database_dump(): def test_restore_database_dump_runs_psql_for_plain_database_dump(): - databases_config = [{'name': 'foo', 'format': 'plain', 'schemas': None}] + hook_config = [{'name': 'foo', 'format': 'plain', 'schemas': None}] extract_process = flexmock(stdout=flexmock()) flexmock(module).should_receive('make_extra_environment').and_return({'PGSSLMODE': 'disable'}) @@ -948,10 +924,10 @@ def test_restore_database_dump_runs_psql_for_plain_database_dump(): ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database=hook_config[0], dry_run=False, extract_process=extract_process, connection_params={ @@ -964,7 +940,7 @@ def test_restore_database_dump_runs_psql_for_plain_database_dump(): def test_restore_database_dump_runs_non_default_pg_restore_and_psql(): - databases_config = [ + hook_config = [ { 'name': 'foo', 'pg_restore_command': 'docker exec mycontainer pg_restore', @@ -1013,10 +989,10 @@ def test_restore_database_dump_runs_non_default_pg_restore_and_psql(): ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database=hook_config[0], dry_run=False, extract_process=extract_process, connection_params={ @@ -1029,7 +1005,7 @@ def test_restore_database_dump_runs_non_default_pg_restore_and_psql(): def test_restore_database_dump_with_dry_run_skips_restore(): - databases_config = [{'name': 'foo', 'schemas': None}] + hook_config = [{'name': 'foo', 'schemas': None}] flexmock(module).should_receive('make_extra_environment').and_return({'PGSSLMODE': 'disable'}) flexmock(module).should_receive('make_dump_path') @@ -1037,10 +1013,10 @@ def test_restore_database_dump_with_dry_run_skips_restore(): flexmock(module).should_receive('execute_command_with_processes').never() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database={'name': 'foo'}, dry_run=True, extract_process=flexmock(), connection_params={ @@ -1053,7 +1029,7 @@ def test_restore_database_dump_with_dry_run_skips_restore(): def test_restore_database_dump_without_extract_process_restores_from_disk(): - databases_config = [{'name': 'foo', 'schemas': None}] + hook_config = [{'name': 'foo', 'schemas': None}] flexmock(module).should_receive('make_extra_environment').and_return({'PGSSLMODE': 'disable'}) flexmock(module).should_receive('make_dump_path') @@ -1089,10 +1065,10 @@ def test_restore_database_dump_without_extract_process_restores_from_disk(): ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database={'name': 'foo'}, dry_run=False, extract_process=None, connection_params={ @@ -1105,7 +1081,7 @@ def test_restore_database_dump_without_extract_process_restores_from_disk(): def test_restore_database_dump_with_schemas_restores_schemas(): - databases_config = [{'name': 'foo', 'schemas': ['bar', 'baz']}] + hook_config = [{'name': 'foo', 'schemas': ['bar', 'baz']}] flexmock(module).should_receive('make_extra_environment').and_return({'PGSSLMODE': 'disable'}) flexmock(module).should_receive('make_dump_path') @@ -1145,10 +1121,10 @@ def test_restore_database_dump_with_schemas_restores_schemas(): ).once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='foo', + database=hook_config[0], dry_run=False, extract_process=None, connection_params={ diff --git a/tests/unit/hooks/test_sqlite.py b/tests/unit/hooks/test_sqlite.py index 761a0557..708e444f 100644 --- a/tests/unit/hooks/test_sqlite.py +++ b/tests/unit/hooks/test_sqlite.py @@ -1,6 +1,5 @@ import logging -import pytest from flexmock import flexmock from borgmatic.hooks import sqlite as module @@ -93,7 +92,7 @@ def test_dump_databases_does_not_dump_if_dry_run(): def test_restore_database_dump_restores_database(): - databases_config = [{'path': '/path/to/database', 'name': 'database'}, {'name': 'other'}] + hook_config = [{'path': '/path/to/database', 'name': 'database'}, {'name': 'other'}] extract_process = flexmock(stdout=flexmock()) flexmock(module).should_receive('execute_command_with_processes').with_args( @@ -109,10 +108,10 @@ def test_restore_database_dump_restores_database(): flexmock(module.os).should_receive('remove').once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='database', + database=hook_config[0], dry_run=False, extract_process=extract_process, connection_params={'restore_path': None}, @@ -120,7 +119,7 @@ def test_restore_database_dump_restores_database(): def test_restore_database_dump_with_connection_params_uses_connection_params_for_restore(): - databases_config = [ + hook_config = [ {'path': '/path/to/database', 'name': 'database', 'restore_path': 'config/path/to/database'} ] extract_process = flexmock(stdout=flexmock()) @@ -138,10 +137,10 @@ def test_restore_database_dump_with_connection_params_uses_connection_params_for flexmock(module.os).should_receive('remove').once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='database', + database={'name': 'database'}, dry_run=False, extract_process=extract_process, connection_params={'restore_path': 'cli/path/to/database'}, @@ -149,7 +148,7 @@ def test_restore_database_dump_with_connection_params_uses_connection_params_for def test_restore_database_dump_without_connection_params_uses_restore_params_in_config_for_restore(): - databases_config = [ + hook_config = [ {'path': '/path/to/database', 'name': 'database', 'restore_path': 'config/path/to/database'} ] extract_process = flexmock(stdout=flexmock()) @@ -167,10 +166,10 @@ def test_restore_database_dump_without_connection_params_uses_restore_params_in_ flexmock(module.os).should_receive('remove').once() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='database', + database=hook_config[0], dry_run=False, extract_process=extract_process, connection_params={'restore_path': None}, @@ -178,34 +177,18 @@ def test_restore_database_dump_without_connection_params_uses_restore_params_in_ def test_restore_database_dump_does_not_restore_database_if_dry_run(): - databases_config = [{'path': '/path/to/database', 'name': 'database'}] + hook_config = [{'path': '/path/to/database', 'name': 'database'}] extract_process = flexmock(stdout=flexmock()) flexmock(module).should_receive('execute_command_with_processes').never() flexmock(module.os).should_receive('remove').never() module.restore_database_dump( - databases_config, + hook_config, {}, 'test.yaml', - database_name='database', + database={'name': 'database'}, dry_run=True, extract_process=extract_process, connection_params={'restore_path': None}, ) - - -def test_restore_database_dump_raises_error_if_database_config_is_empty(): - databases_config = [] - extract_process = flexmock(stdout=flexmock()) - - with pytest.raises(ValueError): - module.restore_database_dump( - databases_config, - {}, - 'test.yaml', - database_name='database', - dry_run=False, - extract_process=extract_process, - connection_params={'restore_path': None}, - )