openstack
/
puppet-ceilometer
Code Issues Proposed changes

Put all the logging related parameters to the logging class 

Currently logging configuration is splitted in two distinct classes, the init.pp and the
logging.pp classes.

This review aims to centralize all logging related parameters in a
single class, the logging.pp one.

The impacted parameters are :

  * use_syslog
  * use_stderr
  * log_facility
  * verbose
  * debug
  * log_dir

This change remains backward compatible with what is currently in place

Change-Id: I1132395295f542f4ba5459769a5b4b829a9cd7a7
This commit is contained in:
Yanis Guenane 2015-09-25 09:39:58 +02:00
parent 392da101bb
commit 577593222a
4 changed files with 100 additions and 78 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
manifests/init.pp
View File

@ -27,22 +27,22 @@
# [*package_ensure*]
# ensure state for package. Optional. Defaults to 'present'
# [*debug*]
# should the daemons log debug messages. Optional. Defaults to 'False'
# should the daemons log debug messages. Optional. Defaults to undef
# [*log_dir*]
# (optional) directory to which ceilometer logs are sent.
# If set to boolean false, it will not log to any directory.
# Defaults to '/var/log/ceilometer'
# Defaults to undef
# [*verbose*]
# should the daemons log verbose messages. Optional. Defaults to 'False'
# should the daemons log verbose messages. Optional. Defaults to undef
# [*use_syslog*]
# (optional) Use syslog for logging
# Defaults to false
# Defaults to undef
# [*use_stderr*]
# (optional) Use stderr for logging
# Defaults to true
# Defaults to undef
# [*log_facility*]
# (optional) Syslog facility to receive log lines.
# Defaults to 'LOG_USER'
# Defaults to undef
# [*rpc_backend*]
# (optional) what rpc/queuing service to use
# Defaults to 'rabbit'
@ -119,12 +119,12 @@ class ceilometer(
$metering_secret = false,
$notification_topics = ['notifications'],
$package_ensure = 'present',
$debug = false,
$log_dir = '/var/log/ceilometer',
$verbose = false,
$use_syslog = false,
$use_stderr = true,
$log_facility = 'LOG_USER',
$debug = undef,
$log_dir = undef,
$verbose = undef,
$use_syslog = undef,
$use_stderr = undef,
$log_facility = undef,
$rpc_backend = 'rabbit',
$rabbit_host = '127.0.0.1',
$rabbit_port = 5672,
@ -157,6 +157,7 @@ class ceilometer(
validate_string($metering_secret)
include ::ceilometer::logging
include ::ceilometer::params
if $kombu_ssl_ca_certs and !$rabbit_use_ssl {
@ -302,38 +303,12 @@ class ceilometer(
'DEFAULT/http_timeout' : value => $http_timeout;
'DEFAULT/rpc_backend' : value => $rpc_backend;
'publisher/metering_secret' : value => $metering_secret, secret => true;
'DEFAULT/debug' : value => $debug;
'DEFAULT/verbose' : value => $verbose;
'DEFAULT/use_stderr' : value => $use_stderr;
'DEFAULT/notification_topics' : value => join($notification_topics, ',');
'database/event_time_to_live' : value => $event_time_to_live;
'database/metering_time_to_live' : value => $metering_time_to_live;
'database/alarm_history_time_to_live' : value => $alarm_history_time_to_live;
}
# Log configuration
if $log_dir {
ceilometer_config {
'DEFAULT/log_dir' : value => $log_dir;
}
} else {
ceilometer_config {
'DEFAULT/log_dir' : ensure => absent;
}
}
# Syslog configuration
if $use_syslog {
ceilometer_config {
'DEFAULT/use_syslog': value => true;
'DEFAULT/syslog_log_facility': value => $log_facility;
}
} else {
ceilometer_config {
'DEFAULT/use_syslog': value => false;
}
}
if $memcached_servers {
validate_array($memcached_servers)
}

51
manifests/logging.pp
View File

@ -1,9 +1,34 @@
# Class ceilometer::logging
#
# ceilometer extended logging configuration
# ceilometer logging configuration
#
# == parameters
#
# [*verbose*]
# (Optional) Should the daemons log verbose messages
# Defaults to 'false'
#
# [*debug*]
# (Optional) Should the daemons log debug messages
# Defaults to 'false'
#
# [*use_syslog*]
# (Optional) Use syslog for logging.
# Defaults to 'false'
#
# [*use_stderr*]
# (optional) Use stderr for logging
# Defaults to 'true'
#
# [*log_facility*]
# (Optional) Syslog facility to receive log lines.
# Defaults to 'LOG_USER'
#
# [*log_dir*]
# (optional) Directory where logs should be stored.
# If set to boolean false, it will not log to any directory.
# Defaults to '/var/log/ceilometer'
#
# [*logging_context_format_string*]
# (optional) Format string to use for log messages with context.
# Defaults to undef.
@ -66,6 +91,12 @@
# Example: 'Y-%m-%d %H:%M:%S'
class ceilometer::logging(
$use_syslog = false,
$use_stderr = true,
$log_facility = 'LOG_USER',
$log_dir = '/var/log/ceilometer',
$verbose = false,
$debug = false,
$logging_context_format_string = undef,
$logging_default_format_string = undef,
$logging_debug_format_suffix = undef,
@ -79,6 +110,24 @@ class ceilometer::logging(
$log_date_format = undef,
) {
# NOTE(spredzy): In order to keep backward compatibility we rely on the pick function
# to use ceilometer::<myparam> first then ceilometer::logging::<myparam>.
$use_syslog_real = pick($::ceilometer::use_syslog,$use_syslog)
$use_stderr_real = pick($::ceilometer::use_stderr,$use_stderr)
$log_facility_real = pick($::ceilometer::log_facility,$log_facility)
$log_dir_real = pick($::ceilometer::log_dir,$log_dir)
$verbose_real = pick($::ceilometer::verbose,$verbose)
$debug_real = pick($::ceilometer::debug,$debug)
ceilometer_config {
'DEFAULT/debug' : value => $debug_real;
'DEFAULT/verbose' : value => $verbose_real;
'DEFAULT/use_stderr' : value => $use_stderr_real;
'DEFAULT/use_syslog' : value => $use_syslog_real;
'DEFAULT/log_dir' : value => $log_dir_real;
'DEFAULT/syslog_log_facility': value => $log_facility_real;
}
if $logging_context_format_string {
ceilometer_config {
'DEFAULT/logging_context_format_string' :

40
spec/classes/ceilometer_init_spec.rb
View File

@ -88,6 +88,7 @@ describe 'ceilometer' do
shared_examples_for 'a ceilometer base installation' do
it { is_expected.to contain_class('ceilometer::logging') }
it { is_expected.to contain_class('ceilometer::params') }
it 'configures ceilometer group' do
@ -143,49 +144,10 @@ describe 'ceilometer' do
it { expect { is_expected.to raise_error(Puppet::Error) } }
end
it 'configures debug and verbosity' do
is_expected.to contain_ceilometer_config('DEFAULT/debug').with_value( params[:debug] )
is_expected.to contain_ceilometer_config('DEFAULT/verbose').with_value( params[:verbose] )
end
it 'configures use_stderr option' do
is_expected.to contain_ceilometer_config('DEFAULT/use_stderr').with_value( params[:use_stderr] )
end
it 'configures logging directory by default' do
is_expected.to contain_ceilometer_config('DEFAULT/log_dir').with_value( params[:log_dir] )
end
context 'with logging directory disabled' do
before { params.merge!( :log_dir => false) }
it { is_expected.to contain_ceilometer_config('DEFAULT/log_dir').with_ensure('absent') }
end
it 'configures notification_topics' do
is_expected.to contain_ceilometer_config('DEFAULT/notification_topics').with_value('notifications')
end
it 'configures syslog to be disabled by default' do
is_expected.to contain_ceilometer_config('DEFAULT/use_syslog').with_value('false')
end
context 'with syslog enabled' do
before { params.merge!( :use_syslog => 'true' ) }
it { is_expected.to contain_ceilometer_config('DEFAULT/use_syslog').with_value('true') }
it { is_expected.to contain_ceilometer_config('DEFAULT/syslog_log_facility').with_value('LOG_USER') }
end
context 'with syslog enabled and custom settings' do
before { params.merge!(
:use_syslog => 'true',
:log_facility => 'LOG_LOCAL0'
) }
it { is_expected.to contain_ceilometer_config('DEFAULT/use_syslog').with_value('true') }
it { is_expected.to contain_ceilometer_config('DEFAULT/syslog_log_facility').with_value('LOG_LOCAL0') }
end
context 'with overriden notification_topics parameter' do
before { params.merge!( :notification_topics => ['notifications', 'custom']) }

36
spec/classes/ceilometer_logging_spec.rb
View File

@ -24,11 +24,26 @@ describe 'ceilometer::logging' do
:instance_format => '[instance: %(uuid)s] ',
:instance_uuid_format => '[instance: %(uuid)s] ',
:log_date_format => '%Y-%m-%d %H:%M:%S',
:use_syslog => true,
:use_stderr => false,
:log_facility => 'LOG_FOO',
:log_dir => '/var/log',
:verbose => true,
:debug => true,
}
end
shared_examples_for 'ceilometer-logging' do
context 'with basic logging options and default settings' do
it_configures 'basic default logging settings'
end
context 'with basic logging options and non-default settings' do
before { params.merge!( log_params ) }
it_configures 'basic non-default logging settings'
end
context 'with extended logging options' do
before { params.merge!( log_params ) }
it_configures 'logging params set'
@ -40,6 +55,27 @@ describe 'ceilometer::logging' do
end
shared_examples 'basic default logging settings' do
it 'configures ceilometer logging settins with default values' do
is_expected.to contain_ceilometer_config('DEFAULT/use_syslog').with(:value => 'false')
is_expected.to contain_ceilometer_config('DEFAULT/use_stderr').with(:value => 'true')
is_expected.to contain_ceilometer_config('DEFAULT/log_dir').with(:value => '/var/log/ceilometer')
is_expected.to contain_ceilometer_config('DEFAULT/verbose').with(:value => 'false')
is_expected.to contain_ceilometer_config('DEFAULT/debug').with(:value => 'false')
end
end
shared_examples 'basic non-default logging settings' do
it 'configures ceilometer logging settins with non-default values' do
is_expected.to contain_ceilometer_config('DEFAULT/use_syslog').with(:value => 'true')
is_expected.to contain_ceilometer_config('DEFAULT/use_stderr').with(:value => 'false')
is_expected.to contain_ceilometer_config('DEFAULT/syslog_log_facility').with(:value => 'LOG_FOO')
is_expected.to contain_ceilometer_config('DEFAULT/log_dir').with(:value => '/var/log')
is_expected.to contain_ceilometer_config('DEFAULT/verbose').with(:value => 'true')
is_expected.to contain_ceilometer_config('DEFAULT/debug').with(:value => 'true')
end
end
shared_examples_for 'logging params set' do
it 'enables logging params' do
is_expected.to contain_ceilometer_config('DEFAULT/logging_context_format_string').with_value(