Fix mysqldump ignoring errors

changelogs/fragments/fix-256-mysql_dump-errors.yml

@@ -0,0 +1,7 @@
+---
+
+bugfixes:
+  - mysql_dump - Fixes issue 256. Using compression masks errors messages from
+    mysql_dump. By default the fix is inactiv to ensure retro-compatibility
+    with system without bash. To activate the fix, use the module option
+    `pipefail=true`.

plugins/modules/mysql_db.py

@@ -156,6 +156,14 @@
     - Can be useful, for example, when I(state=import) and a dump file contains relative paths.
     type: path
     version_added: '3.4.0'
+  pipefail:
+    description:
+    - Use bash instead of sh and add -o pipefail to catch errors from the
+      mysql_dump command when compression is used. The default is false to
+      prevent issue on system without bash. The default may change in a future release
+    type: bool
+    default: no
+    version_added: '3.3.1'
 
 seealso:
 - module: community.mysql.mysql_info
@@ -355,7 +363,7 @@ def db_dump(module, host, user, password, db_name, target, all_databases, port,
             single_transaction=None, quick=None, ignore_tables=None, hex_blob=None,
             encoding=None, force=False, master_data=0, skip_lock_tables=False,
             dump_extra_args=None, unsafe_password=False, restrict_config_file=False,
-            check_implicit_admin=False):
+            check_implicit_admin=False, pipefail=False):
     cmd = module.get_bin_path('mysqldump', True)
     # If defined, mysqldump demands --defaults-extra-file be the first option
     if config_file:
