Akosiaris has submitted this change and it was merged.
Change subject: Refactoring nrpe module (round 2/??)
......................................................................
Refactoring nrpe module (round 2/??)
/usr/local/lib/nagios/plugins directory for plugins shipped by module
Merge nrpe::packages and nrpe::service classes in nrpe class
nrpe class now accepts an allowed_hosts argument. It defaults to undef.
Cleaned-up Service['nagios-nrpe-server']
hasrestart defaults to true in puppet 2.7
hasstatus defaults to true in puppet 2.7 so pattern is not needed
restart not needed (init script works fine)
Updated rspec examples
Updated puppet tests
Removed spec fixtures that caused a problem with puppet doc(cyclic deps)
Added Rake code to create and purge above fixtures before each run
Updated nrpe_local.cfg.erb
nrpe::check now ships virtual resources which are realized by nrpe class
virtual monitor_service resources tagged nrpe are no longer realized by
nrpe class (none existed anyway)
Change-Id: I96bcccae226c0b308ec528a52259d4c1bf764afb
---
M manifests/base.pp
A modules/nrpe/.rspec
M modules/nrpe/Rakefile
R modules/nrpe/files/plugins/check_dpkg
M modules/nrpe/manifests/check.pp
M modules/nrpe/manifests/init.pp
M modules/nrpe/manifests/monitor_service.pp
D modules/nrpe/manifests/packages.pp
D modules/nrpe/manifests/service.pp
M modules/nrpe/spec/classes/nrpe_spec.rb
D modules/nrpe/spec/classes/packages_spec.rb
D modules/nrpe/spec/classes/service_spec.rb
M modules/nrpe/spec/defines/check_spec.rb
M modules/nrpe/spec/defines/monitor_service_spec.rb
D modules/nrpe/spec/fixtures/modules/nrpe/files
D modules/nrpe/spec/fixtures/modules/nrpe/manifests
D modules/nrpe/spec/fixtures/modules/nrpe/templates
M modules/nrpe/templates/nrpe_local.cfg.erb
M modules/nrpe/tests/check.pp
D modules/nrpe/tests/packages.pp
D modules/nrpe/tests/service.pp
21 files changed, 201 insertions(+), 164 deletions(-)
Approvals:
Akosiaris: Looks good to me, approved
jenkins-bot: Verified
diff --git a/manifests/base.pp b/manifests/base.pp
index 311f766..fa55bfe 100644
--- a/manifests/base.pp
+++ b/manifests/base.pp
@@ -447,7 +447,7 @@
sudo_user { [ "nagios", "icinga" ]: privileges => ['ALL =
NOPASSWD: /usr/local/bin/check-raid.py'] }
nrpe::monitor_service { "raid" : description => "RAID",
nrpe_command => "sudo /usr/local/bin/check-raid.py" }
nrpe::monitor_service { "disk_space" : description => "Disk
space", nrpe_command => "/usr/lib/nagios/plugins/check_disk -w 6% -c 3% -l -e"
}
- nrpe::monitor_service { "dpkg" : description => "DPKG",
nrpe_command => "/usr/lib/nagios/plugins/check_dpkg" }
+ nrpe::monitor_service { "dpkg" : description => "DPKG",
nrpe_command => "/usr/local/lib/nagios/plugins/check_dpkg" }
## this is only needed for the raid checks.
## should be able to move into sudo_user def above once puppet
is caught up
diff --git a/modules/nrpe/.rspec b/modules/nrpe/.rspec
new file mode 100644
index 0000000..f449dae
--- /dev/null
+++ b/modules/nrpe/.rspec
@@ -0,0 +1,2 @@
+--format doc
+--color
diff --git a/modules/nrpe/Rakefile b/modules/nrpe/Rakefile
index e6443dc..ed517ca 100644
--- a/modules/nrpe/Rakefile
+++ b/modules/nrpe/Rakefile
@@ -1,7 +1,35 @@
require 'rake'
+require 'fileutils'
require 'rspec/core/rake_task'
-RSpec::Core::RakeTask.new(:spec) do |t|
+symlinks = { 'spec/fixtures/modules/nrpe/files' => '../../../../files',
+ 'spec/fixtures/modules/nrpe/manifests' => '../../../../manifests',
+ 'spec/fixtures/modules/nrpe/templates' => '../../../../templates',
+}
+
+
+task :setup do
+ FileUtils.mkdir_p('spec/fixtures/modules/nrpe')
+ symlinks.each do |x|
+ if !File.exist?(x[0])
+ FileUtils.ln_s(x[1], x[0])
+ end
+ end
+end
+
+task :teardown do
+ symlinks.each { |x| FileUtils.rm(x[0], :force => true) }
+ FileUtils.rmdir('spec/fixtures/modules/nrpe')
+ FileUtils.rmdir('spec/fixtures/modules')
+end
+
+RSpec::Core::RakeTask.new(:realspec) do |t|
+ t.fail_on_error = false
t.pattern = 'spec/*/*_spec.rb'
end
+
+task :spec => [ :setup, :realspec, :teardown]
+
+task :default => :spec do
+end
diff --git a/modules/nrpe/files/check_dpkg
b/modules/nrpe/files/plugins/check_dpkg
similarity index 100%
rename from modules/nrpe/files/check_dpkg
rename to modules/nrpe/files/plugins/check_dpkg
old mode 100644
new mode 100755
diff --git a/modules/nrpe/manifests/check.pp b/modules/nrpe/manifests/check.pp
index 20cf66b..1324dc3 100644
--- a/modules/nrpe/manifests/check.pp
+++ b/modules/nrpe/manifests/check.pp
@@ -1,6 +1,9 @@
# Definition: nrpe::check
#
# Installs a single NRPE check in /etc/nagios/nrpe.d/
+# Please do note that this definition might be used on machines where the nrpe
+# class is not included. In that case it will be a no-op since the definition
+# will not be realized
#
# Parameters:
# $title (implicit parameter)
@@ -13,21 +16,23 @@
# Install a NRPE check definition in /etc/nagios/nrpe.d/
#
# Requires:
-# Class[nrpe::packages]
+# Class[nrpe] (optionally)
#
# Sample Usage:
# nrpe::check { 'check_myprocess':
# $command => '/usr/lib/nagios/plugins/check_procs -c 1:1 -C myprocess'
# }
-define nrpe::check($command) {
- Class[nrpe::packages] -> Nrpe::Check[$title]
-
- file { "/etc/nagios/nrpe.d/${title}.cfg":
- owner => root,
- group => root,
+define nrpe::check($command, $ensure='present') {
+ # If the nrpe class is not included, this entire definition will never be
+ # realized making it a no-op
+ @file { "/etc/nagios/nrpe.d/${title}.cfg":
+ ensure => $ensure,
+ owner => 'root',
+ group => 'root',
mode => '0444',
content => template('nrpe/check.erb'),
- notify => Service['nagios-nrpe-server']
+ notify => Service['nagios-nrpe-server'],
+ tag => 'nrpe::check',
}
}
diff --git a/modules/nrpe/manifests/init.pp b/modules/nrpe/manifests/init.pp
index 022eda6..901ced4 100644
--- a/modules/nrpe/manifests/init.pp
+++ b/modules/nrpe/manifests/init.pp
@@ -1,6 +1,6 @@
# Class: nrpe
#
-# This installes nrpe packages, ensures service in running and collects all
+# This installes nrpe packages, ensures service is running and collects all
# configuration
#
# Parameters:
@@ -11,17 +11,71 @@
# Collect all needed exported resources
#
# Requires:
-# Class[nrpe::packages]
-# Class[nrpe::service]
-# Define[monitor_service]
+# Definition[monitor_service]
#
# Sample Usage:
# include nrpe
-class nrpe {
- include nrpe::packages
- include nrpe::service
+class nrpe($allowed_hosts=undef) {
+ package { [ 'nagios-nrpe-server',
+ 'nagios-plugins',
+ 'nagios-plugins-basic',
+ 'nagios-plugins-extra',
+ 'nagios-plugins-standard',
+ ]:
+ ensure => present,
+ }
- #Collect virtual NRPE nagios service checks
- Monitor_service <| tag == 'nrpe' |>
+ if $allowed_hosts == undef {
+ $nrpe_allowed_hosts = $::realm ? {
+ 'production' =>
'127.0.0.1,208.80.152.185,208.80.152.161,208.80.154.14',
+ 'labs' => '10.4.0.120',
+ default => '127.0.0.1',
+ }
+ } else {
+ $nrpe_allowed_hosts = $allowed_hosts
+ }
+
+ file { '/etc/nagios/nrpe_local.cfg':
+ ensure => present,
+ owner => 'root',
+ group => 'root',
+ mode => '0444',
+ content => template('nrpe/nrpe_local.cfg.erb'),
+ require => Package['nagios-nrpe-server'],
+ notify => Service['nagios-nrpe-server'],
+ }
+
+ # TODO: Remove this after the file has been purged everywhere
+ file { '/usr/lib/nagios/plugins/check_dpkg':
+ ensure => absent,
+ }
+
+ file { '/usr/local/lib/nagios/':
+ ensure => directory,
+ owner => 'root',
+ group => 'root',
+ mode => '0444',
+ }
+
+ # Have a directory with all our plugins.
+ file { '/usr/local/lib/nagios/plugins/':
+ ensure => directory,
+ owner => 'root',
+ group => 'root',
+ mode => '0444',
+ recurse => true,
+ purge => true,
+ force => true,
+ source => 'puppet:///modules/nrpe/plugins',
+ require => File['/usr/local/lib/nagios/'],
+ }
+
+ service { 'nagios-nrpe-server':
+ ensure => running,
+ require => Package['nagios-nrpe-server'],
+ }
+
+ #Collect virtual nrpe checks
+ File <| tag == 'nrpe::check' |>
}
diff --git a/modules/nrpe/manifests/monitor_service.pp
b/modules/nrpe/manifests/monitor_service.pp
index e2655fb..40411e2 100644
--- a/modules/nrpe/manifests/monitor_service.pp
+++ b/modules/nrpe/manifests/monitor_service.pp
@@ -9,33 +9,31 @@
# $description
# Service check description
# $nrpe_command
-# If defined, installs this NRPE command as check_${title}
+# The path to the actual binary/script. A stanza for nrpe daemon will be
+# added with that path and a nagios_service check will be exported to
+# nagios server.
# $contact_group
-# Defaults to admins, the nagios contact groupo for the service
+# Defaults to admins, the nagios contact group for the service
# $retries
# Defaults to 3. The number of times a service will be retried before
# notifying
# $ensure
# Defaults to present
#
-define nrpe::monitor_service(
- $description,
- $nrpe_command = undef,
- $contact_group = 'admins',
- $retries = 3,
- $ensure = 'present') {
- if $nrpe_command != undef {
- nrpe::check { "check_${title}":
- command => $nrpe_command,
- before => ::Monitor_service[$title],
- }
- }
- else {
- # TODO: Figure out why this is here
- Nrpe::Check["check_${title}"] -> Nrpe::Monitor_service[$title]
+define nrpe::monitor_service( $description,
+ $nrpe_command,
+ $contact_group = 'admins',
+ $retries = 3,
+ $ensure = 'present') {
+
+ nrpe::check { "check_${title}":
+ command => $nrpe_command,
+ before => ::Monitor_service[$title],
}
- ::monitor_service{ $title:
+ # TODO: Refactor this to make a call to nagios::monitor_service (or
similar) after nagios
+ # has been refactored to a module. It is known to cause rspec tests to fail
+ ::monitor_service { $title:
ensure => $ensure,
description => $description,
check_command => "nrpe_check!check_${title}",
diff --git a/modules/nrpe/manifests/packages.pp
b/modules/nrpe/manifests/packages.pp
deleted file mode 100644
index 0b0b2b8..0000000
--- a/modules/nrpe/manifests/packages.pp
+++ /dev/null
@@ -1,50 +0,0 @@
-# Class: nrpe::packages
-#
-# Install and configure nrpe package
-#
-# Parameters:
-#
-# Actions:
-# Install rnpe package
-# Configure nrpe
-#
-# Requires:
-#
-# Sample Usage:
-# include nrpe::packages
-
-class nrpe::packages {
- # TODO: Parameterize this.
- $nrpe_allowed_hosts = $::realm ? {
- 'production' =>
'127.0.0.1,208.80.152.185,208.80.152.161,208.80.154.14',
- 'labs' => '10.4.0.120',
- default => '127.0.0.1',
- }
-
- package { [ 'nagios-nrpe-server',
- 'nagios-plugins',
- 'nagios-plugins-basic',
- 'nagios-plugins-extra',
- 'nagios-plugins-standard',
- 'libssl0.9.8' ]:
- ensure => present,
- }
-
- file { '/etc/nagios/nrpe_local.cfg':
- ensure => present,
- owner => root,
- group => root,
- mode => '0444',
- content => template('nrpe/nrpe_local.cfg.erb'),
- require => Package[nagios-nrpe-server],
- }
-
- # TODO: Move this somewhere else
- file { '/usr/lib/nagios/plugins/check_dpkg':
- ensure => present,
- owner => root,
- group => root,
- mode => '0555',
- source => 'puppet:///modules/nrpe/check_dpkg',
- }
-}
diff --git a/modules/nrpe/manifests/service.pp
b/modules/nrpe/manifests/service.pp
deleted file mode 100644
index a2f8c88..0000000
--- a/modules/nrpe/manifests/service.pp
+++ /dev/null
@@ -1,29 +0,0 @@
-# Class: nrpe::service
-#
-# Ensures service is running
-#
-# Parameters:
-#
-# Actions:
-# Ensure service is running
-#
-# Requires:
-# Class[nrpe::packages]
-#
-# Sample Usage:
-# include nrpe::service
-
-class nrpe::service {
- Class[nrpe::packages] -> Class[nrpe::service]
-
- # TODO: Clear the pattern, hasrestart, restart parameters because they are
- # ugly
- service { 'nagios-nrpe-server':
- ensure => running,
- pattern => '/usr/sbin/nrpe',
- hasrestart => true,
- restart => 'killall nrpe; sleep 2; /etc/init.d/nagios-nrpe-server
start',
- require => Package['nagios-nrpe-server'],
- subscribe => File['/etc/nagios/nrpe_local.cfg'],
- }
-}
diff --git a/modules/nrpe/spec/classes/nrpe_spec.rb
b/modules/nrpe/spec/classes/nrpe_spec.rb
index 469a495..0644f69 100644
--- a/modules/nrpe/spec/classes/nrpe_spec.rb
+++ b/modules/nrpe/spec/classes/nrpe_spec.rb
@@ -1,6 +1,36 @@
require 'spec_helper'
describe 'nrpe', :type => :class do
- it { should include_class('nrpe::packages') }
- it { should include_class('nrpe::service') }
+ it { should contain_package('nagios-nrpe-server') }
+ it { should contain_package('nagios-plugins') }
+ it { should contain_package('nagios-plugins-basic') }
+ it { should contain_package('nagios-plugins-extra') }
+ it { should contain_package('nagios-plugins-standard') }
+ it { should contain_file('/etc/nagios/nrpe_local.cfg') }
+ it { should contain_file('/usr/local/lib/nagios/plugins/') }
+ it { should contain_service('nagios-nrpe-server') }
+end
+
+describe 'nrpe', :type => :class do
+ let(:facts) { { :realm => 'production' } }
+
+ it 'should generate valid content for nrpe_local.cfg' do
+ should
contain_file('/etc/nagios/nrpe_local.cfg').with_content(/allowed_hosts=127.0.0.1/)
+ end
+end
+
+describe 'nrpe', :type => :class do
+ let(:facts) { { :realm => 'labs' } }
+ it 'should generate valid content for nrpe_local.cfg' do
+ should
contain_file('/etc/nagios/nrpe_local.cfg').with_content(/allowed_hosts=10.4.0.120/)
+ end
+end
+
+describe 'nrpe', :type => :class do
+ let(:facts) { { :realm => 'labs' } }
+ let(:params) { { :allowed_hosts => '10.10.10.10' } }
+
+ it 'should generate valid content for nrpe_local.cfg' do
+ should
contain_file('/etc/nagios/nrpe_local.cfg').with_content(/allowed_hosts=10.10.10.10/)
+ end
end
diff --git a/modules/nrpe/spec/classes/packages_spec.rb
b/modules/nrpe/spec/classes/packages_spec.rb
deleted file mode 100644
index 6c9fa02..0000000
--- a/modules/nrpe/spec/classes/packages_spec.rb
+++ /dev/null
@@ -1,13 +0,0 @@
-require 'spec_helper'
-
-describe 'nrpe::packages', :type => :class do
- let(:facts) { { :realm => 'production' } }
- it { should contain_package('nagios-nrpe-server') }
- it { should contain_package('nagios-plugins') }
- it { should contain_package('nagios-plugins-basic') }
- it { should contain_package('nagios-plugins-extra') }
- it { should contain_package('nagios-plugins-standard') }
- it { should contain_package('libssl0.9.8') }
- it { should contain_file('/etc/nagios/nrpe_local.cfg') }
- it { should contain_file('/usr/lib/nagios/plugins/check_dpkg') }
-end
diff --git a/modules/nrpe/spec/classes/service_spec.rb
b/modules/nrpe/spec/classes/service_spec.rb
deleted file mode 100644
index e56d804..0000000
--- a/modules/nrpe/spec/classes/service_spec.rb
+++ /dev/null
@@ -1,5 +0,0 @@
-require 'spec_helper'
-
-describe 'nrpe::service', :type => :class do
- it { should contain_service('nagios-nrpe-server') }
-end
diff --git a/modules/nrpe/spec/defines/check_spec.rb
b/modules/nrpe/spec/defines/check_spec.rb
index 6bcdee1..8c9c1dd 100644
--- a/modules/nrpe/spec/defines/check_spec.rb
+++ b/modules/nrpe/spec/defines/check_spec.rb
@@ -4,7 +4,23 @@
let(:title) { 'something' }
let(:params) { { :command => '/usr/local/bin/mycommand -i this -o that' } }
- it do
- should contain_file('/etc/nagios/nrpe.d/something.cfg')
+ context 'with nrpe class not defined' do
+ it 'should not create /etc/nagios/nrpe.d/something.cfg' do
+ should_not contain_file('/etc/nagios/nrpe.d/something.cfg')
+ end
end
+
+ context 'with nrpe class defined' do
+ let(:pre_condition) { "class { 'nrpe': }" }
+ it 'should create /etc/nagios/nrpe.d/something.cfg' do
+ should contain_file('/etc/nagios/nrpe.d/something.cfg')
+ end
+ end
+
+ context 'with ensure absent' do
+ it 'should not create /etc/nagios/nrpe.d/something.cfg' do
+ should_not contain_file('/etc/nagios/nrpe.d/something.cfg')
+ end
+ end
+
end
diff --git a/modules/nrpe/spec/defines/monitor_service_spec.rb
b/modules/nrpe/spec/defines/monitor_service_spec.rb
index 932cba2..12c7743 100644
--- a/modules/nrpe/spec/defines/monitor_service_spec.rb
+++ b/modules/nrpe/spec/defines/monitor_service_spec.rb
@@ -2,19 +2,26 @@
describe 'nrpe::monitor_service', :type => :define do
let(:title) { 'something' }
- let(:params) { {
- :description => 'this is a description',
- :contact_group => 'noone',
- } }
- #let(:params) { { :command => '/usr/local/bin/mycommand -i this -o that' }
}
+ let(:params) do {
+ :description => 'this is a description',
+ :contact_group => 'none',
+ :nrpe_command => '/usr/local/bin/mycommand -i this -o that',
+ }
+ end
it do
- should contain_monitor_service('something').with(
- :description => 'this is a description',
- :contact_group => 'noone',
- :retries => 3,
- :ensure => 'present',
- :check_command => 'nrpe!check_something'
- )
+ should contain_nrpe__check('check_something').with(
+ :command => '/usr/local/bin/mycommand -i this -o that',
+ :ensure => 'present'
+ )
+ end
+ it do
+ should contain_monitor_service('something').with(
+ :description => 'this is a description',
+ :contact_group => 'none',
+ :retries => 3,
+ :ensure => 'present',
+ :check_command => 'nrpe!check_something'
+ )
end
end
diff --git a/modules/nrpe/spec/fixtures/modules/nrpe/files
b/modules/nrpe/spec/fixtures/modules/nrpe/files
deleted file mode 120000
index 84dafe8..0000000
--- a/modules/nrpe/spec/fixtures/modules/nrpe/files
+++ /dev/null
@@ -1 +0,0 @@
-../../../../files
\ No newline at end of file
diff --git a/modules/nrpe/spec/fixtures/modules/nrpe/manifests
b/modules/nrpe/spec/fixtures/modules/nrpe/manifests
deleted file mode 120000
index 373b992..0000000
--- a/modules/nrpe/spec/fixtures/modules/nrpe/manifests
+++ /dev/null
@@ -1 +0,0 @@
-../../../../manifests
\ No newline at end of file
diff --git a/modules/nrpe/spec/fixtures/modules/nrpe/templates
b/modules/nrpe/spec/fixtures/modules/nrpe/templates
deleted file mode 120000
index f8a06d1..0000000
--- a/modules/nrpe/spec/fixtures/modules/nrpe/templates
+++ /dev/null
@@ -1 +0,0 @@
-../../../../templates
\ No newline at end of file
diff --git a/modules/nrpe/templates/nrpe_local.cfg.erb
b/modules/nrpe/templates/nrpe_local.cfg.erb
index bd11095..a97b96b 100644
--- a/modules/nrpe/templates/nrpe_local.cfg.erb
+++ b/modules/nrpe/templates/nrpe_local.cfg.erb
@@ -1,4 +1,11 @@
-allowed_hosts=<%= scope.lookupvar("nrpe::packages::nrpe_allowed_hosts") %>
+# File generated by puppet. DO NOT EDIT manually
+
+# We override allowed_hosts from nrpe.cfg here
+<% if @nrpe_allowed_hosts -%>
+allowed_hosts=<%= @nrpe_allowed_hosts %>
+<% else -%>
+allowed_hosts=127.0.0.1 # Created by fallback. Check your puppet calls are ok
+<% end -%>
command[check_disk_5_2]=/usr/lib/nagios/plugins/check_disk -w 5% -c 2% -l -e
command[check_disk_6_3]=/usr/lib/nagios/plugins/check_disk -w 6% -c 3% -l -e
diff --git a/modules/nrpe/tests/check.pp b/modules/nrpe/tests/check.pp
index 74517a1..c97453f 100644
--- a/modules/nrpe/tests/check.pp
+++ b/modules/nrpe/tests/check.pp
@@ -1,8 +1,5 @@
#
-include nrpe::packages
-include nrpe::service
-
nrpe::check { 'myproc':
command => 'mycommand',
}
diff --git a/modules/nrpe/tests/packages.pp b/modules/nrpe/tests/packages.pp
deleted file mode 100644
index 1f0930d..0000000
--- a/modules/nrpe/tests/packages.pp
+++ /dev/null
@@ -1,3 +0,0 @@
-#
-
-include nrpe::packages
diff --git a/modules/nrpe/tests/service.pp b/modules/nrpe/tests/service.pp
deleted file mode 100644
index 743ef24..0000000
--- a/modules/nrpe/tests/service.pp
+++ /dev/null
@@ -1,4 +0,0 @@
-#
-
-include nrpe::packages
-include nrpe::service
--
To view, visit https://gerrit.wikimedia.org/r/79329
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I96bcccae226c0b308ec528a52259d4c1bf764afb
Gerrit-PatchSet: 6
Gerrit-Project: operations/puppet
Gerrit-Branch: production
Gerrit-Owner: Akosiaris <[email protected]>
Gerrit-Reviewer: Akosiaris <[email protected]>
Gerrit-Reviewer: Andrew Bogott <[email protected]>
Gerrit-Reviewer: Faidon <[email protected]>
Gerrit-Reviewer: Lcarr <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
