Skip to content
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

Implement Backblaze for Backup #1812

Merged
merged 15 commits into from
Nov 26, 2020
Merged

Implement Backblaze for Backup #1812

merged 15 commits into from
Nov 26, 2020

Conversation

hija
Copy link
Contributor

@hija hija commented Aug 26, 2020

Hello :)

I started implementing Backblaze as backup option for miab.

There is one major change I would like to point out: I added the duplicity PPA, so the lastest version of duplicity gets installed. I struggled to get b2 working with the version of duplicity shipped in the default repository and it worked without any problems with the duplicity PPA.

How I tested:

  • Created new VM and installed MIAB and configured it to use B2 for backup
  • Ran sudo ./management/backup.py && sudo ./management/backup.py --verify
  • Result: Verify complete: 84 files compared, 0 differences found.
  • Looked at Web UI and saw that the backup is stored and the size of backup is determined correctly

Where I need help:

  • Verify that it works correctly on other machines as well
  • Make sure that the duplicity PPA does not break the already working mechanisms, e.g. Amazon s3. However, I do not have an Amazon S3 account and would appreciate help with this.

I would love to get feedback on this PR.

@hija hija marked this pull request as draft August 26, 2020 15:27
@hija hija marked this pull request as ready for review August 30, 2020 12:26
@hija
Copy link
Contributor Author

hija commented Aug 30, 2020

I tested the change on a development server (since four days) and it is running since two days on my production server. 👍

@hija hija changed the title [WIP] Implement Backblaze for Backup Implement Backblaze for Backup Aug 30, 2020
Copy link
Contributor

@fspoettel fspoettel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! 🙂 Love seeing B2 support for backups. I ran into some problems with S3 backups after the ppa change (duplicity couldn't find boto) and added some minor suggestions

@hija
Copy link
Contributor Author

hija commented Sep 4, 2020

Hey @fspoettel,

I think I integrated your changes 👍 Thanks for your review and your suggestions. I really appreciate them!

@nomandera
Copy link
Contributor

Devils advocate. Would it be worth considering deploying this as two pulls:

  1. Update duplicity
  2. Add Backblaze custom config options

These would be the smallest possible iterations and has the advantage of allowing quicker feedback if the duplicity upgrade breaks any existing edge case backups.

Very keen to see this deployed as Backblaze is legitimately 3-4 times cheaper than almost everyone else whilst being a open and professional company.

@hija
Copy link
Contributor Author

hija commented Sep 9, 2020

Thanks for your idea, @anoma :) I did not think about splitting this up before, but it might be worth it 👍

@JoshData Do you think this pull request is interesting for the project? If so: Should I split it up or are you ok with one "big" pull request?

@fspoettel
Copy link
Contributor

PR relates to #717

@fspoettel
Copy link
Contributor

recent discourse thread where b2 came up again. it seems like this is a feature that some users really want and have gone the length of implementing homebrew solutions.

@JoshData
Copy link
Member

Ok. I appreciate the work here. I am merging it. If it breaks anything, you all are on the hook for fixing it quickly. :)

@barrybingo
Copy link
Contributor

barrybingo commented Dec 12, 2020

So my B2 Application Key had a slash in it and this causes a "substring not found" error.
Quick fix was to generate a new API key.
Here is the error POC given app key = 'firstpartofappkey/secondpartofappkey'

>>> import urllib.parse
>>> target = urllib.parse.urlparse('b2://00KeyID00000:firstpartofappkey/secondpartofappkey@somebucket')
>>> b2_application_keyid = target.netloc[:target.netloc.index(':')]
>>> b2_application_key = target.netloc[target.netloc.index(':')+1:target.netloc.index('@')]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: substring not found
>>> 
>>> 
>>> print(target)
ParseResult(scheme='b2', netloc='00KeyID00000:firstpartofappkey', path='/secondpartofappkey@somebucket', params='', query='', fragment='')

@hija
Copy link
Contributor Author

hija commented Dec 12, 2020

Hey @barrybingo,

unfortunately I don't see how to fix this issue on our side since duplicity cannot parse the URL if the API key contains a slash (so even if we would fix the code you pointed out above the backup would still not work):

Maybe we should disallow entering application keys and application ids if they contain a slash and ask to regenerate the API if it contains a slash?

@barrybingo
Copy link
Contributor

Yeah I vote for disallow and give instructions.
The problem at the moment is the error "substring not found" has zero meaning.

@barrybingo
Copy link
Contributor

Also if you're going to do some changes then you could add backup-target-b2 to this line so retention days is available for edit

<div class="form-group backup-target-local backup-target-rsync backup-target-s3">

So far first backup is working fine so thumbs up

@hija
Copy link
Contributor Author

hija commented Dec 12, 2020

Also if you're going to do some changes then you could add backup-target-b2 to this line so retention days is available for edit

<div class="form-group backup-target-local backup-target-rsync backup-target-s3">

So far first backup is working fine so thumbs up

Could you clarify what you mean with that?

@barrybingo
Copy link
Contributor

That div is hidden for b2 as the div class does not contain backup-target-b2

@hija
Copy link
Contributor Author

hija commented Dec 12, 2020

Ahhh okay. So your point is that we should be able to set a custom retention time because there is no reason to not do so, right?

@barrybingo
Copy link
Contributor

Yup. That setting's value is passed to duplicity so should be configurable.

@hija
Copy link
Contributor Author

hija commented Dec 12, 2020

Got ya! Good catch! Do you want to do a PR or should I do one?

@barrybingo
Copy link
Contributor

barrybingo commented Dec 12, 2020

I don't want to step on your toes and it is Saturday night here 🍻
If you haven't submitted one by tomorrow I'll do it.

But can't you just add the commits onto your branch then update this PR for Josh to review and re-merge?
Keep everything in one place.

@fspoettel
Copy link
Contributor

But can't you just add the commits onto your branch then update this PR for Josh to review and re-merge?
Keep everything in one place.

no, once a PR is merged, it cannot be re-opened. Depending on the repository's merge strategy and activity, it's also not a good idea to re-use stale branches.

@hija
Copy link
Contributor Author

hija commented Dec 12, 2020

Deleted the fork anyways (don't remember why exactly). I will see if I can do a new PR today / tonight, otherwise barrybingo can do one :)

@hija
Copy link
Contributor Author

hija commented Dec 12, 2020

I don't want to step on your toes and it is Saturday night here 🍻
If you haven't submitted one by tomorrow I'll do it.

But can't you just add the commits onto your branch then update this PR for Josh to review and re-merge?
Keep everything in one place.

I won't make it today. Go ahead if you want to :) @barrybingo

@barrybingo
Copy link
Contributor

First draft
#1882

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants