- Refacted log file locations to use log_path properly

- Added lot of additional validation
- Added additional spec test

lib/puppet/parser/functions/array2csv.rb

@@ -1,14 +1,12 @@
 #
 # Converts the array to a csv string
 #
-#
 # $array = [ 'string1', 'string2', 'string3' ]
 #
 # becomes:
 #
 # $string = "string1,string2,string3"
 #
-
 module Puppet::Parser::Functions
   newfunction(:array2csv, :type => :rvalue, :doc => <<-'EOS'
     Returns a sorted csv formatted string from an array in the form

lib/puppet/parser/functions/hash2csv.rb

@@ -1,7 +1,6 @@
 #
 # Converts the hash to a csv string
 #
-#
 # $hash = {
 #   HOME   => '/home/user',
 #   ENV1   => 'env1',
@@ -12,7 +11,6 @@
 #
 # $string = "HOME='/home/user',ENV1='env1',SECRET='secret'"
 #
-
 module Puppet::Parser::Functions
   newfunction(:hash2csv, :type => :rvalue, :doc => <<-'EOS'
     Returns a csv formatted string from an hash in the form

manifests/config.pp

@@ -31,7 +31,7 @@
       mode    => '0755',
       content => template("supervisord/init/${::osfamily}/init.erb")
     }
-    
+
     if $supervisord::init_defaults {
       file { $supervisord::init_defaults:
         ensure  => present,

manifests/eventlistener.pp

@@ -27,11 +27,11 @@
   $killasgroup             = undef,
   $user                    = undef,
   $redirect_stderr         = undef,
-  $stdout_logfile          = "${supervisord::log_path}/eventlistener_${name}.log",
+  $stdout_logfile          = "eventlistener_${name}.log",
   $stdout_logfile_maxbytes = undef,
   $stdout_logfile_backups  = undef,
   $stdout_events_enabled   = undef,
-  $stderr_logfile          = "${supervisord::log_path}/eventlistener_${name}.error",
+  $stderr_logfile          = "eventlistener_${name}.error",
   $stderr_logfile_maxbytes = undef,
   $stderr_logfile_backups  = undef,
   $stderr_events_enabled   = undef,
@@ -43,6 +43,7 @@
 
   include supervisord
 
+  # parameter validation
   if $env_var {
     $env_hash = hiera_hash($env_var)
     validate_hash($env_hash)

manifests/fcgi_program.pp

@@ -27,12 +27,12 @@
   $killasgroup             = undef,
   $user                    = undef,
   $redirect_stderr         = undef,
-  $stdout_logfile          = "${supervisord::log_path}/fcgi-program_${name}.log",
+  $stdout_logfile          = "fcgi-program_${name}.log",
   $stdout_logfile_maxbytes = undef,
   $stdout_logfile_backups  = undef,
   $stdout_capture_maxbytes = undef,
   $stdout_events_enabled   = undef,
-  $stderr_logfile          = "${supervisord::log_path}/fcgi-program_${name}.error",
+  $stderr_logfile          = "fcgi-program_${name}.error",
   $stderr_logfile_maxbytes = undef,
   $stderr_logfile_backups  = undef,
   $stderr_capture_maxbytes = undef,
@@ -45,11 +45,19 @@
 
   include supervisord
 
+  # parameter validation
+  validate_string($command)
+  validate_re($socket, ['^tcp:\/\/.*:\d+$', '^unix:\/\/\/'])
+  if $priority { validate_re($priority, '^\d+', "invalid priority value of: ${priority}") }
+  if $umask { validate_re($umask, '^0[0-7][0-7]$', "invalid umask: ${umask}.") }
+
   if $env_var {
     $env_hash = hiera_hash($env_var)
+    validate_hash($env_hash)
     $env_string = hash2csv($env_hash)
   }
   elsif $environment {
+    validate_hash($environment)
     $env_string = hash2csv($environment)
   }
 

manifests/group.pp

@@ -13,7 +13,10 @@
 
   include supervisord
 
+  # parameter validation
   validate_array($programs)
+  if $priority { validate_re($priority, '^\d+', "invalid priority value of: ${priority}") }
+
   $progstring = array2csv($programs)
   $conf = "${supervisord::config_include}/group_${name}.conf"
 

manifests/init.pp

@@ -33,8 +33,8 @@
   $unix_scoket_group    = $supervisord::params::unix_socket_group,
 
   $inet_server          = $supervisord::params::inet_server,
-  $inet_server_hostname = $supervisord::params::inet_hostname,
-  $inet_server_port     = $supervisord::params::inet_port,
+  $inet_server_hostname = $supervisord::params::inet_server_hostname,
+  $inet_server_port     = $supervisord::params::inet_server_port,
 
   $unix_auth            = false,
   $unix_username        = undef,
@@ -66,6 +66,16 @@
   validate_absolute_path($log_path)
   validate_absolute_path($run_path)
 
+  $log_levels = ['^critical$', '^error$', '^warn$', '^info$', '^debug$', '^trace$', '^blather$']
+  validate_re($log_level, $log_levels, "invalid log_level: ${log_level}")
+  validate_re($umask, '^0[0-7][0-7]$', "invalid umask: ${umask}.")
+  validate_re($unix_socket_mode, '^[0-7][0-7][0-7][0-7]$', "invalid unix_socket_mode: ${unix_socket_mode}")
+
+  if ! is_integer($logfile_backups) { fail("invalid logfile_backups: ${logfile_backups}.")}
+  if ! is_integer($minfds) { fail("invalid minfds: ${minfds}.")}
+  if ! is_integer($minprocs) { fail("invalid minprocs: ${minprocs}.")}
+  if ! is_integer($inet_server_port) { fail("invalid inet_server_port: ${inet_server_port}.")}
+
   if $env_var {
     validate_hash($env_var)
     $env_hash = hiera($env_var)

manifests/params.pp

@@ -10,9 +10,9 @@
   $executable           = '/usr/local/bin/supervisord'
 
   $run_path             = '/var/run'
-  $pid_file             = "${run_path}/supervisord.pid"
+  $pid_file             = 'supervisord.pid'
   $log_path             = '/var/log/supervisor'
-  $log_file             = "${log_path}/supervisord.log"
+  $log_file             = 'supervisord.log'
   $logfile_maxbytes     = '50MB'
   $logfile_backups      = '10'
   $log_level            = 'info'
@@ -25,7 +25,7 @@
   $setuptools_url       = 'https://bitbucket.org/pypa/setuptools/raw/bootstrap/ez_setup.py'
 
   $unix_socket          = true
-  $unix_socket_file     = "${run_path}/supervisor.sock"
+  $unix_socket_file     = 'supervisor.sock'
   $unix_socket_mode     = '0700'
   $unix_socket_owner    = 'nobody'
 

manifests/program.pp

@@ -24,12 +24,12 @@
   $killasgroup             = undef,
   $user                    = undef,
   $redirect_stderr         = undef,
-  $stdout_logfile          = "${supervisord::log_path}/program_${name}.log",
+  $stdout_logfile          = "program_${name}.log",
   $stdout_logfile_maxbytes = undef,
   $stdout_logfile_backups  = undef,
   $stdout_capture_maxbytes = undef,
   $stdout_events_enabled   = undef,
-  $stderr_logfile          = "${supervisord::log_path}/program_${name}.error",
+  $stderr_logfile          = "program_${name}.error",
   $stderr_logfile_maxbytes = undef,
   $stderr_logfile_backups  = undef,
   $stderr_capture_maxbytes = undef,
@@ -42,6 +42,40 @@
 
   include supervisord
 
+  # parameter validation
+  validate_string($command)
+  if $process_name { validate_string($process_name) }
+  if $numprocs { validate_re($numprocs, '^\d+', "invalid numprocs: ${numprocs}.")}
+  if $numprocs_start { validate_re($numprocs_start, '^\d+', "invalid numprocs_start: ${numprocs_start}.")}
+  if $priority { validate_re($priority, '^\d+', "invalid priority value of: ${priority}") }
+  if $autostart { validate_boolean($autostart) }
+  if $autorestart { validate_re($autorestart, ['true', 'false', 'unexpected'], "invalid autorestart: ${autorestart}") }
+  if $startsecs { validate_re($startsecs, '^\d+', "invalid startsecs: ${startsecs}.")}
+  if $startretries { validate_re($startretries, '^\d+', "invalid startretries: ${startretries}.")}
+  if $exitcodes { validate_string($exitcodes)}
+  if $stopsignal {
+    $stop_signals = ['TERM', 'HUP', 'INT', 'QUIT', 'KILL', 'USR1', 'USR2']
+    validate_re($stopsignal, $stop_signals, "invalid stopsignal: ${stopsignal}")
+  }
+  if $stopwaitsec { validate_re($stopwaitsec, '^\d+', "invalid stopwaitsec: ${stopwaitsec}.")}
+  if $stopasgroup { validate_boolean($stopasgroup) }
+  if $killasgroup { validate_boolean($killasgroup) }
+  if $user { validate_string($user) }
+  if $redirect_stderr { validate_boolean($redirect_stderr) }
+  validate_string($stdout_logfile)
+  if $stdout_logfile_maxbytes { validate_string($stdout_logfile_maxbytes) }
+  if $stdout_logfile_backups { validate_re($stdout_logfile_backups, '^\d+', "invalid stdout_logfile_backups: ${stdout_logfile_backups}.")}
+  if $stdout_capture_maxbytes { validate_string($stdout_capture_maxbytes) }
+  if $stdout_events_enabled { validate_string($stdout_events_enabled) }
+  validate_string($stderr_logfile)
+  if $stderr_logfile_maxbytes { validate_string($stderr_logfile_maxbytes) }
+  if $stderr_logfile_backups { validate_re($stderr_logfile_backups, '^\d+', "invalid stderr_logfile_backups: ${stderr_logfile_backups}.")}
+  if $stderr_capture_maxbytes { validate_string($stderr_capture_maxbytes) }
+  if $stderr_events_enabled { validate_string($stderr_events_enabled) }
+  if $directory { validate_absolute_path($directory) }
+  if $umask { validate_re($umask, '^0[0-7][0-7]$', "invalid umask: ${umask}.") }
+
+  # convert environment data into a csv
   if $env_var {
     $env_hash = hiera_hash($env_var)
     validate_hash($env_hash)

spec/classes/supervisord_spec.rb

@@ -9,7 +9,9 @@
   it { should contain_class('supervisord::install') }
   it { should contain_class('supervisord::config') }
   it { should contain_class('supervisord::service') }
-  it { should contain_concat__fragment('supervisord_main').with_content(/logfile/) }
+  it { should contain_class('supervisord::params') }
+  it { should contain_service('supervisord') }
+  it { should contain_package('supervisor') }
 
   describe '#install_pip' do
     context 'default' do
@@ -20,18 +22,20 @@
       let(:params) {{ :install_pip => true }}
       it { should contain_class('supervisord::pip') }
     end
+
+    context 'true and RedHat' do
+      let(:params) {{ :install_pip => true }}
+      let(:facts) {{ :osfamily => 'RedHat', :concat_basedir => concatdir }}
+      it { should contain_class('supervisord::pip') }
+      it { should contain_exec('pip_provider_name_fix') }
+    end
   end
 
   describe '#env_var' do
     context 'default' do
       it { should contain_class('supervisord').without_env_hash }
       it { should contain_class('supervisord').without_env_string }
     end
-    #context 'is specified' do
-    #  let(:params) {{ :env_var => 'foovars' }}
-    #  let(:hiera_data) {{ :foovars => { 'key1' => 'value1', 'key2' => 'value2' } }}
-    #  it { should contain_concat__fragment('supervisord_main').with_content(/environment=key1='value1',key2='value2'/) }
-    #end
   end
 
   describe '#environment' do
@@ -40,7 +44,8 @@
     end
     context 'is specified' do
       let(:params) {{ :environment => { 'key1' => 'value1', 'key2' => 'value2' } }}
-      it { should contain_concat__fragment('supervisord_main').with_content(/environment=key1='value1',key2='value2'/) }
+      it { should contain_concat__fragment('supervisord_main')\
+        .with_content(/environment=key1='value1',key2='value2'/) }
     end
   end
 
@@ -57,11 +62,13 @@
       context 'with Debian' do
         let(:facts) {{ :osfamily => 'Debian', :concat_basedir => concatdir }}
         it { should contain_file('/etc/init.d/supervisord') }
+        it { should contain_file('/etc/default/supervisor') }
       end
 
       context 'with RedHat' do
         let(:facts) {{ :osfamily => 'RedHat', :concat_basedir => concatdir }}
         it { should contain_file('/etc/init.d/supervisord') }
+        it { should contain_file('/etc/sysconfig/supervisord') }
       end
     end
   end
@@ -89,10 +96,49 @@
   describe '#run_path' do
     context 'default' do
       it { should_not contain_file('/var/run') }
+      it { should contain_concat__fragment('supervisord_main') \
+        .with_content(/pidfile=\/var\/run\/supervisord.pid$/) }
+    end
+    context 'is specified' do
+      let(:params) {{ :run_path => '/opt/supervisord/run' }}
+      it { should contain_file('/opt/supervisord/run') }
+      it { should contain_concat__fragment('supervisord_main') \
+        .with_content(/pidfile=\/opt\/supervisord\/run\/supervisord.pid$/) }
+    end
+  end
+
+  describe '#log_path' do
+    context 'default' do
+      it { should contain_file('/var/log/supervisor') }
+      it { should contain_concat__fragment('supervisord_main') \
+        .with_content(/logfile=\/var\/log\/supervisor\/supervisord.log$/) }
+    end
+    context 'is specified' do
+      let(:params) {{ :log_path => '/opt/supervisord/logs' }}
+      it { should contain_file('/opt/supervisord/logs')}
+      it { should contain_concat__fragment('supervisord_main') \
+        .with_content(/logfile=\/opt\/supervisord\/logs\/supervisord.log$/) }
     end
-    context 'custom setting' do
-      let(:params) {{ :run_path => '/var/run/supervisord'}}
-      it { should contain_file('/var/run/supervisord') }
+  end
+
+  describe '#config_include' do
+    context 'default' do
+      it { should contain_file('/etc/supervisor.d') }
+    end
+    context 'is specified' do
+      let(:params) {{ :config_include => '/opt/supervisord/conf.d' }}
+      it { should contain_file('/opt/supervisord/conf.d') }
+    end
+  end
+
+  describe '#config_file' do
+    context 'default' do
+      it { should contain_file('/etc/supervisord.conf') }
+    end
+    context 'is specified' do
+      let(:params) {{ :config_file => '/opt/supervisord/supervisor.conf' }}
+      it { should contain_file('/opt/supervisord/supervisor.conf') }
     end
   end
+
 end

spec/defines/eventlistener_spec.rb

@@ -4,8 +4,8 @@
   let(:title) {'foo'}
   let(:default_params) {{ 
     :command        => 'bar',
-    :stdout_logfile => '/var/log/supervisor/eventlistener_foo.log',
-    :stderr_logfile => '/var/log/supervisor/eventlistener_foo.error' 
+    :stdout_logfile => 'eventlistener_foo.log',
+    :stderr_logfile => 'eventlistener_foo.error'
   }}
   let(:params) { default_params }
   let(:facts) {{ :concat_basedir => '/var/lib/puppet/concat' }}

spec/defines/fcgi_program_spec.rb

@@ -5,8 +5,8 @@
   let(:default_params) {{ 
     :command        => 'bar',
     :socket         => 'tcp://localhost:1000',
-    :stdout_logfile => '/var/log/supervisor/fcgi-program_foo.log',
-    :stderr_logfile => '/var/log/supervisor/fcgi-program_foo.error',
+    :stdout_logfile => 'fcgi-program_foo.log',
+    :stderr_logfile => 'fcgi-program_foo.error',
     :user           => 'baz' 
   }}
   let(:params) { default_params }

spec/defines/program_spec.rb

@@ -4,8 +4,8 @@
   let(:title) {'foo'}
   let(:default_params) {{ 
     :command => 'bar',
-    :stdout_logfile  => '/var/log/supervisor/program_foo.log',
-    :stderr_logfile  => '/var/log/supervisor/program_foo.error',
+    :stdout_logfile  => 'program_foo.log',
+    :stderr_logfile  => 'program_foo.error',
     :user            => 'baz'
   }}
   let(:params) { default_params }

templates/conf/eventlistener.erb

@@ -54,7 +54,7 @@ user=<%= @user %>
 <% if @redirect_stderr -%>
 redirect_stderr=<%= @redirect_stderr %>
 <% end -%>
-stdout_logfile=<%= @stdout_logfile %>
+stdout_logfile=<%= scope.lookupvar('supervisord::log_path') %>/<%= @stdout_logfile %>
 <% if @stdout_logfile_maxbytes -%>
 stdout_logfile_maxbytes=<%= @stdout_logfile_maxbytes %>
 <% end -%>
@@ -64,7 +64,7 @@ stdout_logfile_backups=<%= @stdout_logfile_backups %>
 <% if @stdout_events_enabled -%>
 stdout_events_enabled=<%= @stdout_events_enabled %>
 <% end -%>
-stderr_logfile=<%= @stderr_logfile %>
+stderr_logfile=<%= scope.lookupvar('supervisord::log_path') %>/<%= @stderr_logfile %>
 <% if @stderr_logfile_maxbytes -%>
 stderr_logfile_maxbytes=<%= @stderr_logfile_maxbytes %>
 <% end -%>

templates/conf/fcgi_program.erb

@@ -53,7 +53,7 @@ user=<%= @user %>
 redirect_stderr=<%= @redirect_stderr %>
 <% end -%>
 <% if @stdout_logfile -%>
-stdout_logfile=<%= @stdout_logfile %>
+stdout_logfile=<%= scope.lookupvar('supervisord::log_path') %>/<%= @stdout_logfile %>
 <% end -%>
 <% if @stdout_logfile_maxbytes -%>
 stdout_logfile_maxbytes=<%= @stdout_logfile_maxbytes %>
@@ -68,7 +68,7 @@ stdout_capture_maxbytes=<%= @stdout_capture_maxbytes %>
 stdout_events_enabled=<%= @stdout_events_enabled %>
 <% end -%>
 <% if @stderr_logfile -%>
-stderr_logfile=<%= @stderr_logfile %>
+stderr_logfile=<%= scope.lookupvar('supervisord::log_path') %>/<%= @stderr_logfile %>
 <% end -%>
 <% if @stderr_logfile_maxbytes -%>
 stderr_logfile_maxbytes=<%= @stderr_logfile_maxbytes %>