launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03517
[Merge] lp:~jtv/launchpad/bug-777941 into lp:launchpad
Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/bug-777941 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #55211 in Launchpad itself: "shouldn't require careful publisher run when cloning new released distrorelease"
https://bugs.launchpad.net/launchpad/+bug/55211
Bug #777941 in Launchpad itself: "Create distroseries indexes from regular publisher run"
https://bugs.launchpad.net/launchpad/+bug/777941
For more details, see:
https://code.launchpad.net/~jtv/launchpad/bug-777941/+merge/60116
= Summary =
Whenever we create a distroseries that we intend to publish, the publish-distro script needs to be run on it with the -A option to create its indexes in the filesystem.
== Proposed fix ==
This is currently a manual step in the Ubuntu release procedure. I had a new job type for it coded up, but we realized that there would be several advantages to running it from the publish-ftpmaster script instead:
* It's less work, of course. No need for a job type, config change, cron entry, etc.
* No more need to disable the cron jobs for fear that this might run concurrently with the cron script: it *is* the cron script, and it has a script lock.
* It's much more obvious how to deal with failure: better luck next time.
Most of the diff in this branch is deletion of the old, now unnecessary job work.
== Pre-implementation notes ==
The change was discussed with Julian, but also with Colin on the Ubuntu side. My job type sent notifications to the release manager, who could then (as per the release procedure) verify archive states manually. As it turns out, the release managers have no need for notifications because monitoring publisher progress is a routine part of the release procedure anyway. They will watch and verify as a matter of course.
Eventually we hope the need to disable cron jobs will disappear altogether. Until then, as Julian figured out, the Ubuntu case (which is the only distribution that we really want the verification step for) does not need the cron jobs disabled for verification. We can keep the publisher running regularly while waiting for the release managers to do the verification work.
== Implementation details ==
To ensure that we can absolutely, definitely, certainly, unequivocally, and reliably(*) see whether index creation has been completed, I made the script write a marker file in the archive root to indicate the fact. However the script only looks for this marker file for series that are in Frozen state (which happens when a series is created and again later for feature freeze). This will reduce the chance of indexes being re-created unnecessarily for existing series, or for series that are not in an inappropriate state. Julian agreed that this seemed sane.
(*) Unless the system went down at an awkward moment and left the filesystem in a really bad state. But then we'll probably have worse things to worry about.
Once this change is deployed, the current step 13 (have the LP team run publish-distro.py -A) can be dropped from the Ubuntu release procedure. Steps 12 & 14 (verification of the results) must be moved behind step 15 (re-enabling the cron jobs), and a wait of up to an hour and a half will be needed to ensure that the work has been done. We hope that disabling the cron jobs will become unnecessary altogether, but that also involves other changes.
== Tests ==
{{{
./bin/test -vvc lp.archivepublisher.tests.test_publish_ftpmaster
}}}
== Demo and Q/A ==
Create a new distroseries, in the Frozen state, and go through applicable steps of the Ubuntu release process.
Run cronscripts/publish-ftpmaster.py on the new series' distribution; this should log "Creating archive indexes for series <name>." It will churn for a while (somewhere between 5 and 30 minutes, I think) and complete.
Run the script again. This time it will not log that it's creating archive indexes, but instead go through its regular publishing work.
= Launchpad lint =
I cleaned up some pre-existing lint. The stuff in schema-lazr.conf isn't something I can do much about.
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/canonical/config/schema-lazr.conf
lib/lp/archivepublisher/scripts/publish_ftpmaster.py
lib/lp/archivepublisher/zcml/configure.zcml
lib/lp/soyuz/interfaces/distributionjob.py
lib/lp/archivepublisher/tests/test_publish_ftpmaster.py
./lib/canonical/config/schema-lazr.conf
536: Line exceeds 78 characters.
619: Line exceeds 78 characters.
995: Line exceeds 78 characters.
1088: Line exceeds 78 characters.
--
https://code.launchpad.net/~jtv/launchpad/bug-777941/+merge/60116
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/bug-777941 into lp:launchpad.
=== modified file 'lib/canonical/config/schema-lazr.conf'
--- lib/canonical/config/schema-lazr.conf 2011-05-03 04:39:43 +0000
+++ lib/canonical/config/schema-lazr.conf 2011-05-05 20:21:38 +0000
@@ -596,16 +596,6 @@
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
-
-# Utility interface to iterate jobs for job_runner to run.
-# datatype: string
-source_interface: lp.archivepublisher.interfaces.createdistroseriesindexesjob.ICreateDistroSeriesIndexesJobSource
-
-
[create_merge_proposals]
# The database user which will be used by this process.
# datatype: string
=== removed file 'lib/lp/archivepublisher/interfaces/createdistroseriesindexesjob.py'
--- lib/lp/archivepublisher/interfaces/createdistroseriesindexesjob.py 2011-04-27 08:20:37 +0000
+++ lib/lp/archivepublisher/interfaces/createdistroseriesindexesjob.py 1970-01-01 00:00:00 +0000
@@ -1,26 +0,0 @@
-# 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 `DistroSeries` that needs its archive
- indexes created.
- :return: An `ICreateDistroSeriesIndexesJob`, or None.
- """
=== removed file 'lib/lp/archivepublisher/model/createdistroseriesindexesjob.py'
--- lib/lp/archivepublisher/model/createdistroseriesindexesjob.py 2011-04-28 15:33:57 +0000
+++ lib/lp/archivepublisher/model/createdistroseriesindexesjob.py 1970-01-01 00:00:00 +0000
@@ -1,155 +0,0 @@
-# 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.registry.model.person import get_recipients
-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"
-
-
-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.CREATE_DISTROSERIES_INDEXES
-
- @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
-
- 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 indices
- 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)
-
- def getMailRecipients(self):
- """List email addresses to notify of success or failure."""
- recipient = self.distroseries.driver or self.distribution.owner
- return [
- format_address_for_person(recipient)
- for person in get_recipients(recipient)]
-
- def notifySuccess(self):
- """Notify the distribution's owners of success."""
- subject = "Launchpad has created archive indexes for %s %s" % (
- self.distribution.displayname, self.distroseries.displayname)
- 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
- MailController(
- from_addr, self.getMailRecipients(), subject, message).send()
-
- def getErrorRecipients(self):
- """See `BaseRunnableJob`."""
- return self.getMailRecipients()
-
- def destroySelf(self):
- """See `IDistributionJob`."""
- job = self.context.job
- Store.of(self.context).remove(self.context)
- Store.of(job).remove(job)
=== modified file 'lib/lp/archivepublisher/scripts/publish_ftpmaster.py'
--- lib/lp/archivepublisher/scripts/publish_ftpmaster.py 2011-04-13 04:16:19 +0000
+++ lib/lp/archivepublisher/scripts/publish_ftpmaster.py 2011-05-05 20:21:38 +0000
@@ -8,17 +8,22 @@
'PublishFTPMaster',
]
+from datetime import datetime
from optparse import OptionParser
import os
+from pytz import utc
from zope.component import getUtility
from canonical.config import config
from lp.archivepublisher.config import getPubConfig
+from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfigSet
from lp.registry.interfaces.distribution import IDistributionSet
+from lp.registry.interfaces.pocket import pocketsuffix
from lp.services.scripts.base import (
LaunchpadCronScript,
LaunchpadScriptFailure,
)
+from lp.registry.interfaces.series import SeriesStatus
from lp.services.utils import file_exists
from lp.soyuz.enums import ArchivePurpose
from lp.soyuz.scripts import publishdistro
@@ -94,6 +99,7 @@
"""
return get_dists(archive_config) + ".in-progress"
+
def extend_PATH():
"""Produce env dict for extending $PATH.
@@ -222,6 +228,59 @@
(archive.purpose, getPubConfig(archive))
for archive in self.archives)
+ def locateIndexesMarker(self, distroseries):
+ """Give path for marker file whose presence marks index creation.
+
+ The file will be created once the archive indexes for
+ `distroseries` have been created. This is how future runs will
+ know that this work is done.
+ """
+ archive_root = self.configs[ArchivePurpose.PRIMARY].archiveroot
+ return os.path.join(
+ archive_root, ".created-indexes-for-%s" % distroseries.name)
+
+ def needsIndexesCreated(self, distroseries):
+ """Does `distroseries` still need its archive indexes created?
+
+ Checks for the marker left by `markIndexCreationComplete`.
+ """
+ if distroseries.status != SeriesStatus.FROZEN:
+ # DistroSeries are created in Frozen state. If the series
+ # is in any other state yet has not been marked as having
+ # its indexes created, that's because it predates automatic
+ # index creation.
+ return False
+ distro = distroseries.distribution
+ publisher_config_set = getUtility(IPublisherConfigSet)
+ if publisher_config_set.getByDistribution(distro) is None:
+ # We won't be able to do a thing without a publisher config,
+ # but that's alright: we have those for all distributions
+ # that we want to publish.
+ return False
+ return not file_exists(self.locateIndexesMarker(distroseries))
+
+ def markIndexCreationComplete(self, distroseries):
+ """Note that archive indexes for `distroseries` have been created.
+
+ This tells `needsIndexesCreated` that no, this series no longer needs
+ archive indexes to be set up.
+ """
+ marker = file(self.locateIndexesMarker(distroseries), "w")
+ marker.write(
+ "Indexes for %s were created on %s.\n"
+ % (distroseries, datetime.now(utc)))
+ marker.close()
+
+ def createIndexes(self, distroseries):
+ """Create archive indexes for `distroseries`."""
+ self.logger.info(
+ "Creating archive indexes for series %s.", distroseries)
+ suites = [
+ distroseries.getSuite(pocket)
+ for pocket in pocketsuffix.iterkeys()]
+ self.runPublishDistro(args=['-A'], suites=suites)
+ self.markIndexCreationComplete(distroseries)
+
def processAccepted(self):
"""Run the process-accepted script."""
self.logger.debug(
@@ -294,6 +353,20 @@
"Creating backup dists directory %s", distscopy)
os.makedirs(distscopy)
+ def runPublishDistro(self, args=[], suites=None):
+ """Execute `publish-distro`."""
+ if suites is None:
+ suites = []
+ arguments = (
+ ['-d', self.distribution.name] +
+ args +
+ sum([['-s', suite] for suite in suites], []))
+
+ parser = OptionParser()
+ publishdistro.add_options(parser)
+ options, args = parser.parse_args(arguments)
+ publishdistro.run_publisher(options, self.txn, log=self.logger)
+
def publishDistroArchive(self, archive, security_suites=None):
"""Publish the results for an archive.
@@ -311,26 +384,13 @@
# for the duration.
temporary_dists = get_working_dists(archive_config)
- arguments = [
- '-v', '-v',
- '-d', self.distribution.name,
- '-R', temporary_dists,
- ]
-
+ arguments = ['-R', temporary_dists]
if archive.purpose == ArchivePurpose.PARTNER:
arguments.append('--partner')
- if security_suites is not None:
- arguments += sum([['-s', suite] for suite in security_suites], [])
-
- parser = OptionParser()
- publishdistro.add_options(parser)
-
os.rename(get_backup_dists(archive_config), temporary_dists)
try:
- options, args = parser.parse_args(arguments)
- publishdistro.run_publisher(
- options, txn=self.txn, log=self.logger)
+ self.runPublishDistro(args=arguments, suites=security_suites)
finally:
os.rename(temporary_dists, get_backup_dists(archive_config))
@@ -495,6 +555,14 @@
"""See `LaunchpadScript`."""
self.setUp()
self.recoverWorkingDists()
+
+ for series in self.distribution.series:
+ if self.needsIndexesCreated(series):
+ self.createIndexes(series)
+ # Don't try to do too much in one run. Leave the rest
+ # of the work for next time.
+ return
+
self.processAccepted()
self.setUpDirs()
=== removed file 'lib/lp/archivepublisher/tests/test_createdistroseriesindexesjob.py'
--- lib/lp/archivepublisher/tests/test_createdistroseriesindexesjob.py 2011-04-28 15:33:57 +0000
+++ lib/lp/archivepublisher/tests/test_createdistroseriesindexesjob.py 1970-01-01 00:00:00 +0000
@@ -1,309 +0,0 @@
-# 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
-
-from logging import (
- FATAL,
- getLogger,
- )
-import os.path
-from zope.component import getUtility
-from zope.security.proxy import removeSecurityProxy
-
-from canonical.config import config
-from canonical.launchpad.interfaces.lpstorm import IMasterStore
-from canonical.launchpad.webapp.testing import verifyObject
-from canonical.testing.layers import LaunchpadZopelessLayer
-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,
- CreateDistroSeriesIndexesJob,
- )
-from lp.registry.interfaces.pocket import pocketsuffix
-from lp.services.job.model.job import Job
-from lp.services.features.testing import FeatureFixture
-from lp.services.job.interfaces.job import (
- IRunnableJob,
- JobStatus,
- )
-from lp.services.job.runner import JobCronScript
-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.soyuz.model.distributionjob import DistributionJob
-from lp.testing import TestCaseWithFactory
-from lp.testing.fakemethod import FakeMethod
-from lp.testing.mail_helpers import run_mail_jobs
-
-
-def silence_publisher_logger():
- """Silence the logger that `run_publisher` creates."""
- getLogger("publish-distro").setLevel(FATAL)
-
-
-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)
- IMasterStore(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()
- return self.getJobSource().makeFor(distroseries)
-
- def getDistsRoot(self, distribution):
- """Get distsroot directory for `distribution`."""
- archive = removeSecurityProxy(distribution.main_archive)
- pub_config = getPubConfig(archive)
- return pub_config.distsroot
-
- def makeDistsDirs(self, distroseries):
- """Create dists directories in `distsroot` for `distroseries`."""
- distsroot = self.getDistsRoot(distroseries.distribution)
- base = os.path.join(distsroot, distroseries.name)
- for suffix in pocketsuffix.itervalues():
- os.makedirs(base + suffix)
-
- def makeCredibleJob(self):
- """Create a job with fixtures required for running it."""
- silence_publisher_logger()
- distro = self.factory.makeDistribution(
- publish_root_dir=unicode(self.makeTemporaryDirectory()))
- distroseries = self.factory.makeDistroSeries(distribution=distro)
- self.makeDistsDirs(distroseries)
- return self.makeJob(distroseries)
-
- def becomeArchivePublisher(self):
- """Become the archive publisher database user (and clean up later)."""
- self.becomeDbUser(config.archivepublisher.dbuser)
- self.addCleanup(self.becomeDbUser, 'launchpad')
-
- def getSuites(self, distroseries):
- """Get the list of suites for `distroseries`."""
- return [
- distroseries.name + suffix
- for suffix in pocketsuffix.itervalues()]
-
- 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.
- job = self.makeJob()
- self.assertContentEqual(
- self.getSuites(job.distroseries),
- removeSecurityProxy(job).getSuites())
-
- def test_getSuites_ignores_suites_for_other_distroseries(self):
- # getSuites does not list suites in the distribution that do not
- # belong to the right distroseries.
- job = self.makeJob()
- self.assertContentEqual(
- self.getSuites(job.distroseries),
- removeSecurityProxy(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()
- naked_job = removeSecurityProxy(job)
- naked_job.runPublishDistro = FakeMethod()
- job.run()
- args, kwargs = naked_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)
- naked_job = removeSecurityProxy(job)
- naked_job.runPublishDistro = FakeMethod()
- job.run()
- self.assertIn(
- ('--partner', ),
- [args for args, kwargs in naked_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()
- naked_job = removeSecurityProxy(job)
- naked_job.runPublishDistro = FakeMethod()
- job.run()
- self.assertEqual(1, naked_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()
- naked_job = removeSecurityProxy(job)
- naked_job.runPublishDistro = FakeMethod()
- naked_job.notifySuccess = FakeMethod()
- job.run()
- self.assertEqual(1, naked_job.notifySuccess.call_count)
-
- def test_failure_notifies_recipients(self):
- # Failure notices are sent to the addresses returned by
- # getMailRecipients.
- job = self.makeJob()
- removeSecurityProxy(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.
- job = self.makeJob()
- naked_job = removeSecurityProxy(job)
- naked_job.getMailRecipients = FakeMethod(result=["bar@xxxxxxxxxxx"])
- naked_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.
- job = self.makeJob()
- removeSecurityProxy(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()
- distroseries.driver = self.factory.makePerson()
- job = self.makeJob(distroseries)
- self.assertIn(
- format_address_for_person(distroseries.driver),
- removeSecurityProxy(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)
- self.assertIn(
- format_address_for_person(distroseries.distribution.owner),
- removeSecurityProxy(job).getMailRecipients())
-
- def test_destroySelf_destroys_job(self):
- job = self.makeJob()
- job_id = removeSecurityProxy(job).job.id
- self.becomeArchivePublisher()
- job.destroySelf()
- store = IMasterStore(Job)
- self.assertIs(None, store.find(Job, Job.id == job_id).one())
-
- def test_destroySelf_destroys_DistributionJob(self):
- job = self.makeJob()
- job_id = job.id
- self.becomeArchivePublisher()
- job.destroySelf()
- store = IMasterStore(DistributionJob)
- self.assertIs(
- None,
- store.find(DistributionJob, DistributionJob.id == job_id).one())
-
- def test_run_does_the_job(self):
- # The job runs publish_distro and generates the expected output
- # files.
- job = self.makeCredibleJob()
- self.becomeArchivePublisher()
- job.run()
- distsroot = self.getDistsRoot(job.distribution)
- output = os.path.join(distsroot, job.distroseries.name, "Release")
- self.assertTrue(file_exists(output))
-
- def test_job_runner_runs_jobs(self):
- # The generic job runner can set itself up to run these jobs.
- job = self.makeCredibleJob()
- script = JobCronScript(
- test_args=["create_distroseries_indexes"],
- commandline_config=True)
- script.logger = DevNullLogger()
- script.main()
- self.assertEqual(
- JobStatus.COMPLETED, removeSecurityProxy(job).context.job.status)
=== modified file 'lib/lp/archivepublisher/tests/test_publish_ftpmaster.py'
--- lib/lp/archivepublisher/tests/test_publish_ftpmaster.py 2011-04-13 04:16:19 +0000
+++ lib/lp/archivepublisher/tests/test_publish_ftpmaster.py 2011-05-05 20:21:38 +0000
@@ -8,11 +8,13 @@
from apt_pkg import TagFile
import logging
import os
+from testtools.matchers import StartsWith
from textwrap import dedent
from zope.component import getUtility
from canonical.config import config
from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
+from canonical.launchpad.interfaces.lpstorm import IMasterStore
from canonical.testing.layers import (
LaunchpadZopelessLayer,
ZopelessDatabaseLayer,
@@ -23,6 +25,7 @@
PackagePublishingPocket,
pocketsuffix,
)
+from lp.registry.interfaces.series import SeriesStatus
from lp.services.log.logger import (
BufferLogger,
DevNullLogger,
@@ -125,6 +128,30 @@
self.addCleanup(config.pop, "run-parts")
+ def makeDistro(self):
+ """Create a `Distribution` for testing.
+
+ The distribution will have a publishing directory set up, which
+ will be cleaned up after the test.
+ """
+ return self.factory.makeDistribution(
+ publish_root_dir=unicode(self.makeTemporaryDirectory()))
+
+ def makeScript(self, distro=None, extra_args=[]):
+ """Produce instance of the `PublishFTPMaster` script."""
+ if distro is None:
+ distro = self.makeDistro()
+ script = PublishFTPMaster(test_args=["-d", distro.name] + extra_args)
+ script.txn = self.layer.txn
+ script.logger = DevNullLogger()
+ return script
+
+ def setUpForScriptRun(self, distro):
+ """Mock up config to run the script on `distro`."""
+ pub_config = getUtility(IPublisherConfigSet).getByDistribution(distro)
+ pub_config.root_dir = unicode(
+ self.makeTemporaryDirectory())
+
class TestPublishFTPMasterHelpers(TestCase):
@@ -205,21 +232,6 @@
# Location of shell script.
SCRIPT_PATH = "cronscripts/publish-ftpmaster.py"
- def setUpForScriptRun(self, distro):
- """Mock up config to run the script on `distro`."""
- pub_config = getUtility(IPublisherConfigSet).getByDistribution(distro)
- pub_config.root_dir = unicode(
- self.makeTemporaryDirectory())
-
- def makeDistro(self):
- """Create a `Distribution` for testing.
-
- The distribution will have a publishing directory set up, which
- will be cleaned up after the test.
- """
- return self.factory.makeDistribution(
- publish_root_dir=unicode(self.makeTemporaryDirectory()))
-
def prepareUbuntu(self):
"""Obtain a reference to Ubuntu, set up for testing.
@@ -230,15 +242,6 @@
self.setUpForScriptRun(ubuntu)
return ubuntu
- def makeScript(self, distro=None, extra_args=[]):
- """Produce instance of the `PublishFTPMaster` script."""
- if distro is None:
- distro = self.makeDistro()
- script = PublishFTPMaster(test_args=["-d", distro.name] + extra_args)
- script.txn = self.layer.txn
- script.logger = DevNullLogger()
- return script
-
def readReleaseFile(self, filename):
"""Read a Release file, return as a keyword/value dict."""
sections = list(TagFile(file(filename)))
@@ -274,7 +277,7 @@
os.chmod(script_path, 0755)
def test_script_runs_successfully(self):
- ubuntu = self.prepareUbuntu()
+ self.prepareUbuntu()
self.layer.txn.commit()
stdout, stderr, retval = run_script(
self.SCRIPT_PATH + " -d ubuntu")
@@ -294,7 +297,6 @@
test_publisher = SoyuzTestPublisher()
distroseries = test_publisher.setUpDefaultDistroSeries()
distro = distroseries.distribution
- pub_config = get_pub_config(distro)
self.factory.makeComponentSelection(
distroseries=distroseries, component="main")
self.factory.makeArchive(
@@ -400,15 +402,15 @@
def test_getDirtySecuritySuites_ignores_non_security_suites(self):
distroseries = self.factory.makeDistroSeries()
- spphs = [
+ pockets = [
+ PackagePublishingPocket.RELEASE,
+ PackagePublishingPocket.UPDATES,
+ PackagePublishingPocket.PROPOSED,
+ PackagePublishingPocket.BACKPORTS,
+ ]
+ for pocket in pockets:
self.factory.makeSourcePackagePublishingHistory(
distroseries=distroseries, pocket=pocket)
- for pocket in [
- PackagePublishingPocket.RELEASE,
- PackagePublishingPocket.UPDATES,
- PackagePublishingPocket.PROPOSED,
- PackagePublishingPocket.BACKPORTS,
- ]]
script = self.makeScript(distroseries.distribution)
script.setUp()
self.assertEqual([], script.getDirtySecuritySuites())
@@ -759,3 +761,160 @@
read_marker_file([archive_root, "marker file"]).rstrip(),
"Did not find expected marker for %s."
% archive.purpose.title)
+
+
+class TestCreateDistroSeriesIndexes(TestCaseWithFactory, HelpersMixin):
+ """Test initial creation of archive indexes for a `DistroSeries`."""
+ layer = LaunchpadZopelessLayer
+
+ def createIndexesMarkerDir(self, script, distroseries):
+ """Create the directory for `distroseries`'s indexes marker."""
+ marker = script.locateIndexesMarker(distroseries)
+ os.makedirs(os.path.dirname(marker))
+
+ def test_new_frozen_series_needs_indexes_created(self):
+ # If a distroseries is Frozen and has not had its indexes
+ # created yet, needsIndexesCreated returns True for it.
+ series = self.factory.makeDistroSeries(status=SeriesStatus.FROZEN)
+ script = self.makeScript(series.distribution)
+ script.setUp()
+ self.assertTrue(script.needsIndexesCreated(series))
+
+ def test_new_nonfrozen_series_does_not_need_indexes_created(self):
+ # needsIndexesCreated only returns True for Frozen distroseries.
+ series = self.factory.makeDistroSeries()
+ script = self.makeScript(series.distribution)
+ self.assertFalse(script.needsIndexesCreated(series))
+
+ def test_distro_without_publisher_config_gets_no_indexes(self):
+ # needsIndexesCreated returns False for distributions that have
+ # no publisher config, such as Debian. We don't want to publish
+ # these distributions.
+ series = self.factory.makeDistroSeries(status=SeriesStatus.FROZEN)
+ pub_config = get_pub_config(series.distribution)
+ IMasterStore(pub_config).remove(pub_config)
+ script = self.makeScript(series.distribution)
+ self.assertFalse(script.needsIndexesCreated(series))
+
+ def test_markIndexCreationComplete_tells_needsIndexesCreated_no(self):
+ # The effect of markIndexCreationComplete is to make
+ # needsIndexesCreated for that distroseries return False.
+ distro = self.makeDistro()
+ series = self.factory.makeDistroSeries(
+ status=SeriesStatus.FROZEN, distribution=distro)
+ script = self.makeScript(distro)
+ script.setUp()
+ self.createIndexesMarkerDir(script, series)
+
+ self.assertTrue(script.needsIndexesCreated(series))
+ script.markIndexCreationComplete(series)
+ self.assertFalse(script.needsIndexesCreated(series))
+
+ def test_createIndexes_marks_index_creation_complete(self):
+ # createIndexes calls markIndexCreationComplete for the
+ # distroseries.
+ distro = self.makeDistro()
+ series = self.factory.makeDistroSeries(distribution=distro)
+ script = self.makeScript(distro)
+ script.markIndexCreationComplete = FakeMethod()
+ script.runPublishDistro = FakeMethod()
+ script.createIndexes(series)
+ self.assertEqual(
+ [((series, ), {})], script.markIndexCreationComplete.calls)
+
+ def test_failed_index_creation_is_not_marked_complete(self):
+ # If index creation fails, it is not marked as having been
+ # completed. The next run will retry.
+ class Boom(Exception):
+ """Simulated failure."""
+
+ series = self.factory.makeDistroSeries()
+ script = self.makeScript(series.distribution)
+ script.markIndexCreationComplete = FakeMethod()
+ script.runPublishDistro = FakeMethod(failure=Boom("Sorry!"))
+ try:
+ script.createIndexes(series)
+ except:
+ pass
+ self.assertEqual([], script.markIndexCreationComplete.calls)
+
+ def test_locateIndexesMarker_places_file_in_archive_root(self):
+ # The marker file for index creation is in the distribution's
+ # archive root.
+ series = self.factory.makeDistroSeries()
+ script = self.makeScript(series.distribution)
+ script.setUp()
+ archive_root = script.configs[ArchivePurpose.PRIMARY].archiveroot
+ self.assertThat(
+ script.locateIndexesMarker(series),
+ StartsWith(os.path.normpath(archive_root)))
+
+ def test_locateIndexesMarker_uses_separate_files_per_series(self):
+ # Different release series of the same distribution get separate
+ # marker files for index creation.
+ distro = self.makeDistro()
+ series1 = self.factory.makeDistroSeries(distribution=distro)
+ series2 = self.factory.makeDistroSeries(distribution=distro)
+ script = self.makeScript(distro)
+ script.setUp()
+ self.assertNotEqual(
+ script.locateIndexesMarker(series1),
+ script.locateIndexesMarker(series2))
+
+ def test_locateIndexMarker_uses_hidden_file(self):
+ # The index-creation marker file is a "dot file," so it's not
+ # visible in normal directory listings.
+ series = self.factory.makeDistroSeries()
+ script = self.makeScript(series.distribution)
+ script.setUp()
+ self.assertThat(
+ os.path.basename(script.locateIndexesMarker(series)),
+ StartsWith("."))
+
+ def test_script_calls_createIndexes_for_new_series(self):
+ # If the script's main() finds a distroseries that needs its
+ # indexes created, it calls createIndexes on that distroseries.
+ distro = self.makeDistro()
+ series = self.factory.makeDistroSeries(
+ status=SeriesStatus.FROZEN, distribution=distro)
+ script = self.makeScript(distro)
+ script.createIndexes = FakeMethod()
+ script.main()
+ self.assertEqual([((series, ), {})], script.createIndexes.calls)
+
+ def test_createIndexes_ignores_other_series(self):
+ # createIndexes does not accidentally also touch other
+ # distroseries than the one it's meant to.
+ distro = self.makeDistro()
+ series = self.factory.makeDistroSeries(distribution=distro)
+ self.factory.makeDistroSeries(distribution=distro)
+ script = self.makeScript(distro)
+ script.setUp()
+ script.runPublishDistro = FakeMethod()
+ self.createIndexesMarkerDir(script, series)
+
+ script.createIndexes(series)
+
+ args, kwargs = script.runPublishDistro.calls[0]
+ suites = kwargs['suites']
+ self.assertEqual(len(pocketsuffix), len(suites))
+ for suite in suites:
+ self.assertThat(suite, StartsWith(series.name))
+
+ def test_script_creates_indexes(self):
+ # End-to-end test: the script creates indexes for distroseries
+ # that need them.
+ test_publisher = SoyuzTestPublisher()
+ series = test_publisher.setUpDefaultDistroSeries()
+ series.status = SeriesStatus.FROZEN
+ self.factory.makeComponentSelection(
+ distroseries=series, component="main")
+ self.layer.txn.commit()
+ self.setUpForScriptRun(series.distribution)
+ script = self.makeScript(series.distribution)
+ script.main()
+ self.assertFalse(script.needsIndexesCreated(series))
+ sources = os.path.join(
+ script.configs[ArchivePurpose.PRIMARY].distsroot,
+ series.name, "main", "source", "Sources")
+ self.assertTrue(file_exists(sources))
=== modified file 'lib/lp/archivepublisher/zcml/configure.zcml'
--- lib/lp/archivepublisher/zcml/configure.zcml 2011-04-26 04:31:38 +0000
+++ lib/lp/archivepublisher/zcml/configure.zcml 2011-05-05 20:21:38 +0000
@@ -28,16 +28,4 @@
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-29 07:37:38 +0000
+++ lib/lp/soyuz/interfaces/distributionjob.py 2011-05-05 20:21:38 +0000
@@ -28,7 +28,6 @@
Int,
List,
Object,
- TextLine,
Tuple,
)
@@ -89,13 +88,6 @@
distribution release series and its parent series.
""")
- CREATE_DISTROSERIES_INDEXES = 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-26 05:04:45 +0000
+++ lib/lp/soyuz/scripts/initialise_distroseries.py 2011-05-05 20:21:38 +0000
@@ -17,9 +17,6 @@
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
@@ -56,9 +53,6 @@
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
@@ -143,7 +137,6 @@
self._copy_architectures()
self._copy_packages()
self._copy_packagesets()
- self._request_index_creation()
transaction.commit()
def _set_parent(self):
@@ -336,8 +329,3 @@
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/tests/test_initialise_distroseries.py'
--- lib/lp/soyuz/scripts/tests/test_initialise_distroseries.py 2011-04-28 13:21:34 +0000
+++ lib/lp/soyuz/scripts/tests/test_initialise_distroseries.py 2011-05-05 20:21:38 +0000
@@ -9,7 +9,6 @@
import subprocess
import sys
-from storm.locals import Store
from testtools.content import Content
from testtools.content_type import UTF8_TEXT
import transaction
@@ -18,10 +17,8 @@
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 (
@@ -32,10 +29,6 @@
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,
@@ -258,7 +251,7 @@
# When initialising a new series within a distro, the copied
# packagesets have ownership preserved.
ps_owner = self.factory.makePerson()
- ps = getUtility(IPackagesetSet).new(
+ getUtility(IPackagesetSet).new(
u'ps', u'packageset', ps_owner, distroseries=self.parent)
child = self._full_initialise(distribution=self.parent.distribution)
child_ps = getUtility(IPackagesetSet).getByName(
@@ -268,7 +261,7 @@
def test_packageset_owner_not_preserved_cross_distro(self):
# In the case of a cross-distro initialisation, the new
# packagesets are owned by the new distro owner.
- ps = getUtility(IPackagesetSet).new(
+ getUtility(IPackagesetSet).new(
u'ps', u'packageset', self.factory.makePerson(),
distroseries=self.parent)
child = self._full_initialise()
@@ -282,7 +275,7 @@
test1 = getUtility(IPackagesetSet).new(
u'test1', u'test 1 packageset', self.parent.owner,
distroseries=self.parent)
- test2 = getUtility(IPackagesetSet).new(
+ getUtility(IPackagesetSet).new(
u'test2', u'test 2 packageset', self.parent.owner,
distroseries=self.parent)
packages = ('udev', 'chromium', 'libc6')
@@ -321,7 +314,7 @@
test1 = getUtility(IPackagesetSet).new(
u'test1', u'test 1 packageset', self.parent.owner,
distroseries=self.parent)
- test2 = getUtility(IPackagesetSet).new(
+ getUtility(IPackagesetSet).new(
u'test2', u'test 2 packageset', self.parent.owner,
distroseries=self.parent)
packages = ('udev', 'chromium')
@@ -356,21 +349,6 @@
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.CREATE_DISTROSERIES_INDEXES,
- 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
