Skip to content

Commit d41fcd7

Browse files
author
Jarret Lavallee
committed
(GH-108) Only manage system collection when sysstat is installed
Prior to this commit, the default was to manage the sysstat package by default. This commit changes the default to not manage the sysstat package. It includes logic to only include the system metrics collection classes when the sysstat package is installed and adds a notify when the packge is not installed.
1 parent 03df8ee commit d41fcd7

File tree

5 files changed

+175
-34
lines changed

5 files changed

+175
-34
lines changed

‎lib/facter/puppet_metrics_collector.rb‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,14 @@
1717
end
1818
end
1919

20+
chunk(:have_sysstat) do
21+
if Facter::Core::Execution.which('sar')
22+
{ have_sysstat: true }
23+
else
24+
{ have_sysstat: false }
25+
end
26+
end
27+
2028
chunk(:pe_psql) do
2129
if File.executable?('/opt/puppetlabs/server/bin/psql')
2230
{ have_pe_psql: true }

‎manifests/system.pp‎

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
Integer $collection_frequency = 5, # minutes
66
Integer $retention_days = 90,
77
Integer $polling_frequency_seconds = 1,
8-
Boolean $manage_sysstat = true,
8+
Boolean $manage_sysstat = false,
99
Boolean $manage_vmware_tools = false,
1010
String $vmware_tools_pkg = 'open-vm-tools',
1111
) {
@@ -32,27 +32,34 @@
3232
}
3333
}
3434

