Added evobackup-client role #83

Manually merged
Ghost merged 17 commits from evobackup-client into unstable 2020-02-06 22:31:45 +01:00
First-time contributor

Already tested on a few machines.

Already tested on a few machines.
jlecour was assigned by Ghost 2019-08-30 20:46:57 +02:00
jlecour reviewed 2019-09-02 08:49:15 +02:00
jlecour left a comment
Owner

Why do you mix "" and "_" in variable names ?

Why do you mix "__" and "___" in variable names ?
jlecour requested changes 2019-09-02 09:06:19 +02:00
jlecour left a comment
Owner

I think we need to spend more time on this before releasing it. There are some architectural changes that need to be done and I don't think it is automated enough to replace the very simple manual process we have right now.

I think we need to spend more time on this before releasing it. There are some architectural changes that need to be done and I don't think it is automated enough to replace the very simple manual process we have right now.
@ -0,0 +1,24 @@
# evobackup-client
Allows the configuration of backups to a pair of bkctld(8) hosts.
Owner

s/a pair of bkctld(8) hosts/one or more backup servers/

The server side has nothing to do with bkctld as a command, so no need to confuse people with the name of the command and a man section. We can backup to anything as long as it is accessible via SSH.

s/a pair of bkctld(8) hosts/one or more backup servers/ The server side has nothing to do with bkctld as a command, so no need to confuse people with the name of the command and a man section. We can backup to anything as long as it is accessible via SSH.
@ -0,0 +4,4 @@
The backup hosts in use need to be defined in evobackup-client___hosts
and the bkctld jail ssh port has to be defined in
evobackup-client___ssh_port before running it.
Owner

see my other comment below, the ssh port can be different for each server.

see my other comment below, the ssh port can be different for each server.
@ -0,0 +6,4 @@
and the bkctld jail ssh port has to be defined in
evobackup-client___ssh_port before running it.
The default zzz_evobackup.sh configures a system backup, but the
Owner

s/system/"system only"/

s/system/"system only"/
Author
First-time contributor

This is fixed, not sure why it's not marked as outdated.

This is fixed, not sure why it's not marked as outdated.
@ -0,0 +9,4 @@
The default zzz_evobackup.sh configures a system backup, but the
template can be overriden to configure a full backup instead. If
you change the variables in defaults/main.yml you can easily run
this again and configure backups to a second set of bkctld(8) hosts.
Owner

see my comment above about the command name and man section.

see my comment above about the command name and man section.
@ -0,0 +6,4 @@
evobackup-client__pid_path: "/var/run/evobackup.pid"
evobackup-client___log_path: "/var/log/evobackup.log"
evobackup-client__backup_path: "/home/backup"
evobackup-client___ssh_port: null
Owner

each backup server can have a different ssh port. Itr should be in the evobackup-client___hosts dictionary.

each backup server can have a different ssh port. Itr should be in the `evobackup-client___hosts` dictionary.
Author
First-time contributor

I havent met that case in the wild. It would be annoying to have to define the full dict for every host though. Not sure there is a clean way around this.

I havent met that case in the wild. It would be annoying to have to define the full dict for every host though. Not sure there is a clean way around this.
@ -0,0 +8,4 @@
- evobackup-client
- evobackup-client-backup-firewall
- name: backup ssh port
Owner

We can have multiple backup sections (with heterogenous ssh ports) in the minifirewall file. Let's be extra careful with what we insert and/ore replace.

We can have multiple backup sections (with heterogenous ssh ports) in the minifirewall file. Let's be extra careful with what we insert and/ore replace.
Author
First-time contributor

Again, heterogeneous ports is kind of an outlier case. But the main question is how to define it without it being a chore.

Again, heterogeneous ports is kind of an outlier case. But the main question is how to define it without it being a chore.
Author
First-time contributor

So I removed this task, instead setting the port directly in the rule.

So I removed this task, instead setting the port directly in the rule.
@ -0,0 +21,4 @@
MAIL={{ evobackup-client___mail }}
# list of hosts (hostname or IP) and SSH port for Rsync
SERVERS="{% for host in evobackup-client___hosts %}{{ host.name }}:{{ evobackup-client___ssh_port }} {% endfor %}"
Owner

