← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~jtv/launchpad/db-bug-766807 into lp:launchpad/db-devel

 

Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/db-bug-766807 into lp:launchpad/db-devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #766807 in Launchpad itself: "Auto-initialize indexes in new distroseries"
  https://bugs.launchpad.net/launchpad/+bug/766807

For more details, see:
https://code.launchpad.net/~jtv/launchpad/db-bug-766807/+merge/59027

= Summary =

When a new distroseries is created (and we plan to publish it), several things need to happen.  One of them is an initial run of the publish-distro script for the main and, if present, the partner archives.  This is steps 12 & 14 in https://wiki.ubuntu.com/NewReleaseCycleProcess

As part of our derivation work, we need to automate more of this process and generalize it beyond Ubuntu.


== Proposed fix ==

Use the job system.  Set up a new DistributionJob-derived job type that creates the indexes.

The job is created in the initialise-from-parent.py script.  If you look at the Ubuntu release process you'll note that this is in a section where publishing cron jobs for the distribution are disabled.  Creation of the indexes also falls within this section.  Once the indexes have been created, the release manager is notified so that they know that this job is no longer a blocker for re-enabling the publishing scripts.

For the Ubuntu case, the documented contacts are Lamont and Steve L.  Steve is documented as the Ubuntu release manager, along with a very few others, so that makes this notification set a good fit to the existing process design.

We don't publish Debian releases, nor do we want to send notifications to its owners just because their releases show up in Launchpad.  We have no publisher configuration in the database for Debian, which would break publish-distro.  So in this branch I don't produce index-creation jobs for distributions that have no publisher config; I hope that is a compromise that will make everyone happy.

We may need either 1 or 2 publish-distro runs for a series.  For the 2-run case, we could either generate a single job that performs both runs, or two separate jobs for the two respective runs.  I opted for the former: humans are waiting for the news that this job is completed.  Tracking successful completion of both gets much harder with two independent jobs.


== Pre-implementation notes ==

Without Julian available, we have to make do as best we can.  Some of the details and caveats were discussed with wgrant, SteveK, and lifeless.  Points that spring to mind:

 * As a matter of design, the publish_distro script should remain unaware of distroseries.

 * There's no particular reason to run publish-distro after creating the symlinks in the Ubuntu release process.

 * All pockets should be generated, though only for the new distroseries.  By default publish_distro processes the entire distribution.

In future we'll clearly want to automate a lot more of the Ubuntu release process.  For now, creating individual jobs for each step should still be manageable.  In future we may be able to automate steps 5, 6, 9, 10, and 18 relatively easily.


== Implementation details ==

The new job type was originally called InitializeDistroSeriesIndexesJob, but this clashed with the more generically-named InitialiseDistroSeriesJob.  Also note the difference in spelling.  In the end I opted for a shorter, more distinct name.  It wasn't worth creating a dedicated interface for this new job type.

You'll notice that I removed try_and_commit from the publisher code.  This was a holdover from obsolete memory micro-management practices, but with those gone it's been stripped down to the point of uselessness.  Ditching it made the code a bit easier to read.

The new job runner will have to run on the server that hosts the Ubuntu archives.


== Tests ==

{{{
./bin/test -vvc lp.archivepublisher.tests.test_createdistroseriesindexesjob
./bin/test -vvc lp.soyuz.scripts.tests.test_initialise_distroseries
}}}


== Demo and Q/A ==

The creation of the new jobs is controlled by a feature flag: archivepublisher.auto_create_indexes.enabled.

To test this feature on dogfood or similar:
 * Enable the feature flag.
 * Create a new distro release based on an existing release.
 * You may need to go through some of the steps in the Ubuntu release process.
 * Run initialise-from-parent.py on the new release.
 * Check that a DistributionJob of type 4 has been created for the new release.
 * Run the job runner: cronscripts/create-distroseries-indexes.py -vvv
 * This should run publish_distro and generate indexes in the distribution's dists root.


= Launchpad lint =

I left a few bits of lint in place: some unused variables because I don't want to damage the artistic integrity of the tests(*), and a long line because I think another concurrent branch already fixes it in a way that would cause conflicts.

(*) Meaning that whoever wrote them may have introduced the variables for legibility or consistency and I don't want to mess with what I do not understand (unless it's actually fun).


Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/soyuz/scripts/initialise_distroseries.py
  lib/lp/soyuz/scripts/tests/test_initialise_distroseries.py
  lib/lp/soyuz/scripts/publishdistro.py
  lib/lp/archivepublisher/zcml/configure.zcml
  lib/lp/soyuz/interfaces/distributionjob.py
  lib/lp/archivepublisher/tests/test_createdistroseriesindexesjob.py
  lib/lp/archivepublisher/interfaces/createdistroseriesindexesjob.py
  lib/lp/archivepublisher/model/createdistroseriesindexesjob.py

