fiaasco / borgbackup

Borg backup role
MIT License
16 stars 19 forks source link

Allow users to install borgbackup package instead of downloading binary #25

Closed TravisWhitehead closed 4 years ago

TravisWhitehead commented 4 years ago

This replaces PR https://github.com/fiaasco/borgbackup/pull/18. Original PR description:

I think it would be helpful to have support for installing borg backup from the package manager.

I've tested this installing from both package and web in a local vagrant environment on CentOS 7, and was able to take a backup successfully both ways.

This offers all of the changes from our fork at https://github.com/tag1consulting/ansible-role-borgbackup that aren't Tag1-specific.

The primary addition here is the ability to install borgbackup from a package (via EPEL), but this also includes a couple ansible-lint fixes.

I tested #18 back when I made those changes, but I don't have the cycles to test this again, so hopefully someone else can take that on.

Happy to see that this repo is active again. :)

stroobl commented 4 years ago

Thanks. I'll try to run some tests so that we can get this ready to merge.

noplanman commented 4 years ago

Hi @TravisWhitehead, thanks for working on this!

I'm running into issues with your changes though, as the geerlingguy.repo-epel dependency in meta/main.yml seems to be forced, which obviously fails on my Debian server.

The dependency should only apply for RedHat systems, but not sure if that can be defined in the meta directly.

Any ideas?

noplanman commented 4 years ago

This is how my play fails:

TASK [Gathering Facts] *****************************************************************************************************************************************
ok: [my-server]

TASK [geerlingguy.repo-epel : Check if EPEL repo is already configured.] ***************************************************************************************
ok: [my-server]

TASK [geerlingguy.repo-epel : Install EPEL repo.] **************************************************************************************************************
FAILED - RETRYING: Install EPEL repo. (5 retries left).
FAILED - RETRYING: Install EPEL repo. (4 retries left).
FAILED - RETRYING: Install EPEL repo. (3 retries left).
FAILED - RETRYING: Install EPEL repo. (2 retries left).
FAILED - RETRYING: Install EPEL repo. (1 retries left).
fatal: [my-server]: FAILED! => {"ansible_facts": {"pkg_mgr": "apt"}, "attempts": 5, "changed": false, "msg": "No package matching 'https://dl.fedoraproject.org/pub/epel/epel-release-latest-10.noarch.rpm' is available"}
stroobl commented 4 years ago

I just ran some tests and CentOS works fine but for Ubuntu I had the same issue as @noplanman just reported. I believe it should be fixed like this:

  - role: geerlingguy.repo-epel
    when: >
      ansible_os_family == 'Redhat'
      and borgbackup_install_epel
TravisWhitehead commented 4 years ago

Good catch y'all. @stroobl I've added that conditional to the requirement (but haven't tested it myself).

stroobl commented 4 years ago

I tested it already, so I'll merge so that we can move on with the other changes. I'll wait to add a new release tag till we merged @dverhelst testing changes and ran more tests.

noplanman commented 4 years ago

@TravisWhitehead @stroobl

Conditional needs to be set in meta/main.yml, not requirements.yml. This PR still fails as it is.

Should be:

dependencies:
  - name: geerlingguy.repo-epel
    when:
      - ansible_os_family == 'Redhat'
      - borgbackup_install_epel
TravisWhitehead commented 4 years ago

@noplanman Thanks, I opened a PR.