Add MySQL database hook "add_drop_database" configuration option to control whether dumped MySQL databases get dropped right before restore (#642).

Reviewed-on: https://projects.torsion.org/borgmatic-collective/borgmatic/pulls/641

NEWS

@@ -1,6 +1,17 @@
+1.7.7
+ * #642: Add MySQL database hook "add_drop_database" configuration option to control whether dumped
+   MySQL databases get dropped right before restore.
+ * #643: Fix for potential data loss (data not getting backed up) when dumping large "directory"
+   format PostgreSQL/MongoDB databases. Prior to the fix, these dumps would not finish writing to
+   disk before Borg consumed them. Now, the dumping process completes before Borg starts. This only
+   applies to "directory" format databases; other formats still stream to Borg without using
+   temporary disk space.
+ * Fix MongoDB "directory" format to work with mongodump/mongorestore without error. Prior to this
+   fix, only the "archive" format worked.
+
 1.7.6
- * #393, #438, #560: Optionally dump "all" PostgreSQL databases to separate files instead of one
-   combined dump file, allowing more convenient restores of individual databases. You can enable
+ * #393, #438, #560: Optionally dump "all" PostgreSQL/MySQL databases to separate files instead of
+   one combined dump file, allowing more convenient restores of individual databases. You can enable
    this by specifying the database dump "format" option when the database is named "all".
  * #602: Fix logs that interfere with JSON output by making warnings go to stderr instead of stdout.
  * #622: Fix traceback when include merging configuration files on ARM64.

borgmatic/config/schema.yaml

@@ -892,6 +892,13 @@ properties:
                                 file of that format, allowing more convenient
                                 restores of individual databases.
                             example: directory
+                        add_drop_database:
+                            type: boolean
+                            description: |
+                                Use the "--add-drop-database" flag with
+                                mysqldump, causing the database to be dropped
+                                right before restore. Defaults to true.
+                            example: false
                         options:
                             type: string
                             description: |

borgmatic/hooks/mongodb.py

@@ -45,13 +45,14 @@ def dump_databases(databases, log_prefix, location_config, dry_run):
         if dry_run:
             continue
 
+        command = build_dump_command(database, dump_filename, dump_format)
+
         if dump_format == 'directory':
             dump.create_parent_directory_for_dump(dump_filename)
+            execute_command(command, shell=True)
         else:
             dump.create_named_pipe_for_dump(dump_filename)
-
-        command = build_dump_command(database, dump_filename, dump_format)
-        processes.append(execute_command(command, shell=True, run_to_completion=False))
+            processes.append(execute_command(command, shell=True, run_to_completion=False))
 
     return processes
 
@@ -61,9 +62,9 @@ def build_dump_command(database, dump_filename, dump_format):
     Return the mongodump command from a single database configuration.
     '''
     all_databases = database['name'] == 'all'
-    command = ['mongodump', '--archive']
+    command = ['mongodump']
     if dump_format == 'directory':
-        command.append(dump_filename)
+        command.extend(('--out', dump_filename))
     if 'hostname' in database:
         command.extend(('--host', database['hostname']))
     if 'port' in database:
@@ -79,7 +80,7 @@ def build_dump_command(database, dump_filename, dump_format):
     if 'options' in database:
         command.extend(database['options'].split(' '))
     if dump_format != 'directory':
-        command.extend(('>', dump_filename))
+        command.extend(('--archive', '>', dump_filename))
     return command
 
 
@@ -145,9 +146,11 @@ def build_restore_command(extract_process, database, dump_filename):
     '''
     Return the mongorestore command from a single database configuration.
     '''
-    command = ['mongorestore', '--archive']
-    if not extract_process:
-        command.append(dump_filename)
+    command = ['mongorestore']
+    if extract_process:
+        command.append('--archive')
+    else:
+        command.extend(('--dir', dump_filename))
     if database['name'] != 'all':
         command.extend(('--drop', '--db', database['name']))
     if 'hostname' in database:

borgmatic/hooks/mysql.py

@@ -81,7 +81,7 @@ def execute_dump_command(
     dump_command = (
         ('mysqldump',)
         + (tuple(database['options'].split(' ')) if 'options' in database else ())
-        + ('--add-drop-database',)
+        + (('--add-drop-database',) if database.get('add_drop_database', True) else ())
         + (('--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 ())

borgmatic/hooks/postgresql.py

@@ -141,17 +141,19 @@ def dump_databases(databases, log_prefix, location_config, dry_run):
 
             if dump_format == 'directory':
                 dump.create_parent_directory_for_dump(dump_filename)
+                execute_command(
+                    command, shell=True, extra_environment=extra_environment,
+                )
             else:
                 dump.create_named_pipe_for_dump(dump_filename)
-
-            processes.append(
-                execute_command(
-                    command,
-                    shell=True,
-                    extra_environment=extra_environment,
-                    run_to_completion=False,
+                processes.append(
+                    execute_command(
+                        command,
+                        shell=True,
+                        extra_environment=extra_environment,
+                        run_to_completion=False,
+                    )
                 )
-            )
 
     return processes
 

docs/Dockerfile

@@ -1,4 +1,4 @@
-FROM alpine:3.16.0 as borgmatic
+FROM alpine:3.17.1 as borgmatic
 
 COPY . /app
 RUN apk add --no-cache py3-pip py3-ruamel.yaml py3-ruamel.yaml.clib
@@ -8,7 +8,7 @@ RUN borgmatic --help > /command-line.txt \
            echo -e "\n--------------------------------------------------------------------------------\n" >> /command-line.txt \
            && borgmatic "$action" --help >> /command-line.txt; done
 
-FROM node:18.4.0-alpine as html
+FROM node:19.5.0-alpine as html
 
 ARG ENVIRONMENT=production
 
@@ -27,7 +27,7 @@ COPY . /source
 RUN NODE_ENV=${ENVIRONMENT} npx eleventy --input=/source/docs --output=/output/docs \
   && mv /output/docs/index.html /output/index.html
 
-FROM nginx:1.22.0-alpine
+FROM nginx:1.22.1-alpine
 
 COPY --from=html /output /usr/share/nginx/html
 COPY --from=borgmatic /etc/borgmatic/config.yaml /usr/share/nginx/html/docs/reference/config.yaml

docs/how-to/backup-your-databases.md

@@ -240,7 +240,7 @@ To restore all databases:
 borgmatic restore --archive host-2023-... --database all
 ```
 
-Or just omit the `--database` flag entirely:
+Or omit the `--database` flag entirely:
 
 
 ```bash

docs/how-to/deal-with-very-large-backups.md

@@ -60,7 +60,15 @@ consistency:
         - name: repository
 ```
 
-(Prior to borgmatic 1.6.2, `checks` was a plain list of strings without the `name:` part.)
+<span class="minilink minilink-addedin">Prior to version 1.6.2</span> `checks`
+was a plain list of strings without the `name:` part. For example:
+
+```yaml
+consistency:
+    checks:
+        - repository
+```
+
 
 Here are the available checks from fastest to slowest:
 
@@ -125,7 +133,16 @@ consistency:
         - name: disabled
 ```
 
-Or, if you have multiple repositories in your borgmatic configuration file,
+<span class="minilink minilink-addedin">Prior to version 1.6.2</span> `checks`
+was a plain list of strings without the `name:` part. For instance:
+
+```yaml
+consistency:
+    checks:
+        - disabled
+```
+
+If you have multiple repositories in your borgmatic configuration file,
 you can keep running consistency checks, but only against a subset of the
 repositories:
 

docs/how-to/set-up-backups.md

@@ -93,6 +93,7 @@ installing borgmatic:
  * [OpenBSD](http://ports.su/sysutils/borgmatic)
  * [openSUSE](https://software.opensuse.org/package/borgmatic)
  * [macOS (via Homebrew)](https://formulae.brew.sh/formula/borgmatic)
+ * [macOS (via MacPorts)](https://ports.macports.org/port/borgmatic/)
  * [Ansible role](https://github.com/borgbase/ansible-role-borgbackup)
  * [virtualenv](https://virtualenv.pypa.io/en/stable/)
 

docs/reference/command-line.md

@@ -13,9 +13,3 @@ each action sub-command:
 ```
 {% include borgmatic/command-line.txt %}
 ```
-
-
-## Related documentation
-
- * [Set up backups with borgmatic](https://torsion.org/borgmatic/docs/how-to/set-up-backups/)
- * [borgmatic configuration reference](https://torsion.org/borgmatic/docs/reference/configuration/)

docs/reference/configuration.md

@@ -15,9 +15,3 @@ Here is a full sample borgmatic configuration file including all available optio
 
 Note that you can also [download this configuration
 file](https://torsion.org/borgmatic/docs/reference/config.yaml) for use locally.
-
-
-## Related documentation
-
- * [Set up backups with borgmatic](https://torsion.org/borgmatic/docs/how-to/set-up-backups/)
- * [borgmatic command-line reference](https://torsion.org/borgmatic/docs/reference/command-line/)

setup.py

@@ -1,6 +1,6 @@
 from setuptools import find_packages, setup
 
-VERSION = '1.7.6'
+VERSION = '1.7.7'
 
 
 setup(

tests/end-to-end/test_database.py

@@ -14,6 +14,7 @@ def write_configuration(
     repository_path,
     borgmatic_source_directory,
     postgresql_dump_format='custom',
+    mongodb_dump_format='archive',
 ):
     '''
     Write out borgmatic configuration into a file at the config path. Set the options so as to work
@@ -67,6 +68,7 @@ hooks:
           username: root
           password: test
           authentication_database: admin
+          format: {mongodb_dump_format}
         - name: all
           hostname: mongodb
           username: root
@@ -136,6 +138,7 @@ def test_database_dump_and_restore_with_directory_format():
             repository_path,
             borgmatic_source_directory,
             postgresql_dump_format='directory',
+            mongodb_dump_format='directory',
         )
 
         subprocess.check_call(

tests/unit/hooks/test_mongodb.py

@@ -17,7 +17,7 @@ def test_dump_databases_runs_mongodump_for_each_database():
 
     for name, process in zip(('foo', 'bar'), processes):
         flexmock(module).should_receive('execute_command').with_args(
-            ['mongodump', '--archive', '--db', name, '>', 'databases/localhost/{}'.format(name)],
+            ['mongodump', '--db', name, '--archive', '>', 'databases/localhost/{}'.format(name)],
             shell=True,
             run_to_completion=False,
         ).and_return(process).once()
@@ -49,13 +49,13 @@ def test_dump_databases_runs_mongodump_with_hostname_and_port():
     flexmock(module).should_receive('execute_command').with_args(
         [
             'mongodump',
-            '--archive',
             '--host',
             'database.example.org',
             '--port',
             '5433',
             '--db',
             'foo',
+            '--archive',
             '>',
             'databases/database.example.org/foo',
         ],
@@ -85,7 +85,6 @@ def test_dump_databases_runs_mongodump_with_username_and_password():
     flexmock(module).should_receive('execute_command').with_args(
         [
             'mongodump',
-            '--archive',
             '--username',
             'mongo',
             '--password',
@@ -94,6 +93,7 @@ def test_dump_databases_runs_mongodump_with_username_and_password():
             'admin',
             '--db',
             'foo',
+            '--archive',
             '>',
             'databases/localhost/foo',
         ],
@@ -106,7 +106,6 @@ def test_dump_databases_runs_mongodump_with_username_and_password():
 
 def test_dump_databases_runs_mongodump_with_directory_format():
     databases = [{'name': 'foo', 'format': 'directory'}]
-    process = flexmock()
     flexmock(module).should_receive('make_dump_path').and_return('')
     flexmock(module.dump).should_receive('make_database_dump_filename').and_return(
         'databases/localhost/foo'
@@ -115,12 +114,10 @@ def test_dump_databases_runs_mongodump_with_directory_format():
     flexmock(module.dump).should_receive('create_named_pipe_for_dump').never()
 
     flexmock(module).should_receive('execute_command').with_args(
-        ['mongodump', '--archive', 'databases/localhost/foo', '--db', 'foo'],
-        shell=True,
-        run_to_completion=False,
-    ).and_return(process).once()
+        ['mongodump', '--out', 'databases/localhost/foo', '--db', 'foo'], shell=True,
+    ).and_return(flexmock()).once()
 
-    assert module.dump_databases(databases, 'test.yaml', {}, dry_run=False) == [process]
+    assert module.dump_databases(databases, 'test.yaml', {}, dry_run=False) == []
 
 
 def test_dump_databases_runs_mongodump_with_options():
@@ -133,7 +130,7 @@ 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', '--archive', '--db', 'foo', '--stuff=such', '>', 'databases/localhost/foo'],
+        ['mongodump', '--db', 'foo', '--stuff=such', '--archive', '>', 'databases/localhost/foo'],
         shell=True,
         run_to_completion=False,
     ).and_return(process).once()
@@ -305,12 +302,12 @@ def test_restore_database_dump_with_dry_run_skips_restore():
 
 
 def test_restore_database_dump_without_extract_process_restores_from_disk():
-    database_config = [{'name': 'foo'}]
+    database_config = [{'name': 'foo', 'format': 'directory'}]
 
     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', '--archive', '/dump/path', '--drop', '--db', 'foo'],
+        ['mongorestore', '--dir', '/dump/path', '--drop', '--db', 'foo'],
         processes=[],
         output_log_level=logging.DEBUG,
         input_file=None,

tests/unit/hooks/test_mysql.py

@@ -159,6 +159,33 @@ def test_execute_dump_command_runs_mysqldump():
     )
 
 
+def test_execute_dump_command_runs_mysqldump_without_add_drop_database():
+    process = flexmock()
+    flexmock(module.dump).should_receive('make_database_dump_filename').and_return('dump')
+    flexmock(module.os.path).should_receive('exists').and_return(False)
+    flexmock(module.dump).should_receive('create_named_pipe_for_dump')
+
+    flexmock(module).should_receive('execute_command').with_args(
+        ('mysqldump', '--databases', 'foo', '>', 'dump',),
+        shell=True,
+        extra_environment=None,
+        run_to_completion=False,
+    ).and_return(process).once()
+
+    assert (
+        module.execute_dump_command(
+            database={'name': 'foo', 'add_drop_database': False},
+            log_prefix='log',
+            dump_path=flexmock(),
+            database_names=('foo',),
+            extra_environment=None,
+            dry_run=False,
+            dry_run_label='',
+        )
+        == process
+    )
+
+
 def test_execute_dump_command_runs_mysqldump_with_hostname_and_port():
     process = flexmock()
     flexmock(module.dump).should_receive('make_database_dump_filename').and_return('dump')

tests/unit/hooks/test_postgresql.py

@@ -270,7 +270,6 @@ def test_make_extra_environment_maps_options_to_environment():
 
 def test_dump_databases_runs_pg_dump_with_directory_format():
     databases = [{'name': 'foo', 'format': 'directory'}]
-    process = flexmock()
     flexmock(module).should_receive('make_extra_environment').and_return({'PGSSLMODE': 'disable'})
     flexmock(module).should_receive('make_dump_path').and_return('')
     flexmock(module).should_receive('database_names_to_dump').and_return(('foo',))
@@ -295,10 +294,9 @@ def test_dump_databases_runs_pg_dump_with_directory_format():
         ),
         shell=True,
         extra_environment={'PGSSLMODE': 'disable'},
-        run_to_completion=False,
-    ).and_return(process).once()
+    ).and_return(flexmock()).once()
 
-    assert module.dump_databases(databases, 'test.yaml', {}, dry_run=False) == [process]
+    assert module.dump_databases(databases, 'test.yaml', {}, dry_run=False) == []
 
 
 def test_dump_databases_runs_pg_dump_with_options():