(GH-108) Disable sysstat management by default

lib/facter/puppet_metrics_collector.rb

@@ -17,6 +17,14 @@
     end
   end
 
+  chunk(:have_sysstat) do
+    if Facter::Core::Execution.which('sar')
+      { have_sysstat: true }
+    else
+      { have_sysstat: false }
+    end
+  end
+
   chunk(:pe_psql) do
     if File.executable?('/opt/puppetlabs/server/bin/psql')
       { have_pe_psql: true }

manifests/system.pp

@@ -5,7 +5,7 @@
   Integer $collection_frequency      = 5, # minutes
   Integer $retention_days            = 90,
   Integer $polling_frequency_seconds = 1,
-  Boolean $manage_sysstat            = true,
+  Boolean $manage_sysstat            = false,
   Boolean $manage_vmware_tools       = false,
   String  $vmware_tools_pkg          = 'open-vm-tools',
 ) {
@@ -32,27 +32,34 @@
     }
   }
 
-  file { "${scripts_dir}/system_metrics":
-    ensure => file,
-    mode   => '0755',
-    source => 'puppet:///modules/puppet_metrics_collector/system_metrics'
+  exec { 'puppet_metrics_collector_system_daemon_reload':
+    command     => 'systemctl daemon-reload',
+    path        => ['/bin', '/usr/bin'],
+    refreshonly => true,
   }
 
   if $manage_sysstat {
     package { 'sysstat':
-      ensure => installed,
+      ensure => $system_metrics_ensure,
     }
   }
 
-  exec { 'puppet_metrics_collector_system_daemon_reload':
-    command     => 'systemctl daemon-reload',
-    path        => ['/bin', '/usr/bin'],
-    refreshonly => true,
-  }
+  if $manage_sysstat or $facts.dig('puppet_metrics_collector', 'have_sysstat') {
+    file { "${scripts_dir}/system_metrics":
+      ensure => file,
+      mode   => '0755',
+      source => 'puppet:///modules/puppet_metrics_collector/system_metrics'
+    }
 
-  include puppet_metrics_collector::system::cpu
-  include puppet_metrics_collector::system::memory
-  include puppet_metrics_collector::system::processes
+    contain puppet_metrics_collector::system::cpu
+    contain puppet_metrics_collector::system::memory
+    contain puppet_metrics_collector::system::processes
+  } else {
+    notify { 'sysstat_missing_warning':
+      message  => 'System collection disabled. Set `puppet_metrics_collector::system::manage_sysstat: true` to enable system metrics',
+      loglevel => warning,
+    }
+  }
 
   if $facts['virtual'] == 'vmware' {
     if $manage_vmware_tools and ($system_metrics_ensure == 'present') {

spec/acceptance/pe_metric_spec.rb

@@ -1,48 +1,34 @@
 require 'spec_helper_acceptance'
 
-describe 'test default and system includes' do
+describe 'default includes' do
   before(:all) do
     pp = <<-MANIFEST
         include puppet_metrics_collector
-        include puppet_metrics_collector::system
         MANIFEST
     idempotent_apply(pp)
   end
-  it 'all puppet_* metric services should be active or inactive' do
-    run_shell('systemctl list-units --type=service | grep "puppet_.*metrics"') do |r|
-      expect(r.stdout).to match(%r{activ})
-    end
-  end
+
   context 'all of the timers are running' do
     it { expect(service('puppet_ace-metrics.timer')).to be_running }
     it { expect(service('puppet_ace-tidy.timer')).to be_running }
     it { expect(service('puppet_bolt-metrics.timer')).to be_running }
     it { expect(service('puppet_bolt-tidy.timer')).to be_running }
     it { expect(service('puppet_orchestrator-metrics.timer')).to be_running }
     it { expect(service('puppet_orchestrator-tidy.timer')).to be_running }
-    it { expect(service('puppet_postgres-metrics.timer')).to be_running }
-    it { expect(service('puppet_postgres-tidy.timer')).to be_running }
     it { expect(service('puppet_puppetdb-metrics.timer')).to be_running }
     it { expect(service('puppet_puppetdb-tidy.timer')).to be_running }
     it { expect(service('puppet_puppetserver-metrics.timer')).to be_running }
     it { expect(service('puppet_puppetserver-tidy.timer')).to be_running }
-    it { expect(service('puppet_system_cpu-metrics.timer')).to be_running }
-    it { expect(service('puppet_system_cpu-tidy.timer')).to be_running }
-    it { expect(service('puppet_system_memory-metrics.timer')).to be_running }
-    it { expect(service('puppet_system_memory-tidy.timer')).to be_running }
-    it { expect(service('puppet_system_processes-metrics.timer')).to be_running }
-    it { expect(service('puppet_system_processes-tidy.timer')).to be_running }
   end
+
   it 'creates tidy services files' do
     files = run_shell('ls /etc/systemd/system/puppet_*-tidy.service').stdout
-    expect(files.split("\n").count).to eq(9)
+    expect(files.split("\n").count).to eq(5)
   end
+
   it 'creates the timer files' do
     files = run_shell('ls /etc/systemd/system/puppet_*-tidy.timer').stdout
-    expect(files.split("\n").count).to eq(9)
-  end
-  it 'sysstat package is installed' do
-    expect(package('sysstat')).to be_installed
+    expect(files.split("\n").count).to eq(5)
   end
 
   describe file('/opt/puppetlabs/puppet-metrics-collector/puppetserver/127.0.0.1') do

spec/acceptance/pe_system_spec.rb

@@ -0,0 +1,105 @@
+require 'spec_helper_acceptance'
+
+describe 'system class' do
+  context 'sysstat not installed and not managed' do
+    before(:all) do
+      run_shell('puppet resource package sysstat ensure=absent')
+      pp = <<-MANIFEST
+          include puppet_metrics_collector::system
+          MANIFEST
+      # The notify makes this non idempotent
+      expect(apply_manifest(pp).exit_code).not_to eq(1)
+      expect(apply_manifest(pp).exit_code).not_to eq(1)
+    end
+
+    context 'postgres timers are running' do
+      it { expect(service('puppet_postgres-metrics.timer')).to be_running }
+      it { expect(service('puppet_postgres-tidy.timer')).to be_running }
+    end
+
+    it 'creates tidy service file for postgres' do
+      files = run_shell('ls /etc/systemd/system/puppet_postgres-tidy.service').stdout
+      expect(files.split("\n")).not_to be_empty
+    end
+
+    it 'creates the service file for postgres' do
+      files = run_shell('ls /etc/systemd/system/puppet_postgres-metrics.service').stdout
+      expect(files.split("\n")).not_to be_empty
+    end
+
+    it 'sysstat package is not installed by default' do
+      expect(package('sysstat')).not_to be_installed
+    end
+
+    it 'have_sysstat is false without the package installed' do
+      expect(host_inventory['facter']['puppet_metrics_collector']['have_sysstat']).to eq false
+    end
+  end
+
+  context 'sysstat installed and not managed' do
+    before(:all) do
+      run_shell('puppet resource package sysstat ensure=installed')
+      pp = <<-MANIFEST
+          include puppet_metrics_collector::system
+          MANIFEST
+      expect(apply_manifest(pp).exit_code).not_to eq(1)
+      expect(apply_manifest(pp).exit_code).not_to eq(1)
+    end
+
+    it 'system puppet_* metric services should be active or inactive' do
+      run_shell('systemctl list-units --type=service | grep "puppet_system.*metrics"') do |r|
+        expect(r.stdout).to match(%r{activ})
+      end
+    end
+
+    context 'system timers are running' do
+      it { expect(service('puppet_system_cpu-metrics.timer')).to be_running }
+      it { expect(service('puppet_system_cpu-tidy.timer')).to be_running }
+      it { expect(service('puppet_system_memory-metrics.timer')).to be_running }
+      it { expect(service('puppet_system_memory-tidy.timer')).to be_running }
+      it { expect(service('puppet_system_processes-metrics.timer')).to be_running }
+      it { expect(service('puppet_system_processes-tidy.timer')).to be_running }
+    end
+
+    it 'creates system tidy services files' do
+      files = run_shell('ls /etc/systemd/system/puppet_system*-tidy.service').stdout
+      expect(files.split("\n").count).to eq(3)
+    end
+  end
+
+  context 'managing sysstat' do
+    before(:all) do
+      pp = <<-MANIFEST
+          class { 'puppet_metrics_collector::system':
+            manage_sysstat => true,
+          }
+          MANIFEST
+      expect(apply_manifest(pp).exit_code).not_to eq(1)
+      expect(apply_manifest(pp).exit_code).not_to eq(1)
+    end
+
+    it 'sysstat package is installed' do
+      expect(package('sysstat')).to be_installed
+    end
+
+    it 'system puppet_* metric services should be active or inactive' do
+      run_shell('systemctl list-units --type=service | grep "puppet_system.*metrics"') do |r|
+        expect(r.stdout).to match(%r{activ})
+      end
+    end
+
+    context 'system timers are running' do
+      it { expect(service('puppet_system_cpu-metrics.timer')).to be_running }
+      it { expect(service('puppet_system_cpu-tidy.timer')).to be_running }
+      it { expect(service('puppet_system_memory-metrics.timer')).to be_running }
+      it { expect(service('puppet_system_memory-tidy.timer')).to be_running }
+      it { expect(service('puppet_system_processes-metrics.timer')).to be_running }
+      it { expect(service('puppet_system_processes-tidy.timer')).to be_running }
+    end
+
+    it 'creates system tidy services files' do
+      files = run_shell('ls /etc/systemd/system/puppet_system*-tidy.service').stdout
+      expect(files.split("\n").count).to eq(3)
+    end
+  end
+end

spec/classes/puppet_metrics_collector_system_spec.rb

@@ -1,6 +1,41 @@
 require 'spec_helper'
 
 describe 'puppet_metrics_collector::system' do
+  context 'with default parameters' do
+    it { is_expected.not_to contain_package('sysstat') }
+    it { is_expected.not_to contain_package('open-vm-tools') }
+  end
+
+  context 'with sysstat' do
+    context 'already installed' do
+      let(:pre_condition) { 'package{"sysstat": }' }
+      let(:facts) { { puppet_metrics_collector: { have_sysstat: true, have_systemd: true } } }
+
+      it { is_expected.not_to contain_notify('sysstat_missing_warning') }
+      it { is_expected.to contain_class('puppet_metrics_collector::system::cpu') }
+      it { is_expected.to contain_class('puppet_metrics_collector::system::memory') }
+      it { is_expected.to contain_class('puppet_metrics_collector::system::processes') }
+    end
+
+    context 'not installed and managed' do
+      let(:params) { { manage_sysstat: true } }
+      let(:facts) { { puppet_metrics_collector: { have_sysstat: false, have_systemd: true } } }
+
+      it { is_expected.not_to contain_notify('sysstat_missing_warning') }
+      it { is_expected.to contain_package('sysstat') }
+      it { is_expected.to contain_class('puppet_metrics_collector::system::cpu') }
+      it { is_expected.to contain_class('puppet_metrics_collector::system::memory') }
+      it { is_expected.to contain_class('puppet_metrics_collector::system::processes') }
+    end
+
+    context 'not installed and not managed' do
+      it { is_expected.to contain_notify('sysstat_missing_warning') }
+      it { is_expected.not_to contain_package('sysstat') }
+      it { is_expected.not_to contain_class('puppet_metrics_collector::system::cpu') }
+      it { is_expected.not_to contain_class('puppet_metrics_collector::system::memory') }
+      it { is_expected.not_to contain_class('puppet_metrics_collector::system::processes') }
+    end
+  end
   context 'when the virtual fact does not report vmware' do
     let(:facts) { { virtual: 'physical' } }