From 872b17744f3d971bd5bf894eccb8bab41a552448 Mon Sep 17 00:00:00 2001 From: Yanis Guenane Date: Fri, 25 Sep 2015 15:39:04 +0200 Subject: [PATCH] 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: I2e81af8e1739d05cdfb061f22f1100dae699f3d8 --- manifests/init.pp | 53 +++++++--------------------- manifests/logging.pp | 51 ++++++++++++++++++++++++++- spec/classes/nova_init_spec.rb | 57 +++---------------------------- spec/classes/nova_logging_spec.rb | 36 +++++++++++++++++++ 4 files changed, 103 insertions(+), 94 deletions(-) diff --git a/manifests/init.pp b/manifests/init.pp index b0fd3f347..125a6d680 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -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; diff --git a/manifests/logging.pp b/manifests/logging.pp index ce4bd06c9..4145cdd6d 100644 --- a/manifests/logging.pp +++ b/manifests/logging.pp @@ -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:: first then nova::logging::. + $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' : diff --git a/spec/classes/nova_init_spec.rb b/spec/classes/nova_init_spec.rb index 3e1720413..a536c4a34 100644 --- a/spec/classes/nova_init_spec.rb +++ b/spec/classes/nova_init_spec.rb @@ -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 diff --git a/spec/classes/nova_logging_spec.rb b/spec/classes/nova_logging_spec.rb index a0b9f4790..6ee48b8af 100644 --- a/spec/classes/nova_logging_spec.rb +++ b/spec/classes/nova_logging_spec.rb @@ -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(