← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~cjwatson/launchpad/germinate-all-dev-series into lp:launchpad

 

Colin Watson has proposed merging lp:~cjwatson/launchpad/germinate-all-dev-series into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #904538 in Launchpad itself: "generate-extra-overrides is confused by multiple DEVELOPMENT/FROZEN distroseries"
  https://bugs.launchpad.net/launchpad/+bug/904538

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/germinate-all-dev-series/+merge/86909

== Summary ==

My recent refactoring of cron.germinate caused some dogfood-specific problems, because dogfood has multiple Ubuntu distroseries in the DEVELOPMENT or FROZEN states.

== Proposed fix ==

Generate extra overrides for all DEVELOPMENT and FROZEN series, rather than for just one.  Log but otherwise ignore any cases where missing seeds would previously have caused us to abort.

This seems like logically the right thing to do, and removes the obstacle to testing this easily on dogfood.

== Implementation details ==

I introduced a simple new CachedDistroSeries class to preserve the current property that most of generate-extra-overrides runs without a database transaction, without having to pass around more clumsy piles of arguments (otherwise, I was going to have to add a list of lists of components somewhere).

== Tests ==

bin/test -vvct generate_extra_overrides

== Demo and Q/A ==

Run cron.germinate on mawson.  Given the current state of dogfood's database, it should emit log entries about the lack of seeds for rusty, and generate updated extra override files (in /srv/launchpad.net/ubuntu-archive/ubuntu-misc/more-extra.override.{oneiric,precise}.main) for precise and oneiric.  Previously, it would fail due to the lack of seeds for rusty.

== lint ==

None.
-- 
https://code.launchpad.net/~cjwatson/launchpad/germinate-all-dev-series/+merge/86909
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/germinate-all-dev-series into lp:launchpad.
=== modified file 'lib/lp/archivepublisher/scripts/generate_extra_overrides.py'
--- lib/lp/archivepublisher/scripts/generate_extra_overrides.py	2011-12-24 17:49:30 +0000
+++ lib/lp/archivepublisher/scripts/generate_extra_overrides.py	2011-12-26 17:31:25 +0000
@@ -17,7 +17,10 @@
 from germinate.archive import TagFile
 from germinate.germinator import Germinator
 from germinate.log import GerminateFormatter
