voxpupuli / puppet-yum

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

Allow to realize Yumrepo #193

Closed kristofvandam closed 3 years ago

kristofvandam commented 3 years ago

Pull Request (PR) description

Allow to realize yumrepo in other modules like so:


somewhere.yaml
yum::repos:
  custom-repository:
    baseurl: dev.domain.null

somewhere.pp
realize(Yumrepo['custom-repository'])

#### This Pull Request (PR) fixes the following issues
 * Unable to include a repository multiple times acros different modules
bastelfreak commented 3 years ago

Could you explain the benefit you see in this? Why would you like to use realize?

kristofvandam commented 3 years ago

Hi, I would benefit the ability to add the repositories with this module, but be able to add those elsewhere in other modules by realizing them (instead of having a hieradata file for every module so it can be merged like that, that doesn't play nice with profiles).

On slack.puppet there was already a small discussion on why (not) to use realize. I'm convinced that another solution would be safer to use. The risk is that when someone collects all Yumrepo resources, all repositories are going to be added.

If you have any other suggestion on how you would implement such functionality, I would be glad to implement that.

TJM commented 3 years ago

I would also vote against anything that uses realize. (but know how much my vote counts for)

We had some simple code that ensures all of our custom repos are added before any packages... such as:

      Yumrepo <| |> -> Yum::Gpgkey <| |> -> Package <| provider != 'rpm' |>

However due to this "realize" pattern that the puppet enterprise module uses (at least in 2018), our code now looks something like:

      # Resource Ordering
      Yumrepo <| |> -> Yum::Gpgkey <| |> -> Package <| provider != 'rpm' and
        title != 'pe-activemq' and
        title != 'pe-backup-tools' and
        title != 'pe-client-tools' and
        title != 'pe-console-services' and
        title != 'pe-console-services-termini' and
        title != 'pe-java' and
        title != 'pe-java-devel' and
        title != 'pe-license' and
        title != 'pe-modules' and
        title != 'pe-orchestration-services' and
        title != 'pe-postgresql-pglogical' and
        title != 'pe-puppet-enterprise-release' and
        title != 'pe-puppetdb' and
        title != 'pe-puppetdb-termini' and
        title != 'pe-puppetserver' and
        title != 'pe-razor-libs' and
        title != 'pe-razor-server' and
        title != 'pe-tasks' |>
        # Added PE packages based on Puppet Support KB#0211 (updated list from @natemccurdy in Slack)

Inevitably, that list becomes outdated with each new update to the module, so it is bothersome.

We usually have some hieradata for whatever application is using that profile anyhow, so its not too hard to add a line for yum::managed_repos

~tommy

kristofvandam commented 3 years ago

It's indeed a edgecase I didn't count for, I'l close this pull request until I figure something better out.

Thx for all the info!