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

[Stuart Bishop]

Enable and disable cronscripts using a configuration file.

Steps towards Bug #607391.

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

[Abel Deuring (adeuring)]

looks good

review: Approve (code)

[Robert Collins (lifeless)]

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

[Stuart Bishop (stub)]

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/

[Robert Collins (lifeless)]

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

=== 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 @@
 
 [canonical]
 chunkydiff: False
+cron_control_file: lib/lp/services/scripts/tests/cronscripts.ini
 
 [archivepublisher]
 base_url: http://ftpmaster.internal/

=== 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 @@
 # datatype: string
 admin_address: [e-mail]
 
+# By default, relative to the root.
+# datatype: filename
+cron_control_file: cronscripts.ini
+
 
 [checkwatches]
 # The database user to run this process as.
@@ -796,7 +800,7 @@
 
 # If true, only the source package names are imported into
 # Launchpad
-# datatype: boolean"
+# datatype: boolean
 sourcepackagenames_only: false
 
 

=== 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 @@
     >>> process.returncode
     0
     >>> print err
-    INFO    Creating lockfile: /var/lock/launchpad-send-bug-notifications.lock
+    DEBUG   ...
     INFO    Notifying [e-mail] about bug 2.
     ...
     INFO    Notifying [e-mail] about bug 2.

=== 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 @@
     0
 
     >>> print err
-    INFO    Creating lockfile: /var/lock/launchpad-checkwatches.lock
+    DEBUG   ...
     DEBUG   No global batch size specified.
     DEBUG   Skipping updating Ubuntu Bugzilla watches.
     DEBUG   No watches to update on http://bugs.debian.org

=== 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 @@
     0
 
     >>> print err
-    INFO    Creating lockfile: /var/lock/launchpad-updateremoteproduct.lock
+    DEBUG   ...
     INFO    No Products to update.
     INFO    Time for this run: ... seconds.
     DEBUG   Removing lock file:...

=== 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 @@
 
     >>> print '\n'.join(
     ...     line for line in err.split('\n') if 'INFO' not in line)
+    DEBUG   ...
     DEBUG   Sent warning email to name16 in launchpad-buildd-admins team.
     DEBUG   Sent warning email to name12 in landscape-developers team.
     DEBUG   Removing lock file: /var/lock/launchpad-flag-expired-memberships.lock

=== 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 @@
     'SilentLaunchpadScriptFailure'
     ]
 
+from ConfigParser import SafeConfigParser
 from cProfile import Profile
 import datetime
 import logging
 from optparse import OptionParser
-import os
+import os.path
 import sys
 
 from contrib.glock import GlobalLock, LockAlreadyAcquired
 import pytz
 from zope.component import getUtility
 
+from canonical.config import config
 from canonical.database.sqlbase import ISOLATION_LEVEL_DEFAULT
 from canonical.launchpad import scripts
 from canonical.launchpad.interfaces import IScriptActivitySet
@@ -185,8 +187,8 @@
     def setup_lock(self):
         """Create lockfile.
 
-        Note that this will create a lockfile even if you don't actually use it.
-        GlobalLock.__del__ is meant to clean it up though.
+        Note that this will create a lockfile even if you don't actually
+        use it. GlobalLock.__del__ is meant to clean it up though.
         """
         self.lock = GlobalLock(self.lockfilepath, logger=self.logger)
 
@@ -290,6 +292,22 @@
 class LaunchpadCronScript(LaunchpadScript):
     """Logs successful script runs in the database."""
 
+    def __init__(self, name=None, dbuser=None, test_args=None):
+        """Initialize, and sys.exit() if the cronscript is disabled.
+
+        Rather than hand editing crontab files, cronscripts can be
+        enabled and disabled using a config file.
+
+        The control file location is specified by
+        config.canonical.cron_control_file.
+        """
+        super(LaunchpadCronScript, self).__init__(name, dbuser, test_args)
+
+        enabled = cronscript_enabled(
+            config.canonical.cron_control_file, self.name, self.logger)
+        if not enabled:
+            sys.exit(0)
+
     def record_activity(self, date_started, date_completed):
         """Record the successful completion of the script."""
         self.txn.begin()
@@ -299,3 +317,45 @@
             date_started=date_started,
             date_completed=date_completed)
         self.txn.commit()
