launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/by-hash-prune-immutable into lp:launchpad.

Commit message:
Prune by-hash files from immutable suites without trying to regenerate the Release file.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/by-hash-prune-immutable/+merge/293145

Prune by-hash files from immutable suites without trying to regenerate the Release file.  This fixes OOPSes we saw following the 16.04 release, and for which we currently have a somewhat different cowboy in place (https://pastebin.canonical.com/155032/).
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/by-hash-prune-immutable into lp:launchpad.

=== modified file 'lib/lp/archivepublisher/publishing.py'
--- lib/lp/archivepublisher/publishing.py	2016-04-14 10:26:19 +0000
+++ lib/lp/archivepublisher/publishing.py	2016-04-27 16:22:00 +0000
@@ -758,18 +758,10 @@
             distroseries, pocket = self.distro.getDistroSeriesAndPocket(
                 container[len(u"release:"):])
             archive_file_suites.add((distroseries, pocket))
-            self.release_files_needed.add((distroseries.name, pocket))
 
         for distroseries in self.distro:
             for pocket in self.archive.getPockets():
-                if not is_careful:
-                    if (not self.isDirty(distroseries, pocket) and
-                            (distroseries, pocket) not in archive_file_suites):
-                        self.log.debug("Skipping release files for %s/%s" %
-                                       (distroseries.name, pocket.name))
-                        continue
-                    self.checkDirtySuiteBeforePublishing(distroseries, pocket)
-                else:
+                if is_careful:
                     if not self.isAllowed(distroseries, pocket):
                         continue
                     # If we were asked for careful Release file generation
@@ -783,7 +775,27 @@
                     if file_exists(release_path):
                         self.release_files_needed.add(
                             (distroseries.name, pocket))
-                self._writeSuite(distroseries, pocket)
+
+                if (distroseries.name, pocket) in self.release_files_needed:
+                    if not is_careful:
+                        if not self.isDirty(distroseries, pocket):
+                            self.log.debug("Skipping release files for %s/%s" %
+                                           (distroseries.name, pocket.name))
+                            continue
+                        self.checkDirtySuiteBeforePublishing(
+                            distroseries, pocket)
+                    self._writeSuite(distroseries, pocket)
+                elif ((distroseries, pocket) in archive_file_suites and
+                      distroseries.publish_by_hash):
+                    # We aren't publishing a new Release file for this
+                    # suite, probably because it's immutable, but we still
+                    # need to prune by-hash files from it.
+                    suite = distroseries.getSuite(pocket)
+                    release_path = os.path.join(
+                        self._config.distsroot, suite, "Release")
+                    with open(release_path) as release_file:
+                        release_data = Release(release_file)
+                    self._updateByHash(suite, release_data)
 
     def _allIndexFiles(self, distroseries):
         """Return all index files on disk for a distroseries.
@@ -1131,16 +1143,6 @@
     def _writeSuite(self, distroseries, pocket):
         """Write out the Release files for the provided suite."""
         # XXX: kiko 2006-08-24: Untested method.
-
-        # As we generate file lists for apt-ftparchive we record which
-        # distroseriess and so on we need to generate Release files for.
-        # We store this in release_files_needed and consume the information
-        # when writeReleaseFiles is called.
-        if (distroseries.name, pocket) not in self.release_files_needed:
-            # If we don't need to generate a release for this release
-            # and pocket, don't!
-            return
-
         suite = distroseries.getSuite(pocket)
         suite_dir = os.path.join(self._config.distsroot, suite)
         all_components = [

=== modified file 'lib/lp/archivepublisher/tests/test_publisher.py'
--- lib/lp/archivepublisher/tests/test_publisher.py	2016-04-14 10:26:19 +0000
+++ lib/lp/archivepublisher/tests/test_publisher.py	2016-04-27 16:22:00 +0000
@@ -2797,9 +2797,7 @@
                 MatchesStructure(scheduled_deletion_date=Not(Is(None))),
                 ]))
 
-    def test_prune(self):
-        # The publisher prunes files from by-hash that were condemned more
-        # than a day ago.
+    def setUpPruneableSuite(self):
         self.breezy_autotest.publish_by_hash = True
         self.breezy_autotest.advertise_by_hash = True
         publisher = Publisher(
@@ -2843,16 +2841,44 @@
             suite_path('main', 'source', 'by-hash'),
             Not(ByHashHasContents(main_contents)))
 
-        # Use a fresh Publisher instance to ensure that it doesn't have
-        # dirty-pocket state left over from the last run.
-        publisher = Publisher(
-            self.logger, self.config, self.disk_pool,
-            self.ubuntutest.main_archive)
-        self.runSteps(publisher, step_a2=True, step_c=True, step_d=True)
-        self.assertEqual(set(), publisher.dirty_pockets)
-        self.assertContentEqual(
-            [('breezy-autotest', PackagePublishingPocket.RELEASE)],
-            publisher.release_files_needed)
+        return main_contents
+
+    def test_prune(self):
+        # The publisher prunes files from by-hash that were condemned more
+        # than a day ago.
+        main_contents = self.setUpPruneableSuite()
+        suite_path = partial(
+            os.path.join, self.config.distsroot, 'breezy-autotest')
+
+        # Use a fresh Publisher instance to ensure that it doesn't have
+        # dirty-pocket state left over from the last run.
+        publisher = Publisher(
+            self.logger, self.config, self.disk_pool,
+            self.ubuntutest.main_archive)
+        self.runSteps(publisher, step_a2=True, step_c=True, step_d=True)
+        self.assertEqual(set(), publisher.dirty_pockets)
+        self.assertThat(
+            suite_path('main', 'source', 'by-hash'),
+            ByHashHasContents(main_contents))
+
+    def test_prune_immutable(self):
+        # The publisher prunes by-hash files from immutable suites, but
+        # doesn't regenerate the Release file in that case.
+        main_contents = self.setUpPruneableSuite()
+        suite_path = partial(
+            os.path.join, self.config.distsroot, 'breezy-autotest')
+        release_path = suite_path('Release')
+        release_mtime = os.stat(release_path).st_mtime
+
+        self.breezy_autotest.status = SeriesStatus.CURRENT
+        # Use a fresh Publisher instance to ensure that it doesn't have
+        # dirty-pocket state left over from the last run.
+        publisher = Publisher(
+            self.logger, self.config, self.disk_pool,
+            self.ubuntutest.main_archive)
+        self.runSteps(publisher, step_a2=True, step_c=True, step_d=True)
+        self.assertEqual(set(), publisher.dirty_pockets)
+        self.assertEqual(release_mtime, os.stat(release_path).st_mtime)
         self.assertThat(
             suite_path('main', 'source', 'by-hash'),
             ByHashHasContents(main_contents))