(SUP-2192) Migrate from cron to systemd timers

This commit replaces cron entries with systemd services and timers. It also fixes the existing tests and adds a few to the pe_metric spec tests: * When systemd is not the init provider * Collecting and not collecting Puppet server metrics * Customizing the collection interval
puppetlabs · Jun 17, 2021 · ec0b499 · ec0b499
1 parent f1df737
commit ec0b499
Show file tree

Hide file tree

Showing 29 changed files with 263 additions and 92 deletions.
diff --git a/.sync.yml b/.sync.yml
@@ -28,6 +28,11 @@ spec/spec_helper.rb:
 Rakefile:
   changelog_user: "puppetlabs"
 
+spec/default_facts.yml:
+  extra_facts:
+    pe_server_version: '2019.8.6'
+    puppet_metrics_collector:
+      have_systemd: true
 .rubocop.yml:
   default_configs:
     "RSpec/NamedSubject":

diff --git a/files/metrics_tidy b/files/metrics_tidy
@@ -74,3 +74,4 @@ tar --create --gzip --file "${metrics_directory}/${metrics_type}-$(date +%Y.%m.%
 # We can assume that the json files have no spaces as they are created by our scripts
 # Only run xargs if the file is >0 bytes
 [[ -s $metrics_tmp ]] && xargs -a "$metrics_tmp" rm
+exit 0
diff --git a/files/psql_metrics b/files/psql_metrics
@@ -149,8 +149,8 @@ module PuppetMetricsCollector
                       '--single-transaction', '--tuples-only', '--quiet']
       psql_command += ["--dbname=#{database}"] unless database.nil?
 
-      command_line = ['/usr/bin/su', '-s', '/bin/bash', 'pe-postgres',
-                      '-c', psql_command.join(' ')]
+      command_line = ['/usr/sbin/runuser', '-u', 'pe-postgres',
+                      '--', *psql_command]
 
       env = { 'PGOPTIONS' => "-c statement_timeout=#{timeout}s",
              'PGTZ' => 'GMT' }

diff --git a/lib/facter/puppet_metrics_collector.rb b/lib/facter/puppet_metrics_collector.rb
@@ -9,6 +9,14 @@
     end
   end
 
+  chunk(:have_systemd) do
+    if Puppet::FileSystem.exist?('/proc/1/comm') && Puppet::FileSystem.read('/proc/1/comm').include?('systemd')
+      { have_systemd: true }
+    else
+      { have_systemd: false }
+    end
+  end
+
   chunk(:pe_psql) do
     if File.executable?('/opt/puppetlabs/server/bin/psql')
       { have_pe_psql: true }

diff --git a/manifests/collect.pp b/manifests/collect.pp
@@ -0,0 +1,60 @@
+# Create systemd units for collecting a given metric
+define puppet_metrics_collector::collect (
+  String $metrics_type    = $title,
+  String $metrics_command = undef,
+  String $tidy_command    = undef,
+  String $metric_ensure   = 'present',
+  String $minute          = '5',
+) {
+
+  $service_ensure = $metric_ensure ? {
+    'present' => running,
+    'absent'  => stopped,
+  }
+
+  $service_enable = $metric_ensure ? {
+    'present' => true,
+    'absent'  => false,
+  }
+
+  file {"/etc/systemd/system/${metrics_type}-metrics.service":
+    ensure  => $metric_ensure,
+    content => epp('puppet_metrics_collector/service.epp',
+      { 'service' => $metrics_type, 'metrics_command' => $metrics_command }
+    ),
+  }
+  file {"/etc/systemd/system/${metrics_type}-metrics.timer":
+    ensure  => $metric_ensure,
+    content => epp('puppet_metrics_collector/timer.epp',
+      { 'service' => $metrics_type, 'minute' => $minute },
+    ),
+  }
+
+  file {"/etc/systemd/system/${metrics_type}-tidy.service":
+    ensure  => $metric_ensure,
+    content => epp('puppet_metrics_collector/tidy.epp',
+      { 'service' => $metrics_type, 'tidy_command' => $tidy_command }
+    ),
+  }
+  file {"/etc/systemd/system/${metrics_type}-tidy.timer":
+    ensure  => $metric_ensure,
+    content => epp('puppet_metrics_collector/tidy_timer.epp',
+      { 'service' => $metrics_type }
+    ),
+  }
+
+  service { "${metrics_type}-metrics.service":
+  }
+  service { "${metrics_type}-metrics.timer":
+    ensure    => $service_ensure,
+    enable    => $service_enable,
+    subscribe => File["/etc/systemd/system/${metrics_type}-metrics.timer"],
+  }
+
+  service { "${metrics_type}-tidy.service": }
+  service { "${metrics_type}-tidy.timer":
+    ensure    => $service_ensure,
+    enable    => $service_enable,
+    subscribe => File["/etc/systemd/system/${metrics_type}-tidy.timer"],
+  }
+}
diff --git a/manifests/init.pp b/manifests/init.pp
@@ -37,6 +37,7 @@
   $config_dir  = "${output_dir}/config"
   $scripts_dir = "${output_dir}/scripts"
 
