feat: add backup and restore scripts with steps #31
Conversation
ulope
left a comment
ulope
left a comment
There was a problem hiding this comment.
Looks good overall but some changes needed.
scripts/RESTORE.md
Outdated
| - Database dump (`keyper.dump`) - Contains full schema and data from the `keyper` database | ||
| - Chain data (`data/chain/`) - Blockchain data and configuration | ||
| - Keyper configuration (`config/`) - Application configuration files | ||
| - Environment variables - Metrics configuration settings |
There was a problem hiding this comment.
This also includes the private key, should be mentioned here.
There was a problem hiding this comment.
now copying whole env file except private key
|
|
||
| echo -e "${B}[4/7] Copying data...${DEF}" | ||
| cp -a "${SCRIPT_DIR}/../data/chain/" "${WORKDIR}/chain" | ||
| cp -a "${SCRIPT_DIR}/../data/db/keyper.dump" "${WORKDIR}/keyper.dump" |
There was a problem hiding this comment.
AFAICS the source path is wrong
There was a problem hiding this comment.
the backup is created in the db folder only as previous docker setup, would not have new volume mapped in the compose (db-dump). The idea is to store the dump in the same dir where postgres data is stored and then copy it to db-dump at the time of restore. LMK, if needs more clarification
There was a problem hiding this comment.
You mean in case an operator updates to the new version but doesn't run compose up before using the backup script?
Yeah I guess that's fine.
Co-authored-by: Ulrich Petri <[email protected]>
Co-authored-by: Ulrich Petri <[email protected]>
jannikluhn
left a comment
jannikluhn
left a comment
There was a problem hiding this comment.
Left a few comments, but looks good in general. My biggest question/concern is about pausing vs stopping.
scripts/backup.sh
Outdated
| mkdir -p "${SCRIPT_DIR}/../data/backups" | ||
|
|
||
| echo -e "${B}[1/7] Pausing all services except database...${DEF}" | ||
| docker compose pause keyper || true |
There was a problem hiding this comment.
this swallows errors, shouldn't we abort in that case? Same for the other pause commands below and for the down command in the recovery script.
scripts/restore.sh
Outdated
|
|
||
| echo -e "${B}[3/6] Restoring chain data...${DEF}" | ||
| if [ -d "$WORKDIR/chain" ]; then | ||
| mkdir -p "${SCRIPT_DIR}/../data/chain" |
There was a problem hiding this comment.
this line seems unnecessary, given that the directory is removed immediately in the next line (that rm -rf doesn't care if the directory exists or not). Similarly below
There was a problem hiding this comment.
This was just to make sure the rm -rf does not fail but I have handled that now
scripts/backup.sh
Outdated
| mkdir -p "${SCRIPT_DIR}/../data/backups" | ||
|
|
||
| echo -e "${B}[1/7] Pausing all services except database...${DEF}" | ||
| docker compose pause keyper || true |
There was a problem hiding this comment.
Have you considered stopping the containers instead of pausing them? This would leave them in a more well defined state (e.g. not in the middle of a db transaction or an RPC call).
There was a problem hiding this comment.
great suggestion, I tried with compose down and it works, changed in the scripts
There was a problem hiding this comment.
The reason I initially went with pause in the dappnode export scripts, which this inherited from, was that it should cause minimal service interruption and for postgres it's important to not have any writes while doing a file level backup.
Arguably the length of service downtime won't be much different, so yes, stopping is probably more safe.
Although I would strongly argue to use stop instead of down since down kills the logs.
…-> docker compose down, readme update
ulope
left a comment
ulope
left a comment
There was a problem hiding this comment.
One more small issue, after that it should be GTG.
|
|
||
| echo -e "${B}[4/7] Copying data...${DEF}" | ||
| cp -a "${SCRIPT_DIR}/../data/chain/" "${WORKDIR}/chain" | ||
| cp -a "${SCRIPT_DIR}/../data/db/keyper.dump" "${WORKDIR}/keyper.dump" |
There was a problem hiding this comment.
You mean in case an operator updates to the new version but doesn't run compose up before using the backup script?
Yeah I guess that's fine.
| exit 1 | ||
| fi | ||
|
|
||
| LATEST_BACKUP=$(find "$BACKUPS_DIR" -name "shutter-api-keyper-*.tar.xz" -type f | sort | tail -n 1) |
There was a problem hiding this comment.
Not necessary to do anything here now, but a note for future improvement:
Some people like to run their containers on really stripped down OSs, therefore it would be more robust to use an ad-hoc container to do these shell actions (i.e. not relying on find, sort and tail to be present on the host). Similar to how it's done below with the tar command.
scripts/restore.sh
Outdated
| if [ -n "$CURRENT_SIGNING_KEY" ]; then | ||
| echo "$CURRENT_SIGNING_KEY" >> "${SCRIPT_DIR}/../.env" | ||
| fi |
There was a problem hiding this comment.
Did you test that this works?
Unless I'm missing something this will lead to there being two SIGNING_KEY lines in the file, one at the top with the placeholder and the actual one at the bottom.
This will at least be really confusing to the user if it works at all.
There was a problem hiding this comment.
ahh, nice catch, fixed it to just have a placeholder for signing key after restore.
Co-authored-by: Ulrich Petri <[email protected]>
Co-authored-by: Ulrich Petri <[email protected]>
| - **Backup file available** - The backup archive should be present in the specified backup directory | ||
| - **Signing key available** - You must have the original `SIGNING_KEY` value from your deployment | ||
|
|
||
| ### Restore Steps |
There was a problem hiding this comment.
If this is a fresh install, the data dir doesn't exist, so users would need to create it manually.
There was a problem hiding this comment.
added command to create data directory, if it does not exist already in restore.sh
blockchainluffy
force-pushed
the
feat/api-keyper-backup-scripts
branch
from
ba5ce7e to
18df3c5
Compare
…reate data dir when not available
| docker compose stop chain | ||
|
|
||
| echo -e "${B}[2/6] Creating database dump...${DEF}" | ||
| docker compose exec db pg_dump -U postgres -d keyper -Fc --create --clean -f /var/lib/postgresql/data/keyper.dump |
There was a problem hiding this comment.
I think we also need to delete this dump from the container during the clean and restart exit, to avoid growing the volume with every backup
There was a problem hiding this comment.
the dump file gets overwritten on every backup, anyway I can add command to delete the dump file when backup archive is created
| @@ -0,0 +1,114 @@ | |||
| # Backup and Restore Guide | |||
There was a problem hiding this comment.
Could you rename this file to BACKUP_RESTORE.md so that it is clear that it contains the instructions for both? Thank you.
blockchainluffy
force-pushed
the
feat/api-keyper-backup-scripts
branch
from
708776f to
3f16a25
Compare
closes #29