-
Notifications
You must be signed in to change notification settings - Fork 616
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
Add remote borg backup support #4804
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Tim Diels <tim@diels.me>
Signed-off-by: Tim Diels <tim@diels.me>
Signed-off-by: Tim Diels <tim@diels.me>
Signed-off-by: Tim Diels <tim@diels.me>
Signed-off-by: Tim Diels <tim@diels.me>
Signed-off-by: Tim Diels <tim@diels.me>
I.e. do not support aio-lockfile for remote borg repos. Let's wait and see if if somebody requests it. Signed-off-by: Tim Diels <tim@diels.me>
Signed-off-by: Tim Diels <tim@diels.me>
Signed-off-by: Tim Diels <tim@diels.me>
Signed-off-by: Tim Diels <tim@diels.me>
4d3e48c
to
7e96d1d
Compare
Hi, first of all thank you for your contribution! I've thought a bit about this the last week and came to the conclusion that I do not want to maintain this since it requires an additional server for me for testing (with a not so easy to reproduce setup) and has the potential of many users needing help getting this to work (and thus only a limited usecase). So I would still only test local backup also in the future. However I would be fine with merging this if you @timdiels would step in as the maintainer of this feature. That means if bug reports or any questions regarding this feature come in, I would ask you for help on the topic. If that is fine for you, I would continue with the review.
Yes, this is expected and documented.
Sounds good!
Yeah, we could create a dev instance from this if we proceed...
Yes |
if ! rsync --archive --human-readable -vv \ | ||
/tmp/borg/nextcloud_aio_volumes/nextcloud_aio_mastercontainer/data/configuration.json \ | ||
/nextcloud_aio_volumes/nextcloud_aio_mastercontainer/data/configuration.json; then |
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 would personally like to keep rsync at least for local archives since as you found out borg extract does not allow for deleting existing files and rsync is much faster at least for local archives... but we could split the logic and use rsync for local archives and borg extract for remote archives...
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.
Sure, I'll bring back the borg mount and rsync for local repos.
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.
Done
Hi,
I'm willing to maintain it, mostly on weekends, for as long as I'm a nextcloud user (which I have been for a couple of years so far).
I will do it on the dev build after I've made the change you suggested.
Is there an easier way to develop than the hacks I did (in a different branch) for local testing? Not sure how you normally test AIO? If it's always via push to github, wait for build and deploy, I will probably keep my debug branch for later. Note to self:
|
Cool, then we can go ahead with this PR. I've invited you to the repo for easier collaboration :)
Great :)
Usually it is indeed always via push to github, wait for build and deploy. So probably it is easier for you to keep your debug branch 👍 |
Very much appreciate the PR guys! |
echo "Could not initialize borg repository." | ||
rm -f "$BORG_BACKUP_DIRECTORY/config" | ||
if [ -z "$BORG_REMOTE_REPO" ]; then | ||
# Originally we checked for presence of the config file instead of calling `borg info`. Likely `borg info` |
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.
Should this be "Likely borg init
will error on a partially initialized repo"?
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.
Probably most borg commands would fail but it's referring to the borg info on line 120. If we ever end up with a partially initialized repo (sounds very rare), I think manual intervention is required anyway? The important thing is I believe borg info
will fail on such a corrupt repo and will end up at line 124 instead of trying to initialize it again. The config file was used as a sentinel to detect whether the repo has been initialized already (see the deleted line 107), which why it is removed here and why it's probably no longer necessary to do this.
Signed-off-by: Tim Diels <tim@diels.me>
Signed-off-by: Tim Diels <tim@diels.me>
I tried out local backups on my machine and made all the requested changes I believe. Could we get a a dev build going? |
This fixes: borg config: error: argument REPOSITORY: "::": Repository must be local Signed-off-by: Tim Diels <tim@diels.me>
Created a backup on my server, I'll enable autobackup/update and try out restoring after some changes later (might be next weekend):
(the duration is pretty much optimal considering my 19 Mbps ISP upload speed) |
I rolled back to an old snapshot because apparently the dev build was older than the latest released nextcloud, so it wouldn't start anymore because e.g. elastic can't roll back. I'll try again on the weekend with a tmp vm instead. |
Appreciate all the work you are doing on this @timdiels. Very much looking forward to this addition! |
@szaimen Is something missing before this pull request can be merged? I see it keeps being pushed back to the next release. I'm not sure of the significance of this as I'm not very familiar with the release process that Nextcloud AIO follows. Thanks! |
Mostly I am waiting for the results of the tests that @timdiels was doing... Afterwards this needs conflict resolution, review and testing... |
Is there a way to get it to rebuild the develop-4804 image? My last change is not included in the image. |
Retested remote and local backups with the dev image and it all seems to work. This time I added a local caddy so I could actually test with adding a user, some files, some notes through the UI. |
…-backup3 Signed-off-by: Tim Diels <tim@diels.me>
Up to date again, I mostly only saw a switch from There's the one change in the backupscript that I haven't been able to test because I need a rebuild of the dev image for that. Other than that it's a go for me. |
Hi @szaimen, my turn for a gentle ask on the status of this one? @timdiels seems to have brought things up to date and is ready to test. Really looking forward to this feature so I can roll out Nextcloud for my organization. Easy backup to a remote location is the only thing holding me up. I've tried several of the other documented methods but found them to either be unreliable or too complicated. Please let us know. Thanks! |
Hi @timdiels, sorry for my late reply. I was on vacation. So my plan would be to include this for the next major release of AIO which will likely be released after Nc30.0.1 or 30.0.2 is out. Until then we have a bit of time to finish this. I for now rebased your branch and pushed my attempts to #5206. If possible, I would like to continue working on that branch. Please review my rebase, possibly some details where going wrong as I did resolve the conflicts manually. I also already built containers for that branch, using the However if you would like to continue working on this branch though, then I can say that yesterday evening, I updated the containers using the existing |
Hi @timdiels can you please decide on which branch we should continue to work on? |
Hi @szaimen This branch just needs one more manual check on the newly built image but other than that it's ready to be merged as far as I'm concerned so it would seem less work to stick with this branch, no? Or you mean that during the next steps more work may pop up that needs to be done before it can be released? |
On the other hand, if we then would need to repeat the work to merge it into the next major release, then I would prefer using the new branch. |
I manually tested my last change, it fixes the problem. Looks good to merge for me. |
This adds support for backing up directly to a remote borg repo. (I tried reaching out earlier)
Why?
I'd like to backup/restore via the UI, resist ransomware and store the backups remotely without storing a local (compressed) copy of the nextcloud data.
Alternatives (https://github.com/nextcloud/all-in-one?tab=readme-ov-file#are-remote-borg-backups-supported):
How to use it
Instead of entering a local repo path (which remains supported), you can choose to instead enter a remote borg repo url. (These screenshots are from a disaster recovery but it's analogous, just ignore the passphrase):
The first you try to initialise the repo, the backup container will create an ssh key and foolishly try to init the borg repo resulting in error because you have to authorise the ssh key it generated first. So, e.g. I copy paste the public key shown here to my borgbase repo:
Then I can try again as instructed and it should work. The following backup info is shown:
Changes
borg extract
because, at least with a remote repo,borg mount
is very slow (20 seconds vs 24 minutes restore for pretty much an empty nextcloud install).FYI Oddly the original code never restores host-mounts.
Testing
I hacked the code til I could run local deployments of nextcloud-aio without any builds and manually tested:
TODO