launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #17863
[Merge] lp:~cjwatson/launchpad/contents-race into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/contents-race into lp:launchpad.
Commit message:
Update Contents files in a way that doesn't suffer from races between generate-contents-files and publish-ftpmaster.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1384797 in Launchpad itself: "Contents generation races with publisher"
https://bugs.launchpad.net/launchpad/+bug/1384797
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/contents-race/+merge/249547
Update Contents files in a way that doesn't suffer from races between generate-contents-files and publish-ftpmaster.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/contents-race into lp:launchpad.
=== modified file 'lib/lp/archivepublisher/scripts/generate_contents_files.py'
--- lib/lp/archivepublisher/scripts/generate_contents_files.py 2012-11-10 02:25:07 +0000
+++ lib/lp/archivepublisher/scripts/generate_contents_files.py 2015-02-12 17:31:49 +0000
@@ -1,4 +1,4 @@
-# Copyright 2011-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2011-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Archive Contents files generator."""
@@ -16,7 +16,6 @@
from lp.archivepublisher.config import getPubConfig
from lp.registry.interfaces.distribution import IDistributionSet
from lp.registry.interfaces.pocket import PackagePublishingPocket
-from lp.registry.interfaces.series import SeriesStatus
from lp.services.command_spawner import (
CommandSpawner,
OutputLineHandler,
@@ -132,20 +131,9 @@
if not file_exists(path):
os.makedirs(path)
- def getSupportedSeries(self):
- """Return suites that are supported in this distribution.
-
- "Supported" means not EXPERIMENTAL or OBSOLETE.
- """
- unsupported_status = (SeriesStatus.EXPERIMENTAL,
- SeriesStatus.OBSOLETE)
- for series in self.distribution:
- if series.status not in unsupported_status:
- yield series
-
def getSuites(self):
"""Return suites that are actually supported in this distribution."""
- for series in self.getSupportedSeries():
+ for series in self.distribution.getSupportedSeries():
for pocket in PackagePublishingPocket.items:
suite = series.getSuite(pocket)
if file_exists(os.path.join(self.config.distsroot, suite)):
@@ -274,7 +262,7 @@
new_contents = os.path.join(
contents_dir, "%s.gz" % contents_filename)
contents_dest = os.path.join(
- self.config.distsroot, suite, "%s.gz" % contents_filename)
+ contents_dir, ".%s.gz" % contents_filename)
os.rename(current_contents, last_contents)
os.rename(new_contents, contents_dest)
=== modified file 'lib/lp/archivepublisher/scripts/publish_ftpmaster.py'
--- lib/lp/archivepublisher/scripts/publish_ftpmaster.py 2014-08-09 19:45:00 +0000
+++ lib/lp/archivepublisher/scripts/publish_ftpmaster.py 2015-02-12 17:31:49 +0000
@@ -1,4 +1,4 @@
-# Copyright 2011-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2011-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Master distro publishing script."""
@@ -10,6 +10,7 @@
from datetime import datetime
import os
+import shutil
from pytz import utc
from zope.component import getUtility
@@ -156,6 +157,19 @@
for purpose, config in candidates if config is not None)
+def newer_mtime(one_file, other_file):
+ """Is one_file newer than other_file, or is other_file missing?"""
+ try:
+ one_mtime = os.stat(one_file).st_mtime
+ except OSError:
+ return False
+ try:
+ other_mtime = os.stat(other_file).st_mtime
+ except OSError:
+ return True
+ return one_mtime > other_mtime
+
+
class PublishFTPMaster(LaunchpadCronScript):
"""Publish a distro (update).
@@ -539,6 +553,32 @@
# most of its time.
self.publishDistroArchive(distribution, archive)
+ def updateContentsFile(self, distribution, suite, arch):
+ """Update a single Contents file if necessary."""
+ config = self.configs[distribution][ArchivePurpose.PRIMARY]
+ backup_dists = get_backup_dists(config)
+ content_dists = os.path.join(
+ config.distroroot, "contents-generation", distribution.name,
+ "dists")
+ contents_filename = "Contents-%s" % arch.architecturetag
+ current_contents = os.path.join(
+ backup_dists, suite, "%s.gz" % contents_filename)
+ new_contents = os.path.join(
+ content_dists, suite, ".%s.gz" % contents_filename)
+ if newer_mtime(new_contents, current_contents):
+ self.logger.debug(
+ "Installing new Contents file for %s/%s.", suite,
+ arch.architecturetag)
+ shutil.copy2(new_contents, current_contents)
+
+ def updateContentsFiles(self, distribution):
+ """Pick up updated Contents files if necessary."""
+ for series in distribution.getSupportedSeries():
+ for pocket in PackagePublishingPocket.items:
+ suite = series.getSuite(pocket)
+ for arch in series.enabled_architectures:
+ self.updateContentsFile(distribution, suite, arch)
+
def publish(self, distribution, security_only=False):
"""Do the main publishing work.
@@ -558,6 +598,8 @@
# Let's assume the main archive is always modified
has_published = True
+ self.updateContentsFiles(distribution)
+
# Swizzle the now-updated backup dists and the current dists
# around.
self.installDists(distribution)
=== modified file 'lib/lp/archivepublisher/tests/test_generate_contents_files.py'
--- lib/lp/archivepublisher/tests/test_generate_contents_files.py 2013-09-11 08:17:34 +0000
+++ lib/lp/archivepublisher/tests/test_generate_contents_files.py 2015-02-12 17:31:49 +0000
@@ -1,4 +1,4 @@
-# Copyright 2011-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2011-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test for the `generate-contents-files` script."""
@@ -15,6 +15,7 @@
execute,
GenerateContentsFiles,
)
+from lp.archivepublisher.scripts.publish_ftpmaster import PublishFTPMaster
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.services.log.logger import DevNullLogger
from lp.services.osutils import write_file
@@ -183,14 +184,6 @@
self.assertEqual(
[das.architecturetag], script.getArchs(distroseries.name))
- def test_getSupportedSeries(self):
- # getSupportedSeries returns the supported distroseries in the
- # distribution.
- script = self.makeScript()
- distroseries = self.factory.makeDistroSeries(
- distribution=script.distribution)
- self.assertIn(distroseries, script.getSupportedSeries())
-
def test_getSuites(self):
# getSuites returns the full names (distroseries-pocket) of the
# pockets that have packages to publish.
@@ -268,7 +261,8 @@
script.content_archive, StartsWith(script.config.distroroot))
def test_main(self):
- # If run end-to-end, the script generates Contents.gz files.
+ # If run end-to-end, the script generates Contents.gz files, and a
+ # following publisher run will put those files in their final place.
distro = self.makeDistro()
distroseries = self.factory.makeDistroSeries(distribution=distro)
processor = self.factory.makeProcessor()
@@ -287,6 +281,13 @@
fake_overrides(script, distroseries)
script.process()
self.assertTrue(file_exists(os.path.join(
+ script.content_archive, distro.name, "dists", suite,
+ ".Contents-%s.gz" % das.architecturetag)))
+ publisher_script = PublishFTPMaster(test_args=["-d", distro.name])
+ publisher_script.txn = self.layer.txn
+ publisher_script.logger = DevNullLogger()
+ publisher_script.main()
+ self.assertTrue(file_exists(os.path.join(
script.config.distsroot, suite,
"Contents-%s.gz" % das.architecturetag)))
=== modified file 'lib/lp/archivepublisher/tests/test_publish_ftpmaster.py'
--- lib/lp/archivepublisher/tests/test_publish_ftpmaster.py 2014-04-15 16:32:03 +0000
+++ lib/lp/archivepublisher/tests/test_publish_ftpmaster.py 2015-02-12 17:31:49 +0000
@@ -1,4 +1,4 @@
-# Copyright 2011-2012 Canonical Ltd. This software is licensed under the
+# Copyright 2011-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test publish-ftpmaster cron script."""
@@ -8,6 +8,7 @@
import logging
import os
from textwrap import dedent
+import time
from apt_pkg import TagFile
from testtools.matchers import (
@@ -24,6 +25,7 @@
compose_shell_boolean,
find_run_parts_dir,
get_working_dists,
+ newer_mtime,
PublishFTPMaster,
shell_quote,
)
@@ -38,6 +40,7 @@
BufferLogger,
DevNullLogger,
)
+from lp.services.osutils import write_file
from lp.services.scripts.base import LaunchpadScriptFailure
from lp.services.utils import file_exists
from lp.soyuz.enums import (
@@ -208,6 +211,46 @@
self.assertEqual("no", compose_shell_boolean(False))
+class TestNewerMtime(TestCase):
+
+ def setUp(self):
+ super(TestCase, self).setUp()
+ tempdir = self.useTempDir()
+ self.a = os.path.join(tempdir, "a")
+ self.b = os.path.join(tempdir, "b")
+
+ def test_both_missing(self):
+ self.assertFalse(newer_mtime(self.a, self.b))
+
+ def test_one_missing(self):
+ write_file(self.b, "")
+ self.assertFalse(newer_mtime(self.a, self.b))
+
+ def test_other_missing(self):
+ write_file(self.a, "")
+ self.assertTrue(newer_mtime(self.a, self.b))
+
+ def test_older(self):
+ write_file(self.a, "")
+ os.utime(self.a, (0, 0))
+ write_file(self.b, "")
+ self.assertFalse(newer_mtime(self.a, self.b))
+
+ def test_equal(self):
+ now = time.time()
+ write_file(self.a, "")
+ os.utime(self.a, (now, now))
+ write_file(self.b, "")
+ os.utime(self.b, (now, now))
+ self.assertFalse(newer_mtime(self.a, self.b))
+
+ def test_newer(self):
+ write_file(self.a, "")
+ write_file(self.b, "")
+ os.utime(self.b, (0, 0))
+ self.assertTrue(newer_mtime(self.a, self.b))
+
+
class TestFindRunPartsDir(TestCaseWithFactory, HelpersMixin):
layer = ZopelessDatabaseLayer
@@ -796,6 +839,31 @@
"Did not find expected marker for %s."
% archive.purpose.title)
+ def test_updateContentsFile_installs_changed(self):
+ distro = self.makeDistroWithPublishDirectory()
+ distroseries = self.factory.makeDistroSeries(distribution=distro)
+ das = self.factory.makeDistroArchSeries(distroseries=distroseries)
+ script = self.makeScript(distro)
+ script.setUp()
+ script.setUpDirs()
+ archive_config = getPubConfig(distro.main_archive)
+ contents_filename = "Contents-%s.gz" % das.architecturetag
+ backup_suite = os.path.join(
+ archive_config.archiveroot + "-distscopy", "dists",
+ distroseries.name)
+ os.makedirs(backup_suite)
+ write_marker_file([backup_suite, contents_filename], "Old Contents")
+ os.utime(os.path.join(backup_suite, contents_filename), (0, 0))
+ content_suite = os.path.join(
+ archive_config.distroroot, "contents-generation", distro.name,
+ "dists", distroseries.name)
+ os.makedirs(content_suite)
+ write_marker_file(
+ [content_suite, ".%s" % contents_filename], "Contents")
+ script.updateContentsFile(distro, distroseries.name, das)
+ self.assertEqual(
+ "Contents", read_marker_file([backup_suite, contents_filename]))
+
def test_publish_always_returns_true_for_primary(self):
script = self.makeScript()
script.publishDistroUploads = FakeMethod()
=== modified file 'lib/lp/registry/interfaces/distribution.py'
--- lib/lp/registry/interfaces/distribution.py 2014-11-17 18:36:16 +0000
+++ lib/lp/registry/interfaces/distribution.py 2015-02-12 17:31:49 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Interfaces including and related to IDistribution."""
@@ -408,6 +408,12 @@
def getDevelopmentSeries():
"""Return the DistroSeries which are marked as in development."""
+ def getSupportedSeries():
+ """Return the DistroSeries that are supported in this distribution.
+
+ "Supported" means not EXPERIMENTAL or OBSOLETE.
+ """
+
def resolveSeriesAlias(name):
"""Resolve a series alias.
=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py 2014-12-08 04:20:17 +0000
+++ lib/lp/registry/model/distribution.py 2015-02-12 17:31:49 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Database classes for implementing distribution items."""
@@ -785,6 +785,14 @@
distribution=self,
status=SeriesStatus.DEVELOPMENT)
+ def getSupportedSeries(self):
+ """See `IDistribution`."""
+ unsupported_status = (SeriesStatus.EXPERIMENTAL,
+ SeriesStatus.OBSOLETE)
+ for series in self.series:
+ if series.status not in unsupported_status:
+ yield series
+
def getMilestone(self, name):
"""See `IDistribution`."""
return Milestone.selectOne("""
=== modified file 'lib/lp/registry/tests/test_distribution.py'
--- lib/lp/registry/tests/test_distribution.py 2013-08-01 14:09:45 +0000
+++ lib/lp/registry/tests/test_distribution.py 2015-02-12 17:31:49 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for Distribution."""
@@ -383,7 +383,7 @@
class SeriesTests(TestCaseWithFactory):
- """Test IDistribution.getSeries().
+ """Test IDistribution.getSeries() and friends.
"""
layer = LaunchpadFunctionalLayer
@@ -416,6 +416,19 @@
self.assertEqual(
series, distro.getSeries("devel", follow_aliases=True))
+ def test_getSupportedSeries(self):
+ distro = self.factory.makeDistribution()
+ self.factory.makeDistroSeries(
+ distribution=distro, status=SeriesStatus.OBSOLETE)
+ current = self.factory.makeDistroSeries(
+ distribution=distro, status=SeriesStatus.CURRENT)
+ development = self.factory.makeDistroSeries(
+ distribution=distro, status=SeriesStatus.DEVELOPMENT)
+ self.factory.makeDistroSeries(
+ distribution=distro, status=SeriesStatus.EXPERIMENTAL)
+ self.assertContentEqual(
+ [current, development], list(distro.getSupportedSeries()))
+
class DerivativesTests(TestCaseWithFactory):
"""Test IDistribution.derivatives.
Follow ups
