launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/livefs-keep-base-images into lp:launchpad with lp:~cjwatson/launchpad/livefs-keep-binary-files-interval as a prerequisite.

Commit message:
Exclude files that are set as base images from LiveFSFile pruning.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/livefs-keep-base-images/+merge/368714
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/livefs-keep-base-images into lp:launchpad.

=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py	2019-06-12 15:09:24 +0000
+++ lib/lp/scripts/garbo.py	2019-06-12 15:09:24 +0000
@@ -1567,9 +1567,10 @@
     """A BulkPruner to remove old `LiveFSFile`s.
 
     We remove binary files attached to `LiveFSBuild`s that are more than
-    `LiveFS.keep_binary_files_interval` old; these files are very large and
-    are only useful for builds in progress.  Text files are typically small
-    (<1MiB) and useful for retrospective analysis, so we preserve those
+    `LiveFS.keep_binary_files_interval` old and that are not set as base
+    images for a `DistroArchSeries`; these files are very large and are only
+    useful for builds in progress.  Text files are typically small (<1MiB)
+    and useful for retrospective analysis, so we preserve those
     indefinitely.
     """
     target_table_class = LiveFSFile
@@ -1586,6 +1587,10 @@
                 CURRENT_TIMESTAMP AT TIME ZONE 'UTC'
                 - LiveFS.keep_binary_files_interval
             AND LibraryFileAlias.mimetype != 'text/plain'
+        EXCEPT
+            SELECT LiveFSFile.id
+            FROM LiveFSFile, PocketChroot
+            WHERE LiveFSFile.libraryfile = PocketChroot.chroot
         """
 
 

=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py	2019-06-12 15:09:24 +0000
+++ lib/lp/scripts/tests/test_garbo.py	2019-06-12 15:09:24 +0000
@@ -1518,33 +1518,40 @@
 
     def _test_LiveFSFilePruner(self, content_type, interval,
                                keep_binary_files_days=_default,
-                               expected_count=0):
+                               base_image=False, expected_count=0,
+                               **livefsbuild_kwargs):
         # Garbo should (or should not, if `expected_count=1`) remove LiveFS
         # files of MIME type `content_type` that finished more than
         # `interval` days ago.  If `keep_binary_files_days` is given, set
-        # that on the test LiveFS.
+        # that on the test LiveFS.  If `base_image` is True, install the
+        # test LiveFS file as a base image for its DAS.
         now = datetime.now(UTC)
         switch_dbuser('testadmin')
         self.useFixture(FeatureFixture({LIVEFS_FEATURE_FLAG: u'on'}))
         store = IMasterStore(LiveFSFile)
+        initial_count = store.find(LiveFSFile).count()
 
-        livefs_kwargs = {}
+        livefsbuild_kwargs = dict(livefsbuild_kwargs)
         if keep_binary_files_days is not _default:
-            livefs_kwargs['keep_binary_files_days'] = keep_binary_files_days
+            livefsbuild_kwargs['keep_binary_files_days'] = (
+                keep_binary_files_days)
         db_build = self.factory.makeLiveFSBuild(
             date_created=now - timedelta(days=interval, minutes=15),
             status=BuildStatus.FULLYBUILT, duration=timedelta(minutes=10),
-            **livefs_kwargs)
+            **livefsbuild_kwargs)
         db_lfa = self.factory.makeLibraryFileAlias(content_type=content_type)
         db_file = self.factory.makeLiveFSFile(
             livefsbuild=db_build, libraryfile=db_lfa)
-        Store.of(db_file).flush()
-        self.assertEqual(1, store.find(LiveFSFile).count())
+        if base_image:
+            db_build.distro_arch_series.setChrootFromBuild(
+                db_build, db_file.libraryfile.filename)
+        store.flush()
 
         self.runDaily()
 
         switch_dbuser('testadmin')
-        self.assertEqual(expected_count, store.find(LiveFSFile).count())
+        self.assertEqual(
+            initial_count + expected_count, store.find(LiveFSFile).count())
 
     def test_LiveFSFilePruner_old_binary_files(self):
         # By default, LiveFS binary files attached to builds over a day old
@@ -1580,6 +1587,31 @@
             'application/octet-stream', 100, keep_binary_files_days=None,
             expected_count=1)
 
+    def test_LiveFSFilePruner_base_image(self):
+        # An old LiveFS binary file is not pruned if it is a base image.
+        self._test_LiveFSFilePruner(
+            'application/octet-stream', 100, base_image=True, expected_count=1)
+
+    def test_LiveFSFilePruner_other_base_image(self):
+        # An old LiveFS binary file is pruned even if some other base image
+        # exists.
+        switch_dbuser('testadmin')
+        self.useFixture(FeatureFixture({LIVEFS_FEATURE_FLAG: u'on'}))
+        store = IMasterStore(LiveFSFile)
+        other_build = self.factory.makeLiveFSBuild(
+            status=BuildStatus.FULLYBUILT, duration=timedelta(minutes=10))
+        other_lfa = self.factory.makeLibraryFileAlias(
+            content_type='application/octet-stream')
+        other_file = self.factory.makeLiveFSFile(
+            livefsbuild=other_build, libraryfile=other_lfa)
+        other_build.distro_arch_series.setChrootFromBuild(
+            other_build, other_file.libraryfile.filename)
+        store.flush()
+        self._test_LiveFSFilePruner(
+            'application/octet-stream', 100,
+            distroarchseries=other_build.distro_arch_series)
+        self.assertContentEqual([other_file], store.find(LiveFSFile))
+
     def _test_SnapFilePruner(self, filename, job_status, interval,
                              expected_count=0):
         # Garbo should (or should not, if `expected_count=1`) remove snap