search

hubspotdevops / puppet-nexus

Puppet module for Sonatype Nexus
MIT License
24 stars 93 forks source link

RHEL/CentOS systemd unit handling #74

Closed CarpathianUA closed 7 years ago

CarpathianUA commented 7 years ago

For now there are no systemd unit handling in service.pp. If Nexus service is stopped, puppet agent sync can't start it. I think if following code will be added to service.pp is should fix this. elsif ($::operatingsystem == 'RedHat') or ($::operatingsystem == 'CentOS') { file { '/etc/systemd/system/nexus.service': mode => '0644', owner => 'root', group => 'root', content => template('nexus/nexus.systemd.erb'), } -> service { 'nexus': ensure => running, name => 'nexus', enable => true, } }

PaulFurtado commented 7 years ago

This would also need to include a centos/redhat version in the if-statement since CentOS/RedHat 6 doesn't use systemd.

Since we already handle systemd for Ubuntu/Debian, we can likely add:

($::operatingsystem == 'CentOS' and versioncmp($::operatingsystemrelease, '7.0') > 0) or
($::operatingsystem == 'RedHat' and versioncmp($::operatingsystemrelease, '7.0') > 0)

here: https://github.com/hubspotdevops/puppet-nexus/blob/master/manifests/service.pp#L45

We would need to test that though.

fatmcgav commented 7 years ago

Are you averse to adding an additional module dependency?

As camptocamp-systemd module adds some additional facts which can be used to massively simplify 'supports systemd' logic.

As an example of use, I've done the same in my puppet-glassfish module - https://github.com/fatmcgav/fatmcgav-glassfish/commit/62887677b76cbe13688a13d046b969ea9b580db9

Thoughts?

PaulFurtado commented 7 years ago

Hey @fatmcgav, I wouldn't say we have an aversion to adding additional dependencies, but we do carefully consider each one. That module looks pretty useful, but for now, we're not going to go out of our way to use it since this particular issue is solved in #76 and it would only save us one (ugly) if-statement here. If more issues for systemd support on other distributions come in, we'll probably switch to that module to save us from making similar changes in the future

PaulFurtado commented 7 years ago

(Also closing this since we should have closed it when we merged #76)

fatmcgav commented 7 years ago

@PaulFurtado No worries, I noticed the fix in #76 after commenting above :)

Cheers