Fix MySQL restore error on "all" database dump by excluding system tables (#301).

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

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,
             )
 
 

setup.py

@@ -1,6 +1,6 @@
 from setuptools import find_packages, setup
 
-VERSION = '1.5.1'
+VERSION = '1.5.2.dev0'
 
 
 setup(

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
     )

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()