juanluisbaptiste / docker-otrs

The unofficial Znuny/OTRS Ticketing System docker image
https://www.juanbaptiste.tech/category/otrs
GNU Lesser General Public License v3.0
173 stars 101 forks source link

Improve documentation on restoring a backup #48

Closed kordianbruck closed 5 years ago

kordianbruck commented 5 years ago

from what I gather from this file, using a folder will not work in this case. (https://github.com/juanluisbaptiste/docker-otrs/blob/58b3c0302b98c59953cc8d34cc1d5ae487c40f3e/otrs/functions.sh#L96)

Please also see the difference between -e and -f: https://stackoverflow.com/questions/10204562/difference-between-if-e-and-if-f

I actually would also prefer to simplify the script at this point and getting rid of the folder option, which is currently not working, alltogether. It was confusing why I suddenly need to have a folder, while the backup script gives me a single tarball.

In addition: the backup script also restores Kernel/Config.pm which might have different database parameters than the "new db" with compose. I had to manually edit those parameters in the config file after the restore in order to get things working.

juanluisbaptiste commented 5 years ago

Please also see the difference between -e and -f: https://stackoverflow.com/questions/10204562/difference-between-if-e-and-if-f

I'm aware of it and the use of -f is correct.

I actually would also prefer to simplify the script at this point and getting rid of the folder option, which is currently not working, alltogether. It was confusing why I suddenly need to have a folder, while the backup script gives me a single tarball.

The script accepts both the directory and a tarball as stated in the README file:

OTRS_BACKUP_DATE is the backup name to restore. It can have two values: Uncompressed backup file: A directory with its name in the same date_time format that the OTRS backup script uses, for example OTRS_BACKUP_DATE="2015-05-26_00-32". A backup file created with this image or with any OTRS installation will work (the backup script creates the directory with that name). Compressed backup file: A gzip tarball file created by OTRS.

If you have done an OTRS backup before you would know that its output is a directory named with the backup date and contains the backup files (db dump, config files and data files). The tarball backups is that directory compressed and is created by this container (not by OTRS as the README wrongly says), just for convenience. It was done like this because the first use of this project was to migrate customer installs to docker. But indeed the name of OTRS_BACKUP_DATE as of now that can hold two values can be confusing, it should be renamed as OTRS_BACKUP or OTRS_BACKUP_FILE.

Reviewing the code, the directory restore got broken at some point (probably after a refactor), if you look down further the restore_backup function you will see the directory handling code which needs fixing.

In addition: the backup script also restores Kernel/Config.pm which might have different database parameters than the "new db" with compose. I had to manually edit those parameters in the config file after the restore in order to get things working.

No it doesn't, Kernel/Config.pm is directly updated by _add_configvalue function:

function restore_backup() {
  [ -z $1 ] && print_error "OTRS_BACKUP_DATE not set." && exit 1
  #Check if a host-mounted volume for configuration storage was added to this
  #container
  check_host_mount_dir
  add_config_value "DatabaseUser" ${OTRS_DB_USER}
  add_config_value "DatabasePw" ${OTRS_DB_PASSWORD}
  add_config_value "DatabaseHost" ${OTRS_DB_HOST}
  add_config_value "DatabasePort" ${OTRS_DB_PORT}
  add_config_value "Database" ${OTRS_DB_NAME}
...
kordianbruck commented 5 years ago

@juanluisbaptiste I just used the backup script from this repo to generate a tar from an older OTRS installation to be migrated. I didn't know that OTRS uses the folder structure itself. Then of course offering both is a good idea.

I'm aware of it and the use of -f is correct.

It wouldn't allow a directory on my system. Only a file, so that's what I tried to point out: if it should be either then we probably should use -e no?

No it doesn't, Kernel/Config.pm is directly updated by add_config_value function: Interesting, it didn't work for me, but maybe it was some sort of hickup. Anyway, not a big deal.

Thanks for looking into these issues!