Merge lp:~soren/nova/logdir into lp:~hudson-openstack/nova/trunk

[Soren Hansen]

Add back --logdir=DIR option. If set, a logfile named after the binary (e.g. nova-api.log) will be kept in DIR.

Resurrect the logdir option.

With The Big Eventlet Merge™, the logdir option went missing. I've
cried myself to sleep ever since.

Setting this option will create a logfile in the named directory
with a filename based on the binary's name. E.g. "nova-api --logdir=/var/log/nova"
will will create logfile /var/log/nova/nova-api.log.

[Devin Carlen (devcamcar)]

lgtm

review: Approve

[Jay Pipes (jaypipes)]

Hi!

35 + if FLAGS.logfile or FLAGS.logdir:
36 + if FLAGS.logfile:
37 + logfile = FLAGS.logfile
38 + else:
39 + binary = os.path.basename(inspect.stack()[-1][1])
40 + logpath = '%s.log' % (os.path.join(FLAGS.logdir, binary),)
41 + logfile = FileHandler(logpath)

Looks like line 37 abve should be:

logpath = FLAGS.logfile

Otherwise line 41 will fail with a NameError is logfile is passed and logpath isn't defined.

Cheers,
jay

review: Needs Fixing

[Soren Hansen (soren)]

Good catch, Jay. I've fixed that, refactored it somewhat, and added unit tests. This will never happen again :)

[Jay Pipes (jaypipes)]

test cases, ftw.

review: Approve

[OpenStack Infra (hudson-openstack)]

The attempt to merge lp:~soren/nova/logdir into lp:nova failed. Below is the output from the failed tests.

AdminAPITest
    test_admin_disabled ok
    test_admin_enabled ok
APITest
    test_exceptions_are_converted_to_faults ok
Test
    test_authorize_token ok
    test_authorize_user ok
    test_bad_token ok
    test_bad_user ok
    test_no_user ok
    test_token_expiry ok
TestLimiter
    test_authorize_token ok
TestFaults
    test_fault_parts ok
    test_raise ok
    test_retry_header ok
FlavorsTest
    test_get_flavor_by_id ok
    test_get_flavor_list ok
GlanceImageServiceTest
    test_create ok
    test_create_and_show_non_existing_image ok
    test_delete ok
    test_update ok
ImageControllerWithGlanceServiceTest
    test_get_image_details ok
    test_get_image_index ok
LocalImageServiceTest
    test_create ok
    test_create_and_show_non_existing_image ok
    test_delete ok
    test_update ok
LimiterTest
    test_minute ok
    test_one_per_period ok
    test_second ok
    test_users_get_separate_buckets ok
    test_we_can_go_indefinitely_if_we_spread_out_requests ok
WSGIAppProxyTest
    test_200 ok
    test_403 ok
    test_failure ok
WSGIAppTest
    test_escaping ok
    test_good_urls ok
    test_invalid_methods ok
    test_invalid_urls ok
    test_response_to_delays ok
ServersTest
    test_create_backup_schedules ok
    test_create_instance ok
    test_delete_backup_schedules ok
    test_delete_server_instance ok
    test_get_all_server_details ok
    test_get_se...

[Jay Pipes (jaypipes)]

lgtm.

review: Approve

[OpenStack Infra (hudson-openstack)]

The attempt to merge lp:~soren/nova/logdir into lp:nova failed. Below is the output from the failed tests.

AdminAPITest
    test_admin_disabled ok
    test_admin_enabled ok
APITest
    test_exceptions_are_converted_to_faults ok
Test
    test_authorize_token ok
    test_authorize_user ok
    test_bad_token ok
    test_bad_user ok
    test_no_user ok
    test_token_expiry ok
TestLimiter
    test_authorize_token ok
TestFaults
    test_fault_parts ok
    test_raise ok
    test_retry_header ok
FlavorsTest
    test_get_flavor_by_id ok
    test_get_flavor_list ok
GlanceImageServiceTest
    test_create ok
    test_create_and_show_non_existing_image ok
    test_delete ok
    test_update ok
ImageControllerWithGlanceServiceTest
    test_get_image_details ok
    test_get_image_index ok
LocalImageServiceTest
    test_create ok
    test_create_and_show_non_existing_image ok
    test_delete ok
    test_update ok
LimiterTest
    test_minute ok
    test_one_per_period ok
    test_second ok
    test_users_get_separate_buckets ok
    test_we_can_go_indefinitely_if_we_spread_out_requests ok
WSGIAppProxyTest
    test_200 ok
    test_403 ok
    test_failure ok
WSGIAppTest
    test_escaping ok
    test_good_urls ok
    test_invalid_methods ok
    test_invalid_urls ok
    test_response_to_delays ok
ServersTest
    test_create_backup_schedules ok
    test_create_instance ok
    test_delete_backup_schedules ok
    test_delete_server_instance ok
    test_get_all_server_details ok
    test_get_se...

=== modified file 'nova/flags.py'
--- nova/flags.py	2011-02-10 19:40:21 +0000
+++ nova/flags.py	2011-02-16 09:49:27 +0000
@@ -282,6 +282,8 @@
 
 DEFINE_string('state_path', os.path.join(os.path.dirname(__file__), '../'),
               "Top-level directory for maintaining nova's state")
+DEFINE_string('logdir', None, 'output to a per-service log file in named '
+                              'directory')
 
 DEFINE_string('sql_connection',
               'sqlite:///$state_path/nova.sqlite',
 
=== modified file 'nova/log.py'
--- nova/log.py	2011-01-28 17:46:46 +0000
+++ nova/log.py	2011-02-16 09:49:27 +0000
@@ -28,9 +28,11 @@
 
 
 import cStringIO
+import inspect
 import json
 import logging
 import logging.handlers
+import os
 import sys
 import traceback
 
@@ -111,6 +113,18 @@
     return context
 
 
+def _get_binary_name():
+    return os.path.basename(inspect.stack()[-1][1])
+
+
+def get_log_file_path(binary=None):
+    if FLAGS.logfile:
+        return FLAGS.logfile
+    if FLAGS.logdir:
+        binary = binary or _get_binary_name()
+        return '%s.log' % (os.path.join(FLAGS.logdir, binary),)
+
+
 def basicConfig():
     logging.basicConfig()
     for handler in logging.root.handlers:
@@ -123,8 +137,9 @@
         syslog = SysLogHandler(address='/dev/log')
         syslog.setFormatter(_formatter)
         logging.root.addHandler(syslog)
-    if FLAGS.logfile:
-        logfile = FileHandler(FLAGS.logfile)
+    logpath = get_log_file_path()
+    if logpath:
+        logfile = FileHandler(logpath)
         logfile.setFormatter(_formatter)
         logging.root.addHandler(logfile)
 
 
=== modified file 'nova/tests/test_log.py'
--- nova/tests/test_log.py	2011-01-14 19:00:47 +0000
+++ nova/tests/test_log.py	2011-02-16 09:49:27 +0000
@@ -46,6 +46,27 @@
         self.assert_(True)  # didn't raise exception
 
 
+class LogHandlerTestCase(test.TestCase):
+    def test_log_path_logdir(self):
+        self.flags(logdir='/some/path')
+        self.assertEquals(log.get_log_file_path(binary='foo-bar'),
+                         '/some/path/foo-bar.log')
+
+    def test_log_path_logfile(self):
+        self.flags(logfile='/some/path/foo-bar.log')
+        self.assertEquals(log.get_log_file_path(binary='foo-bar'),
+                         '/some/path/foo-bar.log')
+
+    def test_log_path_none(self):
+        self.assertTrue(log.get_log_file_path(binary='foo-bar') is None)
+
+    def test_log_path_logfile_overrides_logdir(self):
+        self.flags(logdir='/some/other/path',
+                   logfile='/some/path/foo-bar.log')
+        self.assertEquals(log.get_log_file_path(binary='foo-bar'),
+                         '/some/path/foo-bar.log')
+
+
 class NovaFormatterTestCase(test.TestCase):
     def setUp(self):
         super(NovaFormatterTestCase, self).setUp()
 
=== modified file 'nova/twistd.py'
--- nova/twistd.py	2011-01-20 17:52:02 +0000
+++ nova/twistd.py	2011-02-16 09:49:27 +0000
@@ -43,8 +43,6 @@
 
 
 FLAGS = flags.FLAGS
-flags.DEFINE_string('logdir', None, 'directory to keep log files in '
-                                     '(will be prepended to $logfile)')
 
 
 class TwistdServerOptions(ServerOptions):