+
+
+def cronscript_enabled(control_path, name, log):
+    """Return True if the cronscript is enabled."""
+    if not os.path.isabs(control_path):
+        control_path = os.path.abspath(
+            os.path.join(config.root, control_path))
+
+    cron_config = SafeConfigParser({'enabled': str(True)})
+
+    if not os.path.exists(control_path):
+        # No control file exists. Everything enabled by default.
+        log.debug("Cronscript control file not found at %s", control_path)
+    else:
+        log.debug("Cronscript control file found at %s", control_path)
+
+        # Try reading the config file. If it fails, we log the
+        # traceback and continue on using the defaults.
+        try:
+            cron_config.read(control_path)
+        except:
+            log.exception("Error parsing %s", control_path)
+
+    if cron_config.has_option(name, 'enabled'):
+        section = name
+    else:
+        section = 'DEFAULT'
+
+    try:
+        enabled = cron_config.getboolean(section, 'enabled')
+    except:
+        log.exception(
+            "Failed to load value from %s section of %s",
+            section, control_path)
+        enabled = True
+
+    if enabled:
+        log.debug("Enabled by %s section", section)
+    else:
+        log.info("Disabled by %s section", section)
+
+    return enabled

=== 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 @@
+[example-cronscript-disabled]
+enabled: False

=== 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 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""An example cronscript. If it runs, it returns 42 as its return code."""
+
+__metaclass__ = type
+__all__ = []
+
+import sys
+
+from lp.services.scripts.base import (
+    LaunchpadCronScript, SilentLaunchpadScriptFailure)
+
+class Script(LaunchpadCronScript):
+    def main(self):
+        if self.name == 'example-cronscript-enabled':
+            raise SilentLaunchpadScriptFailure(42)
+        else:
+            # Raise a non-standard error code, as if the
+            # script was invoked as disabled the main()
+            # method should never be invoked.
+            raise SilentLaunchpadScriptFailure(999)
+
+if __name__ == '__main__':
+    if sys.argv[-1] == 'enabled':
+        name = 'example-cronscript-enabled'
+    else:
+        name = 'example-cronscript-disabled'
+    script = Script(name)
+    script.lock_and_run()

=== 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 @@
+# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test the cronscript_enabled function in scripts/base.py."""
+
+__metaclass__ = type
+
+from cStringIO import StringIO
+from logging import DEBUG
+import os.path
+import subprocess
+import sys
+from tempfile import NamedTemporaryFile
+from textwrap import dedent
+import unittest
+
+from lp.services.scripts.base import cronscript_enabled
+from lp.testing import TestCase
+from lp.testing.logger import MockLogger
+
+
+class TestCronscriptEnabled(TestCase):
+    def setUp(self):
+        super(TestCronscriptEnabled, self).setUp()
+        self.log_output = StringIO()
+        self.log = MockLogger(self.log_output)
+        self.log.setLevel(DEBUG)
+
+    def makeConfig(self, body):
+        tempfile = NamedTemporaryFile(suffix='.ini')
+        tempfile.write(body)
+        tempfile.flush()
+        # Ensure a reference is kept until the test is over.
+        # tempfile will then clean itself up.
+        self.addCleanup(lambda x: None, tempfile)
+        return tempfile.name
+
+    def test_noconfig(self):
+        enabled = cronscript_enabled('/idontexist.ini', 'foo', self.log)
+        self.assertIs(True, enabled)
+
+    def test_emptyconfig(self):
+        config = self.makeConfig('')
+        enabled = cronscript_enabled(config, 'foo', self.log)
+        self.assertIs(True, enabled)
+
+    def test_default_true(self):
+        config = self.makeConfig(dedent("""\
+            [DEFAULT]
+            enabled: True
+            """))
+        enabled = cronscript_enabled(config, 'foo', self.log)
+        self.assertIs(True, enabled)
+
+    def test_default_false(self):
+        config = self.makeConfig(dedent("""\
+            [DEFAULT]
+            enabled: False
+            """))
+        enabled = cronscript_enabled(config, 'foo', self.log)
+        self.assertIs(False, enabled)
+
+    def test_specific_true(self):
+        config = self.makeConfig(dedent("""\
+            [DEFAULT]
+            enabled: False
+            [foo]
+            enabled: True
+            """))
+        enabled = cronscript_enabled(config, 'foo', self.log)
+        self.assertIs(True, enabled)
+
+    def test_specific_false(self):
+        config = self.makeConfig(dedent("""\
+            [DEFAULT]
+            enabled: True
+            [foo]
+            enabled: False
+            """))
+        enabled = cronscript_enabled(config, 'foo', self.log)
+        self.assertIs(False, enabled)
+
+    def test_broken_true(self):
+        config = self.makeConfig(dedent("""\
+            # This file is unparsable
+            [DEFAULT
+            enabled: False
+            [foo
+            enabled: False
+            """))
+        enabled = cronscript_enabled(config, 'foo', self.log)
+        self.assertIs(True, enabled)
+
+    def test_invalid_boolean_true(self):
+        config = self.makeConfig(dedent("""\
+            [DEFAULT]
+            enabled: whoops
+            """))
+        enabled = cronscript_enabled(config, 'foo', self.log)
+        self.assertIs(True, enabled)
+
+    def test_specific_missing_fallsback(self):
+        config = self.makeConfig(dedent("""\
+            [DEFAULT]
+            enabled: False
+            [foo]
+            # There is a typo in the next line.
+            enobled: True
+            """))
+        enabled = cronscript_enabled(config, 'foo', self.log)
+        self.assertIs(False, enabled)
+
+    def test_default_missing_fallsback(self):
+        config = self.makeConfig(dedent("""\
+            [DEFAULT]
+            # There is a typo in the next line. Fallsback to hardcoded
+            # default.
+            enobled: False
+            [foo]
+            # There is a typo in the next line.
+            enobled: False
+            """))
+        enabled = cronscript_enabled(config, 'foo', self.log)
+        self.assertIs(True, enabled)
+
+    def test_enabled_cronscript(self):
+        cmd = [
+            sys.executable,
+            os.path.join(os.path.dirname(__file__), 'example-cronscript.py'),
+            '-qqqqq', 'enabled',
+            ]
+        self.assertEqual(42, subprocess.call(cmd))
+
+    def test_disabled_cronscript(self):
+        cmd = [
+            sys.executable,
+            os.path.join(os.path.dirname(__file__), 'example-cronscript.py'),
+            '-qqqqq', 'disabled',
+            ]
+        self.assertEqual(0, subprocess.call(cmd))
+
+
+def test_suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)

