diff --git a/changelogs/fragments/fix-256-mysql_dump-errors.yml b/changelogs/fragments/fix-256-mysql_dump-errors.yml new file mode 100644 index 00000000..85fc0af0 --- /dev/null +++ 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). diff --git a/plugins/modules/mysql_db.py b/plugins/modules/mysql_db.py index 5acdb652..0830a129 100644 --- a/plugins/modules/mysql_db.py +++ b/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 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 @@ 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, diff --git a/tests/integration/targets/setup_mysql/vars/main.yml b/tests/integration/targets/setup_mysql/vars/main.yml index 94b43b46..ecc21e38 100644 --- 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 diff --git 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 new file mode 100644 index 00000000..58285b32 --- /dev/null +++ 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 diff --git a/tests/integration/targets/test_mysql_db/tasks/main.yml b/tests/integration/targets/test_mysql_db/tasks/main.yml index 958e3413..df6bb077 100644 --- 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 diff --git a/tests/integration/targets/test_mysql_db/tasks/state_present_absent.yml b/tests/integration/targets/test_mysql_db/tasks/state_present_absent.yml index 02411f03..e5c5f336 100644 --- 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 }}"