add systemd dropin for slave mode

With currently-supported OSes, smokeping uses systemd. The systemd service unit doesn't use /etc/default/smokeping at all. In order to run smokeping in slave mode, you have to use a systemd service unit dropin file. This commit also converts the slave template to EPP, corrects the data type of the slave_color parameter, and makes some code cleanups.
voxpupuli · Sep 3, 2023 · 3dc6bbd · 3dc6bbd
1 parent 4ce7679
commit 3dc6bbd
Show file tree

Hide file tree

Showing 12 changed files with 111 additions and 88 deletions.
diff --git a/.fixtures.yml b/.fixtures.yml
@@ -5,3 +5,4 @@ fixtures:
     firewall: https://github.com/puppetlabs/puppetlabs-firewall
     concat: https://github.com/puppetlabs/puppetlabs-concat.git
     selinux: https://github.com/voxpupuli/puppet-selinux.git
+    systemd: https://github.com/voxpupuli/puppet-systemd.git
diff --git a/README.md b/README.md
@@ -29,6 +29,7 @@ Standalone only (no master/slave support yet):
 
 - [puppetlabs-concat](https://github.com/puppetlabs/puppet-concat)
 - [puppetlabs-stdlib](https://github.com/puppetlabs/puppet-stdlib)
+- [puppet-systemd](https://github.com/voxpupuli/puppet-systemd)
 
 ## Example
 

diff --git a/REFERENCE.md b/REFERENCE.md
@@ -134,9 +134,9 @@ Default value: `undef`
 
 ##### <a name="-smokeping--slave_color"></a>`slave_color`
 
-Data type: `Optional[String[1]]`
+Data type: `Optional[Integer[0]]`
 
-Color of slave in the graph
+Color of slave in the graph, as a RRGGBB hex integer.
 
 Default value: `undef`
 
@@ -447,19 +447,21 @@ The following parameters are available in the `smokeping::slave` defined type:
 
 ##### <a name="-smokeping--slave--location"></a>`location`
 
-Data type: `Any`
+Data type: `Optional[String[1]]`
 
 Location of slave
 
+Default value: `undef`
+
 ##### <a name="-smokeping--slave--display_name"></a>`display_name`
 
-Data type: `Any`
+Data type: `String[1]`
 
 Name of the slave
 
 ##### <a name="-smokeping--slave--color"></a>`color`
 
-Data type: `Any`
+Data type: `Integer[0]`
 
 Color of this slave
 

diff --git a/manifests/config.pp b/manifests/config.pp
@@ -4,7 +4,6 @@
 
   $mode          = $smokeping::mode
   $master_url    = $smokeping::master_url
-  $shared_secret = $smokeping::shared_secret
   $slave_name    = $smokeping::slave_name
   $master_name   = $smokeping::master_name
 
@@ -34,7 +33,6 @@
   $default_probe  = $smokeping::default_probe
   $cgi_remark_top = $smokeping::cgi_remark_top
   $cgi_title_top  = $smokeping::cgi_title_top
-  $targets        = $smokeping::targets
 
   # Pathnames
   $path_sendmail  = $smokeping::path_sendmail
@@ -75,60 +73,51 @@
       content => template('smokeping/probes.erb');
   }
 
-  ## Platform Specific
-  if $facts['os']['family'] == 'Debian' or $facts['os']['name'] == 'Ubuntu' {
-    # Defaults file allows smokeping to be launched in different modes (eg slave vs master)
-    file { '/etc/default/smokeping':
-      content => template('smokeping/defaults.erb');
-    }
-  } else {
-    # TODO: Add master/slave support to non-Debian distros
-    #
-    # We don't yet support modes other than standalone on other platforms
-    # such as RHEL - to offer it, we need to start replacing the systemd
-    # service file loaded by the package manager with a custom one that
-    # has the alternative parameters set, like in the default file above
-    # for Debian/Ubuntu systems.
-
-    if ($mode != 'standalone') {
-      fail('Currently master/slave mode not supported for this OS family')
-    }
-  }
-
   ## mode specific
   case $mode {
-    ## Slave configuration
     'slave': {
-      # Check if slave_display_name is unset.
-      # --> use FQDN if not set.
-      $display_name = pick($smokeping::slave_display_name, $facts['networking']['fqdn'])
+      smokeping::slave { $facts['networking']['hostname']:
+        location     => $smokeping::slave_location,
+        display_name => pick($smokeping::slave_display_name, $facts['networking']['hostname']),
+        color        => pick($smokeping::slave_color, fqdn_rand(0xffffff)),
+      }
 
-      $slave_color = pick($smokeping::slave_color, sprintf('%06d', fqdn_rand('999999')))
+      $dropin = @("EOT")
+        [Service]
+        ExecStart=
+        ExecStart=/usr/sbin/smokeping --master-url=${smokeping::master_url} --cache-dir=${smokeping::path_datadir} --shared-secret=${smokeping::shared_secret} --pid-dir=/run/smokeping
+        | EOT
 
-      smokeping::slave { $facts['networking']['fqdn']:
-        location     => $smokeping::slave_location,
-        display_name => $display_name,
-        color        => $slave_color,
+      systemd::dropin_file { 'slave.conf':
+        unit    => 'smokeping.service',
+        content => $dropin,
       }
     }
+
     ## Master/Standalone configuration
     ## collect slaves if mode is master and create Targets
     ## if mode is standalone, just create targets...
     /^(master|standalone)$/: {
-      if $mode =~ /^master$/ {
+      if $mode == 'master' {
         # collect slaves
         File <<| tag == "smokeping-slave-${master_name}" |>>
-        file { $smokeping::slave_dir: ensure => directory; }
+
+        file { $smokeping::slave_dir:
+          ensure => directory,
+        }
+
         concat { '/etc/smokeping/config.d/Slaves':
           owner => root,
           group => root,
           mode  => '0644',
         }
+
         concat::fragment { 'slaves-header':
           target  => '/etc/smokeping/config.d/Slaves',
           order   => 10,
           content => "*** Slaves ***\nsecrets=${smokeping::slave_secrets}\n\n",
         }
+
         Concat::Fragment <<| tag == "smokeping-slave-${master_name}" |>>
 
         # collect shared secrets from slaves
@@ -137,9 +126,9 @@
           group => $webserver_group,
           mode  => '0640',
         }
+
         Concat::Fragment <<| tag == "smokeping-slave-secret-${master_name}" |>>
       } else {
-        # ensure $smokeping::slave_secret is there
         file { $smokeping::slave_secrets:
           ensure => file,
           owner  => $daemon_user,
@@ -155,18 +144,28 @@
         purge   => true,
         force   => true,
       }
+
       concat { '/etc/smokeping/config.d/Targets':
         owner => root,
         group => root,
         mode  => '0644',
       }
+
       concat::fragment { 'targets-header':
         target  => '/etc/smokeping/config.d/Targets',
         order   => 10,
         content => template('smokeping/targets-header.erb'),
       }
-      create_resources('smokeping::target', $targets, {})
+
+      $smokeping::targets.each |$key, $value| {
+        smokeping::target { $key:
+          * => $value,
+        }
+      }
+    }
+
+    default: {
+      fail("mode ${mode} unknown")
     }
-    default: { fail("mode ${mode} unknown") }
   }
 }
diff --git a/manifests/init.pp b/manifests/init.pp
@@ -25,7 +25,7 @@
 #   Name of slave, displayed on Webpage
 #
 # @param slave_color
-#   Color of slave in the graph
+#   Color of slave in the graph, as a RRGGBB hex integer.
 #
 # @param master_name
 #   Name of the master, in case of more than one SmokePing Master/Slave
@@ -140,7 +140,7 @@
   Stdlib::Absolutepath $slave_dir = '/etc/smokeping/config.d/slaves.d',
   Optional[String[1]] $slave_location = undef,
   Optional[String[1]] $slave_display_name = undef,
-  Optional[String[1]] $slave_color = undef,
+  Optional[Integer[0]] $slave_color = undef,
   String[1] $master_name = 'default',
   String[1] $owner = 'Peter Random',
   Stdlib::Email $contact = 'root@localhost',

diff --git a/manifests/install.pp b/manifests/install.pp
@@ -13,26 +13,32 @@
     ensure_packages([$smokeping::package_perldoc])
   }
 
-  # correct permissions
-  file { $smokeping::path_datadir:
-    ensure  => directory,
-    owner   => $smokeping::daemon_user,
-    group   => $smokeping::daemon_group,
-    require => Package['smokeping'],
-    recurse => true,
+  $datadir_group = $smokeping::mode ? {
+    'slave' => $smokeping::daemon_group,
+    default => $smokeping::webserver_group,
+  }
+
+  $datadir_mode = $smokeping::mode ? {
+    'slave' => undef,
+    default => 'g+wX',
   }
-  file { $smokeping::path_piddir:
+
+  file { $smokeping::path_datadir:
     ensure  => directory,
     owner   => $smokeping::daemon_user,
-    group   => $smokeping::daemon_group,
+    group   => $datadir_group,
+    mode    => $datadir_mode,
     require => Package['smokeping'],
     recurse => true,
   }
-  file { $smokeping::path_imgcache:
-    ensure  => directory,
-    owner   => $smokeping::webserver_user,
-    group   => $smokeping::webserver_group,
-    require => Package['smokeping'],
-    recurse => true,
+
+  if $smokeping::mode =~ /(master|standalone)/ {
+    file { $smokeping::path_imgcache:
+      ensure  => directory,
+      owner   => $smokeping::webserver_user,
+      group   => $smokeping::webserver_group,
+      require => Package['smokeping'],
+      recurse => true,
+    }
   }
 }
diff --git a/manifests/slave.pp b/manifests/slave.pp
@@ -12,34 +12,44 @@
 # @param master
 #   Name of the smokeping master, in case there are more than one.
 define smokeping::slave (
-  $location,
-  $display_name,
-  $color,
+  String[1] $display_name,
+  Integer[0] $color,
+  Optional[String[1]] $location = undef,
   $master = 'default',
 ) {
   File {
-    owner   => root,
-    group   => root,
-    mode    => '0644',
+    owner => root,
+    group => root,
+    mode  => '0644',
   }
 
-  $random_value = fqdn_rand_string(60, 'abcdefghjkelmnopqrstuvwxyaABCDEFGHJKELMNOPQRSTUVWXYA0123456789')
+  $random_value = fqdn_rand_string(60)
+
   file { $smokeping::shared_secret:
     mode    => '0600',
     owner   => $smokeping::daemon_user,
     group   => $smokeping::daemon_group,
     content => $random_value,
   }
+
   @@concat::fragment { "${facts['networking']['fqdn']}-secret":
     target  => $smokeping::slave_secrets,
     order   => 10,
-    content => "${facts['networking']['fqdn']}:${random_value}\n",
+    content => "${facts['networking']['hostname']}:${random_value}\n",
     tag     => "smokeping-slave-secret-${master}",
   }
 
   $filename = "${smokeping::slave_dir}/${facts['networking']['fqdn']}"
+
   @@file { $filename:
-    content => template('smokeping/slave.erb'),
+    content => epp("${module_name}/slave.epp",
+      {
+        'slave_name'   => $title,
+        'display_name' => $display_name,
+        'color'        => $color,
+        'location'     => $location,
+      }
+    ),
     tag     => "smokeping-slave-${master}",
   }
 

diff --git a/metadata.json b/metadata.json
@@ -51,6 +51,10 @@
     {
       "name": "puppetlabs/stdlib",
       "version_requirement": ">= 4.25.0 < 9.0.0"
+    },
+    {
+      "name": "puppet/systemd",
+      "version_requirement": ">= 3.5.1 < 4.0.0"
     }
   ],
   "requirements": [

diff --git a/spec/classes/init_spec.rb b/spec/classes/init_spec.rb
@@ -3,10 +3,6 @@
 require 'spec_helper'
 
 describe 'smokeping' do
-  let :node do
-    'agent.example.com'
-  end
-
   on_supported_os.each do |os, facts|
     context "on #{os}" do
       let :facts do
@@ -27,6 +23,19 @@
         it { is_expected.to contain_class('smokeping::service') }
         it { is_expected.to contain_class('smokeping::config') }
       end
+
+      context 'slave mode' do
+        let :params do
+          super().merge(
+            mode: 'slave',
+            slave_color: 0xabcdef
+          )
+        end
+
+        it { is_expected.to compile.with_all_deps }
+        it { is_expected.to contain_smokeping__slave(facts[:hostname]).with_color(0xabcdef) }
+        it { is_expected.to contain_systemd__dropin_file('slave.conf').with_unit('smokeping.service') }
+      end
     end
   end
 end
diff --git a/templates/defaults.erb b/templates/defaults.erb
diff --git a/templates/slave.epp b/templates/slave.epp
@@ -0,0 +1,8 @@
+# THIS FILE IS MANAGED BY PUPPET
+
++ <%= $slave_name %>
+display_name = <%= $display_name %>
+color = <%= sprintf("%x", $color) %>
+<% if $location { %>
+location = <%= $location %>
+<% } %>
diff --git a/templates/slave.erb b/templates/slave.erb