-
Notifications
You must be signed in to change notification settings - Fork 95
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add options to disable repo resources when desired #54
Conversation
Is there a specific use case for this request? It's pretty verbose to wrap every resource in the module with |
You may also want to look at the |
The issue is when other modules have a dependency on something like (https://github.com/example42/puppet-yum) and then define another epel repo (because everyone has decided that having a different solution for implementing epel repos is necessary). I've also opened a PR with that module to try to stem off collisions at the source rather than having to chase down and modify a few dozen other modules we're using in our environment. Essentially, this is to try to inhibit bad code from being bad during compilation. We have a single Puppetfile for our environment so all modules need to play nice together and epel is a major pain. |
It seems like the solution would be to have whatever is requiring a I think there's a better way. Honestly, it's a shame there isn't an On Thu, Jun 2, 2016 at 11:54 AM, Riccardo Calixte notifications@github.com
|
The issue becomes when multiple modules have different dependencies for including/defining a yumrepo resource named epel. The catalog issue isn't much of one as they're all defining the same underlying repo (but it should compile consistently and not weave between the configuration). To address it at a higher level would require the community to decide on one epel puppet module rather than the non-standardization. Are you okay with the notion of including the defined checks for at least the epel repo resource? |
We're not comfortable with that, because if you have two modules that contain a resource |
Would it be better to implement this solution with a class variable for each yumrepo resource instead and wrapped each resource with that conditional? Having no way to turn these off really sets a lot of expectations for every other module. |
Do you mean something like 'epel_managed and 'epel_testing_managed'? That might hold value, so long as those default to true. I'm still not convinced this module is what you want if something else is providing epel repos, though. |
c9eb8c9
to
b842d2f
Compare
Those options are precisely defined now. I've updated the PR to reflect those changes. The issue of choice comes down to other modules that have declared this module as a dependency and then yet more modules that declare other epel modules as a dependency. In our environment, we have a single Puppetfile with our modules listed and these issues have been a source of frustration for some time. Hopefully, this can address those problems. |
before => Yumrepo['epel','epel-source','epel-debuginfo','epel-testing','epel-testing-source','epel-testing-debuginfo'], | ||
epel::rpm_gpg_key{ "EPEL-${os_maj_release}": | ||
path => "/etc/pki/rpm-gpg/RPM-GPG-KEY-EPEL-${os_maj_release}", | ||
before => Yumrepo['epel','epel-source','epel-debuginfo','epel-testing','epel-testing-source','epel-testing-debuginfo'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stahnma Should this stanza work if, say, 'epel-debuginfo' is not in the catalog? That's my only concern, and we'll have to add a test for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could add this condition further up in the init.pp outside of the resource definition. I'm thinking something along the lines of
Epel::Rpm_gpg_key -> Yumrepo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using spaceship collectors? I think that would be better, and more future proof.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the code yet again. I'm going to test it as well on a few nodes.
Merged in #68. Sorry this took so long to accept, and thank you for your contribution! |
No description provided.