docker-zulip icon indicating copy to clipboard operation
docker-zulip copied to clipboard

entrypoint: fix broken automatic backups

Open Roosted7 opened this issue 5 years ago • 9 comments

The scheduled database backups, that run by default using a cron timer, have not worked for a while. This seemed to be for 2 reasons:

  • Firstly the date command was not used properly, since both the '+' for the format was missing and %D should be %F (since the slashes produced by %D create new folders due to the usage in the script).
  • Secondly, the password was not passed on to pg_dump. This resulted in it not being able to run from the cron timer.

This PR fixes both those things :)

I have seen plans to drop this backup system in favor of the integrated one, but for the time being it would make sense to at least have this backup function operational?

Roosted7 avatar Nov 25 '20 15:11 Roosted7

Sorry for the slow reply, and thanks for the PR!

@hackerkid can you take a look at this?

timabbott avatar Feb 16 '21 22:02 timabbott

@Roosted7 Do you know when was the last successful backup?

hackerkid avatar Feb 17 '21 19:02 hackerkid

@hackerkid Nope, our installation is quite recent and backups never worked. Only while implementing a custom (internal) backup solution, I noticed this broken built-in functionality and fixed that instead.

Roosted7 avatar Feb 18 '21 13:02 Roosted7

@Roosted7 Okay. Just to verify, with this fix the corn job is correctly creating the backup right?

One thing I am a bit puzzled about is this.

root@de940ecbcfd4:~# more /etc/cron.d/autobackup 
MAILTO=""
30 3 * * * cd /;/entrypoint.sh app:backup
root@de940ecbcfd4:~#  cd /;/entrypoint.sh app:backup

The cronjob try to execute /entrypoint.sh but entrypoint.sh is present in /sbin/ directory. Shouldn't it be cd /;entrypoint.sh app:backup or /sbin/entrypoint.sh app:backup instead?

hackerkid avatar Feb 18 '21 18:02 hackerkid

Oh, I think we should actually just be removing this pg_dump logic entirely in favor of running manage.py backup, which is more carefully implemented and also designed to be portable to a non-docker-zulip system.

timabbott avatar May 05 '21 18:05 timabbott

The cron entry looks wrong as well. Being on the filesystem it should include the username. The resulting schedule should be

30 3 * * * root cd /usr/sbin/;./entrypoint.sh app:backup

maxxer avatar Nov 25 '21 13:11 maxxer

#262

maxxer avatar Nov 26 '21 11:11 maxxer

@alexmv you cool with me closing this in favor of #262? We then have a forking path to decide between taking the PGPASSWORD bits here and fixing (and probably extensively testing, given the discussion above) the cron job, or to migrate to manage.py backup outright, which I think is probably best suited as an Issues ticket for now until we know we have time to take it on (or an interested party to do so).

klardotsh avatar Nov 23 '22 03:11 klardotsh

Fixing the cron entry is probably useful regardless of how the backup runs. I don't know that doing any of the PGPASSWORD work is useful, given #262.

alexmv avatar Nov 29 '22 23:11 alexmv