./lib/lp/soyuz/scripts/tests/test_initialise_distroseries.py
     261: local variable 'ps' is assigned to but never used
     271: local variable 'ps' is assigned to but never used
     285: local variable 'test2' is assigned to but never used
     324: local variable 'test2' is assigned to but never used
./lib/lp/soyuz/scripts/publishdistro.py
     208: E501 line too long (80 characters)
     208: Line exceeds 78 characters.


Jeroen
-- 
https://code.launchpad.net/~jtv/launchpad/db-bug-766807/+merge/59027
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/db-bug-766807 into lp:launchpad/db-devel.
=== added file 'cronscripts/create-distroseries-indexes.py'
--- cronscripts/create-distroseries-indexes.py	1970-01-01 00:00:00 +0000
+++ cronscripts/create-distroseries-indexes.py	2011-04-26 08:25:55 +0000
@@ -0,0 +1,30 @@
+#!/usr/bin/python -S
+#
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Run `CreateDistroSeriesIndexesJob`s.
+
+The jobs write to the file system.  This script should be run _only_ on
+a server that holds the distributions archives.
+"""
+
+__metaclass__ = type
+
+import _pythonpath
+
+from lp.archivepublisher.interfaces.createdistroseriesindexesjob import (
+    ICreateDistroSeriesIndexesJobSource,
+    )
+from lp.services.job.runner import JobCronScript
+
+
+class RunCreateDistroSeriesIndexesJobs(JobCronScript):
+    """Run `CreateDistroSeriesIndexesJobs`s."""
+
+    config_name = 'create_distroseries_indexes'
+    source_interface = ICreateDistroSeriesIndexesJobSource
+
+
+if __name__ == '__main__':
+    RunCreateDistroSeriesIndexesJobs().lock_and_run()

=== modified file 'lib/canonical/config/schema-lazr.conf'
--- lib/canonical/config/schema-lazr.conf	2011-04-26 07:42:38 +0000
+++ lib/canonical/config/schema-lazr.conf	2011-04-26 08:25:55 +0000
@@ -596,6 +596,12 @@
 licensing_policy_url: https://help.launchpad.net/Legal/ProjectLicensing
 
 
+[create_distroseries_indexes]
+# The database role that these jobs will run in.
+# datatype: string
+dbuser: archivepublisher
+
+
 [create_merge_proposals]
 # The database user which will be used by this process.
 # datatype: string

=== added file 'lib/lp/archivepublisher/interfaces/createdistroseriesindexesjob.py'
--- lib/lp/archivepublisher/interfaces/createdistroseriesindexesjob.py	1970-01-01 00:00:00 +0000
+++ lib/lp/archivepublisher/interfaces/createdistroseriesindexesjob.py	2011-04-26 08:25:55 +0000
@@ -0,0 +1,26 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""`IJobSource` for `CreateDistroSeriesIndexesJob`."""
+
+__metaclass__ = type
+__all__ = [
+    'ICreateDistroSeriesIndexesJobSource',
+    ]
+
+from lp.services.job.interfaces.job import IJobSource
+
+
+class ICreateDistroSeriesIndexesJobSource(IJobSource):
+    """Create and manage `ICreateDistroSeriesIndexesJob`s."""
+
+    def makeFor(distroseries):
+        """Create `ICreateDistroSeriesIndexesJob` if appropriate.
+
+        If the `Distribution` that `distroseries` belongs to has no
+        publisher configuration, no job will be returned.
+
+        :param distroseries: A newly created `DistroSeries` that needs
+            its archive indexes created.
+        :return: An `ICreateDistroSeriesIndexesJob`, or None.
+        """