-from germinate.seeds import SeedStructure
+from germinate.seeds import (
+    SeedError,
+    SeedStructure,
+    )
 from zope.component import getUtility
 
 from lp.services.webapp.dbpolicy import (
@@ -50,17 +53,29 @@
             os.rename("%s.new" % self.filename, self.filename)
 
 
+class CachedDistroSeries:
+    """Cache only the fields we need from a DistroSeries."""
+
+    def __init__(self, series):
+        self.name = series.name
+        # Even if DistroSeries.component_names starts including partner,
+        # we don't want it; this applies to the primary archive only.
+        self.components = [component for component in series.component_names]
+        self.architectures = [arch.architecturetag
+                              for arch in series.architectures]
+
+
 def find_operable_series(distribution):
-    """Find a series we can operate on in this distribution.
+    """Find all the series we can operate on in this distribution.
 
     We are allowed to modify DEVELOPMENT or FROZEN series, but should leave
     series with any other status alone.
     """
-    series = distribution.currentseries
-    if series.status in (SeriesStatus.DEVELOPMENT, SeriesStatus.FROZEN):
-        return series
-    else:
-        return None
+    operable_series = []
+    for series in distribution.series:
+        if series.status in (SeriesStatus.DEVELOPMENT, SeriesStatus.FROZEN):
+            operable_series.append(CachedDistroSeries(series))
+    return operable_series
 
 
 class GenerateExtraOverrides(LaunchpadScript):
@@ -96,17 +111,11 @@
                 "Distribution '%s' not found." % self.options.distribution)
 
         self.series = find_operable_series(self.distribution)
-        if self.series is None:
+        if not self.series:
             raise LaunchpadScriptFailure(
-                "There is no DEVELOPMENT distroseries for %s." %
+                "There is no DEVELOPMENT or FROZEN distroseries for %s." %
                 self.options.distribution)
 
-        # Even if DistroSeries.component_names starts including partner, we
-        # don't want it; this applies to the primary archive only.
-        self.components = [component
-                           for component in self.series.component_names
-                           if component != "partner"]
-
     def getConfig(self):
         """Set up a configuration object for this archive."""
         archive = self.distribution.main_archive
@@ -151,8 +160,19 @@
     def makeSeedStructures(self, series_name, flavours, seed_bases=None):
         structures = {}
         for flavour in flavours:
-            structures[flavour] = SeedStructure(
-                "%s.%s" % (flavour, series_name), seed_bases=seed_bases)
+            try:
+                structure = SeedStructure(
+                    "%s.%s" % (flavour, series_name), seed_bases=seed_bases)
+                if len(structure):
+                    structures[flavour] = structure
+                else:
+                    self.logger.warning(
+                        "Skipping empty seed structure for %s.%s",
+                        flavour, series_name)
+            except SeedError, e:
+                self.logger.warning(
+                    "Failed to fetch seeds for %s.%s: %s",
+                    flavour, series_name, e)
         return structures
 
     def logGerminateProgress(self, *args):
@@ -278,58 +298,54 @@
         if "build-essential" in structure.names and primary_flavour:
             write_overrides("build-essential", "Build-Essential", "yes")
 
-    def germinateArch(self, override_file, series_name, arch, flavours,
+    def germinateArch(self, override_file, series, arch, flavours,
                       structures):
         """Germinate seeds on all flavours for a single architecture."""
         germinator = Germinator(arch)
 
         # Read archive metadata.
         archive = TagFile(
-            series_name, self.components, arch,
+            series.name, series.components, arch,
             "file://%s" % self.config.archiveroot, cleanup=True)
         germinator.parse_archive(archive)
 
         for flavour in flavours:
             self.logger.info(
-                "Germinating for %s/%s/%s", flavour, series_name, arch)
+                "Germinating for %s/%s/%s", flavour, series.name, arch)
             # Add this to the germinate log as well so that that can be
             # debugged more easily.  Log a separator line first.
             self.logGerminateProgress("")
             self.logGerminateProgress(
-                "Germinating for %s/%s/%s", flavour, series_name, arch)
+                "Germinating for %s/%s/%s", flavour, series.name, arch)
 
             self.germinateArchFlavour(
-                override_file, germinator, series_name, arch, flavour,
+                override_file, germinator, series.name, arch, flavour,
                 structures[flavour], flavour == flavours[0])
 
-    def generateExtraOverrides(self, series_name, series_architectures,
-                               flavours, seed_bases=None):
+    def generateExtraOverrides(self, series, flavours, seed_bases=None):
         structures = self.makeSeedStructures(
-            series_name, flavours, seed_bases=seed_bases)
+            series.name, flavours, seed_bases=seed_bases)
 
-        override_path = os.path.join(
-            self.config.miscroot,
-            "more-extra.override.%s.main" % series_name)
-        with AtomicFile(override_path) as override_file:
-            for arch in series_architectures:
-                self.germinateArch(
-                    override_file, series_name, arch, flavours, structures)
+        if structures:
+            override_path = os.path.join(
+                self.config.miscroot,
+                "more-extra.override.%s.main" % series.name)
+            with AtomicFile(override_path) as override_file:
+                for arch in series.architectures:
+                    self.germinateArch(
+                        override_file, series, arch, flavours, structures)
 
     def process(self, seed_bases=None):
         """Do the bulk of the work."""
         self.setUp()
 
-        series_name = self.series.name
-        series_architectures = sorted(
-            [arch.architecturetag for arch in self.series.architectures])
-
         # This takes a while.  Ensure that we do it without keeping a
         # database transaction open.
         self.txn.commit()
         with DatabaseBlockedPolicy():
-            self.generateExtraOverrides(
-                series_name, series_architectures, self.args,
-                seed_bases=seed_bases)
+            for series in self.series:
+                self.generateExtraOverrides(
+                    series, self.args, seed_bases=seed_bases)
 
     def main(self):
         """See `LaunchpadScript`."""

=== modified file 'lib/lp/archivepublisher/tests/test_generate_extra_overrides.py'
--- lib/lp/archivepublisher/tests/test_generate_extra_overrides.py	2011-12-21 19:25:05 +0000
+++ lib/lp/archivepublisher/tests/test_generate_extra_overrides.py	2011-12-26 17:31:25 +0000
@@ -23,6 +23,7 @@
     )
 from lp.archivepublisher.scripts.generate_extra_overrides import (
     AtomicFile,
+    CachedDistroSeries,
     GenerateExtraOverrides,
     )
 from lp.archivepublisher.utils import RepositoryIndexFile
@@ -260,7 +261,8 @@
         development_distroseries = self.factory.makeDistroSeries(
             distro, status=SeriesStatus.DEVELOPMENT)
         script = self.makeScript(distro)
-        self.assertEqual(development_distroseries, script.series)
+        observed_series = [series.name for series in script.series]
+        self.assertEqual([development_distroseries.name], observed_series)
 
     def test_permits_frozen_distro_series(self):
         # If there is no DEVELOPMENT series, a FROZEN one will do.
@@ -270,7 +272,8 @@
         frozen_distroseries = self.factory.makeDistroSeries(
             distro, status=SeriesStatus.FROZEN)
         script = self.makeScript(distro)
-        self.assertEqual(frozen_distroseries, script.series)
+        observed_series = [series.name for series in script.series]
+        self.assertEqual([frozen_distroseries.name], observed_series)
 
     def test_requires_development_frozen_distro_series(self):
         # If there is no DEVELOPMENT or FROZEN series, the script fails.
@@ -280,6 +283,28 @@
         script = self.makeScript(distro, run_setup=False)
         self.assertRaises(LaunchpadScriptFailure, script.processOptions)
 
+    def test_multiple_development_frozen_distro_series(self):
+        # If there are multiple DEVELOPMENT or FROZEN series, they are all
+        # used.
+        distro = self.makeDistro()
+        development_distroseries_one = self.factory.makeDistroSeries(
+            distro, status=SeriesStatus.DEVELOPMENT)
+        development_distroseries_two = self.factory.makeDistroSeries(
+            distro, status=SeriesStatus.DEVELOPMENT)
+        frozen_distroseries_one = self.factory.makeDistroSeries(
+            distro, status=SeriesStatus.FROZEN)
+        frozen_distroseries_two = self.factory.makeDistroSeries(
+            distro, status=SeriesStatus.FROZEN)
+        script = self.makeScript(distro)
+        expected_series = [
+            development_distroseries_one.name,
+            development_distroseries_two.name,
+            frozen_distroseries_one.name,
+            frozen_distroseries_two.name,
+            ]
+        observed_series = [series.name for series in script.series]
+        self.assertContentEqual(expected_series, observed_series)
+
     def test_components_exclude_partner(self):
         # If a 'partner' component exists, it is excluded.
         distro = self.makeDistro()
@@ -289,7 +314,8 @@
         self.factory.makeComponentSelection(
             distroseries=distroseries, component="partner")
         script = self.makeScript(distro)
-        self.assertEqual(["main"], script.components)
+        self.assertEqual(1, len(script.series))
+        self.assertEqual(["main"], script.series[0].components)
 
     def test_compose_output_path_in_germinateroot(self):
         # Output files are written to the correct locations under
@@ -308,15 +334,57 @@
                 arch),
             output)
 
-    def fetchGerminatedOverrides(self, script, series_name, arch, flavours):
+    def test_make_seed_structures_missing_seeds(self):
+        # makeSeedStructures ignores missing seeds.
+        distro = self.makeDistro()
+        distroseries = self.factory.makeDistroSeries(distribution=distro)
+        series_name = distroseries.name
+        script = self.makeScript(distro)
+        flavour = self.factory.getUniqueString()
+
+        structures = script.makeSeedStructures(
+            series_name, [flavour], seed_bases=["file://%s" % self.seeddir])
+        self.assertEqual({}, structures)
+
+    def test_make_seed_structures_empty_seed_structure(self):
+        # makeSeedStructures ignores an empty seed structure.
+        distro = self.makeDistro()
+        distroseries = self.factory.makeDistroSeries(distribution=distro)
+        series_name = distroseries.name
+        script = self.makeScript(distro)
+        flavour = self.factory.getUniqueString()
+        self.makeSeedStructure(flavour, series_name, [])
+
+        structures = script.makeSeedStructures(
+            series_name, [flavour], seed_bases=["file://%s" % self.seeddir])
+        self.assertEqual({}, structures)
+
+    def test_make_seed_structures_valid_seeds(self):
+        # makeSeedStructures reads valid seeds successfully.
+        distro = self.makeDistro()
+        distroseries = self.factory.makeDistroSeries(distribution=distro)
+        series_name = distroseries.name
+        script = self.makeScript(distro)
+        flavour = self.factory.getUniqueString()
+        seed = self.factory.getUniqueString()
+        self.makeSeedStructure(flavour, series_name, [seed])
+        self.makeSeed(flavour, series_name, seed, [])
+
+        structures = script.makeSeedStructures(
+            series_name, [flavour], seed_bases=["file://%s" % self.seeddir])
+        self.assertIn(flavour, structures)
+
+    def fetchGerminatedOverrides(self, script, distroseries, arch, flavours):
         """Helper to call script.germinateArch and return overrides."""
         structures = script.makeSeedStructures(
-            series_name, flavours, seed_bases=["file://%s" % self.seeddir])
+            distroseries.name, flavours,
+            seed_bases=["file://%s" % self.seeddir])
 
         override_fd, override_path = tempfile.mkstemp()
         with os.fdopen(override_fd, "w") as override_file:
             script.germinateArch(
-                override_file, series_name, arch, flavours, structures)
+                override_file, CachedDistroSeries(distroseries), arch,
+                flavours, structures)
         return file_contents(override_path).splitlines()
 
     def test_germinate_output(self):
@@ -344,7 +412,7 @@
         self.makeSeed(flavour_two, series_name, seed, [two.name])
 
         overrides = self.fetchGerminatedOverrides(
-            script, series_name, arch, [flavour_one, flavour_two])
+            script, distroseries, arch, [flavour_one, flavour_two])
         self.assertEqual([], overrides)
 
         seed_dir_one = os.path.join(
@@ -402,7 +470,7 @@
             headers=["Task-Description: two"])
 
         overrides = self.fetchGerminatedOverrides(
-            script, series_name, arch, [flavour])
+            script, distroseries, arch, [flavour])
         expected_overrides = [
             "%s/%s  Task  %s" % (one.name, arch, seed_one),
             "%s/%s  Task  %s" % (two.name, arch, seed_one),
@@ -510,10 +578,39 @@
         self.makeSeed(flavour, series_name, seed, [package.name])
 
         overrides = self.fetchGerminatedOverrides(
-            script, series_name, arch, [flavour])
+            script, distroseries, arch, [flavour])
         self.assertContentEqual(
             ["%s/%s  Build-Essential  yes" % (package.name, arch)], overrides)
 
+    def test_process_missing_seeds(self):
+        # The script ignores series with no seed structures.
+        distro = self.makeDistro()
+        distroseries_one = self.factory.makeDistroSeries(distribution=distro)
+        distroseries_two = self.factory.makeDistroSeries(distribution=distro)
+        component = self.factory.makeComponent()
+        self.factory.makeComponentSelection(
+            distroseries=distroseries_one, component=component)
+        self.factory.makeComponentSelection(
+            distroseries=distroseries_two, component=component)
+        das_one = self.factory.makeDistroArchSeries(
+            distroseries=distroseries_one)
+        das_two = self.factory.makeDistroArchSeries(
+            distroseries=distroseries_two)
+        flavour = self.factory.getUniqueString()
+        script = self.makeScript(distro, extra_args=[flavour])
+        self.makeIndexFiles(script, distroseries_two)
+        seed = self.factory.getUniqueString()
+        self.makeSeedStructure(flavour, distroseries_two.name, [seed])
+        self.makeSeed(flavour, distroseries_two.name, seed, [])
+
+        script.process(seed_bases=["file://%s" % self.seeddir])
+        self.assertFalse(os.path.exists(os.path.join(
+            script.config.miscroot,
+            "more-extra.override.%s.main" % distroseries_one.name)))
+        self.assertTrue(os.path.exists(os.path.join(
+            script.config.miscroot,
+            "more-extra.override.%s.main" % distroseries_two.name)))
+
     def test_main(self):
         # If run end-to-end, the script generates override files containing
         # output for all architectures, and sends germinate's log output to


Follow ups