From 86d3e7e214a536a88ee4f4c4e26b9e1e36d09ea8 Mon Sep 17 00:00:00 2001 From: Gael Chamoulaud Date: Thu, 29 Oct 2015 13:44:43 +0100 Subject: [PATCH] Switch aodh to use os_service_default fact This change switches the aodh module to use the os_service_default fact for configuration options that default to ''. Related-bug: #1515273 Change-Id: I05488c8bdcb5f54976a17aa25bf8a3b9d46627bb Signed-off-by: Gael Chamoulaud --- manifests/init.pp | 6 +- manifests/logging.pp | 235 ++++++++-------------------- spec/classes/aodh_api_spec.rb | 22 +-- spec/classes/aodh_evaluator_spec.rb | 4 +- spec/classes/aodh_init_spec.rb | 8 +- spec/classes/aodh_listener_spec.rb | 4 +- spec/classes/aodh_logging_spec.rb | 17 +- spec/classes/aodh_notifier_spec.rb | 4 +- spec/spec_helper.rb | 3 + 9 files changed, 103 insertions(+), 200 deletions(-) diff --git a/manifests/init.pp b/manifests/init.pp index 1fffafb8..93ab7de1 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -91,8 +91,9 @@ # # [*log_dir*] # (optional) Directory where logs should be stored. -# If set to boolean false, it will not log to any directory. -# Defaults to undef +# If set to boolean false or the $::os_service_default, it will not log to +# any directory. +# Defaults to undef. # # [*state_path*] # (optional) Directory for storing state. @@ -222,6 +223,7 @@ class aodh ( $use_syslog = undef, $use_stderr = undef, $log_facility = undef, + $log_dir = undef, $notification_driver = undef, $notification_topics = 'notifications', $database_connection = undef, diff --git a/manifests/logging.pp b/manifests/logging.pp index 88f7cb60..13ece313 100644 --- a/manifests/logging.pp +++ b/manifests/logging.pp @@ -6,59 +6,60 @@ # # [*verbose*] # (Optional) Should the daemons log verbose messages -# Defaults to 'false' +# Defaults to $::os_service_default # # [*debug*] # (Optional) Should the daemons log debug messages -# Defaults to 'false' +# Defaults to $::os_service_default # # [*use_syslog*] # (Optional) Use syslog for logging. -# Defaults to 'false' +# Defaults to $::os_service_default # # [*use_stderr*] # (optional) Use stderr for logging -# Defaults to 'true' +# Defaults to $::os_service_default # # [*log_facility*] # (Optional) Syslog facility to receive log lines. -# Defaults to 'LOG_USER' +# Defaults to $::os_service_default # # [*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/aodh' +# If set to boolean false or the $::os_service_default, it will not log to +# any directory. +# Defaults to '/var/log/aodh'. # # [*logging_context_format_string*] # (optional) Format string to use for log messages with context. -# Defaults to undef. +# Defaults to $::os_service_default # Example: '%(asctime)s.%(msecs)03d %(process)d %(levelname)s %(name)s\ # [%(request_id)s %(user_identity)s] %(instance)s%(message)s' # # [*logging_default_format_string*] # (optional) Format string to use for log messages without context. -# Defaults to undef. +# Defaults to $::os_service_default # Example: '%(asctime)s.%(msecs)03d %(process)d %(levelname)s %(name)s\ # [-] %(instance)s%(message)s' # # [*logging_debug_format_suffix*] # (optional) Formatted data to append to log format when level is DEBUG. -# Defaults to undef. +# Defaults to $::os_service_default # Example: '%(funcName)s %(pathname)s:%(lineno)d' # # [*logging_exception_prefix*] # (optional) Prefix each line of exception output with this format. -# Defaults to undef. +# Defaults to $::os_service_default # Example: '%(asctime)s.%(msecs)03d %(process)d TRACE %(name)s %(instance)s' # # [*log_config_append*] # The name of an additional logging configuration file. -# Defaults to undef. +# Defaults to $::os_service_default # See https://docs.python.org/2/howto/logging.html # # [*default_log_levels*] # (optional) Hash of logger (keys) and level (values) pairs. -# Defaults to undef. +# Defaults to $::os_service_default # Example: # { 'amqp' => 'WARN', 'amqplib' => 'WARN', 'boto' => 'WARN', # 'qpid' => 'WARN', 'sqlalchemy' => 'WARN', 'suds' => 'INFO', @@ -67,191 +68,81 @@ # # [*publish_errors*] # (optional) Publish error events (boolean value). -# Defaults to undef (false if unconfigured). +# Defaults to $::os_service_default # # [*fatal_deprecations*] # (optional) Make deprecations fatal (boolean value) -# Defaults to undef (false if unconfigured). +# Defaults to $::os_service_default # # [*instance_format*] # (optional) If an instance is passed with the log message, format it # like this (string value). -# Defaults to undef. +# Defaults to $::os_service_default # Example: '[instance: %(uuid)s] ' # # [*instance_uuid_format*] # (optional) If an instance UUID is passed with the log message, format # it like this (string value). -# Defaults to undef. +# Defaults to $::os_service_default # Example: instance_uuid_format='[instance: %(uuid)s] ' # [*log_date_format*] # (optional) Format string for %%(asctime)s in log records. -# Defaults to undef. +# Defaults to $::os_service_default # Example: 'Y-%m-%d %H:%M:%S' class aodh::logging( - $use_syslog = false, - $use_stderr = true, - $log_facility = 'LOG_USER', + $use_syslog = $::os_service_default, + $use_stderr = $::os_service_default, + $log_facility = $::os_service_default, $log_dir = '/var/log/aodh', - $verbose = false, - $debug = false, - $logging_context_format_string = undef, - $logging_default_format_string = undef, - $logging_debug_format_suffix = undef, - $logging_exception_prefix = undef, - $log_config_append = undef, - $default_log_levels = undef, - $publish_errors = undef, - $fatal_deprecations = undef, - $instance_format = undef, - $instance_uuid_format = undef, - $log_date_format = undef, + $verbose = $::os_service_default, + $debug = $::os_service_default, + $logging_context_format_string = $::os_service_default, + $logging_default_format_string = $::os_service_default, + $logging_debug_format_suffix = $::os_service_default, + $logging_exception_prefix = $::os_service_default, + $log_config_append = $::os_service_default, + $default_log_levels = $::os_service_default, + $publish_errors = $::os_service_default, + $fatal_deprecations = $::os_service_default, + $instance_format = $::os_service_default, + $instance_uuid_format = $::os_service_default, + $log_date_format = $::os_service_default, ) { # NOTE(spredzy): In order to keep backward compatibility we rely on the pick function # to use aodh:: first then aodh::logging::. - $use_syslog_real = pick($::aodh::use_syslog,$use_syslog) - $use_stderr_real = pick($::aodh::use_stderr,$use_stderr) + $use_syslog_real = pick($::aodh::use_syslog,$use_syslog) + $use_stderr_real = pick($::aodh::use_stderr,$use_stderr) $log_facility_real = pick($::aodh::log_facility,$log_facility) - $log_dir_real = pick($::aodh::log_dir,$log_dir) - $verbose_real = pick($::aodh::verbose,$verbose) - $debug_real = pick($::aodh::debug,$debug) + $log_dir_real = pick($::aodh::log_dir,$log_dir) + $verbose_real = pick($::aodh::verbose,$verbose) + $debug_real = pick($::aodh::debug,$debug) - aodh_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 is_service_default($default_log_levels) { + $default_log_levels_real = $default_log_levels + } else { + $default_log_levels_real = join(sort(join_keys_to_values($default_log_levels, '=')), ',') } - if $logging_context_format_string { - aodh_config { - 'DEFAULT/logging_context_format_string' : - value => $logging_context_format_string; - } - } - else { - aodh_config { - 'DEFAULT/logging_context_format_string' : ensure => absent; - } - } - - if $logging_default_format_string { - aodh_config { - 'DEFAULT/logging_default_format_string' : - value => $logging_default_format_string; - } - } - else { - aodh_config { - 'DEFAULT/logging_default_format_string' : ensure => absent; - } - } - - if $logging_debug_format_suffix { - aodh_config { - 'DEFAULT/logging_debug_format_suffix' : - value => $logging_debug_format_suffix; - } - } - else { - aodh_config { - 'DEFAULT/logging_debug_format_suffix' : ensure => absent; - } - } - - if $logging_exception_prefix { - aodh_config { - 'DEFAULT/logging_exception_prefix' : value => $logging_exception_prefix; - } - } - else { - aodh_config { - 'DEFAULT/logging_exception_prefix' : ensure => absent; - } - } - - if $log_config_append { - aodh_config { - 'DEFAULT/log_config_append' : value => $log_config_append; - } - } - else { - aodh_config { - 'DEFAULT/log_config_append' : ensure => absent; - } - } - - if $default_log_levels { - aodh_config { - 'DEFAULT/default_log_levels' : - value => join(sort(join_keys_to_values($default_log_levels, '=')), ','); - } - } - else { - aodh_config { - 'DEFAULT/default_log_levels' : ensure => absent; - } - } - - if $publish_errors { - aodh_config { - 'DEFAULT/publish_errors' : value => $publish_errors; - } - } - else { - aodh_config { - 'DEFAULT/publish_errors' : ensure => absent; - } - } - - if $fatal_deprecations { - aodh_config { - 'DEFAULT/fatal_deprecations' : value => $fatal_deprecations; - } - } - else { - aodh_config { - 'DEFAULT/fatal_deprecations' : ensure => absent; - } - } - - if $instance_format { - aodh_config { - 'DEFAULT/instance_format' : value => $instance_format; - } - } - else { - aodh_config { - 'DEFAULT/instance_format' : ensure => absent; - } - } - - if $instance_uuid_format { - aodh_config { - 'DEFAULT/instance_uuid_format' : value => $instance_uuid_format; - } - } - else { - aodh_config { - 'DEFAULT/instance_uuid_format' : ensure => absent; - } - } - - if $log_date_format { - aodh_config { - 'DEFAULT/log_date_format' : value => $log_date_format; - } - } - else { - aodh_config { - 'DEFAULT/log_date_format' : ensure => absent; - } - } - - + aodh_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; + 'DEFAULT/logging_context_format_string' : value => $logging_context_format_string; + 'DEFAULT/logging_default_format_string' : value => $logging_default_format_string; + 'DEFAULT/logging_debug_format_suffix' : value => $logging_debug_format_suffix; + 'DEFAULT/logging_exception_prefix' : value => $logging_exception_prefix; + 'DEFAULT/log_config_append' : value => $log_config_append; + 'DEFAULT/default_log_levels' : value => $default_log_levels_real; + 'DEFAULT/publish_errors' : value => $publish_errors; + 'DEFAULT/fatal_deprecations' : value => $fatal_deprecations; + 'DEFAULT/instance_format' : value => $instance_format; + 'DEFAULT/instance_uuid_format' : value => $instance_uuid_format; + 'DEFAULT/log_date_format' : value => $log_date_format; + } } diff --git a/spec/classes/aodh_api_spec.rb b/spec/classes/aodh_api_spec.rb index 3a8f032f..dd320b4d 100644 --- a/spec/classes/aodh_api_spec.rb +++ b/spec/classes/aodh_api_spec.rb @@ -123,12 +123,14 @@ describe 'aodh::api' do context 'on Debian platforms' do let :facts do - { :osfamily => 'Debian', + @default_facts.merge({ + :osfamily => 'Debian', :operatingsystem => 'Debian', :operatingsystemrelease => '8.0', :concat_basedir => '/var/lib/puppet/concat', :fqdn => 'some.host.tld', - :processorcount => 2 } + :processorcount => 2, + }) end let :platform_params do @@ -141,12 +143,14 @@ describe 'aodh::api' do context 'on RedHat platforms' do let :facts do - { :osfamily => 'RedHat', + @default_facts.merge({ + :osfamily => 'RedHat', :operatingsystem => 'RedHat', :operatingsystemrelease => '7.1', :fqdn => 'some.host.tld', :concat_basedir => '/var/lib/puppet/concat', - :processorcount => 2 } + :processorcount => 2, + }) end let :platform_params do @@ -159,7 +163,7 @@ describe 'aodh::api' do describe 'with custom auth_uri' do let :facts do - { :osfamily => 'RedHat' } + @default_facts.merge({ :osfamily => 'RedHat' }) end before do params.merge!({ @@ -173,10 +177,10 @@ describe 'aodh::api' do describe "with custom keystone identity_uri" do let :facts do - { :osfamily => 'RedHat' } + @default_facts.merge({ :osfamily => 'RedHat' }) end before do - params.merge!({ + params.merge!({ :keystone_identity_uri => 'https://foo.bar:1234/', }) end @@ -187,10 +191,10 @@ describe 'aodh::api' do describe "with custom keystone identity_uri and auth_uri" do let :facts do - { :osfamily => 'RedHat' } + @default_facts.merge({ :osfamily => 'RedHat' }) end before do - params.merge!({ + params.merge!({ :keystone_identity_uri => 'https://foo.bar:35357/', :keystone_auth_uri => 'https://foo.bar:5000/v2.0/', }) diff --git a/spec/classes/aodh_evaluator_spec.rb b/spec/classes/aodh_evaluator_spec.rb index 7ada65f6..07450b76 100644 --- a/spec/classes/aodh_evaluator_spec.rb +++ b/spec/classes/aodh_evaluator_spec.rb @@ -87,7 +87,7 @@ describe 'aodh::evaluator' do context 'on Debian platforms' do let :facts do - { :osfamily => 'Debian' } + @default_facts.merge({ :osfamily => 'Debian' }) end let :platform_params do @@ -100,7 +100,7 @@ describe 'aodh::evaluator' do context 'on RedHat platforms' do let :facts do - { :osfamily => 'RedHat' } + @default_facts.merge({ :osfamily => 'RedHat' }) end let :platform_params do diff --git a/spec/classes/aodh_init_spec.rb b/spec/classes/aodh_init_spec.rb index f963fbd8..4ca9f3da 100644 --- a/spec/classes/aodh_init_spec.rb +++ b/spec/classes/aodh_init_spec.rb @@ -214,8 +214,10 @@ describe 'aodh' do context 'on Debian platforms' do let :facts do - { :osfamily => 'Debian', - :operatingsystem => 'Debian' } + @default_facts.merge({ + :osfamily => 'Debian', + :operatingsystem => 'Debian', + }) end let :platform_params do @@ -227,7 +229,7 @@ describe 'aodh' do context 'on RedHat platforms' do let :facts do - { :osfamily => 'RedHat' } + @default_facts.merge({ :osfamily => 'RedHat' }) end let :platform_params do diff --git a/spec/classes/aodh_listener_spec.rb b/spec/classes/aodh_listener_spec.rb index 180b5f34..32f1076e 100644 --- a/spec/classes/aodh_listener_spec.rb +++ b/spec/classes/aodh_listener_spec.rb @@ -73,7 +73,7 @@ describe 'aodh::listener' do context 'on Debian platforms' do let :facts do - { :osfamily => 'Debian' } + @default_facts.merge({ :osfamily => 'Debian' }) end let :platform_params do @@ -86,7 +86,7 @@ describe 'aodh::listener' do context 'on RedHat platforms' do let :facts do - { :osfamily => 'RedHat' } + @default_facts.merge({ :osfamily => 'RedHat' }) end let :platform_params do diff --git a/spec/classes/aodh_logging_spec.rb b/spec/classes/aodh_logging_spec.rb index 17d5e000..4cb4f1a1 100644 --- a/spec/classes/aodh_logging_spec.rb +++ b/spec/classes/aodh_logging_spec.rb @@ -56,12 +56,13 @@ describe 'aodh::logging' do end shared_examples 'basic default logging settings' do - it 'configures aodh logging settins with default values' do - is_expected.to contain_aodh_config('DEFAULT/use_syslog').with(:value => 'false') - is_expected.to contain_aodh_config('DEFAULT/use_stderr').with(:value => 'true') + it 'configures aodh logging settings with default values' do + is_expected.to contain_aodh_config('DEFAULT/use_syslog').with(:value => '') + is_expected.to contain_aodh_config('DEFAULT/use_stderr').with(:value => '') + is_expected.to contain_aodh_config('DEFAULT/syslog_log_facility').with(:value => '') is_expected.to contain_aodh_config('DEFAULT/log_dir').with(:value => '/var/log/aodh') - is_expected.to contain_aodh_config('DEFAULT/verbose').with(:value => 'false') - is_expected.to contain_aodh_config('DEFAULT/debug').with(:value => 'false') + is_expected.to contain_aodh_config('DEFAULT/verbose').with(:value => '') + is_expected.to contain_aodh_config('DEFAULT/debug').with(:value => '') end end @@ -120,13 +121,13 @@ describe 'aodh::logging' do :default_log_levels, :fatal_deprecations, :instance_format, :instance_uuid_format, :log_date_format, ].each { |param| - it { is_expected.to contain_aodh_config("DEFAULT/#{param}").with_ensure('absent') } + it { is_expected.to contain_aodh_config("DEFAULT/#{param}").with(:value => '') } } end context 'on Debian platforms' do let :facts do - { :osfamily => 'Debian' } + @default_facts.merge({ :osfamily => 'Debian' }) end it_configures 'aodh-logging' @@ -134,7 +135,7 @@ describe 'aodh::logging' do context 'on RedHat platforms' do let :facts do - { :osfamily => 'RedHat' } + @default_facts.merge({ :osfamily => 'RedHat' }) end it_configures 'aodh-logging' diff --git a/spec/classes/aodh_notifier_spec.rb b/spec/classes/aodh_notifier_spec.rb index 94d7b9cc..6c786c2a 100644 --- a/spec/classes/aodh_notifier_spec.rb +++ b/spec/classes/aodh_notifier_spec.rb @@ -73,7 +73,7 @@ describe 'aodh::notifier' do context 'on Debian platforms' do let :facts do - { :osfamily => 'Debian' } + @default_facts.merge({ :osfamily => 'Debian' }) end let :platform_params do @@ -86,7 +86,7 @@ describe 'aodh::notifier' do context 'on RedHat platforms' do let :facts do - { :osfamily => 'RedHat' } + @default_facts.merge({ :osfamily => 'RedHat' }) end let :platform_params do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 3df4cede..9bc7bcf9 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -5,6 +5,9 @@ require 'webmock/rspec' RSpec.configure do |c| c.alias_it_should_behave_like_to :it_configures, 'configures' c.alias_it_should_behave_like_to :it_raises, 'raises' + c.before :each do + @default_facts = { :os_service_default => '' } + end end at_exit { RSpec::Puppet::Coverage.report! }