=== added file 'lib/lp/archivepublisher/model/createdistroseriesindexesjob.py'
--- lib/lp/archivepublisher/model/createdistroseriesindexesjob.py	1970-01-01 00:00:00 +0000
+++ lib/lp/archivepublisher/model/createdistroseriesindexesjob.py	2011-04-26 08:25:55 +0000
@@ -0,0 +1,164 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Job to create a distroseries' archive indexes."""
+
+__metaclass__ = type
+__all__ = [
+    'CreateDistroSeriesIndexesJob',
+    ]
+
+from optparse import OptionParser
+from storm.locals import Store
+from textwrap import dedent
+import transaction
+from zope.component import getUtility
+from zope.interface import (
+    classProvides,
+    implements,
+    )
+
+from canonical.config import config
+from canonical.launchpad.interfaces.lpstorm import IMasterStore
+from lp.archivepublisher.interfaces.createdistroseriesindexesjob import (
+    ICreateDistroSeriesIndexesJobSource,
+    )
+from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfigSet
+from lp.registry.interfaces.pocket import pocketsuffix
+from lp.services.features import getFeatureFlag
+from lp.services.job.interfaces.job import IRunnableJob
+from lp.services.mail.sendmail import (
+    format_address_for_person,
+    MailController,
+    )
+from lp.soyuz.interfaces.distributionjob import (
+    DistributionJobType,
+    IDistributionJob,
+    )
+from lp.soyuz.model.distributionjob import (
+    DistributionJob,
+    DistributionJobDerived,
+    )
+from lp.soyuz.scripts import publishdistro
+
+
+FEATURE_FLAG_ENABLE_MODULE = u"archivepublisher.auto_create_indexes.enabled"
+
+
+def get_addresses_for(person):
+    """Get list of contact email address(es) for `person`."""
+    if person.is_team:
+        recipients = person.getMembersWithPreferredEmails()
+    else:
+        recipients = [person]
+
+    return [format_address_for_person(recipient) for recipient in recipients]
+
+
+class CreateDistroSeriesIndexesJob(DistributionJobDerived):
+    """Job to create a distroseries's archive indexes.
+
+    To do this it runs publish-distro on the distribution, in careful mode.
+    """
+    implements(IDistributionJob, IRunnableJob)
+    classProvides(ICreateDistroSeriesIndexesJobSource)
+
+    class_job_type = DistributionJobType.CREATEDISTROSERIESINDEXES
+
+    # Injection point for tests: optional publish_distro logger.
+    logger = None
+
+    @classmethod
+    def create(cls, distroseries):
+        job = DistributionJob(
+            distroseries.distribution, distroseries, cls.class_job_type,
+            metadata={})
+        IMasterStore(DistributionJob).add(job)
+        return cls(job)
+
+    @classmethod
+    def makeFor(cls, distroseries):
+        """See `ICreateDistroSeriesIndexesJob`."""
+        if not getFeatureFlag(FEATURE_FLAG_ENABLE_MODULE):
+            return None
+
+        distro = distroseries.distribution
+        config_set = getUtility(IPublisherConfigSet)
+        publisher_config = config_set.getByDistribution(distro)
+        if publisher_config is None:
+            return None
+        else:
+            return cls.create(distroseries)
+
+    def run(self):
+        """See `IRunnableJob`."""
+        self.runPublishDistro()
+        if self.distribution.getArchiveByComponent('partner') is not None:
+            self.runPublishDistro('--partner')
+
+        self.notifySuccess()
+
+    def getOperationDescription(self):
+        """See `IRunnableJob`."""
+        return "initializing archive indexes for %s" % self.distroseries.title
+
+    def getSuites(self):
+        """List the suites for this `DistroSeries`."""
+        series_name = self.distroseries.name
+        return [series_name + suffix for suffix in pocketsuffix.itervalues()]
+
+    def runPublishDistro(self, extra_args=None):
+        """Invoke the publish-distro script to create indexes.
+
+        Publishes only the distroseries in question, in careful mode.
+        """
+        arguments = [
+            "-A",
+            "-d", self.distribution.name,
+            ]
+        for suite in self.getSuites():
+            arguments += ["-s", suite]
+        if extra_args is not None:
+            arguments.append(extra_args)
+
+        parser = OptionParser()
+        publishdistro.add_options(parser)
+        options, args = parser.parse_args(arguments)
+        publishdistro.run_publisher(options, transaction, self.logger)
+
+    def getMailRecipients(self):
+        """List email addresses to notify of success or failure."""
+        recipient = self.distroseries.driver or self.distribution.owner
+        return get_addresses_for(recipient)
+
+    def notifySuccess(self):
+        """Notify the distribution's owners of success."""
+        subject = "Launchpad has created archive indexes for %s." % (
+            self.distroseries.name)
+        message = dedent("""\
+            You are receiving this email because you are registered in
+            Launchpad as a release manager for %s.
+
+            The archive indexes for %s have been successfully created.
+
+            This automated process is one of many steps in setting up a
+            new distribution release series in Launchpad.  The fact that
+            this part of the work is now done may mean that you can now
+            proceed with subsequent steps.
+
+            This is an automated email; please do not reply.  Contact
+            the Launchpad development team if you have any problems.
+            """ % (self.distribution.displayname, self.distroseries.title))
+        from_addr = config.canonical.noreply_from_address
+        controller = MailController(
+            from_addr, self.getMailRecipients(), subject, message)
+        if controller is not None:
+            controller.send()
+
+    def getErrorRecipients(self):
+        """See `BaseRunnableJob`."""
+        return self.getMailRecipients()
+
+    def destroySelf(self):
+        """See `IDistributionJob`."""
+        Store.of(self.context).remove(self.context)