+  if $facts.dig('puppet_metrics_collector', 'have_systemd') {
     # If the puppet_metrics_collector::system class is evaluted first,
     # File[$output_dir] will already be defined along with common scripts.
     if !defined(File[$output_dir]) {
@@ -85,6 +86,12 @@
       source => 'puppet:///modules/puppet_metrics_collector/tk_metrics'
     }
 
+    exec { 'puppet_metrics_collector_daemon_reload':
+      command     => 'systemctl daemon-reload',
+      path        => ['/bin', '/usr/bin'],
+      refreshonly => true,
+    }
+
     include puppet_metrics_collector::service::puppetserver
     include puppet_metrics_collector::service::puppetdb
     include puppet_metrics_collector::service::orchestrator
@@ -119,4 +126,10 @@
     cron { ['activemq_metrics_collection', 'activemq_metrics_tidy']:
       ensure => absent,
     }
+  } else {
+    notify { 'systemd_provider_warning':
+      message  => 'This module only works with systemd as the provider',
+      loglevel => warning,
+    }
   }
+}
diff --git a/manifests/pe_metric.pp b/manifests/pe_metric.pp
@@ -2,7 +2,7 @@
 define puppet_metrics_collector::pe_metric (
   String                    $metrics_type             = $title,
   Enum['absent', 'present'] $metric_ensure            = 'present',
-  String                    $cron_minute              = '*/5',
+  String                    $cron_minute              = '0/5',
   Integer                   $retention_days           = 90,
   Array[String]             $hosts                    = ['127.0.0.1'],
   Integer                   $metrics_port             = undef,
@@ -101,27 +101,25 @@
   }
   # lint:endignore
 
