Skip to content

Commit

Permalink
Fix systemd unit file not notifying the service
Browse files Browse the repository at this point in the history
The [unreliable](https://puppet.com/docs/puppet/5.5/lang_defaults.html#behavior) resource default
```
File{
  notify => Class['prometheus::run_service']
}
```
is replaced by a `$notify` variable that is set on the relevant file
resources *and* `systemd::unit_file`.  Some care was needed to make sure
the reload behaviour wasn't broken.  ie If the configuration change is
just a new scrape job that is collected, the service should only be
reloaded, not restarted.

Fixes #382
  • Loading branch information
alexjfisher committed Nov 18, 2019
1 parent 20dc6a7 commit 8588d88
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 31 deletions.
51 changes: 24 additions & 27 deletions manifests/config.pp
Original file line number Diff line number Diff line change
Expand Up @@ -154,69 +154,65 @@
}
$daemon_flags = $flags + $extra_options

# the vast majority of files here are init-files
# so any change there should trigger a full service restart
if $prometheus::server::restart_on_change {
File {
notify => Class['prometheus::run_service'],
}
$systemd_notify = [Exec['prometheus-systemd-reload'], Class['prometheus::run_service']]
} else {
$systemd_notify = Exec['prometheus-systemd-reload']
# Service files (init-files/systemd unit files) need to trigger a full service restart
# prometheus.yaml and associated scrape file changes should only trigger a reload (and not use $notify)
$notify = $prometheus::server::restart_on_change ? {
true => Class['prometheus::run_service'],
default => undef,
}

case $prometheus::server::init_style {
'upstart' : {
file { '/etc/init/prometheus.conf':
ensure => file,
mode => '0444',
owner => 'root',
group => 'root',
content => template('prometheus/prometheus.upstart.erb'),
notify => $notify,
}
file { '/etc/init.d/prometheus':
ensure => link,
target => '/lib/init/upstart-job',
owner => 'root',
group => 'root',
mode => '0755',
notify => $notify,
}
}
'systemd' : {
include 'systemd'
systemd::unit_file {'prometheus.service':
content => template('prometheus/prometheus.systemd.erb'),
notify => $notify,
}
}
'sysv', 'redhat' : {
file { '/etc/init.d/prometheus':
mode => '0555',
owner => 'root',
group => 'root',
content => template('prometheus/prometheus.sysv.erb'),
if versioncmp($facts['puppetversion'],'6.1.0') < 0 {
# Puppet 5 doesn't have https://tickets.puppetlabs.com/browse/PUP-3483
# and camptocamp/systemd only creates this relationship when managing the service
Class['systemd::systemctl::daemon_reload'] -> Class['prometheus::run_service']
}
}
'debian' : {
file { '/etc/init.d/prometheus':
mode => '0555',
owner => 'root',
group => 'root',
content => template('prometheus/prometheus.debian.erb'),
'sysv', 'redhat', 'debian', 'sles' : {
$content = $prometheus::server::init_style ? {
'redhat' => template('prometheus/prometheus.sysv.erb'), # redhat and sysv share the same template file
default => template("prometheus/prometheus.${prometheus::server::init_style}.erb"),
}
}
'sles' : {
file { '/etc/init.d/prometheus':
ensure => file,
mode => '0555',
owner => 'root',
group => 'root',
content => template('prometheus/prometheus.sles.erb'),
content => $content,
notify => $notify,
}
}
'launchd' : {
file { '/Library/LaunchDaemons/io.prometheus.daemon.plist':
ensure => file,
mode => '0644',
owner => 'root',
group => 'wheel',
content => template('prometheus/prometheus.launchd.erb'),
notify => $notify,
}
}
default : {
Expand All @@ -232,6 +228,7 @@
group => $prometheus::server::group,
purge => true,
recurse => true,
notify => Class['prometheus::service_reload'], # After purging, a reload is needed
}

$prometheus::server::collect_scrape_jobs.each |Hash $job_definition| {
Expand Down Expand Up @@ -265,7 +262,7 @@

if $prometheus::server::manage_config {
file { 'prometheus.yaml':
ensure => present,
ensure => file,
path => "${prometheus::server::config_dir}/${prometheus::server::configname}",
owner => 'root',
group => $prometheus::server::group,
Expand Down
2 changes: 1 addition & 1 deletion manifests/scrape_job.pp
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
},
])
file { "${collect_dir}/${job_name}_${name}.yaml":
ensure => present,
ensure => file,
owner => 'root',
group => $prometheus::group,
mode => $prometheus::config_mode,
Expand Down
2 changes: 1 addition & 1 deletion manifests/server.pp
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,6 @@

Class['prometheus::install']
-> Class['prometheus::config']
-> Class['prometheus::run_service']
-> Class['prometheus::run_service'] # Note: config must *not* be configured here to notify run_service. Some resources in config.pp need to notify service_reload instead
-> Class['prometheus::service_reload']
}
4 changes: 2 additions & 2 deletions spec/classes/prometheus_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@

it {
is_expected.to contain_file('prometheus.yaml').with(
'ensure' => 'present',
'ensure' => 'file',
'path' => '/etc/prometheus/prometheus.yaml',
'owner' => 'root',
'group' => 'prometheus',
Expand Down Expand Up @@ -296,7 +296,7 @@
}
it {
is_expected.to contain_file('prometheus.yaml').with(
'ensure' => 'present',
'ensure' => 'file',
'path' => '/etc/prometheus/prometheus.yaml',
'owner' => 'root',
'group' => 'prometheus',
Expand Down

0 comments on commit 8588d88

Please sign in to comment.