=== added file 'lib/lp/archivepublisher/tests/test_createdistroseriesindexesjob.py'
--- lib/lp/archivepublisher/tests/test_createdistroseriesindexesjob.py	1970-01-01 00:00:00 +0000
+++ lib/lp/archivepublisher/tests/test_createdistroseriesindexesjob.py	2011-04-26 08:25:55 +0000
@@ -0,0 +1,308 @@
+# Copyright 2011 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for `CreateDistroSeriesIndexesJob`."""
+
+__metaclass__ = type
+
+import os.path
+from storm.locals import Store
+import transaction
+from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
+
+from canonical.config import config
+from canonical.launchpad.scripts.tests import run_script
+from canonical.launchpad.webapp.testing import verifyObject
+from canonical.testing.layers import (
+    LaunchpadZopelessLayer,
+    ZopelessDatabaseLayer,
+    )
+from lp.archivepublisher.config import getPubConfig
+from lp.archivepublisher.interfaces.createdistroseriesindexesjob import (
+    ICreateDistroSeriesIndexesJobSource,
+    )
+from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfigSet
+from lp.archivepublisher.model.createdistroseriesindexesjob import (
+    FEATURE_FLAG_ENABLE_MODULE,
+    get_addresses_for,
+    CreateDistroSeriesIndexesJob,
+    )
+from lp.registry.interfaces.pocket import pocketsuffix
+from lp.services.features.testing import FeatureFixture
+from lp.services.job.interfaces.job import IRunnableJob
+from lp.services.log.logger import DevNullLogger
+from lp.services.mail import stub
+from lp.services.mail.sendmail import format_address_for_person
+from lp.services.utils import file_exists
+from lp.soyuz.enums import ArchivePurpose
+from lp.soyuz.interfaces.distributionjob import IDistributionJob
+from lp.testing import (
+    celebrity_logged_in,
+    TestCaseWithFactory,
+    )
+from lp.testing.fakemethod import FakeMethod
+from lp.testing.mail_helpers import run_mail_jobs
+
+
+class TestHelpers(TestCaseWithFactory):
+    """Test module's helpers."""
+
+    layer = ZopelessDatabaseLayer
+
+    def test_get_addresses_for_person_returns_person_address(self):
+        # For a single person, get_addresses_for returns that person's
+        # email address.
+        person = self.factory.makePerson()
+        self.assertEqual(
+            [format_address_for_person(person)], get_addresses_for(person))
+
+    def test_get_addresses_for_team_returns_member_addresses(self):
+        # For a team with members that have preferred email addresses,
+        # get_addresses_for returns the list of email addresses of the
+        # team's members.
+        member = self.factory.makePerson()
+        team = self.factory.makeTeam(
+            owner=self.factory.makePerson(), members=[member])
+        self.assertIn(
+            format_address_for_person(member), get_addresses_for(team))
+
+    def test_get_addresses_for_team_returns_owner_address(self):
+        # For a team, get_addresses_for includes the owner's address.
+        owner = self.factory.makePerson()
+        team = self.factory.makeTeam(owner=owner, members=[])
+        addresses = get_addresses_for(team)
+        self.assertEqual(1, len(addresses))
+        self.assertIn(
+            removeSecurityProxy(owner.preferredemail).email, addresses[0])
+
+
+class TestCreateDistroSeriesIndexesJobSource(TestCaseWithFactory):
+    """Test utility."""
+
+    layer = LaunchpadZopelessLayer
+
+    def setUp(self):
+        super(TestCreateDistroSeriesIndexesJobSource, self).setUp()
+        self.useFixture(FeatureFixture({FEATURE_FLAG_ENABLE_MODULE: u'on'}))
+
+    def removePublisherConfig(self, distribution):
+        """Strip `distribution` of its publisher configuration."""
+        publisher_config = getUtility(IPublisherConfigSet).getByDistribution(
+            distribution)
+        Store.of(publisher_config).remove(publisher_config)
+
+    def test_baseline(self):
+        # The utility conforms to the interfaces it claims to implement.
+        jobsource = getUtility(ICreateDistroSeriesIndexesJobSource)
+        self.assertTrue(
+            verifyObject(ICreateDistroSeriesIndexesJobSource, jobsource))
+
+    def test_creates_job_for_distro_with_publisher_config(self):
+        # The utility can create a job if the distribution has a
+        # publisher configuration.
+        distroseries = self.factory.makeDistroSeries()
+        jobset = getUtility(ICreateDistroSeriesIndexesJobSource)
+        job = jobset.makeFor(distroseries)
+        self.assertIsInstance(job, CreateDistroSeriesIndexesJob)
+
+    def test_does_not_create_job_for_distro_without_publisher_config(self):
+        # If the distribution has no publisher configuration, the
+        # utility creates no job for it.
+        distroseries = self.factory.makeDistroSeries()
+        self.removePublisherConfig(distroseries.distribution)
+        jobset = getUtility(ICreateDistroSeriesIndexesJobSource)
+        job = jobset.makeFor(distroseries)
+        self.assertIs(None, job)
+
+    def test_feature_flag_disables_feature(self):
+        # The creation of jobs is controlled by a feature flag.
+        self.useFixture(FeatureFixture({FEATURE_FLAG_ENABLE_MODULE: u''}))
+        jobset = getUtility(ICreateDistroSeriesIndexesJobSource)
+        self.assertIs(None, jobset.makeFor(self.factory.makeDistroSeries()))
+
+
+class HorribleFailure(Exception):
+    """A sample error for testing purposes."""
+
+
+class TestCreateDistroSeriesIndexesJob(TestCaseWithFactory):
+    """Test job class."""
+
+    layer = LaunchpadZopelessLayer
+
+    def setUp(self):
+        super(TestCreateDistroSeriesIndexesJob, self).setUp()
+        self.useFixture(FeatureFixture({FEATURE_FLAG_ENABLE_MODULE: u'on'}))
+
+    def getJobSource(self):
+        """Shorthand for getting at the job-source utility."""
+        return getUtility(ICreateDistroSeriesIndexesJobSource)
+
+    def makeJob(self, distroseries=None):
+        """Create an `CreateDistroSeriesIndexesJob`."""
+        if distroseries is None:
+            distroseries = self.factory.makeDistroSeries()
+        job = removeSecurityProxy(self.getJobSource().makeFor(distroseries))
+        job.logger = DevNullLogger()
+        return job
+
+    def getSuites(self, distroseries):
+        """Get the list of suites for `distroseries`."""
+        return [
+            distroseries.name + suffix
+            for suffix in pocketsuffix.itervalues()]
+
+    def makeDistsDirs(self, distsroot, distroseries):
+        """Create dists directories in `distsroot` for `distroseries`."""
+        base = os.path.join(distsroot, distroseries.name)
+        for suffix in pocketsuffix.itervalues():
+            os.makedirs(base + suffix)
+
+    def test_baseline(self):
+        # The job class conforms to the interfaces it claims to implement.
+        job = self.makeJob()
+        self.assertTrue(verifyObject(IRunnableJob, job))
+        self.assertTrue(verifyObject(IDistributionJob, job))
+
+    def test_getSuites_identifies_distroseries_suites(self):
+        # getSuites lists all suites in the distroseries.
+        distroseries = self.factory.makeDistroSeries()
+        job = self.makeJob(distroseries)
+        self.assertContentEqual(self.getSuites(distroseries), job.getSuites())
+
+    def test_getSuites_ignores_suites_for_other_distroseries(self):
+        # getSuites does not list suites in the distributio that do not
+        # belong to the right distroseries.
+        distroseries = self.factory.makeDistroSeries()
+        self.factory.makeDistroSeries(distribution=distroseries.distribution)
+        job = self.makeJob(distroseries)
+        self.assertContentEqual(self.getSuites(distroseries), job.getSuites())
+
+    def test_job_runs_publish_distro_for_main(self):
+        # The job always runs publish_distro for the distribution's main
+        # archive.
+        job = self.makeJob()
+        job.runPublishDistro = FakeMethod()
+        job.run()
+        args, kwargs = job.runPublishDistro.calls[-1]
+        self.assertEqual((), args)
+
+    def test_job_runs_publish_distro_for_partner_if_present(self):
+        # If the distribution has a partner archive, the job will run
+        # publish_distro for it.  This differs from the run for the main
+        # archive in that publish_distro receives the --partner option.
+        distroseries = self.factory.makeDistroSeries()
+        self.factory.makeArchive(
+            distribution=distroseries.distribution,
+            purpose=ArchivePurpose.PARTNER)
+        job = self.makeJob(distroseries)
+        job.runPublishDistro = FakeMethod()
+        job.run()
+        self.assertIn(
+            ('--partner', ),
+            [args for args, kwargs in job.runPublishDistro.calls])
+
+    def test_job_does_not_run_publish_distro_for_partner_if_not_present(self):
+        # If the distribution does not have a partner archive,
+        # publish_distro is not run for the partner archive.
+        job = self.makeJob()
+        job.runPublishDistro = FakeMethod()
+        job.run()
+        self.assertEqual(1, job.runPublishDistro.call_count)
+
+    def test_job_notifies_if_successful(self):
+        # Once the indexes have been created, the job calls its
+        # notifySuccess method to let stakeholders know that they may
+        # proceed with their release process.
+        job = self.makeJob()
+        job.runPublishDistro = FakeMethod()
+        job.notifySuccess = FakeMethod()
+        job.run()
+        self.assertEqual(1, job.notifySuccess.call_count)
+
+    def test_failure_notifies_recipients(self):
+        # Failure notices are sent to the addresses returned by
+        # getMailRecipients.
+        distroseries = self.factory.makeDistroSeries()
+        job = self.makeJob(distroseries)
+        job.getMailRecipients = FakeMethod(result=["foo@xxxxxxxxxxx"])
+        job.notifyUserError(HorribleFailure("Boom!"))
+        run_mail_jobs()
+        sender, recipients, body = stub.test_emails.pop()
+        self.assertIn("foo@xxxxxxxxxxx", recipients)
+
+    def test_success_notifies_recipients(self):
+        # Success notices are sent to the addresses returned by
+        # getMailRecipients.
+        distroseries = self.factory.makeDistroSeries()
+        job = self.makeJob(distroseries)
+        job.getMailRecipients = FakeMethod(result=["bar@xxxxxxxxxxx"])
+        job.notifySuccess()
+        run_mail_jobs()
+        sender, recipients, body = stub.test_emails.pop()
+        self.assertIn("bar@xxxxxxxxxxx", recipients)
+
+    def test_notifySuccess_sends_email(self):
+        # notifySuccess sends out a success notice by email.
+        distroseries = self.factory.makeDistroSeries()
+        job = self.makeJob(distroseries)
+        job.notifySuccess()
+        run_mail_jobs()
+        sender, recipients, body = stub.test_emails.pop()
+        self.assertIn("success", body)
+
+    def test_release_manager_gets_notified(self):
+        # The release manager gets notified.  This role is represented
+        # by the driver for the distroseries.
+        distroseries = self.factory.makeDistroSeries()
+        driver = self.factory.makePerson()
+        distroseries.driver = driver
+        job = self.makeJob(distroseries)
+        self.assertIn(
+            format_address_for_person(driver), job.getMailRecipients())
+
+    def test_distribution_owner_gets_notified_if_no_release_manager(self):
+        # If no release manager is available, the distribution owners
+        # are notified.
+        distroseries = self.factory.makeDistroSeries()
+        distroseries.driver = None
+        job = self.makeJob(distroseries)
+        owner = distroseries.distribution.owner
+        self.assertIn(
+            format_address_for_person(owner), job.getMailRecipients())
+
+    def test_run_does_the_job(self):
+        # The job runs publish_distro and generates the expected output
+        # files.
+        distro = self.factory.makeDistribution(
+            publish_root_dir=unicode(self.makeTemporaryDirectory()))
+        distroseries = self.factory.makeDistroSeries(distribution=distro)
+        job = self.makeJob(distroseries)
+
+        with celebrity_logged_in('admin'):
+            distsroot = getPubConfig(distro.main_archive).distsroot
+            self.makeDistsDirs(distsroot, distroseries)
+            self.becomeDbUser(config.archivepublisher.dbuser)
+            self.addCleanup(self.becomeDbUser, 'launchpad')
+            job.run()
+
+        output = os.path.join(distsroot, distroseries.name, "Release")
+        self.assertTrue(file_exists(output))
+
+    def test_job_runner_runs_jobs(self):
+        # The job runner script successfully runs the jobs.
+        distro = self.factory.makeDistribution(
+            publish_root_dir=unicode(self.makeTemporaryDirectory()))
+        distroseries = self.factory.makeDistroSeries(distribution=distro)
+        self.makeJob(distroseries)
+        with celebrity_logged_in('admin'):
+            distsroot = getPubConfig(distro.main_archive).distsroot
+            self.makeDistsDirs(distsroot, distroseries)
+        transaction.commit()
+
+        returncode, stdout, stderr = run_script(
+            "cronscripts/create-distroseries-indexes.py", ["-vvvv"])
+
+        output = os.path.join(distsroot, distroseries.name, "Release")
+        self.assertTrue(file_exists(output))

