-
Notifications
You must be signed in to change notification settings - Fork 69
Fix multiple bugs in the restore script #1112
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
Conversation
|
Oops! Looks like you forgot to update the changelog. When updating CHANGELOG.md, please consider the following:
|
bdfe75c to
d72b866
Compare
| --network=$NETWORK \ | ||
| postgres:17.6 \ | ||
| bash -c "psql -h postgres -U $POSTGRES_USER -c 'DROP DATABASE IF EXISTS events;'" | ||
| bash -c "psql -h postgres -U $POSTGRES_USER -c 'DROP DATABASE IF EXISTS events WITH (FORCE);'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@naftis I had to add a force drop here as there might be clients connected to the DB when this restore procedure is ran. Is this ok?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm feeling confident that line was there. It was added by @cibelius about month ago.
I was reviewer on that PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
might be a merge issue once again
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm feeling confident that line was there. It was added by @cibelius about month ago.
I added WITH (FORCE) to clear-all-data.sh but not this restore.sh:
https://github.com/opencrvs/opencrvs-countryconfig/blob/develop/infrastructure/clear-all-data.sh#L147-L148
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the services restarted on restore? Not sure to be honest if we have any long running clients - but they'll get dropped then. At least restart solves it
To be honest it's a database drop so if someone is listening they'll get some funky errors any way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, I think we actually do have in our documentation https://documentation.opencrvs.org/setup/3.-installation/3.3-set-up-a-server-hosted-environment/4.3.7-backup-and-restore/4.3.7.1-restoring-a-backup
OpenCRVS requires to be re-deployed to function properly once a backup has been restored.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was looking into wrong file, sorry for misleading you
| --network=$NETWORK \ | ||
| postgres:17.6 \ | ||
| bash -c "createdb -h postgres -U $POSTGRES_USER events && pg_restore -h postgres -U $POSTGRES_USER -d events /backups/events-${LABEL}.dump" | ||
| bash -c "createdb -h postgres -U $POSTGRES_USER events && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to also create the database back here, with the schema and correct grants @naftis
| psql -h postgres -U $POSTGRES_USER -d events -c 'CREATE SCHEMA app AUTHORIZATION events_migrator; GRANT USAGE ON SCHEMA app TO events_app;' && \ | ||
| pg_restore -h postgres -U $POSTGRES_USER -d events --schema=app /backups/events-${LABEL}.dump" | ||
| echo "Update credentials in Postgres on restore" | ||
| docker service update --force opencrvs_postgres-on-update |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This updates PostgreSQL credentials after restore
| --network=$NETWORK \ | ||
| postgres:17.6 \ | ||
| bash -c "psql -h postgres -U $POSTGRES_USER -c 'DROP DATABASE IF EXISTS events;'" | ||
| bash -c "psql -h postgres -U $POSTGRES_USER -c 'DROP DATABASE IF EXISTS events WITH (FORCE);'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm feeling confident that line was there. It was added by @cibelius about month ago.
I was reviewer on that PR.
We encounted issues with the restore script when testing backup + restore with Ashikul. This should now fix all of them