Ubuntu Distributed Development

Merge lp:~vila/udd/719888-log-config into lp:udd

  1. 719888-log-config
  2. Merge into import-scripts
Proposed by Vincent Ladeuil
Status: Merged
Merged at revision: 418
Proposed branch: lp:~vila/udd/719888-log-config
Merge into: lp:udd
Diff against target: 486 lines (+258/-52) (has conflicts)
12 files modified
categorise_failures.py (+24/-1)
etc-apache2-sites-available-package-import.ubuntu.com (+18/-0)
etc-init.d-mass-import (+1/-2)
icommon.py (+4/-3)
iconfig.py (+32/-0)
import_package.py (+6/-4)
importer.crontab (+9/-6)
importer.logrotate (+0/-17)
logrotate.py (+53/-0)
mass_import.py (+33/-19)
pkgimport.conf (+18/-0)
tests.py (+60/-0)
Text conflict in tests.py
To merge this branch: bzr merge lp:~vila/udd/719888-log-config
Reviewer Review Type Date Requested Status
James Westby Approve
Review via email: mp+54572@code.launchpad.net

Description of the change

This introduces a configuration file for the package importer so that the directory used for log files can be changed more easily.

Tons of yak shaving to get there so I apologize in advance for the diff which will be a bit hard to read.

Further modifications of icommon.py to transfer more constants in pkgimport.conf should be easier now.

Note that I didn't (yet) support modifying the config via locations.conf (for tests) so the needed modifications can be reviewed separately.

This changes significant parts of the configuration and requires
some specific actions when deploying:

- stop the importer gracefully
- empty the crontab
- pull lp:udd
- rsync/delete the log files (for those interested)
  That means debug_log*, process_log* and logs/*
- re-install the crontab
- restart the installer
- run categorize_failures.py once to bootstrap the web server
- update the apache config to use www/ instead of logs/ (see provided file in the branch)
- restart apache ?

Forget about the diff in mass-import-in-etc, it's just a catch-up with the actual jubany config.

To post a comment you must log in.
Revision history for this message
James Westby (james-w) wrote :

210 +17 * * * * /usr/bin/python ${SCRIPTS_DIR}/logrotate.py

I don't see that script in the diff, did you forget to add it?

379 + log_process_path = conf.get('pkgimport.driver.log.process')

The file is "progress_log", not "process_log". If you want to rename that's fine,
otherwise I think we should stay consistent (there is "progress_handler" later
in this function.)

Thanks,

James

review: Needs Fixing
lp:~vila/udd/719888-log-config updated
429. By Vincent Ladeuil

Add missing file and fix typos.

430. By Vincent Ladeuil

Fix more typos :-/

Revision history for this message
Vincent Ladeuil (vila) wrote :

Good catches, sorry about that, I tested the wrong version :-/

So, this has no been tested locally and the log file is still progress_log (weird thinko there).

Revision history for this message
Vincent Ladeuil (vila) wrote :

s/no/now/

Revision history for this message
James Westby (james-w) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'categorise_failures.py'
2--- categorise_failures.py 2011-02-20 15:53:29 +0000
3+++ categorise_failures.py 2011-03-24 08:28:41 +0000
4@@ -8,7 +8,6 @@
5 import sys
6 import time
7
8-sys.path.insert(0, os.path.dirname(__file__))
9 import icommon
10
11 html_head = '''<html>
12@@ -322,12 +321,36 @@
13 remove_obsolete_pages(old_pages, new_pages)
14
15
16+def install_index():
17+ content = '''<html>
18+<head>
19+<title>Ubuntu Bazaar package importer</title>
20+<meta http-equiv="refresh" content="0;url=status/">
21+</head>
22+<body>
23+Hi!
24+
25+See <a href="status">the status page.</a>
26+</body>
27+</html>
28+'''
29+ # FIXME: 'status' in content should stay in sync with 'status' in
30+ # web_status_dir, we could use os.path.basename(icommon.web_status_dir)
31+ # -- vila 2011-03-23
32+ index_path = os.path.join(icommon.web_base_dir, 'index.html')
33+ f = open(index_path, 'w')
34+ try:
35+ f.write(content)
36+ finally:
37+ f.close()
38+
39 def main():
40 lock = icommon.lock_categorise_failures()
41 if lock is None:
42 print "Another instance of categorise_failures is already running."
43 sys.exit(0)
44 try:
45+ install_index()
46 get_info()
47 finally:
48 lock.close()
49
50=== added file 'etc-apache2-sites-available-package-import.ubuntu.com'
51--- etc-apache2-sites-available-package-import.ubuntu.com 1970-01-01 00:00:00 +0000
52+++ etc-apache2-sites-available-package-import.ubuntu.com 2011-03-24 08:28:41 +0000
53@@ -0,0 +1,18 @@
54+NameVirtualHost *
55+<VirtualHost *>
56+ ServerName package-import.ubuntu.com
57+ ServerAlias package-import.canonical.com
58+
59+ ErrorLog /var/log/apache2/package-import.canonical.com-error.log
60+ CustomLog /var/log/apache2/package-import.canonical.com-access.log combi
61+ned
62+
63+ DocumentRoot /srv/package-import.canonical.com/new/www/
64+
65+ <Directory /srv/package-import.canonical.com/new/www/>
66+ Options Indexes
67+ AllowOverride None
68+ IndexOptions DescriptionWidth=* NameWidth=*
69+ </Directory>
70+
71+</VirtualHost>
72
73=== renamed file 'mass-import-in-etc' => 'etc-init.d-mass-import'
74--- mass-import-in-etc 2011-01-18 23:55:51 +0000
75+++ etc-init.d-mass-import 2011-03-24 08:28:41 +0000
76@@ -1,6 +1,6 @@
77 #!/bin/sh
78
79-BASEDIR=/srv/package-import.canonical.com/new/
80+export BASEDIR=/srv/package-import.canonical.com/new/
81 PATH=${BASEDIR}scripts:/usr/local/sbin:/usr/local/bin:/sbin:/bin:/usr/sbin:/usr/bin
82 NAME=mass-import
83 DAEMON=${BASEDIR}scripts/mass_import.py
84@@ -56,4 +56,3 @@
85 exit 1
86 ;;
87 esac
88-
89
90=== modified file 'icommon.py'
91--- icommon.py 2011-03-18 07:40:09 +0000
92+++ icommon.py 2011-03-24 08:28:41 +0000
93@@ -43,6 +43,8 @@
94
95 SERVICE_ROOT = EDGE_SERVICE_ROOT.replace("edge.", "")
96
97+# FIXME: This is also defined in pkgimport.conf. Almost all the constants
98+# defined below should be migrated to pkgimport.conf RSN -- vila 2011-03-22
99 base_dir = "/srv/package-import.canonical.com/new/"
100 if not os.path.exists(base_dir):
101 base_dir = "."
102@@ -53,8 +55,6 @@
103 # -- vila 20110219
104 lp_creds_file = os.path.join(base_dir, "lp_creds.txt")
105 stop_file = os.path.join(base_dir, "STOP_PLEASE")
106-debug_log_file = os.path.join(base_dir, "debug_log")
107-progress_log_file = os.path.join(base_dir, "progress_log")
108 max_threads_file = os.path.join(base_dir, "max_threads")
109 locks_dir = os.path.join(base_dir, "locks")
110 updates_dir = os.path.join(base_dir, "updates")
111@@ -67,7 +67,7 @@
112 # bug #714614: keep 'explanations' in sync with the scripts
113 explanations_file = os.path.join(os.path.dirname(__file__), 'explanations')
114 sqlite_timeout = 30
115-web_base_dir = logs_dir
116+web_base_dir = os.path.join(base_dir, "www")
117 web_status_dir = os.path.join(web_base_dir, 'status')
118 debian_diffs_dir = os.path.join(web_base_dir, "diffs")
119 ubuntu_merge_dir = os.path.join(web_base_dir, "merges")
120@@ -78,6 +78,7 @@
121
122 for directory in (logs_dir, lists_dir,
123 locks_dir, updates_dir,
124+ web_base_dir,
125 web_status_dir,
126 debian_diffs_dir, ubuntu_merge_dir,
127 ):
128
129=== added file 'iconfig.py'
130--- iconfig.py 1970-01-01 00:00:00 +0000
131+++ iconfig.py 2011-03-24 08:28:41 +0000
132@@ -0,0 +1,32 @@
133+from bzrlib import (
134+ config,
135+ osutils,
136+ )
137+
138+# Directory containing all python scripts and the configuration file
139+_scripts_dir = osutils.dirname(osutils.realpath(__file__))
140+
141+def _build_path(name):
142+ """Build and return a path in the scripts directory for name"""
143+ return osutils.pathjoin(_scripts_dir, name)
144+
145+
146+def iconfig_file_name():
147+ return _build_path('pkgimport.conf')
148+
149+
150+# FIXME: We don't inherit from LockableConfig because we don't want to create a
151+# 'lock' dir alonside the scripts (we should not require write access there and
152+# we don't need to modify options in the configuration file) -- vila 2011-03-22
153+class Iconfig(config.IniBasedConfig):
154+
155+ def __init__(self, file_name=None):
156+ if file_name is None:
157+ file_name = iconfig_file_name()
158+ super(Iconfig, self).__init__(file_name=file_name)
159+
160+ def config_id(self):
161+ return 'pkgimport'
162+
163+ def get(self, name):
164+ return self.get_user_option(name)
165
166=== modified file 'import_package.py'
167--- import_package.py 2011-03-18 15:14:39 +0000
168+++ import_package.py 2011-03-24 08:28:41 +0000
169@@ -47,8 +47,8 @@
170
171 from bzrlib.plugins.builddeb import import_dsc
172
173-sys.path.insert(0, os.path.dirname(__file__))
174 import icommon
175+import iconfig
176
177 push_lock = threading.Lock()
178
179@@ -950,9 +950,11 @@
180
181 def main(package, push=True, check=False, extra_debian=None, no_existing=False,
182 keep_temp=False, local_branches=False, persistent_download_cache=False):
183- if not os.path.exists(icommon.logs_dir):
184- os.makedirs(icommon.logs_dir)
185- log_name = os.path.join(icommon.logs_dir, package)
186+ conf = iconfig.Iconfig()
187+ log_dir = conf.get('pkgimport.packages.log_dir')
188+ if not os.path.exists(log_dir):
189+ os.makedirs(log_dir)
190+ log_name = os.path.join(log_dir, package)
191 f = open(log_name, "ab", 1)
192 log_token = trace.push_log_file(f)
193 try:
194
195=== modified file 'importer.crontab'
196--- importer.crontab 2011-02-01 17:56:12 +0000
197+++ importer.crontab 2011-03-24 08:28:41 +0000
198@@ -1,8 +1,11 @@
199 MAILTO=james.westby@canonical.com,canonical-bazaar@lists.canonical.com
200+# FIXME: This duplicates some config option in pkgimport.conf -- vila 2011-03023
201+SCRIPTS_DIR=/srv/package-import.canonical.com/new/scripts
202+PYTHONPATH=/srv/package-import.canonical.com/new/scripts/python-debian
203 # m h dom mon dow command
204-17 * * * * /usr/sbin/logrotate -s /srv/package-import.canonical.com/new/logrotate.state /srv/package-import.canonical.com/new/scripts/importer.logrotate
205-*/5 * * * * PYTHONPATH=/srv/package-import.canonical.com/new/scripts/python-debian /usr/bin/python /srv/package-import.canonical.com/new/scripts/categorise_failures.py
206-*/5 * * * * PYTHONPATH=/srv/package-import.canonical.com/new/scripts/python-debian /usr/bin/python /srv/package-import.canonical.com/new/scripts/add_import_jobs.py
207-17 07 * * * PYTHONPATH=/srv/package-import.canonical.com/new/scripts/python-debian /usr/bin/python /srv/package-import.canonical.com/new/scripts/list_packages.py
208-30 03 * * * PYTHONPATH=/srv/package-import.canonical.com/new/scripts/python-debian /usr/bin/python /srv/package-import.canonical.com/new/scripts/email_failures.py
209-21 * * * * PYTHONPATH=/srv/package-import.canonical.com/new/scripts/python-debian /usr/bin/python /srv/package-import.canonical.com/new/scripts/graph_failures.py
210+17 * * * * /usr/bin/python ${SCRIPTS_DIR}/logrotate.py
211+*/5 * * * * /usr/bin/python ${SCRIPTS_DIR}/categorise_failures.py
212+*/5 * * * * /usr/bin/python ${SCRIPTS_DIR}/add_import_jobs.py
213+17 07 * * * /usr/bin/python ${SCRIPTS_DIR}/list_packages.py
214+30 03 * * * /usr/bin/python ${SCRIPTS_DIR}/email_failures.py
215+21 * * * * /usr/bin/python ${SCRIPTS_DIR}/graph_failures.py
216
217=== removed file 'importer.logrotate'
218--- importer.logrotate 2009-09-09 12:26:17 +0000
219+++ importer.logrotate 1970-01-01 00:00:00 +0000
220@@ -1,17 +0,0 @@
221-/srv/package-import.canonical.com/new/debug_log {
222- daily
223- rotate 7
224- delaycompress
225- compress
226- notifempty
227- missingok
228-}
229-
230-/srv/package-import.canonical.com/new/progress_log {
231- daily
232- rotate 28
233- delaycompress
234- compress
235- notifempty
236- missingok
237-}
238
239=== added file 'logrotate.py'
240--- logrotate.py 1970-01-01 00:00:00 +0000
241+++ logrotate.py 2011-03-24 08:28:41 +0000
242@@ -0,0 +1,53 @@
243+#!/usr/bin/python
244+"""Invoke logrotate for the package importer taking configuration into account.
245+"""
246+
247+import os
248+import subprocess
249+import sys
250+
251+import iconfig
252+
253+logrotate_configuration='''%s {
254+ daily
255+ rotate 7
256+ delaycompress
257+ compress
258+ notifempty
259+ missingok
260+}
261+
262+%s {
263+ daily
264+ rotate 28
265+ delaycompress
266+ compress
267+ notifempty
268+ missingok
269+}
270+'''
271+
272+def main():
273+ conf = iconfig.Iconfig()
274+ debug_log = conf.get('pkgimport.driver.log.debug')
275+ progress_log = conf.get('pkgimport.driver.log.progress')
276+
277+ log_dir = conf.get('pkgimport.driver.log_dir')
278+ lrot_conf_path = os.path.join(log_dir, 'logrotate.conf')
279+ # We let the open fail if the directory doesn't exist, either there is no
280+ # log files or the setup is misconfigured anyway.
281+ lrot_conf = open(lrot_conf_path, 'w')
282+ try:
283+ # FIXME: Another use case for config.expand_options -- vila 2011-03-24
284+ lrot_conf.write(logrotate_configuration % (debug_log, progress_log))
285+ finally:
286+ lrot_conf.close()
287+
288+ lrot_state_path = os.path.join(log_dir, 'logrotate.state')
289+ retcode = subprocess.call(['/usr/sbin/logrotate', '-s', lrot_state_path,
290+ lrot_conf_path])
291+ return retcode
292+
293+
294+if __name__ == '__main__':
295+ sys.exit(main())
296
297=== modified file 'mass_import.py'
298--- mass_import.py 2011-03-23 12:21:27 +0000
299+++ mass_import.py 2011-03-24 08:28:41 +0000
300@@ -15,8 +15,8 @@
301
302 from launchpadlib.errors import HTTPError
303
304-sys.path.insert(0, os.path.dirname(__file__))
305 import icommon
306+import iconfig
307
308 #import httplib2
309 #httplib2.debuglevel = 1
310@@ -86,23 +86,35 @@
311 logging.FileHandler.emit(self, record)
312
313
314-logger = logging.getLogger(__name__)
315-logger.setLevel(logging.DEBUG)
316-
317-debug_handler = WatchedFileHandler(icommon.debug_log_file)
318-progress_handler = WatchedFileHandler(icommon.progress_log_file)
319-
320-debug_handler.setLevel(logging.DEBUG)
321-progress_handler.setLevel(logging.INFO)
322-
323-formatter = logging.Formatter("%(asctime)s - %(name)s - %(levelname)s - %(message)s")
324-
325-debug_handler.setFormatter(formatter)
326-progress_handler.setFormatter(formatter)
327-
328-logger.addHandler(debug_handler)
329-logger.addHandler(progress_handler)
330-
331+def create_logger():
332+ logger = logging.getLogger(__name__)
333+ logger.setLevel(logging.DEBUG)
334+
335+ conf = iconfig.Iconfig()
336+ log_debug_path = conf.get('pkgimport.driver.log.debug')
337+ log_progress_path = conf.get('pkgimport.driver.log.progress')
338+ for p in (log_debug_path, log_progress_path):
339+ dir_path = os.path.dirname(p)
340+ if not os.path.exists(dir_path):
341+ os.makedirs(dir_path)
342+
343+ debug_handler = WatchedFileHandler(log_debug_path)
344+ progress_handler = WatchedFileHandler(log_progress_path)
345+
346+ debug_handler.setLevel(logging.DEBUG)
347+ progress_handler.setLevel(logging.INFO)
348+
349+ formatter = logging.Formatter(
350+ '%(asctime)s - %(name)s - %(levelname)s - %(message)s')
351+
352+ debug_handler.setFormatter(formatter)
353+ progress_handler.setFormatter(formatter)
354+
355+ logger.addHandler(debug_handler)
356+ logger.addHandler(progress_handler)
357+ return logger
358+
359+logger = create_logger()
360 logger.info("Starting up")
361 # Some context to ease debug
362 logger.info("PATH: %s" % os.environ.get('PATH', None))
363@@ -349,10 +361,12 @@
364 logger.info("Driver finished: stopping")
365 sys.exit(1)
366
367-
368+# Install signal handlers
369 signal.signal(signal.SIGINT, die)
370 signal.signal(signal.SIGTERM, die)
371
372+
373+# Run the controller
374 try:
375 controller.run()
376 except HTTPError, e:
377
378=== added file 'pkgimport.conf'
379--- pkgimport.conf 1970-01-01 00:00:00 +0000
380+++ pkgimport.conf 2011-03-24 08:28:41 +0000
381@@ -0,0 +1,18 @@
382+# The package importer working directory (note that, currently,
383+# many other config options use this hard-coded value to build
384+# their own, once bzr > 2.4~ is available, we may use the
385+# expand_options feature to define it once and refer to it in
386+# other options) -- vila 2011-03-22
387+pkgimport.base_dir = /srv/package-import.canonical.com/new
388+pkgimport.log_dir = /srv/package-import.canonical.com/new/logs
389+pkgimport.driver.log_dir = /srv/package-import.canonical.com/new/logs/driver
390+pkgimport.driver.log.debug = /srv/package-import.canonical.com/new/logs/driver/debug_log
391+pkgimport.driver.log.progress = /srv/package-import.canonical.com/new/logs/driver/progress_log
392+pkgimport.packages.log_dir = /srv/package-import.canonical.com/new/logs/packages
393+# i.e. the following will be supported
394+# pkgimport.base_dir = /srv/package-import.canonical.com/new
395+# pkgimport.log_dir = {pkgimport.base_dir}/logs
396+# pkgimport.driver.log_dir = {pkgimport.log_dir}/driver
397+# pkgimport.driver.log.debug = {pkgimport.driver.log_dir}/debug_log
398+# pkgimport.driver.log.progress = {pkgimport.driver.log_dir}/progress_log
399+
400
401=== modified file 'tests.py'
402--- tests.py 2011-03-18 07:40:09 +0000
403+++ tests.py 2011-03-24 08:28:41 +0000
404@@ -10,6 +10,7 @@
405 import unittest
406
407 from bzrlib import (
408+ osutils,
409 tests,
410 transform,
411 )
412@@ -20,9 +21,13 @@
413 except ImportError:
414 from debian_bundle import changelog
415
416+<<<<<<< TREE
417
418 sys.path.insert(0, os.path.dirname(__file__))
419+=======
420+>>>>>>> MERGE-SOURCE
421 import icommon
422+import iconfig
423 import import_package
424
425 base_dir = os.path.abspath(os.path.dirname(__file__))
426@@ -902,6 +907,61 @@
427 pass
428
429
430+class TestConfigPath(tests.TestCase):
431+
432+ def setUp(self):
433+ super(TestConfigPath, self).setUp()
434+ self.mydir = osutils.dirname(osutils.realpath(__file__))
435+
436+ def test_default_path(self):
437+ # Ensure the iconfig finds the right path
438+ self.assertEquals(self.mydir, iconfig._scripts_dir)
439+
440+ def test_iconfig_file_name(self):
441+ conf_path = iconfig.iconfig_file_name()
442+ self.assertEquals(self.mydir, osutils.dirname(conf_path))
443+ self.assertEndsWith(conf_path, 'pkgimport.conf')
444+
445+
446+class TestIconfig(tests.TestCaseInTempDir):
447+
448+ def setUp(self):
449+ super(TestIconfig, self).setUp()
450+ self.conf_file_name = osutils.pathjoin(self.test_dir, 'pkgimport.conf')
451+
452+ def test_simple_option(self):
453+ conf = iconfig.Iconfig.from_string(
454+ '''pkgimport.base_dir=/foo/bar''',
455+ file_name=self.conf_file_name, save=True)
456+ self.assertEquals('/foo/bar', conf.get('pkgimport.base_dir'))
457+
458+
459+class TestDefaultIconfig(tests.TestCase):
460+
461+ def setUp(self):
462+ super(TestDefaultIconfig, self).setUp()
463+ self.conf = iconfig.Iconfig()
464+
465+ def test_default_config(self):
466+ self.assertEquals('pkgimport', self.conf.config_id())
467+ self.assertEquals(iconfig.iconfig_file_name(), self.conf.file_name)
468+
469+ def test_default_basedir(self):
470+ self.assertEquals('/srv/package-import.canonical.com/new',
471+ self.conf.get('pkgimport.base_dir'))
472+
473+ def assertOptiondIsDefined(self, name):
474+ self.assertIsNot(None, self.conf.get(name))
475+
476+ def test_required_options(self):
477+ self.assertOptiondIsDefined('pkgimport.base_dir')
478+ self.assertOptiondIsDefined('pkgimport.log_dir')
479+ self.assertOptiondIsDefined('pkgimport.driver.log_dir')
480+ self.assertOptiondIsDefined('pkgimport.driver.log.debug')
481+ self.assertOptiondIsDefined('pkgimport.driver.log.process')
482+ self.assertOptiondIsDefined('pkgimport.packages.log_dir')
483+
484+
485 if __name__ == '__main__':
486 unittest.TestProgram(module=__name__)
487

Subscribers

People subscribed via source and target branches