=== 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 @@
     * Build Log: http://.../...i386.mozilla-firefox_0.9_BUILDING.txt.gz
     ...
     Upload log:
-    INFO    Creating lockfile:...
+    DEBUG   ...
     DEBUG   Initialising connection.
     ...
     DEBUG   Removing lock file: /var/lock/process-upload-buildd.lock
@@ -625,7 +625,7 @@
     ...                                  'uploader.log'))
 
     >>> print uploader_log.read()
-    INFO    Creating lockfile:...
+    DEBUG   ...
     DEBUG   Initialising connection.
     DEBUG   Beginning processing
     DEBUG   Creating directory /var/tmp/builddmaster/accepted

=== 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 @@
         # the process-upload call terminated with ERROR and
         # proper log message
         self.assertEqual(1, returncode)
-        self.assertEqual([
-            ('INFO    Creating lockfile: '
-             '/var/lock/process-upload-insecure.lock'),
-            'DEBUG   Lockfile /var/lock/process-upload-insecure.lock in use',
-             ], err.splitlines())
+        self.assert_(
+            'INFO    Creating lockfile: '
+            '/var/lock/process-upload-insecure.lock' in err.splitlines())
+        self.assert_(
+            'DEBUG   Lockfile /var/lock/process-upload-insecure.lock in use'
+            in err.splitlines())
 
         # release the locally acquired lockfile
         locker.release()

=== 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 @@
 
 __metaclass__ = type
 
+import unittest
+
 from canonical.testing import DatabaseFunctionalLayer
 from lp.testing import TestCase
 
@@ -22,3 +24,7 @@
 
         # XXX: Assertions take expected value first, actual value second.
         self.assertEqual(4, 2 + 2)
+
+
+def test_suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)

=== 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 @@
     ...     )
     >>> (output, empty) = process.communicate()
     >>> print output
+    DEBUG   ...
     INFO    Creating lockfile: /var/lock/launchpad-rosetta-export-queue.lock
     DEBUG   Exporting objects for Happy Downloader, related to template
     evolution-2.2 in Evolution trunk