openstack/puppet-nova
Code Issues Proposed changes

Put all the logging related parameters to the logging class

Browse Source 
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: I2e81af8e1739d05cdfb061f22f1100dae699f3d8
This commit is contained in:
Yanis Guenane
2015-09-25 15:39:04 +02:00
parent 0d57b76bf6
commit 872b17744f
4 changed files with 103 additions and 94 deletions
Download Patch File Download Diff File Expand all files Collapse all files

53
manifests/init.pp
View File

@@ -157,7 +157,7 @@
# [*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/nova'
# Defaults to undef
#
# [*state_path*]
# (optional) Directory for storing state.
@@ -170,11 +170,11 @@
#
# [*verbose*]
# (optional) Set log output to verbose output.
# Defaults to false
# Defaults to undef
#
# [*debug*]
# (optional) Set log output to debug output.
# Defaults to false
# Defaults to undef
#
# [*periodic_interval*]
# (optional) Seconds between running periodic tasks.
@@ -190,15 +190,15 @@
#
# [*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
#
# [*install_utilities*]
# (optional) Install nova utilities (Extra packages used by nova tools)
@@ -306,11 +306,11 @@ class nova(
$qpid_tcp_nodelay = true,
$auth_strategy = 'keystone',
$service_down_time = 60,
$log_dir = '/var/log/nova',
$log_dir = undef,
$state_path = '/var/lib/nova',
$lock_path = $::nova::params::lock_path,
$verbose = false,
$debug = false,
$verbose = undef,
$debug = undef,
$periodic_interval = '60',
$report_interval = '10',
$rootwrap_config = '/etc/nova/rootwrap.conf',
@@ -321,9 +321,9 @@ class nova(
$key_file = false,
$nova_public_key = undef,
$nova_private_key = undef,
$use_syslog = false,
$use_stderr = true,
$log_facility = 'LOG_USER',
$use_syslog = undef,
$use_stderr = undef,
$log_facility = undef,
$install_utilities = true,
$notification_driver = undef,
$notification_topics = 'notifications',
@@ -336,6 +336,7 @@ class nova(
# maintain backward compatibility
include ::nova::db
include ::nova::logging
if $mysql_module {
warning('The mysql_module parameter is deprecated. The latest 2.x mysql module will be used.')
@@ -599,19 +600,6 @@ class nova(
}
}
if $log_dir {
file { $log_dir:
ensure => directory,
mode => '0750',
owner => 'nova',
group => $::nova::params::nova_log_group,
require => Package['nova-common'],
}
nova_config { 'DEFAULT/log_dir': value => $log_dir;}
} else {
nova_config { 'DEFAULT/log_dir': ensure => absent;}
}
if $notification_driver {
nova_config {
'DEFAULT/notification_driver': value => join(any2array($notification_driver), ',');
@@ -621,9 +609,6 @@ class nova(
}
nova_config {
'DEFAULT/verbose': value => $verbose;
'DEFAULT/debug': value => $debug;
'DEFAULT/use_stderr': value => $use_stderr;
'DEFAULT/rpc_backend': value => $rpc_backend;
'DEFAULT/notification_topics': value => $notification_topics;
'DEFAULT/notify_api_faults': value => $notify_api_faults;
@@ -643,18 +628,6 @@ class nova(
nova_config { 'DEFAULT/notify_on_state_change': ensure => absent; }
}
# Syslog configuration
if $use_syslog {
nova_config {
'DEFAULT/use_syslog': value => true;
'DEFAULT/syslog_log_facility': value => $log_facility;
}
} else {
nova_config {
'DEFAULT/use_syslog': value => false;
}
}
if $os_region_name {
nova_config {
'cinder/os_region_name': value => $os_region_name;

51
manifests/logging.pp
View File

@@ -1,9 +1,34 @@
# Class nova::logging
#
# nova extended logging configuration
# nova 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/nova'
#
# [*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 nova::logging(
$use_syslog = false,
$use_stderr = true,
$log_facility = 'LOG_USER',
$log_dir = '/var/log/nova',
$verbose = false,
$debug = false,
$logging_context_format_string = undef,
$logging_default_format_string = undef,
$logging_debug_format_suffix = undef,
@@ -79,6 +110,24 @@ class nova::logging(
$log_date_format = undef,
) {
# NOTE(spredzy): In order to keep backward compatibility we rely on the pick function
# to use nova::<myparam> first then nova::logging::<myparam>.
$use_syslog_real = pick($::nova::use_syslog,$use_syslog)
$use_stderr_real = pick($::nova::use_stderr,$use_stderr)
$log_facility_real = pick($::nova::log_facility,$log_facility)
$log_dir_real = pick($::nova::log_dir,$log_dir)
$verbose_real = pick($::nova::verbose,$verbose)
$debug_real = pick($::nova::debug,$debug)
nova_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 {
nova_config {
'DEFAULT/logging_context_format_string' :

57
spec/classes/nova_init_spec.rb
View File

@@ -6,6 +6,10 @@ describe 'nova' do
context 'with default parameters' do
it 'contains the logging class' do
is_expected.to contain_class('nova::logging')
end
it 'installs packages' do
is_expected.to contain_package('python-greenlet').with(
:ensure => 'present',
@@ -23,12 +27,6 @@ describe 'nova' do
end
it 'creates various files and folders' do
is_expected.to contain_file('/var/log/nova').with(
:ensure => 'directory',
:mode => '0750',
:owner => 'nova',
:require => 'Package[nova-common]'
)
is_expected.to contain_file('/etc/nova/nova.conf').with(
:mode => '0640',
:owner => 'nova',
@@ -68,10 +66,6 @@ describe 'nova' do
end
it 'configures various things' do
is_expected.to contain_nova_config('DEFAULT/verbose').with_value(false)
is_expected.to contain_nova_config('DEFAULT/debug').with_value(false)
is_expected.to contain_nova_config('DEFAULT/use_stderr').with_value(true)
is_expected.to contain_nova_config('DEFAULT/log_dir').with_value('/var/log/nova')
is_expected.to contain_nova_config('DEFAULT/state_path').with_value('/var/lib/nova')
is_expected.to contain_nova_config('DEFAULT/lock_path').with_value(platform_params[:lock_path])
is_expected.to contain_nova_config('DEFAULT/service_down_time').with_value('60')
@@ -86,9 +80,6 @@ describe 'nova' do
is_expected.to contain_class('nova::utilities')
end
it 'disables syslog' do
is_expected.to contain_nova_config('DEFAULT/use_syslog').with_value(false)
end
end
context 'with overridden parameters' do
@@ -150,9 +141,6 @@ describe 'nova' do
end
it 'configures various things' do
is_expected.to contain_nova_config('DEFAULT/verbose').with_value(true)
is_expected.to contain_nova_config('DEFAULT/debug').with_value(true)
is_expected.to contain_nova_config('DEFAULT/log_dir').with_value('/var/log/nova2')
is_expected.to contain_nova_config('DEFAULT/state_path').with_value('/var/lib/nova2')
is_expected.to contain_nova_config('DEFAULT/lock_path').with_value('/var/locky/path')
is_expected.to contain_nova_config('DEFAULT/service_down_time').with_value('120')
@@ -176,11 +164,6 @@ describe 'nova' do
is_expected.to_not contain_class('nova::utilities')
end
context 'with logging directory disabled' do
before { params.merge!( :log_dir => false) }
it { is_expected.to contain_nova_config('DEFAULT/log_dir').with_ensure('absent') }
end
end
context 'with wrong notify_on_state_change parameter' do
@@ -203,29 +186,6 @@ describe 'nova' do
end
end
context 'with syslog enabled' do
let :params do
{ :use_syslog => 'true' }
end
it 'configures syslog' do
is_expected.to contain_nova_config('DEFAULT/use_syslog').with_value(true)
is_expected.to contain_nova_config('DEFAULT/syslog_log_facility').with_value('LOG_USER')
end
end
context 'with syslog enabled and log_facility parameter' do
let :params do
{ :use_syslog => 'true',
:log_facility => 'LOG_LOCAL0' }
end
it 'configures syslog' do
is_expected.to contain_nova_config('DEFAULT/use_syslog').with_value(true)
is_expected.to contain_nova_config('DEFAULT/syslog_log_facility').with_value('LOG_LOCAL0')
end
end
context 'with rabbit_hosts parameter' do
let :params do
{ :rabbit_hosts => ['rabbit:5673', 'rabbit2:5674'] }
@@ -603,9 +563,6 @@ describe 'nova' do
end
it_behaves_like 'nova'
it 'creates the log folder with the right group for Debian' do
is_expected.to contain_file('/var/log/nova').with(:group => 'nova')
end
end
context 'on Ubuntu platforms' do
@@ -620,9 +577,6 @@ describe 'nova' do
end
it_behaves_like 'nova'
it 'creates the log folder with the right group for Ubuntu' do
is_expected.to contain_file('/var/log/nova').with(:group => 'adm')
end
end
context 'on RedHat platforms' do
@@ -637,8 +591,5 @@ describe 'nova' do
it_behaves_like 'nova'
it 'creates the log folder with the right group for RedHat' do
is_expected.to contain_file('/var/log/nova').with(:group => 'nova')
end
end
end

36
spec/classes/nova_logging_spec.rb
View File

@@ -24,11 +24,26 @@ describe 'nova::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 'nova-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 'nova::logging' do
end
shared_examples 'basic default logging settings' do
it 'configures nova logging settins with default values' do
is_expected.to contain_nova_config('DEFAULT/use_syslog').with(:value => 'false')
is_expected.to contain_nova_config('DEFAULT/use_stderr').with(:value => 'true')
is_expected.to contain_nova_config('DEFAULT/log_dir').with(:value => '/var/log/nova')
is_expected.to contain_nova_config('DEFAULT/verbose').with(:value => 'false')
is_expected.to contain_nova_config('DEFAULT/debug').with(:value => 'false')
end
end
shared_examples 'basic non-default logging settings' do
it 'configures nova logging settins with non-default values' do
is_expected.to contain_nova_config('DEFAULT/use_syslog').with(:value => 'true')
is_expected.to contain_nova_config('DEFAULT/use_stderr').with(:value => 'false')
is_expected.to contain_nova_config('DEFAULT/syslog_log_facility').with(:value => 'LOG_FOO')
is_expected.to contain_nova_config('DEFAULT/log_dir').with(:value => '/var/log')
is_expected.to contain_nova_config('DEFAULT/verbose').with(:value => 'true')
is_expected.to contain_nova_config('DEFAULT/debug').with(:value => 'true')
end
end
shared_examples_for 'logging params set' do
it 'enables logging params' do
is_expected.to contain_nova_config('DEFAULT/logging_context_format_string').with_value(