docker-db-backup icon indicating copy to clipboard operation
docker-db-backup copied to clipboard

Restore a db backup doesn't read port (again)

Open Korvenwin opened this issue 2 years ago • 8 comments

Summary

This problem was posted https://github.com/tiredofit/docker-db-backup/issues/121 and solved.

Now with v3.3.2 it occurs again.

When restoring a database backup with 'restore' command, all parameters are accepted until DBPORT that allways stays waiting. Steps to reproduce

Start a restore without parameters What is the expected correct behavior?

DBPORT should be accepted as others parameters. Relevant logs and/or screenshots Environment

Image version / tag: v3.3.2
Host OS: Unraid

Korvenwin avatar May 07 '22 15:05 Korvenwin

New information: I've installed v3.2.5 and it restore ok.

Korvenwin avatar May 08 '22 18:05 Korvenwin

Hi there, not sure what happened - I remember repairing this before and I then went on a long trip- Its now back to the way it was with newly released 3.3.5 - Please let me know if indeed solved.

tiredofit avatar Jun 08 '22 16:06 tiredofit

I've also been experiencing strange behaviour when calling the restore script specifying ALL 7 parameters:

restore <filename> <db_type> <db_hostname> <db_name> <db_user> <db_pass> <db_port> $0......$1.........$2........$3............$4........$5........$6........$7

the script keeps showing questions and returning an error because it puts the password in the username.

I think this is related to this issue with reading the port and I think it is not solved yet.

@tiredofit I noticed that at line 855 of the restore script, the Database Name is retrieved from $3 instead of $4, while at line 843 the Database Host is also retrieved from $3. All the following parameters are shifted by 1 place. In fact at line 891 Database Port is retrieved from $6 instead of $7.

buxm avatar Jun 22 '22 21:06 buxm

Thanks for this sleuthing. I will take a peek here and see if I can resolve.

tiredofit avatar Jun 22 '22 23:06 tiredofit

You are right! I had the arguments duplicated for $3 and that caused everything to shift. I've pushed tiredofit/db-backup:3.3.6 that contains a fix.

tiredofit avatar Jun 23 '22 15:06 tiredofit

It seems to be more consistent now, but there is still something weird. On purpose I tried to pass the wrong db_port parameter to the script (3307), while the environment variable DB_PORT has been set to the correct value (3306) since the container creation. The restore is, unexpectedly, successful because I think it used the correct DB_PORT env variable rather than the incorrect parameter that I passed to the restore script. But shouldn't the script prioritize the parameter over the DB_PORT env variable? Am I not supposed to override the port set at container creation time with a different one passing it as a parameter to the restore script?

I'm not sure whether this is linked..... but at line 892 of the restore script there is: if [ ! -f "${7}" ]; then which, if I'm not getting something wrong, tries to test if $7 is not a file. If it's not a file, it calls get_dbport (which I think gets the db port from the env variable or from an interactive user's input); if it is a file, it assigns $7 to the DB port. ... But how could the db port ever be a file? I suspect the execution flow always goes to calling get_dbport rather than using $7. Potentially, the same could happen for all other parameters apart from $1, since none of them is supposed to be a file but all undergo a -f test. Unless, as I said, I'm getting something completely wrong about the -f test.

buxm avatar Jun 23 '22 18:06 buxm

This is excellent feedback and you are right- arguments are supposed to override any defaults. I need to figure out why I put -f in here as -f is explicitly for the existence of files which would make sense for $1 but not the other. This is definitely me cheating when I put together this script. TBH I've used the restore script once personally (this morning in fact) for a restore, so this is valuable feedback and testing input.

Let me see where I've gone wrong here and push you a new release shortly. EDIT - that was very quick looking at it - I have a 3.3.7 coming now.

tiredofit avatar Jun 23 '22 18:06 tiredofit

@tiredofit it seems to be working to me, now. Many thanks!

buxm avatar Jun 25 '22 11:06 buxm

Something seems to be going weird again in 3.7.3 (and maybe also some earlier version). If I don't set DB_PORT, here is the debug log that is printed:

+ '[' -n '' ']'
+ get_dbport
+ '[' -z 3306 ']'
+ '[' -n 3306 ']'
+ print_debug 'Parsed DBPort Variant: 2 - Env'
+ output_off
+ '[' true = true ']'
+ set +x
2023-01-11.21:57:28 [DEBUG] /usr/local/bin/restore ** [db-backup-restore] Parsed DBPort Variant: 2 - Env
/assets/functions/00-container: line 800: /var/log/container//container.log: No such file or directory
+ q_dbport_variant=2
++ cat
+ q_dbport_menu='
    C ) Custom Entered Database Port
    E ) Environment Variable DB_PORT: '\''3306'\'''
+ cat

What Database Port do you wish to use? MySQL/MariaDB typcially listens on port 3306. Postrgresql port 5432. MongoDB 27017

    C ) Custom Entered Database Port
    E ) Environment Variable DB_PORT: '3306'
    Q ) Quit

+ case "${q_dbport_variant}" in
+ true
++ echo -e '\e[92m**' '\e[90mEnter' Value '(\e[97mC\e[90m)' '|' '(\e[97mE\e[90m)' : '\e[97m\e[0m'
+ read -p '** Enter Value (C) | (E) :  ' q_dbport_menu
+ case "${q_dbport_menu,,}" in
+ r_dbport=3306
+ break
+ print_debug 'Database Port '\''3306'\'''
+ output_off
+ '[' true = true ']'
+ set +x
2023-01-11.21:57:28 [DEBUG] /usr/local/bin/restore ** [db-backup-restore] Database Port '3306'

Essentially the restore script prompts me for the port to use:

What Database Port do you wish to use? MySQL/MariaDB typcially listens on port 3306. Postrgresql port 5432. MongoDB 27017

    C ) Custom Entered Database Port
    E ) Environment Variable DB_PORT: '3306'
    Q ) Quit

but then it keeps running using the default port. So apparently it is more a cosmetic issue than anything else, since, at least in my case, the restore completes successfully anyway.

buxm avatar Jan 11 '23 22:01 buxm