=== modified file 'lib/lp/archivepublisher/zcml/configure.zcml'
--- lib/lp/archivepublisher/zcml/configure.zcml	2011-03-09 17:51:40 +0000
+++ lib/lp/archivepublisher/zcml/configure.zcml	2011-04-26 08:25:55 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2009 Canonical Ltd.  This software is licensed under the
+<!-- Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
      GNU Affero General Public License version 3 (see the file LICENSE).
 -->
 
@@ -28,6 +28,16 @@
             set_schema="lp.archivepublisher.interfaces.publisherconfig.IPublisherConfig"/>
     </class>
 
+    <class class="lp.archivepublisher.model.createdistroseriesindexesjob.CreateDistroSeriesIndexesJob">
+        <allow
+          interface="lp.soyuz.interfaces.distributionjob.IDistributionJob"/>
+        <allow interface="lp.services.job.interfaces.job.IRunnableJob"/>
+    </class>
+    <securedutility
+      component="lp.archivepublisher.model.createdistroseriesindexesjob.CreateDistroSeriesIndexesJob"
+      provides="lp.archivepublisher.interfaces.createdistroseriesindexesjob.ICreateDistroSeriesIndexesJobSource">
+        <allow
+          interface="lp.archivepublisher.interfaces.createdistroseriesindexesjob.ICreateDistroSeriesIndexesJobSource"/>
+    </securedutility>
 
 </configure>
