Fix mysqldump ignoring errors (#403)

* Add schema and tables for the tests * Add tests for full dump with and without compression * Add test for distinct dump with and without compression * Fix sh not seeing errors for command before the pipe sh is missing the pipefail flag. We must use bash for this. * Add cleanup to prevent the following tests from failing * Fix fqcn in module_defaults * Add changelog fragment * Add check to the error message to ensure we captured the right one * Add option to activate the fix on systems with bash * Fix errors when data schema is already absent * Update changelogs/fragments/fix-256-mysql_dump-errors.yml Co-authored-by: Andrew Klychkov <aaklychkov@mail.ru> * Add markup for commands in the documentation string Co-authored-by: Andrew Klychkov <aaklychkov@mail.ru> * Add markup and next release version in the documentation string Co-authored-by: Andrew Klychkov <aaklychkov@mail.ru> * Fix missing dependency for MySQL 8 * Add pipefail to tests of uncompressed dumps to enure it still works * Fix "bash command not found" if pipefail is used for uncompressed dump * Fix sanity pep8 * Document example of dump with pipefail * Add dedpulication to command construct Co-authored-by: Andrew Klychkov <aaklychkov@mail.ru> Co-authored-by: Andrew Klychkov <aaklychkov@mail.ru>
2025-04-05 10:10:32 -07:00 · 2022-06-30 06:54:26 +02:00 · 2022-06-30 06:54:26 +02:00 · 5108ca5e66
commit 5108ca5e66
parent 6f87620d9b
6 changed files with 188 additions and 5 deletions
--- a/changelogs/fragments/fix-256-mysql_dump-errors.yml
+++ b/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`` (https://github.com/ansible-collections/community.mysql/issues/256).
--- a/plugins/modules/mysql_db.py
+++ b/plugins/modules/mysql_db.py
@ -156,6 +156,14 @@ options:
    - 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 C(bash) instead of C(sh) and add C(-o pipefail) to catch errors from the
+      mysql_dump command when I(state=import) and compression is used. The default is I(false) to
+      prevent issue on system without bash. The default may change in a future release.
+    type: bool
+    default: no
+    version_added: '3.4.0'

 seealso:
 - module: community.mysql.mysql_info
@ -295,6 +303,13 @@ EXAMPLES = r'''
    login_password: 123456
    name: bobdata
    state: present
+
+- name: Dump a database with compression and catch errors from mysqldump with bash pipefail
+  community.mysql.mysql_db:
+    state: dump
+    name: foo
+    target: /tmp/dump.sql.gz
+    pipefail: true
 '''

 RETURN = r'''
@ -355,7 +370,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:
@ -424,11 +439,18 @@ def db_dump(module, host, user, password, db_name, target, all_databases, port,

    if path:
        cmd = '%s | %s > %s' % (cmd, path, shlex_quote(target))
+        if pipefail:
+            cmd = 'set -o pipefail && ' + cmd
    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 +591,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 +641,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 +728,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,
--- a/tests/integration/targets/setup_mysql/vars/main.yml
+++ b/tests/integration/targets/setup_mysql/vars/main.yml
@ -19,6 +19,7 @@ install_prereqs:

 install_python_prereqs:
  - python3-dev
+  - python3-cryptography
  - default-libmysqlclient-dev
  - build-essential

--- a/tests/integration/targets/test_mysql_db/tasks/issue_256_mysqldump_errors.yml
+++ b/tests/integration/targets/test_mysql_db/tasks/issue_256_mysqldump_errors.yml
@ -0,0 +1,148 @@
+---
+
+# 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
+        pipefail: true  # This should do nothing
+      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
+        pipefail: true  # This should do nothing
+      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
--- a/tests/integration/targets/test_mysql_db/tasks/main.yml
+++ b/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
--- a/tests/integration/targets/test_mysql_db/tasks/state_present_absent.yml
+++ b/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 }}"