-  cron { "${metrics_type}_metrics_collection" :
-    ensure  => $metric_ensure,
-    command => $metrics_command,
-    user    => 'root',
-    minute  => $cron_minute,
+  $tidy_command = "${puppet_metrics_collector::scripts_dir}/metrics_tidy -d ${metrics_output_dir} -r ${retention_days}"
+
+  puppet_metrics_collector::collect {$metrics_type:
+    metrics_command => $metrics_command,
+    tidy_command    => $tidy_command,
+    metric_ensure   => $metric_ensure,
+    minute          => $cron_minute,
+    notify          => Exec['puppet_metrics_collector_daemon_reload'],
   }
 
-  # The hardcoded numbers with the fqdn_rand calls are to trigger the metrics_tidy
-  # command to run at a randomly selected time between 12:00 AM and 3:00 AM.
-  # NOTE - if adding a new service, the name of the service must be added to the valid_paths array in files/metrics_tidy
+  # LEGACY CLEANUP
+  cron { "${metrics_type}_metrics_collection" :
+    ensure  => absent,
+  }
 
   cron { "${metrics_type}_metrics_tidy" :
-    ensure  => $metric_ensure,
-    command => "${puppet_metrics_collector::scripts_dir}/metrics_tidy -d ${metrics_output_dir} -r ${retention_days}",
-    user    => 'root',
-    hour    => fqdn_rand(3, $metrics_type),
-    minute  => (5 * fqdn_rand(11, $metrics_type)),
+    ensure  => absent,
   }
 
-  # LEGACY CLEANUP
-
   $metric_legacy_files = [
     "${puppet_metrics_collector::scripts_dir}/${metrics_type}_config.yaml",
     "${puppet_metrics_collector::scripts_dir}/${metrics_type}_metrics_tidy",

diff --git a/manifests/sar_metric.pp b/manifests/sar_metric.pp
@@ -2,7 +2,7 @@
 define puppet_metrics_collector::sar_metric (
   String                    $metrics_type              = $title,
   Enum['absent', 'present'] $metric_ensure             = 'present',
-  String                    $cron_minute               = '*/5',
+  String                    $cron_minute               = '0/5',
   Integer                   $retention_days            = 90,
   Integer                   $collection_frequency      = 5, # minutes
   Integer                   $polling_frequency_seconds = 1,
@@ -33,27 +33,30 @@
                             ' > /dev/null',
                             ], '')
 
-  cron { "${metrics_type}_metrics_collection" :
-    ensure  => $metric_ensure,
-    command => $metrics_command,
-    user    => 'root',
-    minute  => $cron_minute,
-  }
-
   # The hardcoded numbers with the fqdn_rand calls are to trigger the metrics_tidy 
   # command to run at a randomly selected time between 12:00 AM and 3:00 AM.
   # NOTE - if adding a new service, the name of the service must be added to the valid_paths array in files/metrics_tidy
 
-  cron { "${metrics_type}_metrics_tidy" :
-    ensure  => $metric_ensure,
-    command => "${puppet_metrics_collector::system::scripts_dir}/metrics_tidy -d ${metrics_output_dir} -r ${retention_days}",
-    user    => 'root',
-    hour    => fqdn_rand(3, $metrics_type),
-    minute  => (5 * fqdn_rand(11, $metrics_type)),
+  $tidy_command = "${puppet_metrics_collector::system::scripts_dir}/metrics_tidy -d ${metrics_output_dir} -r ${retention_days}"
+
+  puppet_metrics_collector::collect {$metrics_type:
+    metrics_command => $metrics_command,
+    tidy_command    => $tidy_command,
+    metric_ensure   => $metric_ensure,
+    minute          => $cron_minute,
+    notify          => Exec['puppet_metrics_collector_system_daemon_reload'],
   }
 
   # LEGACY CLEANUP
 
+  cron { "${metrics_type}_metrics_tidy" :
+    ensure  => absent,
+  }
+
+  cron { "${metrics_type}_metrics_collection" :
+    ensure  => absent,
+  }
+
   $metric_legacy_files = [
     "${puppet_metrics_collector::system::scripts_dir}/${metrics_type}_metrics_tidy",
   ]

diff --git a/manifests/service/ace.pp b/manifests/service/ace.pp
@@ -13,9 +13,9 @@
   Optional[Integer]       $metrics_server_port      = $puppet_metrics_collector::metrics_server_port,
   Optional[String]        $metrics_server_db_name   = $puppet_metrics_collector::metrics_server_db_name,
   ) {
-  puppet_metrics_collector::pe_metric { 'ace' :
+  puppet_metrics_collector::pe_metric { 'puppet_ace' :
     metric_ensure           => $metrics_ensure,
-    cron_minute             => "*/${collection_frequency}",
+    cron_minute             => "0/${collection_frequency}",
     retention_days          => $retention_days,
     hosts                   => $hosts,
     metrics_port            => $port,

diff --git a/manifests/service/bolt.pp b/manifests/service/bolt.pp
@@ -13,9 +13,9 @@
   Optional[Integer]       $metrics_server_port      = $puppet_metrics_collector::metrics_server_port,
   Optional[String]        $metrics_server_db_name   = $puppet_metrics_collector::metrics_server_db_name,
   ) {
-  puppet_metrics_collector::pe_metric { 'bolt' :
+  puppet_metrics_collector::pe_metric { 'puppet_bolt' :
     metric_ensure           => $metrics_ensure,
-    cron_minute             => "*/${collection_frequency}",
+    cron_minute             => "0/${collection_frequency}",
     retention_days          => $retention_days,
     hosts                   => $hosts,
     metrics_port            => $port,

diff --git a/manifests/service/orchestrator.pp b/manifests/service/orchestrator.pp
@@ -13,9 +13,9 @@
   Optional[Integer]       $metrics_server_port      = $puppet_metrics_collector::metrics_server_port,
   Optional[String]        $metrics_server_db_name   = $puppet_metrics_collector::metrics_server_db_name,
   ) {
-  puppet_metrics_collector::pe_metric { 'orchestrator' :
+  puppet_metrics_collector::pe_metric { 'puppet_orchestrator' :
     metric_ensure            => $metrics_ensure,
-    cron_minute              => "*/${collection_frequency}",
+    cron_minute              => "0/${collection_frequency}",
     retention_days           => $retention_days,
     hosts                    => $hosts,
     metrics_port             => $port,

diff --git a/manifests/service/puppetdb.pp b/manifests/service/puppetdb.pp
@@ -349,9 +349,9 @@       'mbean' => 'puppetlabs.puppetdb.ha:name=record-transfer-duration'
     $_port = $port
   }
 
-  puppet_metrics_collector::pe_metric { 'puppetdb' :
+  puppet_metrics_collector::pe_metric { 'puppet_puppetdb' :
     metric_ensure            => $metrics_ensure,
-    cron_minute              => "*/${collection_frequency}",
+    cron_minute              => "0/${collection_frequency}",
     retention_days           => $retention_days,
     hosts                    => $hosts,
     metrics_port             => $_port,

diff --git a/manifests/service/puppetserver.pp b/manifests/service/puppetserver.pp
@@ -13,9 +13,9 @@
   Optional[Integer]       $metrics_server_port      = $puppet_metrics_collector::metrics_server_port,
   Optional[String]        $metrics_server_db_name   = $puppet_metrics_collector::metrics_server_db_name,
   ) {
-  puppet_metrics_collector::pe_metric { 'puppetserver' :
+  puppet_metrics_collector::pe_metric { 'puppet_puppetserver' :
     metric_ensure            => $metrics_ensure,
-    cron_minute              => "*/${collection_frequency}",
+    cron_minute              => "0/${collection_frequency}",
     retention_days           => $retention_days,
     hosts                    => $hosts,
     metrics_port             => $port,

diff --git a/manifests/system.pp b/manifests/system.pp
@@ -32,7 +32,6 @@
     }
   }
 
-
   file { "${scripts_dir}/system_metrics":
     ensure => file,
     mode   => '0755',
@@ -45,6 +44,12 @@
     }
   }
 
+  exec { 'puppet_metrics_collector_system_daemon_reload':
+    command     => 'systemctl daemon-reload',
+    path        => ['/bin', '/usr/bin'],
+    refreshonly => true,
+  }
+
   include puppet_metrics_collector::system::cpu
   include puppet_metrics_collector::system::memory
   include puppet_metrics_collector::system::processes

diff --git a/manifests/system/cpu.pp b/manifests/system/cpu.pp
@@ -5,9 +5,9 @@
   Integer $retention_days            = $puppet_metrics_collector::system::retention_days,
   Integer $polling_frequency_seconds = $puppet_metrics_collector::system::polling_frequency_seconds,
   ) {
-  puppet_metrics_collector::sar_metric { 'system_cpu' :
+  puppet_metrics_collector::sar_metric { 'puppet_system_cpu' :
     metric_ensure             => $metrics_ensure,
-    cron_minute               => "*/${collection_frequency}",
+    cron_minute               => "0/${collection_frequency}",
     retention_days            => $retention_days,
     collection_frequency      => $collection_frequency,
     polling_frequency_seconds => $polling_frequency_seconds,

diff --git a/manifests/system/memory.pp b/manifests/system/memory.pp
@@ -5,9 +5,9 @@
   Integer $retention_days            = $puppet_metrics_collector::system::retention_days,
   Integer $polling_frequency_seconds = $puppet_metrics_collector::system::polling_frequency_seconds,
   ) {
-  puppet_metrics_collector::sar_metric { 'system_memory' :
+  puppet_metrics_collector::sar_metric { 'puppet_system_memory' :
     metric_ensure             => $metrics_ensure,
-    cron_minute               => "*/${collection_frequency}",
+    cron_minute               => "0/${collection_frequency}",
     retention_days            => $retention_days,
     collection_frequency      => $collection_frequency,
     polling_frequency_seconds => $polling_frequency_seconds,

diff --git a/manifests/system/postgres.pp b/manifests/system/postgres.pp
@@ -28,22 +28,18 @@
                       '--output_dir', $metrics_output_dir,
                       '> /dev/null'].join(' ')
 
-  cron { 'postgres_metrics_collection':
-    ensure  => $metrics_ensure,
-    command => $metrics_command,
-    user    => 'root',
-    minute  => "*/${collection_frequency}",
-  }
+  $tidy_command = "${puppet_metrics_collector::system::scripts_dir}/metrics_tidy -d ${metrics_output_dir} -r ${retention_days}"
 
-  # The hardcoded numbers with the fqdn_rand calls are to trigger the metrics_tidy
-  # command to run at a randomly selected time between 12:00 AM and 3:00 AM.
-  # NOTE - if adding a new service, the name of the service must be added to the valid_paths array in files/metrics_tidy
+  puppet_metrics_collector::collect {'puppet_postgres':
+    metrics_command => $metrics_command,
+    tidy_command    => $tidy_command,
+    metric_ensure   => $metrics_ensure,
+    minute          => String($collection_frequency),
+    notify          => Exec['puppet_metrics_collector_system_daemon_reload'],
+  }
 
-  cron { 'postgres_metrics_tidy':
-    ensure  => $metrics_ensure,
-    command => "${puppet_metrics_collector::system::scripts_dir}/metrics_tidy -d ${metrics_output_dir} -r ${retention_days}",
-    user    => 'root',
-    hour    => fqdn_rand(3, 'postgres'),
-    minute  => (5 * fqdn_rand(11, 'postgres')),
+  # Legacy cleanup
+  cron { ['postgres_metrics_tidy', 'postgres_metrics_collection']:
+    ensure => absent
   }
 }
diff --git a/manifests/system/processes.pp b/manifests/system/processes.pp
@@ -5,9 +5,9 @@
   Integer $retention_days            = $puppet_metrics_collector::system::retention_days,
   Integer $polling_frequency_seconds = $puppet_metrics_collector::system::polling_frequency_seconds,
 ) {
-  puppet_metrics_collector::sar_metric { 'system_processes' :
+  puppet_metrics_collector::sar_metric { 'puppet_system_processes' :
     metric_ensure             => $metrics_ensure,
-    cron_minute               => "*/${collection_frequency}",
+    cron_minute               => "0/${collection_frequency}",
     retention_days            => $retention_days,
     collection_frequency      => $collection_frequency,
     polling_frequency_seconds => $polling_frequency_seconds,