i'm not sure this loop produces the intended output.

i'm not sure this loop produces the intended output.
Author
First-time contributor

It produces:

SERVERS="hostname:port hostname:port "

The extra space at the end is not the cleanest, but it does not break the script.

It produces: ``` SERVERS="hostname:port hostname:port " ``` The extra space at the end is not the cleanest, but it does not break the script.
Author
First-time contributor

But this is kind of reason why it's a pull request and not a straight merge.

But this is kind of reason why it's a pull request and not a straight merge.
Author
First-time contributor

This should not be marked as outdated, this conversation is still open.

This should not be marked as outdated, this conversation is still open.
@ -0,0 +341,4 @@
/etc \
/root \
/var \
/home/backup \
Owner

Why /home/backup and not /home like it is in the official script https://gitea.evolix.org/evolix/evobackup/src/branch/master/zzz_evobackup#L344 ?

Why /home/backup and not /home like it is in the official script https://gitea.evolix.org/evolix/evobackup/src/branch/master/zzz_evobackup#L344 ?
Author
First-time contributor

The way I've been using it mostly is for system backups not full backups, hence the '/home/backup'. If you're doing a complete backup, it's almost always going to be more custom, so I feel like the more common use case is more useful. But I dont have strong opinions about this.

The way I've been using it mostly is for system backups not full backups, hence the '/home/backup'. If you're doing a complete backup, it's almost always going to be more custom, so I feel like the more common use case is more useful. But I dont have strong opinions about this.
Author
First-time contributor

This would be the main outstanding question. I feel like /home/backup is a better default, but I can change it. What do you think ?

This would be the main outstanding question. I feel like /home/backup is a better default, but I can change it. What do you think ?
Owner

This reveals a blindspot in our setup.

When we do a "system only" backup, we definitely want to keep the files with the list of packages, mtr output… They currently go to /home/backup by default.

But when we also backup the whole server to another backup server and there are databases dumps in /home/backup then thos files are also added to the "system-only" backup, which is not good.

Maybe we should put lightweight important files to backup to /root/backup and large data dumps to /home/backup. Then we would always add /root/backup to the rsync command, and only add /home to full backups.

This reveals a blindspot in our setup. When we do a "system only" backup, we definitely want to keep the files with the list of packages, mtr output… They currently go to `/home/backup` by default. But when we also backup the whole server to another backup server and there are databases dumps in `/home/backup` then thos files are also added to the "system-only" backup, which is not good. Maybe we should put lightweight important files to backup to `/root/backup` and large data dumps to `/home/backup`. Then we would always add `/root/backup` to the `rsync` command, and only add `/home` to full backups.
Author
First-time contributor

That makes sense. We could then add a boolean switch to enable "full" backups from ansible.

Could this cause any problems with the typical free space on /root ?

That makes sense. We could then add a boolean switch to enable "full" backups from ansible. Could this cause any problems with the typical free space on /root ?
Author
First-time contributor

Fixed the more egregious variable name problem. Somehow a few extra _ slipped in when I was making the names more generic.

Fixed the more egregious variable name problem. Somehow a few extra _ slipped in when I was making the names more generic.
jlecour reviewed 2019-09-03 18:07:24 +02:00
Owner

This probably is a bad copy/paste.

This probably is a bad copy/paste.
Ghost reviewed 2019-09-03 18:12:08 +02:00
Author
First-time contributor

Damnit, thought I had erased that.

Damnit, thought I had erased that.
Author
First-time contributor

Tested on one more machine, worked after latest commits.

Tested on one more machine, worked after latest commits.
Owner

Like we've said IRL, the typical backup of system files (list of packages, mtr output, network state…) takes only a few MB and won't grow.
Saving them to /root/backup/ seems reasonable and does make sense regarding the separation of system and user/data files.

Like we've said IRL, the typical backup of system files (list of packages, mtr output, network state…) takes only a few MB and won't grow. Saving them to `/root/backup/` seems reasonable and does make sense regarding the separation of system and user/data files.
Author
First-time contributor