-

=== modified file 'lib/lp/soyuz/interfaces/distributionjob.py'
--- lib/lp/soyuz/interfaces/distributionjob.py	2011-04-21 13:57:43 +0000
+++ lib/lp/soyuz/interfaces/distributionjob.py	2011-04-26 08:25:55 +0000
@@ -89,6 +89,13 @@
         distribution release series and its parent series.
         """)
 
+    CREATEDISTROSERIESINDEXES = DBItem(4, """
+        Set up a series' archive indexes.
+
+        Performans an initial run of the publish-distro script to
+        create indexes for the distroseries.
+        """)
+
 
 class IInitialiseDistroSeriesJobSource(IJobSource):
     """An interface for acquiring IInitialiseDistroSeriesJobs."""

=== modified file 'lib/lp/soyuz/scripts/initialise_distroseries.py'
--- lib/lp/soyuz/scripts/initialise_distroseries.py	2011-04-14 14:08:08 +0000
+++ lib/lp/soyuz/scripts/initialise_distroseries.py	2011-04-26 08:25:55 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Initialise a distroseries from its parent distroseries."""
@@ -17,6 +17,9 @@
 from canonical.database.sqlbase import sqlvalues
 from canonical.launchpad.helpers import ensure_unicode
 from canonical.launchpad.interfaces.lpstorm import IMasterStore
