Skip to content
This repository was archived by the owner on Jul 19, 2021. It is now read-only.

moar features! - #9

Open
semoule wants to merge 12 commits into
SphericalElephant:masterfrom
semoule:master
Open

semoule wants to merge 12 commits into
SphericalElephant:masterfrom
semoule:master

Conversation

@semoule

@semoule semoule commented May 3, 2018

Copy link
Copy Markdown

Hello,

First : thank you for your amazing job, that give me a good start to use borgbackup everywhere easily!

I made some improvement to your role that you may accept to merge.
My usage context is ~300 clients servers and 6 repository servers.
every client backup on 2 differents repository servers, to avoid bitrot.
This PR contain one breaking change : the script/crontab filenames are changing to fit with borg commands and support multiple backup servers.
here's the exhaustive list of features I added, and a short description of it.

  • add support lan-free backup
    for security reasons, backup servers are sometimes accessible only by the client using a special vlan and not by the deployement system (eg : your workstation that play ansible). So the server IP address to be used in backups scripts is different from the one used by ansible deployement.
    this is not a breaking change.

  • add a repository check job
    your cool role comes with backup and prune job. Borg propose a check job, that verify repository integrity and I really need to regulary and automatically check my backups, so I added this job. I feel important to rename "backup" job as "create" job to fit with borgs native commands
    this is a little breaking change : script names change to fit the borgs native commands.

  • script and cron file naming.
    because I renamed "backup" job as "create" job and because I need to backup to more than one backup server, I needed to rename the script file to have a better convention
    the name pattern is repositoryname_jobtype_backupserver.sh. that give you something like this :
    system_check_backup03-prod.sh
    system_check_backup05-prod.sh
    system_create_backup03-prod.sh
    system_create_backup05-prod.sh
    system_prune_backup03-prod.sh
    system_prune_backup05-prod.sh
    sqldata_check_backup03-prod.sh
    sqldata_check_backup05-prod.sh
    sqldata_create_backup03-prod.sh
    sqldata_create_backup05-prod.sh
    sqldata_prune_backup03-prod.sh
    sqldata_prune_backup05-prod.sh
    I also renamed the cronfiles to keep this convention :
    borgbackup_system_check_backup03-prod
    borgbackup_system_check_backup05-prod
    borgbackup_system_create_backup03-prod
    borgbackup_system_create_backup05-prod
    borgbackup_system_prune_backup03-prod
    borgbackup_system_prune_backup05-prod
    borgbackup_sqldata_check_backup03-prod
    borgbackup_sqldata_check_backup05-prod
    borgbackup_sqldata_create_backup03-prod
    borgbackup_sqldata_create_backup05-prod
    borgbackup_sqldata_prune_backup03-prod
    borgbackup_sqldata_prune_backup05-prod
    This is a little breaking change, as scripts are renamed.

  • add lastlog status into /var/cache/borg and improve logs
    Because I have a lot of backup clients, I use a monitoring tool for supervision. So I need to be able to know the last state of every backup/prune/check jobs. So I added a tee for logs to get a lastlog into /var/cache/borg, and improved a little those logs to have more informations and especially the borg return code.

  • change default version
    borg project has evolved, so I change the default version to 1.1.4 (the one I currently use). for information, latest stable is 1.1.5

  • make role play indempotent and add "random_day"/"random_hour"/"random_minutes".
    if 'random_hour' is set, then 'hour' will be set by a random value within range 'hour' up to 'hour+random_hour'.
    Why ? because you need to separate time slice of "create" jobs from "prune" jobs and even "check jobs"
    Also, you need that every jobs to be launch in a random time of this time slice, to avoid that all your client start together hitting the repository server
    Also, you need the random to be indempotent to avoid false change when you play ansible role again and again.
    This is not a breaking change

  • binary install now support version upgrade

