openstack
/
puppet-keystone
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
  * log_file

This change remains backward compatible with what is currently in place.

Change-Id: I2ad392f8f12be3f3a7d51916e0645531ed1184a2
This commit is contained in:
Yanis Guenane 2015-09-25 15:18:32 +02:00
parent 240301fe01
commit ee2723009e
4 changed files with 124 additions and 110 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

64
manifests/init.pp
View File

@ -31,23 +31,23 @@
#
# [*verbose*]
# (optional) Rather keystone should log at verbose level.
# Defaults to false.
# Defaults to undef.
#
# [*debug*]
# (optional) Rather keystone should log at debug level.
# Defaults to False.
# 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.
#
# [*catalog_type*]
# (optional) Type of catalog that keystone uses to store endpoints,services.
@ -249,11 +249,11 @@
# [*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/keystone'
# Defaults to undef.
#
# [*log_file*]
# (optional) Where to log
# Defaults to false
# Defaults to undef.
#
# [*public_endpoint*]
# (optional) The base public endpoint URL for keystone that are
@ -450,13 +450,13 @@ class keystone(
$admin_bind_host = '0.0.0.0',
$public_port = '5000',
$admin_port = '35357',
$verbose = false,
$debug = false,
$log_dir = '/var/log/keystone',
$log_file = false,
$use_syslog = false,
$use_stderr = true,
$log_facility = 'LOG_USER',
$verbose = undef,
$debug = undef,
$log_dir = undef,
$log_file = undef,
$use_syslog = undef,
$use_stderr = undef,
$log_facility = undef,
$catalog_type = 'sql',
$catalog_driver = false,
$catalog_template_file = '/etc/keystone/default_catalog.templates',
@ -530,6 +530,8 @@ class keystone(
$compute_port = undef,
) inherits keystone::params {
include ::keystone::logging
if ! $catalog_driver {
validate_re($catalog_type, 'template|sql')
}
@ -615,9 +617,6 @@ class keystone(
'DEFAULT/admin_bind_host': value => $admin_bind_host;
'DEFAULT/public_port': value => $public_port;
'DEFAULT/admin_port': value => $admin_port;
'DEFAULT/verbose': value => $verbose;
'DEFAULT/debug': value => $debug;
'DEFAULT/use_stderr': value => $use_stderr;
}
if $compute_port {
@ -934,37 +933,6 @@ class keystone(
Class['::keystone::db::sync'] ~> Service[$service_name]
}
# Syslog configuration
if $use_syslog {
keystone_config {
'DEFAULT/use_syslog': value => true;
'DEFAULT/syslog_log_facility': value => $log_facility;
}
} else {
keystone_config {
'DEFAULT/use_syslog': value => false;
}
}
if $log_file {
keystone_config {
'DEFAULT/log_file': value => $log_file;
'DEFAULT/log_dir': value => $log_dir;
}
} else {
if $log_dir {
keystone_config {
'DEFAULT/log_dir': value => $log_dir;
'DEFAULT/log_file': ensure => absent;
}
} else {
keystone_config {
'DEFAULT/log_dir': ensure => absent;
'DEFAULT/log_file': ensure => absent;
}
}
}
if $paste_config {
keystone_config {
'paste_deploy/config_file': value => $paste_config;

69
manifests/logging.pp
View File

@ -1,9 +1,38 @@
# Class keystone::logging
#
# keystone extended logging configuration
# keystone 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/keystone'
#
# [*log_file*]
# (optional) File where logs should be stored.
# Defaults to false.
#
# [*logging_context_format_string*]
# (optional) Format string to use for log messages with context.
# Defaults to undef.
@ -69,6 +98,13 @@
# Example: 'Y-%m-%d %H:%M:%S'
class keystone::logging(
$use_syslog = false,
$use_stderr = true,
$log_facility = 'LOG_USER',
$log_dir = '/var/log/keystone',
$log_file = false,
$verbose = false,
$debug = false,
$logging_context_format_string = undef,
$logging_default_format_string = undef,
$logging_debug_format_suffix = undef,
@ -82,6 +118,37 @@ class keystone::logging(
$log_date_format = undef,
) {
# NOTE(spredzy): In order to keep backward compatibility we rely on the pick function
# to use keystone::<myparam> first then keystone::logging::<myparam>.
$use_syslog_real = pick($::keystone::use_syslog,$use_syslog)
$use_stderr_real = pick($::keystone::use_stderr,$use_stderr)
$log_facility_real = pick($::keystone::log_facility,$log_facility)
$log_dir_real = pick($::keystone::log_dir,$log_dir)
$log_file_real = pick($::keystone::log_file,$log_file)
$verbose_real = pick($::keystone::verbose,$verbose)
$debug_real = pick($::keystone::debug,$debug)
keystone_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 $log_file_real {
keystone_config {
'DEFAULT/log_file' :
value => $log_file_real;
}
}
else {
keystone_config {
'DEFAULT/log_file' : ensure => absent;
}
}
if $logging_context_format_string {
keystone_config {
'DEFAULT/logging_context_format_string' :

39
spec/classes/keystone_logging_spec.rb
View File

@ -24,11 +24,50 @@ describe 'keystone::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',
:log_file => '/var/tmp/keystone_random.log',
:verbose => true,
:debug => true,
}
end
shared_examples 'basic default logging settings' do
it 'configures glance logging settins with default values' do
is_expected.to contain_keystone_config('DEFAULT/use_syslog').with(:value => 'false')
is_expected.to contain_keystone_config('DEFAULT/use_stderr').with(:value => 'true')
is_expected.to contain_keystone_config('DEFAULT/log_dir').with(:value => '/var/log/keystone')
is_expected.to contain_keystone_config('DEFAULT/log_file').with(:ensure => :absent)
is_expected.to contain_keystone_config('DEFAULT/verbose').with(:value => 'false')
is_expected.to contain_keystone_config('DEFAULT/debug').with(:value => 'false')
end
end
shared_examples 'basic non-default logging settings' do
it 'configures glance logging settins with non-default values' do
is_expected.to contain_keystone_config('DEFAULT/use_syslog').with(:value => 'true')
is_expected.to contain_keystone_config('DEFAULT/use_stderr').with(:value => 'false')
is_expected.to contain_keystone_config('DEFAULT/syslog_log_facility').with(:value => 'LOG_FOO')
is_expected.to contain_keystone_config('DEFAULT/log_dir').with(:value => '/var/log')
is_expected.to contain_keystone_config('DEFAULT/log_file').with(:value => '/var/tmp/keystone_random.log')
is_expected.to contain_keystone_config('DEFAULT/verbose').with(:value => 'true')
is_expected.to contain_keystone_config('DEFAULT/debug').with(:value => 'true')
end
end
shared_examples_for 'keystone-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'

62
spec/classes/keystone_spec.rb
View File

@ -104,6 +104,7 @@ describe 'keystone' do
httpd_params = {'service_name' => 'httpd'}.merge(default_params)
shared_examples_for 'core keystone examples' do |param_hash|
it { is_expected.to contain_class('keystone::logging') }
it { is_expected.to contain_class('keystone::params') }
it { is_expected.to contain_package('keystone').with(
@ -556,67 +557,6 @@ describe 'keystone' do
raise_error(Puppet::Error, /is not an Array/) }
end
describe 'with syslog disabled by default' do
let :params do
default_params
end
it { is_expected.to contain_keystone_config('DEFAULT/use_syslog').with_value(false) }
it { is_expected.to_not contain_keystone_config('DEFAULT/syslog_log_facility') }
end
describe 'with syslog enabled' do
let :params do
default_params.merge({
:use_syslog => 'true',
})
end
it { is_expected.to contain_keystone_config('DEFAULT/use_syslog').with_value(true) }
it { is_expected.to contain_keystone_config('DEFAULT/syslog_log_facility').with_value('LOG_USER') }
end
describe 'with syslog enabled and custom settings' do
let :params do
default_params.merge({
:use_syslog => 'true',
:log_facility => 'LOG_LOCAL0'
})
end
it { is_expected.to contain_keystone_config('DEFAULT/use_syslog').with_value(true) }
it { is_expected.to contain_keystone_config('DEFAULT/syslog_log_facility').with_value('LOG_LOCAL0') }
end
describe 'with log_file disabled by default' do
let :params do
default_params
end
it { is_expected.to contain_keystone_config('DEFAULT/log_file').with_ensure('absent') }
end
describe 'with log_file and log_dir enabled' do
let :params do
default_params.merge({
:log_file => 'keystone.log',
:log_dir => '/var/lib/keystone'
})
end
it { is_expected.to contain_keystone_config('DEFAULT/log_file').with_value('keystone.log') }
it { is_expected.to contain_keystone_config('DEFAULT/log_dir').with_value('/var/lib/keystone') }
end
describe 'with log_file and log_dir disabled' do
let :params do
default_params.merge({
:log_file => false,
:log_dir => false
})
end
it { is_expected.to contain_keystone_config('DEFAULT/log_file').with_ensure('absent') }
it { is_expected.to contain_keystone_config('DEFAULT/log_dir').with_ensure('absent') }
end
describe 'when enabling SSL' do
let :params do
{