+from lp.archivepublisher.interfaces.createdistroseriesindexesjob import (
+    ICreateDistroSeriesIndexesJobSource,
+    )
 from lp.buildmaster.enums import BuildStatus
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.soyuz.adapters.packagelocation import PackageLocation
@@ -53,6 +56,9 @@
       selections will be duplicated, as will any permission-related
       structures.
 
+      A `CreateDistroSeriesIndexesJob` is created if appropriate, so that
+      the series' archive indexes will be initialized.
+
     Note:
       This method will raise a InitialisationError when the pre-conditions
       are not met. After this is run, you still need to construct chroots
@@ -137,6 +143,7 @@
         self._copy_architectures()
         self._copy_packages()
         self._copy_packagesets()
+        self._request_index_creation()
         transaction.commit()
 
     def _set_parent(self):
@@ -329,3 +336,8 @@
                 new_series_ps.add(parent_to_child[old_series_child])
             new_series_ps.add(old_series_ps.sourcesIncluded(
                 direct_inclusion=True))
+
+    def _request_index_creation(self):
+        """Schedule a `CreateDistroSeriesIndexesJob` if appropriate."""
+        getUtility(ICreateDistroSeriesIndexesJobSource).makeFor(
+            self.distroseries)

=== modified file 'lib/lp/soyuz/scripts/publishdistro.py'
--- lib/lp/soyuz/scripts/publishdistro.py	2011-04-04 11:41:52 +0000
+++ lib/lp/soyuz/scripts/publishdistro.py	2011-04-26 08:25:55 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Publisher script functions."""
@@ -97,15 +97,6 @@
             return "Careful"
         return "Normal"
 
-    def try_and_commit(description, func, *args):
-        try:
-            func(*args)
-            txn.commit()
-        except:
-            log.exception("Unexpected exception while %s" % description)
-            txn.abort()
-            raise
-
     exclusive_options = (
         options.partner, options.ppa, options.private_ppa,
         options.primary_debug, options.copy_archive)
@@ -195,34 +186,35 @@
         # Do we need to delete the archive or publish it?
         if archive.status == ArchiveStatus.DELETING:
             if archive.purpose == ArchivePurpose.PPA:
-                try_and_commit("deleting archive", publisher.deleteArchive)
+                publisher.deleteArchive()
+                txn.commit()
             else:
                 # Other types of archives do not currently support deletion.
                 log.warning(
                     "Deletion of %s skipped: operation not supported on %s"
                     % archive.displayname)
         else:
-            try_and_commit(
-                "publishing", publisher.A_publish,
-                options.careful or options.careful_publishing)
+            publisher.A_publish(options.careful or options.careful_publishing)
+            txn.commit()
+
             # Flag dirty pockets for any outstanding deletions.
             publisher.A2_markPocketsWithDeletionsDirty()
-            try_and_commit(
-                "dominating", publisher.B_dominate,
+            publisher.B_dominate(
                 options.careful or options.careful_domination)
+            txn.commit()
 
             # The primary and copy archives use apt-ftparchive to generate the
             # indexes, everything else uses the newer internal LP code.
             if archive.purpose in (ArchivePurpose.PRIMARY, ArchivePurpose.COPY):
-                try_and_commit(
-                    "doing apt-ftparchive", publisher.C_doFTPArchive,
+                publisher.C_doFTPArchive(
                     options.careful or options.careful_apt)
             else:
-                try_and_commit("building indexes", publisher.C_writeIndexes,
-                               options.careful or options.careful_apt)
+                publisher.C_writeIndexes(
+                    options.careful or options.careful_apt)
+            txn.commit()
 
-            try_and_commit(
-                "doing release files", publisher.D_writeReleaseFiles,
+            publisher.D_writeReleaseFiles(
                 options.careful or options.careful_apt)
+            txn.commit()
 
     log.debug("Ciao")

=== modified file 'lib/lp/soyuz/scripts/tests/test_initialise_distroseries.py'
--- lib/lp/soyuz/scripts/tests/test_initialise_distroseries.py	2011-04-14 02:04:18 +0000
+++ lib/lp/soyuz/scripts/tests/test_initialise_distroseries.py	2011-04-26 08:25:55 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test the initialise_distroseries script machinery."""
@@ -9,6 +9,7 @@
 import subprocess
 import sys
 
