From d4d70dd2f8190d81af820fd2a89fa323d029d784 Mon Sep 17 00:00:00 2001
From: Steve Traylen <steve.traylen@cern.ch>
Date: Tue, 10 Mar 2020 09:41:57 +0100
Subject: [PATCH] New utils parameter to install collectdctl

On some operating systems `collectdctl`, `collectd-nagios`
is contained in a seperate sub package, typically `collectd-utils`

Setting `utils` to `true` will install this sub package if it exists
for an operating system.
---
 manifests/init.pp                           |  7 +++++++
 manifests/install.pp                        | 11 ++++++++++-
 manifests/params.pp                         |  1 +
 spec/acceptance/class_plugin_python_spec.rb | 13 +++++--------
 spec/classes/collectd_init_spec.rb          | 17 +++++++++++++++++
 5 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/manifests/init.pp b/manifests/init.pp
index 0bab5ac94..f22f1964b 100644
--- a/manifests/init.pp
+++ b/manifests/init.pp
@@ -1,4 +1,10 @@
+# @summary installs and configures collectd
+# @example Install collectd utilities
+#  class{'collectd':
+#    utils => true,
+#  }
 #
+# @param utils Install collectd utilities package containing collectdctl, collectd-nagios
 class collectd (
   Boolean $autoloadplugin                              = $collectd::params::autoloadplugin,
   String $collectd_hostname                            = $collectd::params::collectd_hostname,
@@ -36,6 +42,7 @@
   Optional[Integer] $write_queue_limit_high            = $collectd::params::write_queue_limit_high,
   Optional[Integer] $write_queue_limit_low             = $collectd::params::write_queue_limit_low,
   Integer[1] $write_threads                            = $collectd::params::write_threads,
+  Boolean    $utils                                    = $collectd::params::utils,
 ) inherits collectd::params {
 
   $collectd_version_real = pick_default($facts['collectd_version'], $minimum_version)
diff --git a/manifests/install.pp b/manifests/install.pp
index ed3522fd4..8efec4cef 100644
--- a/manifests/install.pp
+++ b/manifests/install.pp
@@ -1,4 +1,5 @@
-#
+# @summary installs collectd
+# @api private
 class collectd::install {
 
   assert_private()
@@ -10,4 +11,12 @@
       install_options => $collectd::package_install_options,
     }
   }
+
+  if $collectd::utils and  ( $facts['os']['family'] == 'Debian' or
+    ( $facts['os']['family'] == 'RedHat' and versioncmp($facts['os']['release']['major'],'8') >= 0 )) {
+    package{'collectd-utils':
+      ensure => $collectd::package_ensure,
+    }
+  }
+
 }
diff --git a/manifests/params.pp b/manifests/params.pp
index 8e3fc53a1..0cb76bef8 100644
--- a/manifests/params.pp
+++ b/manifests/params.pp
@@ -29,6 +29,7 @@
   $plugin_conf_dir_mode      = '0750'
   $ci_package_repo           = undef
   $package_keyserver         = 'keyserver.ubuntu.com'
+  $utils                     = false
 
   case $facts['kernel'] {
     'OpenBSD': { $has_wordexp = false }
diff --git a/spec/acceptance/class_plugin_python_spec.rb b/spec/acceptance/class_plugin_python_spec.rb
index e2b2afb34..9fa857d91 100644
--- a/spec/acceptance/class_plugin_python_spec.rb
+++ b/spec/acceptance/class_plugin_python_spec.rb
@@ -27,12 +27,6 @@ class { 'collectd::plugin::csv':}
   context 'trivial pip module connect-time' do
     it 'works idempotently with no errors' do
       pp = <<-EOS
-      if $facts['os']['family'] == 'Debian' or ( $facts['os']['family'] == 'RedHat' and $facts['os']['release']['major'] == '8' )  {
-        # for collectdctl command
-        package{'collectd-utils':
-       	  ensure => present,
-        }
-      }
       if $facts['os']['family'] == 'RedHat' and $facts['os']['release']['major'] == '8'  {
         $_python_pip_package = 'python3-pip'
         $_pip_provider = 'pip3'
@@ -49,6 +43,9 @@ class { 'collectd::plugin::csv':}
 	      require  => Package[$_python_pip_package],
 	      before   => Service['collectd'],
       }
+      class{'collectd':
+        utils => true,
+      }
       class{'collectd::plugin::python':
 	      logtraces   => true,
 	      interactive => false,
@@ -84,8 +81,7 @@ class { 'collectd::plugin::csv':}
     it 'works idempotently with no errors' do
       pp = <<-EOS
       if $facts['os']['family'] == 'Debian' {
-        # for collectdctl command
-        package{['collectd-utils','python-dbus']:
+        package{'python-dbus':
 	        ensure => present,
         }
       }
@@ -116,6 +112,7 @@ class { 'collectd::plugin::csv':}
 	      before   => Service['collectd'],
       }
       class{'collectd':
+        utils   => true,
         typesdb => ['/usr/share/collectd/types.db'],
       }
       class{'collectd::plugin::python':
diff --git a/spec/classes/collectd_init_spec.rb b/spec/classes/collectd_init_spec.rb
index 17c067d26..692fb98dc 100644
--- a/spec/classes/collectd_init_spec.rb
+++ b/spec/classes/collectd_init_spec.rb
@@ -37,6 +37,23 @@
         end
       end
 
+      context 'when utils false' do
+        let(:params) { { utils: false } }
+
+        it { is_expected.not_to contain_package('collectd-utils') }
+      end
+
+      context 'when utils true' do
+        let(:params) { { utils: true } }
+
+        case "#{facts[:os]['family']}-#{facts[:os]['release']['major']}"
+        when %r{^Debian-.+}, 'RedHat-8'
+          it { is_expected.to contain_package('collectd-utils') }
+        else
+          it { is_expected.not_to contain_package('collectd-utils') }
+        end
+      end
+
       context 'when purge_config is enabled' do
         let(:params) { { purge_config: true } }