search

theforeman / puppet-foreman

Puppet module for Foreman
GNU General Public License v3.0
104 stars 270 forks source link

Puppet 4 support for ENC script #367

Closed brandonweeks closed 8 years ago

brandonweeks commented 9 years ago

Filing against the right project this time...

$puppet_etcdir is hardcoded to '/etc/puppet' regardless of Puppet version. In Puppet 4 the default etcdir has changed to '/etc/puppetlabs/puppet'.

ekohl commented 9 years ago

I should really revisit https://github.com/theforeman/puppet-foreman/pull/332 but there are more issues than you just mentioned.

mmoll commented 9 years ago

Maybe we could pull the default for some more params from other modules (theforeman-puppet in this case), like we do in theforeman-foreman_proxy.

ekohl commented 9 years ago

That sounds sane, but I think we should avoid a hard dependency on puppet-puppet.

mmoll commented 9 years ago

It's kind of complicated already which module is needed for specific tasks. -foreman is needed to set up the puppet master's node.rb and reports for example, and I needed to touch -foreman also for implementing FreeBSD support in -puppet (maybe -foreman_proxy, too).

As (IMHO) most deployments of Foreman also use the Puppet master on the same host (or at least have the possibility to use -puppet to configure the agent then), I wouldn't see that specific change really problematic, but I feel that another measure might be better: this is a good moment to think/discuss about general shuffling around of duties between some modules or some "top-level foreman" module with soft dependencies to other modules (or something else I didn't think about).

On the other hand I think finishing #332 to at least pass the tests so all our modules are on the same level is more urgent (and I like to enable Ruby 2.2 testing to get enabled after that) to finish off http://projects.theforeman.org/issues/9822

@brandonweeks sorry to pull you into this discussion that has the potential to get pretty big... ;). I also would like to invite you to hang out in #theforeman-dev on Freenode IRC as we often also discuss there and we would love to see more people involved in general.

ekohl commented 9 years ago

@mmoll this is indeed a pretty old discussion. I'm still convinced puppet-foreman is the wrong place for the ENC and reporting but we never did figure out where to ship them instead.

However, I think it's not a problem to have a hardcoded $puppet_etc_dir since it's contained to foreman::puppetmaster for which every parameter is overrideable. In fact, puppet-puppet already passes all parameters so I'd expect it to already place them in the correct directory. That said, we still need to modify the scripts themselves to become compatible with puppet 4.

GregSutcliffe commented 8 years ago

+1 for the script changes, it does hardcode /etc/puppet for the location for the yaml file. I think we could potentially add similar handling to the path as we do elsewhere to detect whether to use /etc/puppet or /etc/puppetlabs/puppet - it works after i've upgraded to Puppet 4, but that's because /etc/puppet still exists from pre-upgrade and I've set $server_external_nodes = "/etc/puppet/node.rb" in puppet-puppet...

mmoll commented 8 years ago

I guess this can be closed now as #413 and theforeman/puppet-puppet#363 were merged?

ekohl commented 8 years ago

I think you're right.