I told you my client are using 2 differents backup servers.
The way I do this is using a dedicated role that call yours 2 times. I published it here https://github.com/semoule/ansible-role-borgbackup-config.
I could integrate this feature straight in your role, but I needed also other features : install nrpe on the client (for supervision) and automatic subscribe to our supervision solution. This needs are quite specific and shouldn't be integrated in the borgbackup install role, so I created a role on top.

Don't hesitate to reply if you need some more information.

Best regards,

semoule.

@Grauwolf

Grauwolf commented May 3, 2018

Copy link
Copy Markdown
Contributor

Wow! I will need some time to go through the changes.

Adding to the idempotent random time: This makes sense. We are using https://gist.github.com/ptman/9bd8223272e2c0e27b2b which can be used like:

borgbackup_client_jobs:
  - name: system
    hour: "{{ 7 | determrand(ansible_fqdn) }}"
    minute: "{{ 59 | determrand(ansible_fqdn) }}"
    backup_repository: "system"
    directories:
      - "/etc"
    excludes: []

While I like having that functionality in an extra filter_plugin, it makes sense to have it as sane defaults inside the role.

Thank you!

@semoule

semoule commented May 3, 2018

Copy link
Copy Markdown
Author

Here how I have implemented the random_hour feature and idempotent principle :

hour: "{{ (item.hour | default(1)) + (item.random_hour | random(seed=item.name + create_suffix_script_filename + ansible_host))%24 if item.random_hour is defined else item.hour | default(1) }}"

translation :
if random_hour is defined, then :

  • we compute a random hour in the range (hour:hour+random_hour)
  • the seed for randomness is ansible_fqdn + jobname (here create_suffix_script_filename) , because create_job is a list and we may prefer to not get the same random value for each item in the list.

else :
we simply put the hour value

and if no hour is defined, we put the hour value arbitrary to 1.

@Grauwolf Grauwolf left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for your pull request!
After reading through it a bit more thorough I found a bug and two minor issues. After those have been addressed I'd be happy to test it on a couple of our servers.

Comment thread defaults/main.yml Outdated
borgbackup_client_backup_server:

# if defined, IP or FQDN used on backup operation. Usefull in case of LAN-free backup
borgbackup_client_backup_server_lanfreebackup:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

borgbackup_client_backup_server_lanfreebackup needs to be undefined (so commented in here) for the default(borgbackup_client_backup_server) in the scripts in the templates/ folder to work

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

You're right, role execution shouldn't fail because of that.
Change done.

mode: 0600
state: touch

- name: add ssh-pubkey admin users to backup server

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

please make it so that borgbackup_server_admins keys have a field state so that they can not only be added but also removed via borgbackup_server_admins.

I suggest having something like

borgbackup_server_admins: []
#  - key: "ssh-rsa abc123..."
#  - key: "ssh-rsa abc456..."
#    state: present
#  - key: "ssh-rsa abc789..."
#    state: absent

in the defaults/main.yml and

- name: add ssh-pubkey admin users to backup server
  authorized_key:
    user: "{{ borgbackup_server_user }}"
    key: "{{ item.key }}"
    state: "{{ item.state | default('present') }}"
  with_items: "{{ borgbackup_server_admins }}"

in the tasks file

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Nice idea. Change done!

Comment thread defaults/main.yml Outdated
#borgbackup_client_jobs:

# lastlog parsed by supervision
borgbackup_client_lastlog_dir: "/var/cache/borg"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe that this should be /var/log/borg.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The lastlog is overwritten on each create/prune/check execution. It's a lastlog and not a log of all execution. (Thoses are sent to syslog). I need that to get a easier last status seen for supervision purpose. That's why I was thinking that /var/cache was better than /var/log.
Anyway, it's just a default value, so I'm ok to change it.

…ns keys can be removed, change default borg backup log
@semoule

semoule commented May 19, 2018

Copy link
Copy Markdown
Author

Thank you for the review.
I made the change you suggest and add a reference to check job in the readme.md
Tests on my side are successful.
Best regards

@varac

varac commented Aug 2, 2018

Copy link
Copy Markdown

Hej, what's the state of this ? I'd like this PR to get implemented.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants