Merge pull request #248 from zanloy/issue197

A (better) fix for Issue #197
sensu · Sep 23, 2014 · bbaa233 · bbaa233
2 parents f79fae6 + 568fde8
commit bbaa233
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 8 deletions.
diff --git a/manifests/init.pp b/manifests/init.pp
@@ -61,6 +61,11 @@
 #   Default: true
 #   Valid values: true, false
 #
+# [*manage_plugins_dir*]
+#   Boolean. Manage the sensu plusing directory
+#   Default: true
+#   Valid values: true, false
+#
 # [*rabbitmq_port*]
 #   Integer.  Rabbitmq port to be used by sensu
 #   Default: 5672
@@ -143,6 +148,10 @@
 #   String, Array of Strings.  Plugins to install on the node
 #   Default: []
 #
+# [*plugins_dir*]
+#   String. Puppet url to plugins directory
+#   Default: undef
+#
 # [*purge_config*]
 #   Boolean.  If unused configs should be removed from the system
 #   Default: false
@@ -179,6 +188,7 @@
   $api                      = false,
   $manage_services          = true,
   $manage_user              = true,
+  $manage_plugins_dir       = true,
   $rabbitmq_port            = 5672,
   $rabbitmq_host            = 'localhost',
   $rabbitmq_user            = 'sensu',
@@ -200,6 +210,7 @@
   $client_custom            = {},
   $safe_mode                = false,
   $plugins                  = [],
+  $plugins_dir              = undef,
   $purge_config             = false,
   $use_embedded_ruby        = false,
   $rubyopt                  = '',
@@ -239,6 +250,13 @@
     $check_notify = []
   }
 
+  # Because you can't reassign a variable in puppet and we need to set to
+  # false if you specify a directory, we have to use another variable.
+  if $plugins_dir {
+    $_manage_plugins_dir = false
+  } else {
+    $_manage_plugins_dir = $manage_plugins_dir
+  }
 
   # Include everything and let each module determine its state.  This allows
   # transitioning to purged config and stopping/disabling services
@@ -253,6 +271,10 @@
   class { 'sensu::server::service': } ->
   anchor {'sensu::end': }
 
-  sensu::plugin { $plugins: install_path => '/etc/sensu/plugins' }
+  if $plugins_dir {
+    sensu::plugin { $plugins_dir: type => 'directory' }
+  } else {
+    sensu::plugin { $plugins: install_path => '/etc/sensu/plugins' }
+  }
 
 }
diff --git a/manifests/package.pp b/manifests/package.pp
@@ -61,14 +61,24 @@
     require => Package['sensu'],
   }
 
-  file { ['/etc/sensu/plugins', '/etc/sensu/handlers', '/etc/sensu/extensions', '/etc/sensu/mutators', '/etc/sensu/extensions/handlers']:
+  file { ['/etc/sensu/handlers', '/etc/sensu/extensions', '/etc/sensu/mutators', '/etc/sensu/extensions/handlers']:
     ensure  => directory,
     mode    => '0555',
     owner   => 'sensu',
     group   => 'sensu',
     require => Package['sensu'],
   }
 
+  if $sensu::_manage_plugins_dir {
+    file { '/etc/sensu/plugins':
+      ensure  => directory,
+      mode    => '0555',
+      owner   => 'sensu',
+      group   => 'sensu',
+      require => Package['sensu'],
+    }
+  }
+
   if $sensu::manage_user {
     user { 'sensu':
       ensure  => 'present',

diff --git a/manifests/plugin.pp b/manifests/plugin.pp
@@ -50,15 +50,20 @@
     'file':       {
       $filename = inline_template('<%= scope.lookupvar(\'name\').split(\'/\').last %>')
 
+      define_plugins_dir { $install_path: }
+
       file { "${install_path}/${filename}":
         ensure  => file,
         mode    => '0555',
         source  => $name,
+        require => File[$install_path],
       }
     }
     'url' : {
         $filename = inline_template('<%= scope.lookupvar(\'name\').split(\'/\').last %>')
 
+        define_plugins_dir { $install_path: }
+
         wget::fetch { $name :
           destination => "${install_path}/${filename}",
           timeout     => 0,
@@ -68,11 +73,11 @@
         file { "${install_path}/${filename}":
           ensure  => file,
           mode    => '0555',
+          require => File[$install_path],
         }
     }
     'directory':  {
-      file { 'sensu_plugins_dir':
-        path    => $install_path,
+      file { $install_path:
         ensure  => directory,
         mode    => '0555',
         source  => $name,
@@ -92,3 +97,15 @@
 
   }
 }
+# This is to verify the install_dir exists without duplicate declarations
+define define_plugins_dir ($path = $name) {
+  if ! defined(File[$path]) {
+    file { $path:
+      ensure => directory,
+      mode => '0555',
+      owner => 'sensu',
+      group => 'sensu',
+      require => Package['sensu'],
+    }
+  }
+}
diff --git a/spec/defines/sensu_plugin_spec.rb b/spec/defines/sensu_plugin_spec.rb
@@ -61,8 +61,7 @@
     context 'defaults' do
       let(:params) { { :type => 'directory' } }
 
-      it { should contain_file('sensu_plugins_dir').with(
-        'path'    => '/etc/sensu/plugins',
+      it { should contain_file('/etc/sensu/plugins').with(
         'source'  => 'puppet:///data/sensu/plugins',
         'ensure'  => 'directory',
         'recurse' => 'true',
@@ -74,13 +73,13 @@
     context 'set install_path' do
       let(:params) { { :type => 'directory', :install_path => '/opt/sensu/plugins' } }
 
-      it { should contain_file('sensu_plugins_dir').with('path' => '/opt/sensu/plugins') }
+      it { should contain_file('/opt/sensu/plugins') }
     end
 
     context 'set purge params' do
       let(:params) { { :type => 'directory', :recurse => false, :force => false, :purge => false } }
 
-      it { should contain_file('sensu_plugins_dir').with(
+      it { should contain_file('/etc/sensu/plugins').with(
         'recurse' => false,
         'purge'   => false,
         'force'   => false