netz98 / n98-magerun2

The swiss army knife for Magento developers, sysadmins and devops. The tool provides a huge set of well tested command line commands which save hours of work time. All commands are extendable by a module API.
https://magerun.net
Other
870 stars 221 forks source link

Exit code on db:dump is always 0 #929

Open schmengler opened 2 years ago

schmengler commented 2 years ago

Describe the bug

When running the db:dump command and mysqldump fails, magerun will still return with an exit code of 0.

Expected behaviour

Non-zero exit code on error

Steps to reproduce the issue

  1. Change app/etc/env.php to invalid database credentials
  2. Run magerun db:dump
  3. See error like mysqldump: Got error: 2005: "Unknown MySQL server host ''bielefeld'' (-2)" when trying to connect
  4. Check error code, e.g. with echo $?

Technical details

Possible Fix

In script, I currently work around the issue with $(magerun db:dump --only-command) (together with set -e -o pipefail)

Additional context

This is important for scripting, e.g. database backup as part of the deployment pipeline. If a backup fails, the pipeline should not continue.

cmuench commented 2 years ago

Bug confirmed in v4.9.1

cmuench commented 2 years ago

Found an issue in result of the piped command.

Example:

mysqldump --single-transaction --quick -h'db'  -u'db' --password='db1' 'db' | LANG=C LC_CTYPE=C LC_ALL=C sed -E 's/DEFINER[ ]*=[ ]*`[^`]+`@`[^`]+`/DEFINER=CURRENT_USER/g' > 'db.sql' 2>&1

We have a special pipehandling for bash compatible shells included. The check with getenv('SHELL') is not working anymore.

cmuench commented 2 years ago

Tried with Symfony/Process component. Seems that internally any shell_exec or proc_open with piped commands is executed in OS default shell. On some system, like my ddev test environment based on Debian, the "bin/sh" is a link to "dash". Dash does not support set -o pipefail which is currently needed to stop the pipe. In our command line call the last command is a sed which always returns "0" status code. I am still looking for a good solution.

convenient commented 2 years ago

@cmuench Spotted this and wonder if you have come across this before? http://cfajohnson.com/shell/cus-faq-2.html#Q11

Might be useful, may still produce a dump file but allow you to get the exit code somehow

cmuench commented 2 years ago

@cmuench Spotted this and wonder if you have come across this before? http://cfajohnson.com/shell/cus-faq-2.html#Q11

Might be useful, may still produce a dump file but allow you to get the exit code somehow

I am thankful for every hint. Thought it's easy to fix but then I spent hours to try out possible solutions.

convenient commented 2 years ago

🙈 Maybe it's a case of doing it in multiple processes / steps?

mysqldump --single-transaction --quick -h'db'  -u'db' --password='db1' 'db' > db.sql.tmp
# if exit code !=0 delete db.sql.tmp and exit 1

LANG=C LC_CTYPE=C LC_ALL=C sed -i -E 's/DEFINER[ ]*=[ ]*`[^`]+`@`[^`]+`/DEFINER=CURRENT_USER/g' db.sql.tmp
# if exit code !=0 delete db.sql.tmp and exit 1

mv db.sql.tmp db.sql
# exit 0

It's probably more convoluted PHP code though

cmuench commented 2 years ago

see_no_evil Maybe it's a case of doing it in multiple processes / steps?

mysqldump --single-transaction --quick -h'db'  -u'db' --password='db1' 'db' > db.sql.tmp
# if exit code !=0 delete db.sql.tmp and exit 1

LANG=C LC_CTYPE=C LC_ALL=C sed -i -E 's/DEFINER[ ]*=[ ]*`[^`]+`@`[^`]+`/DEFINER=CURRENT_USER/g' db.sql.tmp
# if exit code !=0 delete db.sql.tmp and exit 1

mv db.sql.tmp db.sql
# exit 0

It's probably more convoluted PHP code though

I agree. This command btw. needs some refactoring.