Merge "Put all the logging related parameters to the logging class"

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,
+  $verbose                            = undef,
-  $debug                              = false,
+  $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_syslog                         = undef,
-  $use_stderr                         = true,
+  $use_stderr                         = undef,
-  $log_facility                       = 'LOG_USER',
+  $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;

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::<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' :

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

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(