@@ -422,13 +430,20 @@ def db_dump(module, host, user, password, db_name, target, all_databases, port,
     elif os.path.splitext(target)[-1] == '.xz':
         path = module.get_bin_path('xz', True)
 
-    if path:
+    if pipefail:
+        cmd = 'set -o pipefail && %s | %s > %s' % (cmd, path, shlex_quote(target))
+    elif path:
         cmd = '%s | %s > %s' % (cmd, path, shlex_quote(target))
     else:
         cmd += " > %s" % shlex_quote(target)
 
     executed_commands.append(cmd)
-    rc, stdout, stderr = module.run_command(cmd, use_unsafe_shell=True)
+
+    if pipefail:
+      rc, stdout, stderr = module.run_command(cmd, use_unsafe_shell=True, executable='bash')
+    else:
+      rc, stdout, stderr = module.run_command(cmd, use_unsafe_shell=True)
+
     return rc, stdout, stderr
 
 
@@ -569,6 +584,7 @@ def main():
         check_implicit_admin=dict(type='bool', default=False),
         config_overrides_defaults=dict(type='bool', default=False),
         chdir=dict(type='path'),
+        pipefail=dict(type='bool', default=False),
     )
 
     module = AnsibleModule(
@@ -618,6 +634,7 @@ def main():
     check_implicit_admin = module.params['check_implicit_admin']
     config_overrides_defaults = module.params['config_overrides_defaults']
     chdir = module.params['chdir']
+    pipefail = module.params['pipefail']
 
     if chdir:
         try:
@@ -704,7 +721,7 @@ def main():
                                      ssl_ca, single_transaction, quick, ignore_tables,
                                      hex_blob, encoding, force, master_data, skip_lock_tables,
                                      dump_extra_args, unsafe_login_password, restrict_config_file,
-                                     check_implicit_admin)
+                                     check_implicit_admin, pipefail)
         if rc != 0:
             module.fail_json(msg="%s" % stderr)
         module.exit_json(changed=True, db=db_name, db_list=db, msg=stdout,

tests/integration/targets/test_mysql_db/tasks/issue_256_mysqldump_errors.yml

@@ -0,0 +1,146 @@
+---
+
+# When mysqldump encountered an issue, mysql_db was still happy. But the
+# dump produced was empty or worse, only contained `DROP TABLE IF EXISTS...`
+
+- module_defaults:
+    community.mysql.mysql_db: &mysql_defaults
+      login_user: '{{ mysql_user }}'
+      login_password: '{{ mysql_password }}'
+      login_host: 127.0.0.1
+      login_port: '{{ mysql_primary_port }}'
+    community.mysql.mysql_query: *mysql_defaults
+
+  block:
+
+    - name: Dumps errors | Setup test | Create 2 schemas
+      community.mysql.mysql_db:
+        name:
+          - "db1"
+          - "db2"
+        state: present
+
+    - name: Dumps errors | Setup test | Create 2 tables
+      community.mysql.mysql_query:
+        query:
+          - "CREATE TABLE db1.t1 (id int)"
+          - "CREATE TABLE db1.t2 (id int)"
+          - "CREATE VIEW db2.v1 AS SELECT id from db1.t1"
+
+    - name: Dumps errors | Full dump without compression
+      community.mysql.mysql_db:
+        state: dump
+        name: all
+        target: /tmp/full-dump.sql
+      register: full_dump
+
+    - name: Dumps errors | Full dump with gunzip
+      community.mysql.mysql_db:
+        state: dump
+        name: all
+        target: /tmp/full-dump.sql.gz
+      register: full_dump_gz
+
+    - name: Dumps errors | Distinct dump without compression
+      community.mysql.mysql_db:
+        state: dump
+        name: db2
+        target: /tmp/dump-db2.sql
+      register: dump_db2
+
+    - name: Dumps errors | Distinct dump with gunzip
+      community.mysql.mysql_db:
+        state: dump
+        name: db2
+        target: /tmp/dump-db2.sql.gz
+      register: dump_db2_gz
+
+    - name: Dumps errors | Check distinct dumps are changed
+      ansible.builtin.assert:
+        that:
+          - dump_db2 is changed
+          - dump_db2_gz is changed
+
+    # Now db2.v1 targets an inexistant table so mysqldump will fail
+    - name: Dumps errors | Drop t1
+      community.mysql.mysql_query:
+        query:
+          - "DROP TABLE db1.t1"
+
+    - name: Dumps errors | Full dump after drop t1 without compression
+      community.mysql.mysql_db:
+        state: dump
+        name: all
+        target: /tmp/full-dump-without-t1.sql
+      register: full_dump_without_t1
+      ignore_errors: true
+
+    - name: Dumps errors | Full dump after drop t1 with gzip without the fix
+      community.mysql.mysql_db:
+        state: dump
+        name: all
+        target: /tmp/full-dump-without-t1.sql.gz
+      register: full_dump_without_t1_gz_without_fix
+      ignore_errors: true
+
+    - name: Dumps errors | Full dump after drop t1 with gzip with the fix
+      community.mysql.mysql_db:
+        state: dump
+        name: all
+        target: /tmp/full-dump-without-t1.sql.gz
+        pipefail: true
+      register: full_dump_without_t1_gz_with_fix
+      ignore_errors: true
+
+    - name: Dumps errors | Check full dump
+      ansible.builtin.assert:
+        that:
+          - full_dump_without_t1 is failed
+          - full_dump_without_t1.msg is search(
+              'references invalid table')
+          - full_dump_without_t1_gz_without_fix is changed
+          - full_dump_without_t1_gz_with_fix is failed
+          - full_dump_without_t1_gz_with_fix.msg is search(
+              'references invalid table')
+
+    - name: Dumps errors | Distinct dump after drop t1 without compression
+      community.mysql.mysql_db:
+        state: dump
+        name: db2
+        target: /tmp/dump-db2-without_t1.sql
+      register: dump_db2_without_t1
+      ignore_errors: true
+
+    - name: Dumps errors | Distinct dump after drop t1 with gzip without the fix
+      community.mysql.mysql_db:
+        state: dump
+        name: db2
+        target: /tmp/dump-db2-without_t1.sql.gz
+      register: dump_db2_without_t1_gz_without_fix
+      ignore_errors: true
+
+    - name: Dumps errors | Distinct dump after drop t1 with gzip with the fix
+      community.mysql.mysql_db:
+        state: dump
+        name: db2
+        target: /tmp/dump-db2-without_t1.sql.gz
+        pipefail: true
+      register: dump_db2_without_t1_gz_with_fix
+      ignore_errors: true
+
+    - name: Dumps errors | Check distinct dump
+      ansible.builtin.assert:
+        that:
+          - dump_db2_without_t1 is failed
+          - dump_db2_without_t1.msg is search(
+              'references invalid table')
+          - dump_db2_without_t1_gz_without_fix is changed
+          - dump_db2_without_t1_gz_with_fix is failed
+          - dump_db2_without_t1_gz_with_fix.msg is search(
+              'references invalid table')
+    - name: Dumps errors | Cleanup
+      community.mysql.mysql_db:
+        name:
+          - "db1"
+          - "db2"
+        state: absent

tests/integration/targets/test_mysql_db/tasks/main.yml

@@ -63,3 +63,6 @@
   vars:
     db_name: "{{ item }}"
   loop: "{{ db_names }}"
+
+- name: Check errors from mysqldump are seen issue 256
+  ansible.builtin.include_tasks: issue_256_mysqldump_errors.yml

tests/integration/targets/test_mysql_db/tasks/state_present_absent.yml

@@ -18,8 +18,8 @@
 # ============================================================
 - name: remove database if it exists
   command: >
-    "{{ mysql_command }} -sse 'drop database {{ db_name }}'"
-  ignore_errors: True
+    "{{ mysql_command }} -sse 'DROP DATABASE IF EXISTS {{ db_name }}'"
+  ignore_errors: true
 
 - name: make sure the test database is not there
   command: "{{ mysql_command }} {{ db_name }}"