fix: exclude volume-backups from web server backup rsync command#4015
Conversation
- Updated the rsync command in the runWebServerBackup function to exclude the 'volume-backups/' directory, ensuring that unnecessary data is not copied during the backup process.
|
|
||
| await execAsync( | ||
| `rsync -a --ignore-errors --no-specials --no-devices ${BASE_PATH}/ ${tempDir}/filesystem/`, | ||
| `rsync -a --ignore-errors --no-specials --no-devices --exclude='volume-backups/' ${BASE_PATH}/ ${tempDir}/filesystem/`, |
There was a problem hiding this comment.
Anchor the exclude pattern to source root
The pattern --exclude='volume-backups/' will match a directory named volume-backups/ at any depth within BASE_PATH. While this works correctly today since volume-backups/ only exists at the top level of BASE_PATH, using a leading / anchors the pattern to the rsync source root and is more precise:
| `rsync -a --ignore-errors --no-specials --no-devices --exclude='volume-backups/' ${BASE_PATH}/ ${tempDir}/filesystem/`, | |
| `rsync -a --ignore-errors --no-specials --no-devices --exclude='/volume-backups/' ${BASE_PATH}/ ${tempDir}/filesystem/`, |
| await execAsync( | ||
| `rsync -a --ignore-errors --no-specials --no-devices ${BASE_PATH}/ ${tempDir}/filesystem/`, | ||
| `rsync -a --ignore-errors --no-specials --no-devices --exclude='volume-backups/' ${BASE_PATH}/ ${tempDir}/filesystem/`, | ||
| ); |
There was a problem hiding this comment.
Restore wipes volume-backups not included in backup
Now that volume-backups/ is excluded from the web server backup, the corresponding restore operation in packages/server/src/utils/restore/web-server.ts will permanently destroy all volume backups during a restore. The restore code (line 56) does:
await execAsync(`rm -rf "${BASE_PATH}/"*`);This blanket deletion removes ${BASE_PATH}/volume-backups/, but since the directory is no longer included in the backup, it is never restored. Any volume backups present at the time of restoration will be silently lost.
Consider updating the restore logic to preserve volume-backups/ by moving it aside before wiping BASE_PATH and placing it back afterwards, or by using a targeted delete that skips volume-backups/:
// Delete everything except volume-backups
await execAsync(`find "${BASE_PATH}" -maxdepth 1 -mindepth 1 ! -name 'volume-backups' -exec rm -rf {} +`);
What is this PR about?
Please describe in a short paragraph what this PR is about.
Checklist
Before submitting this PR, please make sure that:
canarybranch.Issues related (if applicable)
closes #3971
Screenshots (if applicable)
Greptile Summary
This PR adds
--exclude='volume-backups/'to the rsync command inrunWebServerBackup, preventing potentially large Docker volume backup files from bloating web server backups. The change is a minimal, targeted fix for issue #3971.Key changes:
packages/server/src/utils/backups/web-server.ts: Excludesvolume-backups/from rsync during web server backup.Issues found:
'volume-backups/'is not anchored to the rsync source root. Using'/volume-backups/'(with a leading/) would be more precise, though the current version works correctly given the known directory structure.restoreWebServerBackup) performs a blanketrm -rf "${BASE_PATH}/"*to clear the destination before restoring. Sincevolume-backups/is now excluded from the backup, any existing volume backups will be permanently deleted during a restore and will not be recovered. The restore logic should be updated to preservevolume-backups/across the restore operation.Confidence Score: 2/5
volume-backups/from backups without updating the restore logic, any user who restores a web server backup will silently lose all their Docker volume backups — the restore wipesBASE_PATHentirely before copying files back, and sincevolume-backups/is no longer in the archive, it is never restored.packages/server/src/utils/restore/web-server.ts— the blanketrm -rf "${BASE_PATH}/"*will destroyvolume-backups/during restore since it is no longer included in backups.Last reviewed commit: a2f1421