search

zammad / zammad-docker-compose

Zammad Docker images for docker-compose
https://hub.docker.com/r/zammad/zammad-docker-compose/
GNU Affero General Public License v3.0
268 stars 207 forks source link

Update backup.sh to include complete /opt/zammad folder for file storage #390

Closed migasQ closed 8 months ago

migasQ commented 9 months ago

See https://github.com/zammad/zammad-docker-compose/issues/389

mgruner commented 9 months ago

Hello @migasQ. Thank you very much for your contribution, we appreciate this very much!

In the current case, I am not sure if it is the right way to go. The zammad directory is part of the docker image, and therefore a backup would be redundant. We could consider adding a separate backup file for the storage/ volume in Zammad. But there may be very much data in there, so it may be a bad idea to force backups of this data on users.

@monotek what's your opinion on this?

MrGeneration commented 9 months ago

I'd like to suggest once again to use our onboard backup script instead of the super custom one. It already supports non full fs dump approaches to only get storage/.

André mentioned that there was incompatibilities in the past, I don't remember exactly. Maybe it would be a great idea to correct this behavior to reduce maintenance.

sstidl commented 9 months ago

I'd like to suggest once again to use our onboard backup script instead of the super custom one. It already supports non full fs dump approaches to only get storage/.

André mentioned that there was incompatibilities in the past, I don't remember exactly. Maybe it would be a great idea to correct this behavior to reduce maintenance.

Which one are you referring to?

MrGeneration commented 9 months ago

Which one are you referring to?

contrib/backup/zammad_backup.sh ?

sstidl commented 9 months ago

Which one are you referring to?

contrib/backup/zammad_backup.sh ?

Thanks. So this needs a config file. Maybe that's why it's not used here.

@mgruner do you remember why you used your own script?

If you like, I could try to use the script from inside the container for backup in a dockerizd setup

mgruner commented 9 months ago

@migasQ that was before my time here, so I cannot answer the question about reasons.

To use the included script would probably take two steps:

migasQ commented 8 months ago

To use the included script would probably take two steps:

* Enhance the backup script in Zammad allow using ENV variables instead of the `config` file (which should still also work, of course). The name of the ENVs would have to be more specific than the ones in the `config` I guess, like `ZAMMAD_BACKUP_*`, and could be mapped internally.
* Changing zammad-docker-compose to set the vars and use the script instead.

I can give it a try in the next weeks. My approach would be to change the compose-backup-entrypoint-script to

mgruner commented 8 months ago

I may have been thinking too complex here. Can't we just modify the existing backup.sh script of zammad-docker-compose to not perform the actual backup itself, but instead first write the conf file from the environment variables, and then invoke the backup script of Zammad to perform the backup? That would avoid a modification in Zammad.

migasQ commented 8 months ago

Jep, that would be my approach.

monotek commented 8 months ago

I may have been thinking too complex here. Can't we just modify the existing backup.sh script of zammad-docker-compose to not perform the actual backup itself, but instead first write the conf file from the environment variables, and then invoke the backup script of Zammad to perform the backup? That would avoid a modification in Zammad.

trying to write config in a read only fs might need other workarounds like new volumes again :/ so if we have to adjust it (i don't see the need anyway) i would go fo a env var based change.

mgruner commented 8 months ago

I'll close this PR now. Feel free to open another one for a new approach.

mgruner commented 3 months ago

Small follow-up: Zammad's backup script zammad_backup.sh is really not written towards being usable within a dockerized environment and thus seems not a good fit.