35-
file { "${scripts_dir}/system_metrics":
36-
ensure => file,
37-
mode => '0755',
38-
source => 'puppet:///modules/puppet_metrics_collector/system_metrics'
35+
exec { 'puppet_metrics_collector_system_daemon_reload':
36+
command => 'systemctl daemon-reload',
37+
path => ['/bin', '/usr/bin'],
38+
refreshonly => true,
3939
}
4040

4141
if $manage_sysstat {
4242
package { 'sysstat':
43-
ensure => installed,
43+
ensure => $system_metrics_ensure,
4444
}
4545
}
4646

47-
exec { 'puppet_metrics_collector_system_daemon_reload':
48-
command => 'systemctl daemon-reload',
49-
path => ['/bin', '/usr/bin'],
50-
refreshonly => true,
51-
}
47+
if $manage_sysstat or $facts.dig('puppet_metrics_collector', 'have_sysstat') {
48+
file { "${scripts_dir}/system_metrics":
49+
ensure => file,
50+
mode => '0755',
51+
source => 'puppet:///modules/puppet_metrics_collector/system_metrics'
52+
}
5253

53-
include puppet_metrics_collector::system::cpu
54-
include puppet_metrics_collector::system::memory
55-
include puppet_metrics_collector::system::processes
54+
contain puppet_metrics_collector::system::cpu
55+
contain puppet_metrics_collector::system::memory
56+
contain puppet_metrics_collector::system::processes
57+
} else {
58+
notify { 'sysstat_missing_warning':
59+
message => 'System collection disabled. Set `puppet_metrics_collector::system::manage_sysstat: true` to enable system metrics',
60+
loglevel => warning,
61+
}
62+
}
5663

5764
if $facts['virtual'] == 'vmware' {
5865
if $manage_vmware_tools and ($system_metrics_ensure == 'present') {

‎spec/acceptance/pe_metric_spec.rb‎

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,34 @@
11
require 'spec_helper_acceptance'
22

3-
describe 'test default and system includes' do
3+
describe 'default includes' do
44
before(:all) do
55
pp = <<-MANIFEST
66
include puppet_metrics_collector
7-
include puppet_metrics_collector::system
87
MANIFEST
98
idempotent_apply(pp)
109
end
11-
it 'all puppet_* metric services should be active or inactive' do
12-
run_shell('systemctl list-units --type=service | grep "puppet_.*metrics"') do |r|
13-
expect(r.stdout).to match(%r{activ})
14-
end
15-
end
10+
1611
context 'all of the timers are running' do
1712
it { expect(service('puppet_ace-metrics.timer')).to be_running }
1813
it { expect(service('puppet_ace-tidy.timer')).to be_running }
1914
it { expect(service('puppet_bolt-metrics.timer')).to be_running }
2015
it { expect(service('puppet_bolt-tidy.timer')).to be_running }
2116
it { expect(service('puppet_orchestrator-metrics.timer')).to be_running }
2217
it { expect(service('puppet_orchestrator-tidy.timer')).to be_running }
23-
it { expect(service('puppet_postgres-metrics.timer')).to be_running }
24-
it { expect(service('puppet_postgres-tidy.timer')).to be_running }
2518
it { expect(service('puppet_puppetdb-metrics.timer')).to be_running }
2619
it { expect(service('puppet_puppetdb-tidy.timer')).to be_running }
2720
it { expect(service('puppet_puppetserver-metrics.timer')).to be_running }
2821
it { expect(service('puppet_puppetserver-tidy.timer')).to be_running }
29-
it { expect(service('puppet_system_cpu-metrics.timer')).to be_running }
30-
it { expect(service('puppet_system_cpu-tidy.timer')).to be_running }
31-
it { expect(service('puppet_system_memory-metrics.timer')).to be_running }
32-
it { expect(service('puppet_system_memory-tidy.timer')).to be_running }
33-
it { expect(service('puppet_system_processes-metrics.timer')).to be_running }
34-
it { expect(service('puppet_system_processes-tidy.timer')).to be_running }
3522
end
23+
3624
it 'creates tidy services files' do
3725
files = run_shell('ls /etc/systemd/system/puppet_*-tidy.service').stdout
38-
expect(files.split("\n").count).to eq(9)
26+
expect(files.split("\n").count).to eq(5)
3927
end
28+
4029
it 'creates the timer files' do
4130
files = run_shell('ls /etc/systemd/system/puppet_*-tidy.timer').stdout
42-
expect(files.split("\n").count).to eq(9)
43-
end
44-
it 'sysstat package is installed' do
45-
expect(package('sysstat')).to be_installed
31+
expect(files.split("\n").count).to eq(5)
4632
end
4733

4834
describe file('/opt/puppetlabs/puppet-metrics-collector/puppetserver/127.0.0.1') do

‎spec/acceptance/pe_system_spec.rb‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
require 'spec_helper_acceptance'
2+
3+
describe 'system class' do
4+
context 'sysstat not installed and not managed' do
5+
before(:all) do
6+
run_shell('puppet resource package sysstat ensure=absent')
7+
pp = <<-MANIFEST
8+
include puppet_metrics_collector::system
9+
MANIFEST
10+
# The notify makes this non idempotent
11+
expect(apply_manifest(pp).exit_code).not_to eq(1)
12+
expect(apply_manifest(pp).exit_code).not_to eq(1)
13+
end
14+
15+
context 'postgres timers are running' do
16+
it { expect(service('puppet_postgres-metrics.timer')).to be_running }
17+
it { expect(service('puppet_postgres-tidy.timer')).to be_running }
18+
end
19+
20+
it 'creates tidy service file for postgres' do
21+
files = run_shell('ls /etc/systemd/system/puppet_postgres-tidy.service').stdout
22+
expect(files.split("\n")).not_to be_empty
23+
end
24+
25+
it 'creates the service file for postgres' do
26+
files = run_shell('ls /etc/systemd/system/puppet_postgres-metrics.service').stdout
27+
expect(files.split("\n")).not_to be_empty
28+
end
29+
30+
it 'sysstat package is not installed by default' do
31+
expect(package('sysstat')).not_to be_installed
32+
end
33+
34+
it 'have_sysstat is false without the package installed' do
35+
expect(host_inventory['facter']['puppet_metrics_collector']['have_sysstat']).to eq false
36+
end
37+
end
38+
39+
context 'sysstat installed and not managed' do
40+
before(:all) do
41+
run_shell('puppet resource package sysstat ensure=installed')
42+
pp = <<-MANIFEST
43+
include puppet_metrics_collector::system
44+
MANIFEST
45+
expect(apply_manifest(pp).exit_code).not_to eq(1)
46+
expect(apply_manifest(pp).exit_code).not_to eq(1)
47+
end
48+
49+
it 'system puppet_* metric services should be active or inactive' do
50+
run_shell('systemctl list-units --type=service | grep "puppet_system.*metrics"') do |r|
51+
expect(r.stdout).to match(%r{activ})
52+
end
53+
end
54+
55+
context 'system timers are running' do
56+
it { expect(service('puppet_system_cpu-metrics.timer')).to be_running }
57+
it { expect(service('puppet_system_cpu-tidy.timer')).to be_running }
58+
it { expect(service('puppet_system_memory-metrics.timer')).to be_running }
59+
it { expect(service('puppet_system_memory-tidy.timer')).to be_running }
60+
it { expect(service('puppet_system_processes-metrics.timer')).to be_running }
61+
it { expect(service('puppet_system_processes-tidy.timer')).to be_running }
62+
end
63+
64+
it 'creates system tidy services files' do
65+
files = run_shell('ls /etc/systemd/system/puppet_system*-tidy.service').stdout
66+
expect(files.split("\n").count).to eq(3)
67+
end
68+
end
69+
70+
context 'managing sysstat' do
71+
before(:all) do
72+
pp = <<-MANIFEST
73+
class { 'puppet_metrics_collector::system':
74+
manage_sysstat => true,
75+
}
76+
MANIFEST
77+
expect(apply_manifest(pp).exit_code).not_to eq(1)
78+
expect(apply_manifest(pp).exit_code).not_to eq(1)
79+
end
80+
81+
it 'sysstat package is installed' do
82+
expect(package('sysstat')).to be_installed
83+
end
84+
85+
it 'system puppet_* metric services should be active or inactive' do
86+
run_shell('systemctl list-units --type=service | grep "puppet_system.*metrics"') do |r|
87+
expect(r.stdout).to match(%r{activ})
88+
end
89+
end
90+
91+
context 'system timers are running' do
92+
it { expect(service('puppet_system_cpu-metrics.timer')).to be_running }
93+
it { expect(service('puppet_system_cpu-tidy.timer')).to be_running }
94+
it { expect(service('puppet_system_memory-metrics.timer')).to be_running }
95+
it { expect(service('puppet_system_memory-tidy.timer')).to be_running }
96+
it { expect(service('puppet_system_processes-metrics.timer')).to be_running }
97+
it { expect(service('puppet_system_processes-tidy.timer')).to be_running }
98+
end
99+
100+
it 'creates system tidy services files' do
101+
files = run_shell('ls /etc/systemd/system/puppet_system*-tidy.service').stdout
102+
expect(files.split("\n").count).to eq(3)
103+
end
104+
end
105+
end

‎spec/classes/puppet_metrics_collector_system_spec.rb‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,41 @@
11
require 'spec_helper'
22

33
describe 'puppet_metrics_collector::system' do
4+
context 'with default parameters' do
5+
it { is_expected.not_to contain_package('sysstat') }
6+
it { is_expected.not_to contain_package('open-vm-tools') }
7+
end
8+
9+
context 'with sysstat' do
10+
context 'already installed' do
11+
let(:pre_condition) { 'package{"sysstat": }' }
12+
let(:facts) { { puppet_metrics_collector: { have_sysstat: true, have_systemd: true } } }
13+
14+
it { is_expected.not_to contain_notify('sysstat_missing_warning') }
15+
it { is_expected.to contain_class('puppet_metrics_collector::system::cpu') }
16+
it { is_expected.to contain_class('puppet_metrics_collector::system::memory') }
17+
it { is_expected.to contain_class('puppet_metrics_collector::system::processes') }
18+
end
19+
20+
context 'not installed and managed' do
21+
let(:params) { { manage_sysstat: true } }
22+
let(:facts) { { puppet_metrics_collector: { have_sysstat: false, have_systemd: true } } }
23+
24+
it { is_expected.not_to contain_notify('sysstat_missing_warning') }
25+
it { is_expected.to contain_package('sysstat') }
26+
it { is_expected.to contain_class('puppet_metrics_collector::system::cpu') }
27+
it { is_expected.to contain_class('puppet_metrics_collector::system::memory') }
28+
it { is_expected.to contain_class('puppet_metrics_collector::system::processes') }
29+
end
30+
31+
context 'not installed and not managed' do
32+
it { is_expected.to contain_notify('sysstat_missing_warning') }
33+
it { is_expected.not_to contain_package('sysstat') }
34+
it { is_expected.not_to contain_class('puppet_metrics_collector::system::cpu') }
35+
it { is_expected.not_to contain_class('puppet_metrics_collector::system::memory') }
36+
it { is_expected.not_to contain_class('puppet_metrics_collector::system::processes') }
37+
end
38+
end
439
context 'when the virtual fact does not report vmware' do
540
let(:facts) { { virtual: 'physical' } }
641

0 commit comments

Comments
 (0)