I fixed a bug in the firewall rule definitions and added support for creating the jail on the host. At this point everything works and is tested, but I still need some pointers on the UI. For instance I feel like I should add a variable to allow someone to configure a client without creating / modifying a jail, there are probably a few other points like this.

I fixed a bug in the firewall rule definitions and added support for creating the jail on the host. At this point everything works and is tested, but I still need some pointers on the UI. For instance I feel like I should add a variable to allow someone to configure a client without creating / modifying a jail, there are probably a few other points like this.
Author
First-time contributor

I played this today to update / create backups on 6 different machines.

These machines each had two evobackup scripts, one for system only backups and one for full backups, with each script talking to different pairs of backup servers. This was done by adding the role twice and overloading the necessary variables, I also added machine specific full backup script templates. It all ran perfectly and quickly, except for one case: machines which had already created their jail on the second backup pair, but not on the first, errored out during the start jail handler because their jail was already started. I think this happened because the creation of the prior pair notifies the handler, but does not flush it aftewards, making the handler active for the second pair as well, although it's not actually needed.

This is not a critical issue, as running the playbook a second time will complete successfully, but it would be nice to fix it. Any tips?

I played this today to update / create backups on 6 different machines. These machines each had two evobackup scripts, one for system only backups and one for full backups, with each script talking to different pairs of backup servers. This was done by adding the role twice and overloading the necessary variables, I also added machine specific full backup script templates. It all ran perfectly and quickly, except for one case: machines which had already created their jail on the second backup pair, but not on the first, errored out during the start jail handler because their jail was already started. I think this happened because the creation of the prior pair notifies the handler, but does not flush it aftewards, making the handler active for the second pair as well, although it's not actually needed. This is not a critical issue, as running the playbook a second time will complete successfully, but it would be nice to fix it. Any tips?
lpoujol requested changes 2019-11-25 11:34:33 +01:00
@ -0,0 +1,13 @@
---
evobackup_client__root_key_path: "/root/.ssh/evobackup_id"
Owner

I like the idea, but I'm not sure if we need a separate key for backups only

I like the idea, but I'm not sure if we need a separate key for backups only
Author
First-time contributor

This idea was brought up by @jlecour I can let him expand on it.

This idea was brought up by @jlecour I can let him expand on it.
@ -0,0 +5,4 @@
name: root
generate_ssh_key: true
ssh_key_file: "{{ evobackup_client__root_key_path }}"
ssh_key_type: rsa
Owner

I do have a preference for ed25519 over rsa. But this isn't mandatory :)

I do have a preference for ed25519 over rsa. But this isn't mandatory :)
Author
First-time contributor

It's what we used up till now, but I have no problem with changing it.

It's what we used up till now, but I have no problem with changing it.
@ -0,0 +69,4 @@
port=$(echo "${item}" | cut -d':' -f2)
# Test if the server is accepting connections
ssh -q -o "ConnectTimeout ${SSH_CONNECT_TIMEOUT}" -i /root/.ssh/evobackup_id "${host}" -p "${port}" -t "exit"
Owner

While you allow to chose a different name for the SSH Key with evobackup_client__root_key_path, it's static here.

While you allow to chose a different name for the SSH Key with evobackup_client__root_key_path, it's static here.
Author
First-time contributor

In practice, I end up overwriting this template almost all the time. But you're right.

In practice, I end up overwriting this template almost all the time. But you're right.
@ -0,0 +305,4 @@
# It breaks the command and destroys data, simply remove (or add) lines.
# Remote shell command
RSH_COMMAND="ssh -i /root/.ssh/evobackup_id -p ${SSH_PORT} -o 'ConnectTimeout ${SSH_CONNECT_TIMEOUT}'"
Owner

Same issue here, you're not using evobackup_client__root_key_path

Same issue here, you're not using evobackup_client__root_key_path
Author
First-time contributor

Will fix

Will fix
Ghost closed this pull request 2020-02-06 22:31:45 +01:00
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: evolix/ansible-roles#83
No description provided.