diff --git a/NEWS b/NEWS index 1cc87146..656d6a4d 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,6 @@ +1.5.2.dev0 + * #301: Fix MySQL restore error on "all" database dump by excluding system tables. + 1.5.1 * #289: Tired of looking up the latest successful archive name in order to pass it to borgmatic actions? Me too. Now you can specify "--archive latest" to all actions that accept an archive diff --git a/borgmatic/hooks/mysql.py b/borgmatic/hooks/mysql.py index b76e2bed..c22eb087 100644 --- a/borgmatic/hooks/mysql.py +++ b/borgmatic/hooks/mysql.py @@ -16,6 +16,43 @@ def make_dump_path(location_config): # pragma: no cover ) +SYSTEM_DATABASE_NAMES = ('information_schema', 'mysql', 'performance_schema', 'sys') + + +def database_names_to_dump(database, extra_environment, log_prefix, dry_run_label): + ''' + Given a requested database name, return the corresponding sequence of database names to dump. + In the case of "all", query for the names of databases on the configured host and return them, + excluding any system databases that will cause problems during restore. + ''' + requested_name = database['name'] + + if requested_name != 'all': + return (requested_name,) + + show_command = ( + ('mysql',) + + (('--host', database['hostname']) if 'hostname' in database else ()) + + (('--port', str(database['port'])) if 'port' in database else ()) + + (('--protocol', 'tcp') if 'hostname' in database or 'port' in database else ()) + + (('--user', database['username']) if 'username' in database else ()) + + ('--skip-column-names', '--batch') + + ('--execute', 'show schemas') + ) + logger.debug( + '{}: Querying for "all" MySQL databases to dump{}'.format(log_prefix, dry_run_label) + ) + show_output = execute_command( + show_command, output_log_level=None, extra_environment=extra_environment + ) + + return tuple( + show_name + for show_name in show_output.strip().splitlines() + if show_name not in SYSTEM_DATABASE_NAMES + ) + + def dump_databases(databases, log_prefix, location_config, dry_run): ''' Dump the given MySQL/MariaDB databases to disk. The databases are supplied as a sequence of @@ -28,30 +65,37 @@ def dump_databases(databases, log_prefix, location_config, dry_run): logger.info('{}: Dumping MySQL databases{}'.format(log_prefix, dry_run_label)) for database in databases: - name = database['name'] + requested_name = database['name'] dump_filename = dump.make_database_dump_filename( - make_dump_path(location_config), name, database.get('hostname') + make_dump_path(location_config), requested_name, database.get('hostname') ) - command = ( + extra_environment = {'MYSQL_PWD': database['password']} if 'password' in database else None + dump_command_names = database_names_to_dump( + database, extra_environment, log_prefix, dry_run_label + ) + + dump_command = ( ('mysqldump', '--add-drop-database') + (('--host', database['hostname']) if 'hostname' in database else ()) + (('--port', str(database['port'])) if 'port' in database else ()) + (('--protocol', 'tcp') if 'hostname' in database or 'port' in database else ()) + (('--user', database['username']) if 'username' in database else ()) + (tuple(database['options'].split(' ')) if 'options' in database else ()) - + (('--all-databases',) if name == 'all' else ('--databases', name)) + + ('--databases',) + + dump_command_names ) - extra_environment = {'MYSQL_PWD': database['password']} if 'password' in database else None logger.debug( '{}: Dumping MySQL database {} to {}{}'.format( - log_prefix, name, dump_filename, dry_run_label + log_prefix, requested_name, dump_filename, dry_run_label ) ) if not dry_run: os.makedirs(os.path.dirname(dump_filename), mode=0o700, exist_ok=True) execute_command( - command, output_file=open(dump_filename, 'w'), extra_environment=extra_environment + dump_command, + output_file=open(dump_filename, 'w'), + extra_environment=extra_environment, ) diff --git a/setup.py b/setup.py index e6a357fa..6bad9305 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,6 @@ from setuptools import find_packages, setup -VERSION = '1.5.1' +VERSION = '1.5.2.dev0' setup( diff --git a/tests/end-to-end/test_database.py b/tests/end-to-end/test_database.py index a011c6f3..482d42eb 100644 --- a/tests/end-to-end/test_database.py +++ b/tests/end-to-end/test_database.py @@ -34,6 +34,10 @@ hooks: hostname: mysql username: root password: test + - name: all + hostname: mysql + username: root + password: test '''.format( config_path, repository_path, borgmatic_source_directory ) diff --git a/tests/unit/hooks/test_mysql.py b/tests/unit/hooks/test_mysql.py index 6465d71d..91c0149e 100644 --- a/tests/unit/hooks/test_mysql.py +++ b/tests/unit/hooks/test_mysql.py @@ -5,6 +5,35 @@ from flexmock import flexmock from borgmatic.hooks import mysql as module +def test_database_names_to_dump_passes_through_name(): + extra_environment = flexmock() + log_prefix = '' + dry_run_label = '' + + names = module.database_names_to_dump( + {'name': 'foo'}, extra_environment, log_prefix, dry_run_label + ) + + assert names == ('foo',) + + +def test_database_names_to_dump_queries_mysql_for_database_names(): + extra_environment = flexmock() + log_prefix = '' + dry_run_label = '' + flexmock(module).should_receive('execute_command').with_args( + ('mysql', '--skip-column-names', '--batch', '--execute', 'show schemas'), + output_log_level=None, + extra_environment=extra_environment, + ).and_return('foo\nbar\nmysql\n').once() + + names = module.database_names_to_dump( + {'name': 'all'}, extra_environment, log_prefix, dry_run_label + ) + + assert names == ('foo', 'bar') + + def test_dump_databases_runs_mysqldump_for_each_database(): databases = [{'name': 'foo'}, {'name': 'bar'}] output_file = flexmock() @@ -12,6 +41,9 @@ def test_dump_databases_runs_mysqldump_for_each_database(): flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/localhost/foo' ).and_return('databases/localhost/bar') + flexmock(module).should_receive('database_names_to_dump').and_return(('foo',)).and_return( + ('bar',) + ) flexmock(module.os).should_receive('makedirs') flexmock(sys.modules['builtins']).should_receive('open').and_return(output_file) @@ -31,6 +63,9 @@ def test_dump_databases_with_dry_run_skips_mysqldump(): flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/localhost/foo' ).and_return('databases/localhost/bar') + flexmock(module).should_receive('database_names_to_dump').and_return(('foo',)).and_return( + ('bar',) + ) flexmock(module.os).should_receive('makedirs').never() flexmock(module).should_receive('execute_command').never() @@ -44,6 +79,7 @@ def test_dump_databases_runs_mysqldump_with_hostname_and_port(): flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/database.example.org/foo' ) + flexmock(module).should_receive('database_names_to_dump').and_return(('foo',)) flexmock(module.os).should_receive('makedirs') flexmock(sys.modules['builtins']).should_receive('open').and_return(output_file) @@ -74,6 +110,7 @@ def test_dump_databases_runs_mysqldump_with_username_and_password(): flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/localhost/foo' ) + flexmock(module).should_receive('database_names_to_dump').and_return(('foo',)) flexmock(module.os).should_receive('makedirs') flexmock(sys.modules['builtins']).should_receive('open').and_return(output_file) @@ -93,6 +130,7 @@ def test_dump_databases_runs_mysqldump_with_options(): flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/localhost/foo' ) + flexmock(module).should_receive('database_names_to_dump').and_return(('foo',)) flexmock(module.os).should_receive('makedirs') flexmock(sys.modules['builtins']).should_receive('open').and_return(output_file) @@ -112,11 +150,12 @@ def test_dump_databases_runs_mysqldump_for_all_databases(): flexmock(module.dump).should_receive('make_database_dump_filename').and_return( 'databases/localhost/all' ) + flexmock(module).should_receive('database_names_to_dump').and_return(('foo', 'bar')) flexmock(module.os).should_receive('makedirs') flexmock(sys.modules['builtins']).should_receive('open').and_return(output_file) flexmock(module).should_receive('execute_command').with_args( - ('mysqldump', '--add-drop-database', '--all-databases'), + ('mysqldump', '--add-drop-database', '--databases', 'foo', 'bar'), output_file=output_file, extra_environment=None, ).once()