search

voxpupuli / puppet-yum

Puppet module for Yum
https://forge.puppet.com/puppet/yum
MIT License
16 stars 101 forks source link

Add yum::copr resource to handle COPR repositories. #215

Closed olifre closed 1 year ago

olifre commented 3 years ago

COPR (Cool Other Package Repo) is a Fedora project for third-party package repositories, see: https://copr.fedorainfracloud.org/

Pull Request (PR) description

This PR adds support to manage COPR repositories via a new yum::copr resource. Documentation and test are included.

This Pull Request (PR) fixes the following issues

Fixes #149

olifre commented 3 years ago

This is a good case for an acceptance test as well I'd say.

Indeed it is. I've addressed all comments apart from the acceptance test, will look into this later this week.

olifre commented 3 years ago

I've now fixed the Rubocop offenses in the existing test, and added an acceptance test. I don't have a box at hand (yet) to run the acceptance test, but I think it should work — CI will show ;-).

olifre commented 3 years ago

Nice, the acceptance test found a real bug: disableing a COPR repository without adding it first will fail, and not be idempotent on dnf-based systems (which leave the repository file in place, but switch it to disabled in this case). yum-based systems effectively don't differentiate between disabled or removed.

I have fixed that in the last force-push by ensuring a COPR repository is added before disabling it, if disabled is requested and it is not disabled yet on dnf-based systems.

Another issue was that the removed acceptance test did not add a repository first, so it was not a real test whether the removing worked. This is now also fixed.

So I hope this might be the last iteration :-).

traylenator commented 3 years ago

One of the problems with this if you are running with

resources{'yumrepo':
   purge => true,
}

The repo file will be deleted won't it?

I don't see a good way around that.

olifre commented 3 years ago

The repo file will be deleted won't it?

Indeed...

I don't see a good way around that.

Same here, at least not as long as we want to use the official way (via dnf / yum) instead of managing the repo "manually", but then all the magic hidden by the underlying package manager plugins is lost and we'd need to follow all updates on that end.

Good ideas are very welcome.

olifre commented 1 year ago

Sorry for the multiple force-pushes, I found a small logic issue in the last commit and fixed it — if there is any good proposal on how to upstream this, fixing / documenting the issue @traylenator pointed out, please let me know.

bastelfreak commented 1 year ago

@olifre force pushes are fine. Can you take a look at the failing CI jobs?

olifre commented 1 year ago

@olifre force pushes are fine. Can you take a look at the failing CI jobs?

Thanls, I think I fixed all CI issues, the remaining issue seems to be a fluke of the GitHub actions runner (read timeout reached). I will trigger CI once more with an empty force-push.

olifre commented 1 year ago

@bastelfreak That did the trick, now all is green :smile: .

olifre commented 1 year ago

@bastelfreak It seems CI went red again after you re-ran it yesterday, but checking the logs, that seems like an issue with the runners, correct?

traylenator commented 1 year ago

One of the problems with this if you are running with

resources{'yumrepo':
   purge => true,
}

The repo file will be deleted won't it?

I don't see a good way around that.

Given there's no sensible way around this and it only affects particular configurations I would just document it. If the resources declaration can be adjusted to ignore the copr repos based on name or something then makes sense to give that example.

https://forge.puppet.com/modules/crayfishx/purge/readme gives an example in fact.

olifre commented 1 year ago

Given there's no sensible way around this and it only affects particular configurations I would just document it. If the resources declaration can be adjusted to ignore the copr repos based on name or something then makes sense to give that example.

Thanks for confirming!

The names of yumrepo resources resulting from COPR commands end up being formed such as: copr:copr.fedorainfracloud.org:jdoss:wireguard copr:copr.fedorainfracloud.org:copart:restic (tested with RHEL 7 and 8). Since resources itself does not seem to allow limiting what is purged, the only way out seems to be to use something like the purge module you linked.

What do you think about the documentation addendum I added in the preceding commit?

olifre commented 1 year ago

I have finally come around to also regenerate REFERENCE.md and rebase onto current master, so things should now be green and mergable.