Merge pull request #390 from alexjfisher/issue_382

Fix prometheus not restarting after config changes on systemd based systems

manifests/config.pp

@@ -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 : {
@@ -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| {
@@ -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,

manifests/scrape_job.pp

@@ -31,7 +31,7 @@
     },
   ])
   file { "${collect_dir}/${job_name}_${name}.yaml":
-    ensure  => present,
+    ensure  => file,
     owner   => 'root',
     group   => $prometheus::group,
     mode    => $prometheus::config_mode,

manifests/server.pp

@@ -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']
 }

spec/acceptance/prometheus_server_spec.rb

@@ -2,7 +2,7 @@
 
 describe 'prometheus server basics' do
   it 'prometheus server via main class works idempotently with no errors' do
-    pp = "class{'prometheus': manage_prometheus_server => true, version => '1.5.2' }"
+    pp = "class{'prometheus': manage_prometheus_server => true }"
 
     # Run it twice and test for idempotency
     apply_manifest(pp, catch_failures: true)
@@ -32,6 +32,29 @@
     it { is_expected.to be_listening.with('tcp6') }
   end
 
+  it 'does not allow admin API' do # rubocop:disable RSpec/MultipleExpectations
+    shell('curl -s -XPOST http://127.0.0.1:9090/api/v1/admin/tsdb/snapshot') do |r|
+      expect(r.stdout).to match(%r{admin APIs disabled})
+      expect(r.exit_code).to eq(0)
+    end
+  end
+
+  describe 'updating configuration to enable Admin API' do
+    it 'prometheus server via main class works idempotently with no errors' do
+      pp = "class{'prometheus': manage_prometheus_server => true, web_enable_admin_api => true }"
+
+      apply_manifest(pp, catch_failures: true)
+      apply_manifest(pp, catch_changes: true)
+    end
+    it 'allows admin API' do # rubocop:disable RSpec/MultipleExpectations
+      shell('curl -s -XPOST http://127.0.0.1:9090/api/v1/admin/tsdb/snapshot') do |r|
+        expect(r.stdout).not_to match(%r{admin APIs disabled})
+        expect(r.stdout).to match(%r{success})
+        expect(r.exit_code).to eq(0)
+      end
+    end
+  end
+
   describe 'prometheus server with options' do
     it 'is idempotent' do
       pp = "class{'prometheus::server': version => '2.4.3', external_url => '/test'}"

spec/classes/prometheus_spec.rb

@@ -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',
@@ -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',