Launchpad itself

Merge lp:~stub/launchpad/cronscripts into lp:launchpad

  1. cronscripts
  2. Merge into devel
Proposed by Stuart Bishop
Status: Merged
Approved by: Stuart Bishop
Approved revision: no longer in the source branch.
Merged at revision: 11395
Proposed branch: lp:~stub/launchpad/cronscripts
Merge into: lp:launchpad
Diff against target: 459 lines (+264/-14)
14 files modified
configs/testrunner/launchpad-lazr.conf (+1/-0)
lib/canonical/config/schema-lazr.conf (+5/-1)
lib/lp/bugs/doc/bugnotification-sending.txt (+1/-1)
lib/lp/bugs/doc/checkwatches.txt (+1/-1)
lib/lp/bugs/doc/sourceforge-remote-products.txt (+1/-1)
lib/lp/registry/doc/teammembership.txt (+1/-0)
lib/lp/services/scripts/base.py (+63/-3)
lib/lp/services/scripts/tests/cronscripts.ini (+2/-0)
lib/lp/services/scripts/tests/example-cronscript.py (+30/-0)
lib/lp/services/scripts/tests/test_cronscript_enabled.py (+144/-0)
lib/lp/soyuz/doc/buildd-slavescanner.txt (+2/-2)
lib/lp/soyuz/scripts/tests/test_processupload.py (+6/-5)
lib/lp/testing/tests/test_standard_test_template.py (+6/-0)
lib/lp/translations/doc/poexport-request.txt (+1/-0)
To merge this branch: bzr merge lp:~stub/launchpad/cronscripts
Reviewer Review Type Date Requested Status
Abel Deuring (community) code Approve
Review via email: mp+31934@code.launchpad.net

Commit message

Enable and disable cronscripts using a configuration file.

Description of the change

Steps towards Bug #607391.

Adds a config file controlling cronscripts. This config file allows them to be selectively or in bulk disabled from running.

To post a comment you must log in.
Revision history for this message
Abel Deuring (adeuring) wrote :

looks good

review: Approve (code)
Revision history for this message
Robert Collins (lifeless) wrote :

Thanks for working on this Stuart, its key to making rollouts safer
and more reliable.

I did have a small question though; our conf system is kindof like an
ini file already; how does using a regular ini file from within it
help?

-Rob

Revision history for this message
Stuart Bishop (stub) wrote :

On Fri, Aug 20, 2010 at 12:28 AM, Robert Collins
<email address hidden> wrote:
> Thanks for working on this Stuart, its key to making rollouts safer
> and more reliable.
>
> I did have a small question though; our conf system is kindof like an
> ini file already; how does using a regular ini file from within it
> help?

Currently, altering our existing config files is a heavyweight
operation. If we can fix this operational issue, we could collapse the
cron .ini file into the main config. I just took a lightweight
approach that we can expand on (scheduling shutdowns requested in the
bug for instance). I suspect we will stick with the .ini approach
until move to a zookeeper type system to control runtime
configuration.

The alternative I considered was using a separate PostgreSQL
configuration database, but after discussion with Gary we decided even
a separate PostgreSQL database doesn't provide the robustness we want
(and is probably NIH other zookeeper type systems).

--
Stuart Bishop <email address hidden>
http://www.stuartbishop.net/

Revision history for this message
Robert Collins (lifeless) wrote :

I'm ok with an ini file; but would like to note that changing our
config is -only- heavyweight because we've chosen to have it so.

We can, in principle, have another config file referenced just like
you are here; I don't particularly care for that though.

I don't like the idea of another pg db either; zookeepr or something
may be fairly far away.

So I'm +0.5 or something on the ini; I think its a shame to have
*another* mechanism in play, but I'm keen on getting this work out
here; I think we should consider the presence of the two config
systems a techdebt bug and file it as such though.

-Rob

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'configs/testrunner/launchpad-lazr.conf'
2--- configs/testrunner/launchpad-lazr.conf 2010-07-16 20:55:29 +0000
3+++ configs/testrunner/launchpad-lazr.conf 2010-08-19 09:52:48 +0000
4@@ -7,6 +7,7 @@
5
6 [canonical]
7 chunkydiff: False
8+cron_control_file: lib/lp/services/scripts/tests/cronscripts.ini
9
10 [archivepublisher]
11 base_url: http://ftpmaster.internal/
12
13=== modified file 'lib/canonical/config/schema-lazr.conf'
14--- lib/canonical/config/schema-lazr.conf 2010-08-18 17:51:27 +0000
15+++ lib/canonical/config/schema-lazr.conf 2010-08-19 09:52:48 +0000
16@@ -209,6 +209,10 @@
17 # datatype: string
18 admin_address: system-error@launchpad.net
19
20+# By default, relative to the root.
21+# datatype: filename
22+cron_control_file: cronscripts.ini
23+
24
25 [checkwatches]
26 # The database user to run this process as.
27@@ -796,7 +800,7 @@
28
29 # If true, only the source package names are imported into
30 # Launchpad
31-# datatype: boolean"
32+# datatype: boolean
33 sourcepackagenames_only: false
34
35
36
37=== modified file 'lib/lp/bugs/doc/bugnotification-sending.txt'
38--- lib/lp/bugs/doc/bugnotification-sending.txt 2010-08-16 13:50:28 +0000
39+++ lib/lp/bugs/doc/bugnotification-sending.txt 2010-08-19 09:52:48 +0000
40@@ -931,7 +931,7 @@
41 >>> process.returncode
42 0
43 >>> print err
44- INFO Creating lockfile: /var/lock/launchpad-send-bug-notifications.lock
45+ DEBUG ...
46 INFO Notifying mark@example.com about bug 2.
47 ...
48 INFO Notifying support@ubuntu.com about bug 2.
49
50=== modified file 'lib/lp/bugs/doc/checkwatches.txt'
51--- lib/lp/bugs/doc/checkwatches.txt 2010-08-02 17:48:13 +0000
52+++ lib/lp/bugs/doc/checkwatches.txt 2010-08-19 09:52:48 +0000
53@@ -44,7 +44,7 @@
54 0
55
56 >>> print err
57- INFO Creating lockfile: /var/lock/launchpad-checkwatches.lock
58+ DEBUG ...
59 DEBUG No global batch size specified.
60 DEBUG Skipping updating Ubuntu Bugzilla watches.
61 DEBUG No watches to update on http://bugs.debian.org
62
63=== modified file 'lib/lp/bugs/doc/sourceforge-remote-products.txt'
64--- lib/lp/bugs/doc/sourceforge-remote-products.txt 2010-05-18 22:36:20 +0000
65+++ lib/lp/bugs/doc/sourceforge-remote-products.txt 2010-08-19 09:52:48 +0000
66@@ -117,7 +117,7 @@
67 0
68
69 >>> print err
70- INFO Creating lockfile: /var/lock/launchpad-updateremoteproduct.lock
71+ DEBUG ...
72 INFO No Products to update.
73 INFO Time for this run: ... seconds.
74 DEBUG Removing lock file:...
75
76=== modified file 'lib/lp/registry/doc/teammembership.txt'
77--- lib/lp/registry/doc/teammembership.txt 2010-08-16 06:45:39 +0000
78+++ lib/lp/registry/doc/teammembership.txt 2010-08-19 09:52:48 +0000
79@@ -681,6 +681,7 @@
80
81 >>> print '\n'.join(
82 ... line for line in err.split('\n') if 'INFO' not in line)
83+ DEBUG ...
84 DEBUG Sent warning email to name16 in launchpad-buildd-admins team.
85 DEBUG Sent warning email to name12 in landscape-developers team.
86 DEBUG Removing lock file: /var/lock/launchpad-flag-expired-memberships.lock
87
88=== modified file 'lib/lp/services/scripts/base.py'
89--- lib/lp/services/scripts/base.py 2010-04-27 06:15:37 +0000
90+++ lib/lp/services/scripts/base.py 2010-08-19 09:52:48 +0000
91@@ -9,17 +9,19 @@
92 'SilentLaunchpadScriptFailure'
93 ]
94
95+from ConfigParser import SafeConfigParser
96 from cProfile import Profile
97 import datetime
98 import logging
99 from optparse import OptionParser
100-import os
101+import os.path
102 import sys
103
104 from contrib.glock import GlobalLock, LockAlreadyAcquired
105 import pytz
106 from zope.component import getUtility
107
108+from canonical.config import config
109 from canonical.database.sqlbase import ISOLATION_LEVEL_DEFAULT
110 from canonical.launchpad import scripts
111 from canonical.launchpad.interfaces import IScriptActivitySet
112@@ -185,8 +187,8 @@
113 def setup_lock(self):
114 """Create lockfile.
115
116- Note that this will create a lockfile even if you don't actually use it.
117- GlobalLock.__del__ is meant to clean it up though.
118+ Note that this will create a lockfile even if you don't actually
119+ use it. GlobalLock.__del__ is meant to clean it up though.
120 """
121 self.lock = GlobalLock(self.lockfilepath, logger=self.logger)
122
123@@ -290,6 +292,22 @@
124 class LaunchpadCronScript(LaunchpadScript):
125 """Logs successful script runs in the database."""
126
127+ def __init__(self, name=None, dbuser=None, test_args=None):
128+ """Initialize, and sys.exit() if the cronscript is disabled.
129+
130+ Rather than hand editing crontab files, cronscripts can be
131+ enabled and disabled using a config file.
132+
133+ The control file location is specified by
134+ config.canonical.cron_control_file.
135+ """
136+ super(LaunchpadCronScript, self).__init__(name, dbuser, test_args)
137+
138+ enabled = cronscript_enabled(
139+ config.canonical.cron_control_file, self.name, self.logger)
140+ if not enabled:
141+ sys.exit(0)
142+
143 def record_activity(self, date_started, date_completed):
144 """Record the successful completion of the script."""
145 self.txn.begin()
146@@ -299,3 +317,45 @@
147 date_started=date_started,
148 date_completed=date_completed)
149 self.txn.commit()
150+
151+
152+def cronscript_enabled(control_path, name, log):
153+ """Return True if the cronscript is enabled."""
154+ if not os.path.isabs(control_path):
155+ control_path = os.path.abspath(
156+ os.path.join(config.root, control_path))
157+
158+ cron_config = SafeConfigParser({'enabled': str(True)})
159+
160+ if not os.path.exists(control_path):
161+ # No control file exists. Everything enabled by default.
162+ log.debug("Cronscript control file not found at %s", control_path)
163+ else:
164+ log.debug("Cronscript control file found at %s", control_path)
165+
166+ # Try reading the config file. If it fails, we log the
167+ # traceback and continue on using the defaults.
168+ try:
169+ cron_config.read(control_path)
170+ except:
171+ log.exception("Error parsing %s", control_path)
172+
173+ if cron_config.has_option(name, 'enabled'):
174+ section = name
175+ else:
176+ section = 'DEFAULT'
177+
178+ try:
179+ enabled = cron_config.getboolean(section, 'enabled')
180+ except:
181+ log.exception(
182+ "Failed to load value from %s section of %s",
183+ section, control_path)
184+ enabled = True
185+
186+ if enabled:
187+ log.debug("Enabled by %s section", section)
188+ else:
189+ log.info("Disabled by %s section", section)
190+
191+ return enabled
192
193=== added file 'lib/lp/services/scripts/tests/cronscripts.ini'
194--- lib/lp/services/scripts/tests/cronscripts.ini 1970-01-01 00:00:00 +0000
195+++ lib/lp/services/scripts/tests/cronscripts.ini 2010-08-19 09:52:48 +0000
196@@ -0,0 +1,2 @@
197+[example-cronscript-disabled]
198+enabled: False
199
200=== added file 'lib/lp/services/scripts/tests/example-cronscript.py'
201--- lib/lp/services/scripts/tests/example-cronscript.py 1970-01-01 00:00:00 +0000
202+++ lib/lp/services/scripts/tests/example-cronscript.py 2010-08-19 09:52:48 +0000
203@@ -0,0 +1,30 @@
204+# Copyright 2010 Canonical Ltd. This software is licensed under the
205+# GNU Affero General Public License version 3 (see the file LICENSE).
206+
207+"""An example cronscript. If it runs, it returns 42 as its return code."""
208+
209+__metaclass__ = type
210+__all__ = []
211+
212+import sys
213+
214+from lp.services.scripts.base import (
215+ LaunchpadCronScript, SilentLaunchpadScriptFailure)
216+
217+class Script(LaunchpadCronScript):
218+ def main(self):
219+ if self.name == 'example-cronscript-enabled':
220+ raise SilentLaunchpadScriptFailure(42)
221+ else:
222+ # Raise a non-standard error code, as if the
223+ # script was invoked as disabled the main()
224+ # method should never be invoked.
225+ raise SilentLaunchpadScriptFailure(999)
226+
227+if __name__ == '__main__':
228+ if sys.argv[-1] == 'enabled':
229+ name = 'example-cronscript-enabled'
230+ else:
231+ name = 'example-cronscript-disabled'
232+ script = Script(name)
233+ script.lock_and_run()
234
235=== added file 'lib/lp/services/scripts/tests/test_cronscript_enabled.py'
236--- lib/lp/services/scripts/tests/test_cronscript_enabled.py 1970-01-01 00:00:00 +0000
237+++ lib/lp/services/scripts/tests/test_cronscript_enabled.py 2010-08-19 09:52:48 +0000
238@@ -0,0 +1,144 @@
239+# Copyright 2010 Canonical Ltd. This software is licensed under the
240+# GNU Affero General Public License version 3 (see the file LICENSE).
241+
242+"""Test the cronscript_enabled function in scripts/base.py."""
243+
244+__metaclass__ = type
245+
246+from cStringIO import StringIO
247+from logging import DEBUG
248+import os.path
249+import subprocess
250+import sys
251+from tempfile import NamedTemporaryFile
252+from textwrap import dedent
253+import unittest
254+
255+from lp.services.scripts.base import cronscript_enabled
256+from lp.testing import TestCase
257+from lp.testing.logger import MockLogger
258+
259+
260+class TestCronscriptEnabled(TestCase):
261+ def setUp(self):
262+ super(TestCronscriptEnabled, self).setUp()
263+ self.log_output = StringIO()
264+ self.log = MockLogger(self.log_output)
265+ self.log.setLevel(DEBUG)
266+
267+ def makeConfig(self, body):
268+ tempfile = NamedTemporaryFile(suffix='.ini')
269+ tempfile.write(body)
270+ tempfile.flush()
271+ # Ensure a reference is kept until the test is over.
272+ # tempfile will then clean itself up.
273+ self.addCleanup(lambda x: None, tempfile)
274+ return tempfile.name
275+
276+ def test_noconfig(self):
277+ enabled = cronscript_enabled('/idontexist.ini', 'foo', self.log)
278+ self.assertIs(True, enabled)
279+
280+ def test_emptyconfig(self):
281+ config = self.makeConfig('')
282+ enabled = cronscript_enabled(config, 'foo', self.log)
283+ self.assertIs(True, enabled)
284+
285+ def test_default_true(self):
286+ config = self.makeConfig(dedent("""\
287+ [DEFAULT]
288+ enabled: True
289+ """))
290+ enabled = cronscript_enabled(config, 'foo', self.log)
291+ self.assertIs(True, enabled)
292+
293+ def test_default_false(self):
294+ config = self.makeConfig(dedent("""\
295+ [DEFAULT]
296+ enabled: False
297+ """))
298+ enabled = cronscript_enabled(config, 'foo', self.log)
299+ self.assertIs(False, enabled)
300+
301+ def test_specific_true(self):
302+ config = self.makeConfig(dedent("""\
303+ [DEFAULT]
304+ enabled: False
305+ [foo]
306+ enabled: True
307+ """))
308+ enabled = cronscript_enabled(config, 'foo', self.log)
309+ self.assertIs(True, enabled)
310+
311+ def test_specific_false(self):
312+ config = self.makeConfig(dedent("""\
313+ [DEFAULT]
314+ enabled: True
315+ [foo]
316+ enabled: False
317+ """))
318+ enabled = cronscript_enabled(config, 'foo', self.log)
319+ self.assertIs(False, enabled)
320+
321+ def test_broken_true(self):
322+ config = self.makeConfig(dedent("""\
323+ # This file is unparsable
324+ [DEFAULT
325+ enabled: False
326+ [foo
327+ enabled: False
328+ """))
329+ enabled = cronscript_enabled(config, 'foo', self.log)
330+ self.assertIs(True, enabled)
331+
332+ def test_invalid_boolean_true(self):
333+ config = self.makeConfig(dedent("""\
334+ [DEFAULT]
335+ enabled: whoops
336+ """))
337+ enabled = cronscript_enabled(config, 'foo', self.log)
338+ self.assertIs(True, enabled)
339+
340+ def test_specific_missing_fallsback(self):
341+ config = self.makeConfig(dedent("""\
342+ [DEFAULT]
343+ enabled: False
344+ [foo]
345+ # There is a typo in the next line.
346+ enobled: True
347+ """))
348+ enabled = cronscript_enabled(config, 'foo', self.log)
349+ self.assertIs(False, enabled)
350+
351+ def test_default_missing_fallsback(self):
352+ config = self.makeConfig(dedent("""\
353+ [DEFAULT]
354+ # There is a typo in the next line. Fallsback to hardcoded
355+ # default.
356+ enobled: False
357+ [foo]
358+ # There is a typo in the next line.
359+ enobled: False
360+ """))
361+ enabled = cronscript_enabled(config, 'foo', self.log)
362+ self.assertIs(True, enabled)
363+
364+ def test_enabled_cronscript(self):
365+ cmd = [
366+ sys.executable,
367+ os.path.join(os.path.dirname(__file__), 'example-cronscript.py'),
368+ '-qqqqq', 'enabled',
369+ ]
370+ self.assertEqual(42, subprocess.call(cmd))
371+
372+ def test_disabled_cronscript(self):
373+ cmd = [
374+ sys.executable,
375+ os.path.join(os.path.dirname(__file__), 'example-cronscript.py'),
376+ '-qqqqq', 'disabled',
377+ ]
378+ self.assertEqual(0, subprocess.call(cmd))
379+
380+
381+def test_suite():
382+ return unittest.TestLoader().loadTestsFromName(__name__)
383
384=== modified file 'lib/lp/soyuz/doc/buildd-slavescanner.txt'
385--- lib/lp/soyuz/doc/buildd-slavescanner.txt 2010-06-02 16:32:10 +0000
386+++ lib/lp/soyuz/doc/buildd-slavescanner.txt 2010-08-19 09:52:48 +0000
387@@ -382,7 +382,7 @@
388 * Build Log: http://.../...i386.mozilla-firefox_0.9_BUILDING.txt.gz
389 ...
390 Upload log:
391- INFO Creating lockfile:...
392+ DEBUG ...
393 DEBUG Initialising connection.
394 ...
395 DEBUG Removing lock file: /var/lock/process-upload-buildd.lock
396@@ -625,7 +625,7 @@
397 ... 'uploader.log'))
398
399 >>> print uploader_log.read()
400- INFO Creating lockfile:...
401+ DEBUG ...
402 DEBUG Initialising connection.
403 DEBUG Beginning processing
404 DEBUG Creating directory /var/tmp/builddmaster/accepted
405
406=== modified file 'lib/lp/soyuz/scripts/tests/test_processupload.py'
407--- lib/lp/soyuz/scripts/tests/test_processupload.py 2010-07-20 12:06:36 +0000
408+++ lib/lp/soyuz/scripts/tests/test_processupload.py 2010-08-19 09:52:48 +0000
409@@ -91,11 +91,12 @@
410 # the process-upload call terminated with ERROR and
411 # proper log message
412 self.assertEqual(1, returncode)
413- self.assertEqual([
414- ('INFO Creating lockfile: '
415- '/var/lock/process-upload-insecure.lock'),
416- 'DEBUG Lockfile /var/lock/process-upload-insecure.lock in use',
417- ], err.splitlines())
418+ self.assert_(
419+ 'INFO Creating lockfile: '
420+ '/var/lock/process-upload-insecure.lock' in err.splitlines())
421+ self.assert_(
422+ 'DEBUG Lockfile /var/lock/process-upload-insecure.lock in use'
423+ in err.splitlines())
424
425 # release the locally acquired lockfile
426 locker.release()
427
428=== modified file 'lib/lp/testing/tests/test_standard_test_template.py'
429--- lib/lp/testing/tests/test_standard_test_template.py 2010-07-17 21:13:21 +0000
430+++ lib/lp/testing/tests/test_standard_test_template.py 2010-08-19 09:52:48 +0000
431@@ -5,6 +5,8 @@
432
433 __metaclass__ = type
434
435+import unittest
436+
437 from canonical.testing import DatabaseFunctionalLayer
438 from lp.testing import TestCase
439
440@@ -22,3 +24,7 @@
441
442 # XXX: Assertions take expected value first, actual value second.
443 self.assertEqual(4, 2 + 2)
444+
445+
446+def test_suite():
447+ return unittest.TestLoader().loadTestsFromName(__name__)
448
449=== modified file 'lib/lp/translations/doc/poexport-request.txt'
450--- lib/lp/translations/doc/poexport-request.txt 2010-07-22 10:14:53 +0000
451+++ lib/lp/translations/doc/poexport-request.txt 2010-08-19 09:52:48 +0000
452@@ -269,6 +269,7 @@
453 ... )
454 >>> (output, empty) = process.communicate()
455 >>> print output
456+ DEBUG ...
457 INFO Creating lockfile: /var/lock/launchpad-rosetta-export-queue.lock
458 DEBUG Exporting objects for Happy Downloader, related to template
459 evolution-2.2 in Evolution trunk