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

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
=== modified file 'configs/testrunner/launchpad-lazr.conf'
--- configs/testrunner/launchpad-lazr.conf 2010-07-16 20:55:29 +0000
+++ configs/testrunner/launchpad-lazr.conf 2010-08-19 09:52:48 +0000
@@ -7,6 +7,7 @@
77
8[canonical]8[canonical]
9chunkydiff: False9chunkydiff: False
10cron_control_file: lib/lp/services/scripts/tests/cronscripts.ini
1011
11[archivepublisher]12[archivepublisher]
12base_url: http://ftpmaster.internal/13base_url: http://ftpmaster.internal/
1314
=== modified file 'lib/canonical/config/schema-lazr.conf'
--- lib/canonical/config/schema-lazr.conf 2010-08-18 17:51:27 +0000
+++ lib/canonical/config/schema-lazr.conf 2010-08-19 09:52:48 +0000
@@ -209,6 +209,10 @@
209# datatype: string209# datatype: string
210admin_address: system-error@launchpad.net210admin_address: system-error@launchpad.net
211211
212# By default, relative to the root.
213# datatype: filename
214cron_control_file: cronscripts.ini
215
212216
213[checkwatches]217[checkwatches]
214# The database user to run this process as.218# The database user to run this process as.
@@ -796,7 +800,7 @@
796800
797# If true, only the source package names are imported into801# If true, only the source package names are imported into
798# Launchpad802# Launchpad
799# datatype: boolean"803# datatype: boolean
800sourcepackagenames_only: false804sourcepackagenames_only: false
801805
802806
803807
=== modified file 'lib/lp/bugs/doc/bugnotification-sending.txt'
--- lib/lp/bugs/doc/bugnotification-sending.txt 2010-08-16 13:50:28 +0000
+++ lib/lp/bugs/doc/bugnotification-sending.txt 2010-08-19 09:52:48 +0000
@@ -931,7 +931,7 @@
931 >>> process.returncode931 >>> process.returncode
932 0932 0
933 >>> print err933 >>> print err
934 INFO Creating lockfile: /var/lock/launchpad-send-bug-notifications.lock934 DEBUG ...
935 INFO Notifying mark@example.com about bug 2.935 INFO Notifying mark@example.com about bug 2.
936 ...936 ...
937 INFO Notifying support@ubuntu.com about bug 2.937 INFO Notifying support@ubuntu.com about bug 2.
938938
=== modified file 'lib/lp/bugs/doc/checkwatches.txt'
--- lib/lp/bugs/doc/checkwatches.txt 2010-08-02 17:48:13 +0000
+++ lib/lp/bugs/doc/checkwatches.txt 2010-08-19 09:52:48 +0000
@@ -44,7 +44,7 @@
44 044 0
4545
46 >>> print err46 >>> print err
47 INFO Creating lockfile: /var/lock/launchpad-checkwatches.lock47 DEBUG ...
48 DEBUG No global batch size specified.48 DEBUG No global batch size specified.
49 DEBUG Skipping updating Ubuntu Bugzilla watches.49 DEBUG Skipping updating Ubuntu Bugzilla watches.
50 DEBUG No watches to update on http://bugs.debian.org50 DEBUG No watches to update on http://bugs.debian.org
5151
=== modified file 'lib/lp/bugs/doc/sourceforge-remote-products.txt'
--- lib/lp/bugs/doc/sourceforge-remote-products.txt 2010-05-18 22:36:20 +0000
+++ lib/lp/bugs/doc/sourceforge-remote-products.txt 2010-08-19 09:52:48 +0000
@@ -117,7 +117,7 @@
117 0117 0
118118
119 >>> print err119 >>> print err
120 INFO Creating lockfile: /var/lock/launchpad-updateremoteproduct.lock120 DEBUG ...
121 INFO No Products to update.121 INFO No Products to update.
122 INFO Time for this run: ... seconds.122 INFO Time for this run: ... seconds.
123 DEBUG Removing lock file:...123 DEBUG Removing lock file:...
124124
=== modified file 'lib/lp/registry/doc/teammembership.txt'
--- lib/lp/registry/doc/teammembership.txt 2010-08-16 06:45:39 +0000
+++ lib/lp/registry/doc/teammembership.txt 2010-08-19 09:52:48 +0000
@@ -681,6 +681,7 @@
681681
682 >>> print '\n'.join(682 >>> print '\n'.join(
683 ... line for line in err.split('\n') if 'INFO' not in line)683 ... line for line in err.split('\n') if 'INFO' not in line)
684 DEBUG ...
684 DEBUG Sent warning email to name16 in launchpad-buildd-admins team.685 DEBUG Sent warning email to name16 in launchpad-buildd-admins team.
685 DEBUG Sent warning email to name12 in landscape-developers team.686 DEBUG Sent warning email to name12 in landscape-developers team.
686 DEBUG Removing lock file: /var/lock/launchpad-flag-expired-memberships.lock687 DEBUG Removing lock file: /var/lock/launchpad-flag-expired-memberships.lock
687688
=== modified file 'lib/lp/services/scripts/base.py'
--- lib/lp/services/scripts/base.py 2010-04-27 06:15:37 +0000
+++ lib/lp/services/scripts/base.py 2010-08-19 09:52:48 +0000
@@ -9,17 +9,19 @@
9 'SilentLaunchpadScriptFailure'9 'SilentLaunchpadScriptFailure'
10 ]10 ]
1111
12from ConfigParser import SafeConfigParser
12from cProfile import Profile13from cProfile import Profile
13import datetime14import datetime
14import logging15import logging
15from optparse import OptionParser16from optparse import OptionParser
16import os17import os.path
17import sys18import sys
1819
19from contrib.glock import GlobalLock, LockAlreadyAcquired20from contrib.glock import GlobalLock, LockAlreadyAcquired
20import pytz21import pytz
21from zope.component import getUtility22from zope.component import getUtility
2223
24from canonical.config import config
23from canonical.database.sqlbase import ISOLATION_LEVEL_DEFAULT25from canonical.database.sqlbase import ISOLATION_LEVEL_DEFAULT
24from canonical.launchpad import scripts26from canonical.launchpad import scripts
25from canonical.launchpad.interfaces import IScriptActivitySet27from canonical.launchpad.interfaces import IScriptActivitySet
@@ -185,8 +187,8 @@
185 def setup_lock(self):187 def setup_lock(self):
186 """Create lockfile.188 """Create lockfile.
187189
188 Note that this will create a lockfile even if you don't actually use it.190 Note that this will create a lockfile even if you don't actually
189 GlobalLock.__del__ is meant to clean it up though.191 use it. GlobalLock.__del__ is meant to clean it up though.
190 """192 """
191 self.lock = GlobalLock(self.lockfilepath, logger=self.logger)193 self.lock = GlobalLock(self.lockfilepath, logger=self.logger)
192194
@@ -290,6 +292,22 @@
290class LaunchpadCronScript(LaunchpadScript):292class LaunchpadCronScript(LaunchpadScript):
291 """Logs successful script runs in the database."""293 """Logs successful script runs in the database."""
292294
295 def __init__(self, name=None, dbuser=None, test_args=None):
296 """Initialize, and sys.exit() if the cronscript is disabled.
297
298 Rather than hand editing crontab files, cronscripts can be
299 enabled and disabled using a config file.
300
301 The control file location is specified by
302 config.canonical.cron_control_file.
303 """
304 super(LaunchpadCronScript, self).__init__(name, dbuser, test_args)
305
306 enabled = cronscript_enabled(
307 config.canonical.cron_control_file, self.name, self.logger)
308 if not enabled:
309 sys.exit(0)
310
293 def record_activity(self, date_started, date_completed):311 def record_activity(self, date_started, date_completed):
294 """Record the successful completion of the script."""312 """Record the successful completion of the script."""
295 self.txn.begin()313 self.txn.begin()
@@ -299,3 +317,45 @@
299 date_started=date_started,317 date_started=date_started,
300 date_completed=date_completed)318 date_completed=date_completed)
301 self.txn.commit()319 self.txn.commit()
320
321
322def cronscript_enabled(control_path, name, log):
323 """Return True if the cronscript is enabled."""
324 if not os.path.isabs(control_path):
325 control_path = os.path.abspath(
326 os.path.join(config.root, control_path))
327
328 cron_config = SafeConfigParser({'enabled': str(True)})
329
330 if not os.path.exists(control_path):
331 # No control file exists. Everything enabled by default.
332 log.debug("Cronscript control file not found at %s", control_path)
333 else:
334 log.debug("Cronscript control file found at %s", control_path)
335
336 # Try reading the config file. If it fails, we log the
337 # traceback and continue on using the defaults.
338 try:
339 cron_config.read(control_path)
340 except:
341 log.exception("Error parsing %s", control_path)
342
343 if cron_config.has_option(name, 'enabled'):
344 section = name
345 else:
346 section = 'DEFAULT'
347
348 try:
349 enabled = cron_config.getboolean(section, 'enabled')
350 except:
351 log.exception(
352 "Failed to load value from %s section of %s",
353 section, control_path)
354 enabled = True
355
356 if enabled:
357 log.debug("Enabled by %s section", section)
358 else:
359 log.info("Disabled by %s section", section)
360
361 return enabled
302362
=== added file 'lib/lp/services/scripts/tests/cronscripts.ini'
--- lib/lp/services/scripts/tests/cronscripts.ini 1970-01-01 00:00:00 +0000
+++ lib/lp/services/scripts/tests/cronscripts.ini 2010-08-19 09:52:48 +0000
@@ -0,0 +1,2 @@
1[example-cronscript-disabled]
2enabled: False
03
=== added file 'lib/lp/services/scripts/tests/example-cronscript.py'
--- lib/lp/services/scripts/tests/example-cronscript.py 1970-01-01 00:00:00 +0000
+++ lib/lp/services/scripts/tests/example-cronscript.py 2010-08-19 09:52:48 +0000
@@ -0,0 +1,30 @@
1# Copyright 2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""An example cronscript. If it runs, it returns 42 as its return code."""
5
6__metaclass__ = type
7__all__ = []
8
9import sys
10
11from lp.services.scripts.base import (
12 LaunchpadCronScript, SilentLaunchpadScriptFailure)
13
14class Script(LaunchpadCronScript):
15 def main(self):
16 if self.name == 'example-cronscript-enabled':
17 raise SilentLaunchpadScriptFailure(42)
18 else:
19 # Raise a non-standard error code, as if the
20 # script was invoked as disabled the main()
21 # method should never be invoked.
22 raise SilentLaunchpadScriptFailure(999)
23
24if __name__ == '__main__':
25 if sys.argv[-1] == 'enabled':
26 name = 'example-cronscript-enabled'
27 else:
28 name = 'example-cronscript-disabled'
29 script = Script(name)
30 script.lock_and_run()
031
=== added file 'lib/lp/services/scripts/tests/test_cronscript_enabled.py'
--- lib/lp/services/scripts/tests/test_cronscript_enabled.py 1970-01-01 00:00:00 +0000
+++ lib/lp/services/scripts/tests/test_cronscript_enabled.py 2010-08-19 09:52:48 +0000
@@ -0,0 +1,144 @@
1# Copyright 2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).
3
4"""Test the cronscript_enabled function in scripts/base.py."""
5
6__metaclass__ = type
7
8from cStringIO import StringIO
9from logging import DEBUG
10import os.path
11import subprocess
12import sys
13from tempfile import NamedTemporaryFile
14from textwrap import dedent
15import unittest
16
17from lp.services.scripts.base import cronscript_enabled
18from lp.testing import TestCase
19from lp.testing.logger import MockLogger
20
21
22class TestCronscriptEnabled(TestCase):
23 def setUp(self):
24 super(TestCronscriptEnabled, self).setUp()
25 self.log_output = StringIO()
26 self.log = MockLogger(self.log_output)
27 self.log.setLevel(DEBUG)
28
29 def makeConfig(self, body):
30 tempfile = NamedTemporaryFile(suffix='.ini')
31 tempfile.write(body)
32 tempfile.flush()
33 # Ensure a reference is kept until the test is over.
34 # tempfile will then clean itself up.
35 self.addCleanup(lambda x: None, tempfile)
36 return tempfile.name
37
38 def test_noconfig(self):
39 enabled = cronscript_enabled('/idontexist.ini', 'foo', self.log)
40 self.assertIs(True, enabled)
41
42 def test_emptyconfig(self):
43 config = self.makeConfig('')
44 enabled = cronscript_enabled(config, 'foo', self.log)
45 self.assertIs(True, enabled)
46
47 def test_default_true(self):
48 config = self.makeConfig(dedent("""\
49 [DEFAULT]
50 enabled: True
51 """))
52 enabled = cronscript_enabled(config, 'foo', self.log)
53 self.assertIs(True, enabled)
54
55 def test_default_false(self):
56 config = self.makeConfig(dedent("""\
57 [DEFAULT]
58 enabled: False
59 """))
60 enabled = cronscript_enabled(config, 'foo', self.log)
61 self.assertIs(False, enabled)
62
63 def test_specific_true(self):
64 config = self.makeConfig(dedent("""\
65 [DEFAULT]
66 enabled: False
67 [foo]
68 enabled: True
69 """))
70 enabled = cronscript_enabled(config, 'foo', self.log)
71 self.assertIs(True, enabled)
72
73 def test_specific_false(self):
74 config = self.makeConfig(dedent("""\
75 [DEFAULT]
76 enabled: True
77 [foo]
78 enabled: False
79 """))
80 enabled = cronscript_enabled(config, 'foo', self.log)
81 self.assertIs(False, enabled)
82
83 def test_broken_true(self):
84 config = self.makeConfig(dedent("""\
85 # This file is unparsable
86 [DEFAULT
87 enabled: False
88 [foo
89 enabled: False
90 """))
91 enabled = cronscript_enabled(config, 'foo', self.log)
92 self.assertIs(True, enabled)
93
94 def test_invalid_boolean_true(self):
95 config = self.makeConfig(dedent("""\
96 [DEFAULT]
97 enabled: whoops
98 """))
99 enabled = cronscript_enabled(config, 'foo', self.log)
100 self.assertIs(True, enabled)
101
102 def test_specific_missing_fallsback(self):
103 config = self.makeConfig(dedent("""\
104 [DEFAULT]
105 enabled: False
106 [foo]
107 # There is a typo in the next line.
108 enobled: True
109 """))
110 enabled = cronscript_enabled(config, 'foo', self.log)
111 self.assertIs(False, enabled)
112
113 def test_default_missing_fallsback(self):
114 config = self.makeConfig(dedent("""\
115 [DEFAULT]
116 # There is a typo in the next line. Fallsback to hardcoded
117 # default.
118 enobled: False
119 [foo]
120 # There is a typo in the next line.
121 enobled: False
122 """))
123 enabled = cronscript_enabled(config, 'foo', self.log)
124 self.assertIs(True, enabled)
125
126 def test_enabled_cronscript(self):
127 cmd = [
128 sys.executable,
129 os.path.join(os.path.dirname(__file__), 'example-cronscript.py'),
130 '-qqqqq', 'enabled',
131 ]
132 self.assertEqual(42, subprocess.call(cmd))
133
134 def test_disabled_cronscript(self):
135 cmd = [
136 sys.executable,
137 os.path.join(os.path.dirname(__file__), 'example-cronscript.py'),
138 '-qqqqq', 'disabled',
139 ]
140 self.assertEqual(0, subprocess.call(cmd))
141
142
143def test_suite():
144 return unittest.TestLoader().loadTestsFromName(__name__)
0145
=== modified file 'lib/lp/soyuz/doc/buildd-slavescanner.txt'
--- lib/lp/soyuz/doc/buildd-slavescanner.txt 2010-06-02 16:32:10 +0000
+++ lib/lp/soyuz/doc/buildd-slavescanner.txt 2010-08-19 09:52:48 +0000
@@ -382,7 +382,7 @@
382 * Build Log: http://.../...i386.mozilla-firefox_0.9_BUILDING.txt.gz382 * Build Log: http://.../...i386.mozilla-firefox_0.9_BUILDING.txt.gz
383 ...383 ...
384 Upload log:384 Upload log:
385 INFO Creating lockfile:...385 DEBUG ...
386 DEBUG Initialising connection.386 DEBUG Initialising connection.
387 ...387 ...
388 DEBUG Removing lock file: /var/lock/process-upload-buildd.lock388 DEBUG Removing lock file: /var/lock/process-upload-buildd.lock
@@ -625,7 +625,7 @@
625 ... 'uploader.log'))625 ... 'uploader.log'))
626626
627 >>> print uploader_log.read()627 >>> print uploader_log.read()
628 INFO Creating lockfile:...628 DEBUG ...
629 DEBUG Initialising connection.629 DEBUG Initialising connection.
630 DEBUG Beginning processing630 DEBUG Beginning processing
631 DEBUG Creating directory /var/tmp/builddmaster/accepted631 DEBUG Creating directory /var/tmp/builddmaster/accepted
632632
=== modified file 'lib/lp/soyuz/scripts/tests/test_processupload.py'
--- lib/lp/soyuz/scripts/tests/test_processupload.py 2010-07-20 12:06:36 +0000
+++ lib/lp/soyuz/scripts/tests/test_processupload.py 2010-08-19 09:52:48 +0000
@@ -91,11 +91,12 @@
91 # the process-upload call terminated with ERROR and91 # the process-upload call terminated with ERROR and
92 # proper log message92 # proper log message
93 self.assertEqual(1, returncode)93 self.assertEqual(1, returncode)
94 self.assertEqual([94 self.assert_(
95 ('INFO Creating lockfile: '95 'INFO Creating lockfile: '
96 '/var/lock/process-upload-insecure.lock'),96 '/var/lock/process-upload-insecure.lock' in err.splitlines())
97 'DEBUG Lockfile /var/lock/process-upload-insecure.lock in use',97 self.assert_(
98 ], err.splitlines())98 'DEBUG Lockfile /var/lock/process-upload-insecure.lock in use'
99 in err.splitlines())
99100
100 # release the locally acquired lockfile101 # release the locally acquired lockfile
101 locker.release()102 locker.release()
102103
=== modified file 'lib/lp/testing/tests/test_standard_test_template.py'
--- lib/lp/testing/tests/test_standard_test_template.py 2010-07-17 21:13:21 +0000
+++ lib/lp/testing/tests/test_standard_test_template.py 2010-08-19 09:52:48 +0000
@@ -5,6 +5,8 @@
55
6__metaclass__ = type6__metaclass__ = type
77
8import unittest
9
8from canonical.testing import DatabaseFunctionalLayer10from canonical.testing import DatabaseFunctionalLayer
9from lp.testing import TestCase11from lp.testing import TestCase
1012
@@ -22,3 +24,7 @@
2224
23 # XXX: Assertions take expected value first, actual value second.25 # XXX: Assertions take expected value first, actual value second.
24 self.assertEqual(4, 2 + 2)26 self.assertEqual(4, 2 + 2)
27
28
29def test_suite():
30 return unittest.TestLoader().loadTestsFromName(__name__)
2531
=== modified file 'lib/lp/translations/doc/poexport-request.txt'
--- lib/lp/translations/doc/poexport-request.txt 2010-07-22 10:14:53 +0000
+++ lib/lp/translations/doc/poexport-request.txt 2010-08-19 09:52:48 +0000
@@ -269,6 +269,7 @@
269 ... )269 ... )
270 >>> (output, empty) = process.communicate()270 >>> (output, empty) = process.communicate()
271 >>> print output271 >>> print output
272 DEBUG ...
272 INFO Creating lockfile: /var/lock/launchpad-rosetta-export-queue.lock273 INFO Creating lockfile: /var/lock/launchpad-rosetta-export-queue.lock
273 DEBUG Exporting objects for Happy Downloader, related to template274 DEBUG Exporting objects for Happy Downloader, related to template
274 evolution-2.2 in Evolution trunk275 evolution-2.2 in Evolution trunk