feat: #638 Add cronjob config for database restore #674
Conversation
MCatherine1994
changed the title
| echo "Running SQL file: $sql_file" | ||
| psql -h ${NAME}-${ZONE}-${COMPONENT} -U ${POSTGRES_USER} -d ${POSTGRES_DB} -f $sql_file | ||
| echo "Finish database restore" | ||
| fi |
There was a problem hiding this comment.
So now the logic is that:
- Try to find the sql file first (in case already unzip it in the past)
- If not found, try to find the zipped sql file, and unzip it
- Get the sql file again, if not found then print "no backup sql found" and do nothing
- If found then run restore script
There was a problem hiding this comment.
Should exit with error if no backup SQL file is found. Should check the return value of the psql commands and report errors.
There was a problem hiding this comment.
The restore finishes with the old schema existing, which is fine. Part of the recovery process could be to remove that. Also, maybe this process should perform a backup first before doing the restore?
There was a problem hiding this comment.
yeah, we should remove that, otherwise we can't run the restore multiple times. Yeah, I like the idea to the cleanup before run the restore, thanks!!
There was a problem hiding this comment.
when no backup file is found, it will just print "Not found" and do nothing for now. I'll check how to report errors.
There was a problem hiding this comment.
So rather than having the cron job finish successfully in the case of no backup SQL file found, we should have the job run marked as a failure (which I assume will happen with a non-zero exit code from the script).
There was a problem hiding this comment.
When no backup file found, exit 1:
When psql command failed, exit 1 (the following example failure happened when I pass an empty file psql -h fom-24-db -U ${POSTGRES_USER} -d ${POSTGRES_DB} -f ''):
There was a problem hiding this comment.
I think it's better to run backup before restore, but not sure if we want to include that in this script as well, it's getting complicated....
There was a problem hiding this comment.
Yes, probably sufficient for now to just have our documented process say to do a manual backup before trying to do the restore. (and bring the app down before doing the backup).
…test-db-backup
db/openshift.deploy.yml
Outdated
| app: ${NAME}-${ZONE} | ||
| cronjob: ${NAME}-${ZONE} | ||
| spec: | ||
| backoffLimit: ${{JOB_BACKOFF_LIMIT}} |
There was a problem hiding this comment.
Where are these limits set?
There was a problem hiding this comment.
these are set for the db backup cronjob, I thought maybe I can use them here as well
There was a problem hiding this comment.
Hmm, not sure if that makes sense, since we have retry = never.
There was a problem hiding this comment.
um... this I'm not so sure... just follow the one for db backup
There was a problem hiding this comment.
I changed to 0. The restart policy sets to never will ensure when the pod fails, it won't be restarted. But we still need to set backofflimit to make sure the job will not be retired.
| echo "Running SQL file: $sql_file" | ||
| psql -h ${NAME}-${ZONE}-${COMPONENT} -U ${POSTGRES_USER} -d ${POSTGRES_DB} -f $sql_file | ||
| echo "Finish database restore" | ||
| fi |
There was a problem hiding this comment.
The restore finishes with the old schema existing, which is fine. Part of the recovery process could be to remove that. Also, maybe this process should perform a backup first before doing the restore?
…test-db-backup
|
Reading the wiki process: "Do a manual backup of the current database (can do that in the database pod, so this temporary backup will be stored in the database volume)" -> the specific commands to run should be provided. I'm a little concerned by doing this versus the normal backup process as if you need to restore from this special backup you need a special restore process, but on the other hand if the regular restore process fails, this approach should still succeed. Just want to be careful that if this special backup isn't stored within a PVC, then if the DB pod restarts the backup will be lost. |
| # use the same image as our database, so we can run the psql command | ||
| image: image-registry.apps.silver.devops.gov.bc.ca/${OC_NAMESPACE}/${NAME}-${ZONE}-${COMPONENT}:${ZONE}-db | ||
| command: ["/bin/sh", "-c"] | ||
| args: |
There was a problem hiding this comment.
I thought we talked about making this a script file that's loaded into the database image. But I'm okay if it is left like this.
| exit 1 | ||
| else | ||
| echo "Found SQL file, rename existing database, and create a new empty database" | ||
| psql -h ${NAME}-${ZONE}-${COMPONENT} -U ${POSTGRES_USER} -c "DROP DATABASE IF EXISTS ${OLD_FOM_DATABASE_NAME};" -c "ALTER DATABASE fom RENAME TO ${OLD_FOM_DATABASE_NAME};" -c "CREATE DATABASE fom;" |
There was a problem hiding this comment.
Still not checking return code from this psql call.
| exit 1 | ||
| else | ||
| echo "Found SQL file, rename existing database, and create a new empty database" | ||
| psql -h ${NAME}-${ZONE}-${COMPONENT} -U ${POSTGRES_USER} -c "DROP DATABASE IF EXISTS ${OLD_FOM_DATABASE_NAME};" -c "ALTER DATABASE fom RENAME TO ${OLD_FOM_DATABASE_NAME};" -c "CREATE DATABASE fom;" |
There was a problem hiding this comment.
The process documented in the wiki could talk more about what to do if the restore fails (or wasn't the correct restore point and needs to be redone), which could mean picking a different value for old_fom_database_name, and/or one option for reverting the restore would be to drop the fom database and rename old_fom to fom...
|
Team Evergreen has in their backlog to look at DB restore/backup as part of STRA-- goal was to not use PVC but object storage. they had buckets created for this purpose, but I am not sure how far they have gone with it yet. @craigyu or @DerekRoberts may know? |
|
@webgismd for FOM at least, the database is small and we store a limited number of backups so storage demands are limited. But there are some nice aspects to using object storage instead of a PVC - better resistance to ransomware-style attacks, especially if you can structure your object storage permissions to be insert-only... I'd be more worried though about every team developing a completely different backup/restore process, versus having a defined method for OpenShift Postgres DBs that all the teams can leverage. |
|
I 100% agree here @basilv , FDS is essentially on its own to develop a pattern for itself. I don't see alot of tactile support from other teams in this area. Perhaps something to discuss with @RMCampos @jazzgrewal @craigyu @paulushcgcj @abschwenker @DerekRoberts |
The base here was derived from client. In our case, we will keep both a PVC and an S3 backup, as the PVC is a "hot" copy, that will be the more recent one while the S3 will have all past and all current ones, but will be handled as a "cold" copy. This was at least the idea/suggestion I gave and is what we're aiming to do on client. |
Full documentation is at: https://github.com/bcgov/nr-fom/wiki/Database-Restore
Thanks for the PR!
Any successful deployments (not always required) will be available below.
Once merged, code will be promoted and handed off to following workflow run.
Thanks for the PR!
Any successful deployments (not always required) will be available below.
Once merged, code will be promoted and handed off to following workflow run.