Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from

Conversation

migasQ
Copy link
Contributor

@migasQ migasQ commented Dec 8, 2023

See #389

@mgruner
Copy link
Collaborator

mgruner commented Dec 11, 2023

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?

Copy link
Collaborator

@mgruner mgruner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.

@MrGeneration
Copy link
Member

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
Copy link

sstidl commented Dec 11, 2023

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
Copy link
Member

Which one are you referring to?

contrib/backup/zammad_backup.sh ?

@sstidl
Copy link

sstidl commented Dec 11, 2023

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
Copy link
Collaborator

mgruner commented Dec 13, 2023

@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:

  • 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.

@migasQ
Copy link
Contributor Author

migasQ commented Dec 22, 2023

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

  • consume some config VARs
  • forward them to the actual backup script
  • take care of the cron part, aka the periodical calling of the actual backup script

@mgruner
Copy link
Collaborator

mgruner commented Dec 22, 2023

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
Copy link
Contributor Author

migasQ commented Dec 22, 2023 via email

@monotek
Copy link
Member

monotek commented Jan 8, 2024

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
Copy link
Collaborator

mgruner commented Jan 12, 2024

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

@mgruner mgruner closed this Jan 12, 2024
@mgruner
Copy link
Collaborator

mgruner commented Jun 11, 2024

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.

  • It requires a config file to read the backup configuration.
  • It reads the database connection settings from the database.yml file.
  • It has more code that works with the zammad services in a package installation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants