OpenStack Compute (nova)

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

  1. logdir
  2. Merge into trunk
Proposed by Soren Hansen
Status: Merged
Approved by: Soren Hansen
Approved revision: 674
Merged at revision: 681
Proposed branch: lp:~soren/nova/logdir
Merge into: lp:~hudson-openstack/nova/trunk
Diff against target: 104 lines (+40/-4)
4 files modified
nova/flags.py (+2/-0)
nova/log.py (+17/-2)
nova/tests/test_log.py (+21/-0)
nova/twistd.py (+0/-2)
To merge this branch: bzr merge lp:~soren/nova/logdir
Reviewer Review Type Date Requested Status
Jay Pipes (community) Approve
Devin Carlen (community) Approve
Review via email: mp+49722@code.launchpad.net

Commit message

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

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Devin Carlen (devcamcar) wrote :

lgtm

review: Approve
Revision history for this message
Jay Pipes (jaypipes) wrote :

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
Revision history for this message
Soren Hansen (soren) wrote :

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

Revision history for this message
Jay Pipes (jaypipes) wrote :

test cases, ftw.

review: Approve
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :
Download full text (21.2 KiB)

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...

Revision history for this message
Jay Pipes (jaypipes) wrote :

lgtm.

review: Approve
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :
Download full text (23.3 KiB)

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...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'nova/flags.py'
2--- nova/flags.py 2011-02-10 19:40:21 +0000
3+++ nova/flags.py 2011-02-16 09:49:27 +0000
4@@ -282,6 +282,8 @@
5
6 DEFINE_string('state_path', os.path.join(os.path.dirname(__file__), '../'),
7 "Top-level directory for maintaining nova's state")
8+DEFINE_string('logdir', None, 'output to a per-service log file in named '
9+ 'directory')
10
11 DEFINE_string('sql_connection',
12 'sqlite:///$state_path/nova.sqlite',
13
14=== modified file 'nova/log.py'
15--- nova/log.py 2011-01-28 17:46:46 +0000
16+++ nova/log.py 2011-02-16 09:49:27 +0000
17@@ -28,9 +28,11 @@
18
19
20 import cStringIO
21+import inspect
22 import json
23 import logging
24 import logging.handlers
25+import os
26 import sys
27 import traceback
28
29@@ -111,6 +113,18 @@
30 return context
31
32
33+def _get_binary_name():
34+ return os.path.basename(inspect.stack()[-1][1])
35+
36+
37+def get_log_file_path(binary=None):
38+ if FLAGS.logfile:
39+ return FLAGS.logfile
40+ if FLAGS.logdir:
41+ binary = binary or _get_binary_name()
42+ return '%s.log' % (os.path.join(FLAGS.logdir, binary),)
43+
44+
45 def basicConfig():
46 logging.basicConfig()
47 for handler in logging.root.handlers:
48@@ -123,8 +137,9 @@
49 syslog = SysLogHandler(address='/dev/log')
50 syslog.setFormatter(_formatter)
51 logging.root.addHandler(syslog)
52- if FLAGS.logfile:
53- logfile = FileHandler(FLAGS.logfile)
54+ logpath = get_log_file_path()
55+ if logpath:
56+ logfile = FileHandler(logpath)
57 logfile.setFormatter(_formatter)
58 logging.root.addHandler(logfile)
59
60
61=== modified file 'nova/tests/test_log.py'
62--- nova/tests/test_log.py 2011-01-14 19:00:47 +0000
63+++ nova/tests/test_log.py 2011-02-16 09:49:27 +0000
64@@ -46,6 +46,27 @@
65 self.assert_(True) # didn't raise exception
66
67
68+class LogHandlerTestCase(test.TestCase):
69+ def test_log_path_logdir(self):
70+ self.flags(logdir='/some/path')
71+ self.assertEquals(log.get_log_file_path(binary='foo-bar'),
72+ '/some/path/foo-bar.log')
73+
74+ def test_log_path_logfile(self):
75+ self.flags(logfile='/some/path/foo-bar.log')
76+ self.assertEquals(log.get_log_file_path(binary='foo-bar'),
77+ '/some/path/foo-bar.log')
78+
79+ def test_log_path_none(self):
80+ self.assertTrue(log.get_log_file_path(binary='foo-bar') is None)
81+
82+ def test_log_path_logfile_overrides_logdir(self):
83+ self.flags(logdir='/some/other/path',
84+ logfile='/some/path/foo-bar.log')
85+ self.assertEquals(log.get_log_file_path(binary='foo-bar'),
86+ '/some/path/foo-bar.log')
87+
88+
89 class NovaFormatterTestCase(test.TestCase):
90 def setUp(self):
91 super(NovaFormatterTestCase, self).setUp()
92
93=== modified file 'nova/twistd.py'
94--- nova/twistd.py 2011-01-20 17:52:02 +0000
95+++ nova/twistd.py 2011-02-16 09:49:27 +0000
96@@ -43,8 +43,6 @@
97
98
99 FLAGS = flags.FLAGS
100-flags.DEFINE_string('logdir', None, 'directory to keep log files in '
101- '(will be prepended to $logfile)')
102
103
104 class TwistdServerOptions(ServerOptions):