+from storm.locals import Store
 from testtools.content import Content
 from testtools.content_type import UTF8_TEXT
 import transaction
@@ -17,8 +18,10 @@
 from canonical.config import config
 from canonical.launchpad.interfaces.lpstorm import IStore
 from canonical.testing.layers import LaunchpadZopelessLayer
+from lp.archivepublisher.model import createdistroseriesindexesjob
 from lp.buildmaster.enums import BuildStatus
 from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.services.features.testing import FeatureFixture
 from lp.soyuz.enums import SourcePackageFormat
 from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet
 from lp.soyuz.interfaces.packageset import (
@@ -29,6 +32,10 @@
 from lp.soyuz.interfaces.sourcepackageformat import (
     ISourcePackageFormatSelectionSet,
     )
+from lp.soyuz.model.distributionjob import (
+    DistributionJob,
+    DistributionJobType,
+    )
 from lp.soyuz.model.distroarchseries import DistroArchSeries
 from lp.soyuz.scripts.initialise_distroseries import (
     InitialisationError,
@@ -349,6 +356,21 @@
         self.assertEqual(
             das[0].architecturetag, self.parent_das.architecturetag)
 
+    def test_schedule_index_creation(self):
+        # One follow-up step is creation of the new series' archive
+        # indexes.  The initialise() method schedules a job for this.
+        feature_flag = createdistroseriesindexesjob.FEATURE_FLAG_ENABLE_MODULE
+        self.useFixture(FeatureFixture({feature_flag: u'on'}))
+        child = self.factory.makeDistroSeries()
+        ids = InitialiseDistroSeries(self.parent, child)
+        ids.initialise()
+        job = Store.of(child).find(
+            DistributionJob,
+            DistributionJob.job_type ==
+                DistributionJobType.CREATEDISTROSERIESINDEXES,
+            DistributionJob.distroseries == child).one()
+        self.assertNotEqual(None, job)
+
     def test_script(self):
         # Do an end-to-end test using the command-line tool.
         uploader = self.factory.makePerson()


Follow ups