Merge lp:~cjwatson/launchpad/livefs-keep-binary-files-interval into lp:launchpad

[Colin Watson]

Allow configuring the binary file retention period of a LiveFS.

This makes it more practical to deal with live filesystems that are expected to produce base images for use by Launchpad itself. Setting the base image of course ensures that the corresponding LFA stays around, but we may very well want to set a base image from a live filesystem build that's more than a day old.

https://code.launchpad.net/~cjwatson/launchpad/db-livefs-keep-binary-files-interval/+merge/368702 is the corresponding DB change.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py	2019-01-30 11:23:33 +0000
+++ lib/lp/scripts/garbo.py	2019-06-12 12:20:38 +0000
@@ -1566,21 +1566,25 @@
 class LiveFSFilePruner(BulkPruner):
     """A BulkPruner to remove old `LiveFSFile`s.
 
-    We remove binary files attached to `LiveFSBuild`s that are more than a
-    day 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 indefinitely.
+    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
+    indefinitely.
     """
     target_table_class = LiveFSFile
+    # Note that a NULL keep_binary_files_interval disables pruning, due to
+    # SQL NULL propagation.
     ids_to_prune_query = """
         SELECT DISTINCT LiveFSFile.id
-        FROM LiveFSFile, LiveFSBuild, LibraryFileAlias
+        FROM LiveFSFile, LiveFSBuild, LiveFS, LibraryFileAlias
         WHERE
             LiveFSFile.livefsbuild = LiveFSBuild.id
+            AND LiveFSBuild.livefs = LiveFS.id
             AND LiveFSFile.libraryfile = LibraryFileAlias.id
             AND LiveFSBuild.date_finished <
                 CURRENT_TIMESTAMP AT TIME ZONE 'UTC'
-                - CAST('1 day' AS interval)
+                - LiveFS.keep_binary_files_interval
             AND LibraryFileAlias.mimetype != 'text/plain'
         """
 
 
=== modified file 'lib/lp/scripts/tests/test_garbo.py'
--- lib/lp/scripts/tests/test_garbo.py	2019-01-30 11:23:33 +0000
+++ lib/lp/scripts/tests/test_garbo.py	2019-06-12 12:20:38 +0000
@@ -154,6 +154,9 @@
     )
 
 
+_default = object()
+
+
 class TestGarboScript(TestCase):
     layer = LaunchpadScriptLayer
 
@@ -1513,18 +1516,25 @@
             'PopulateLatestPersonSourcePackageReleaseCache')
         self.assertEqual(spph_2.id, job_data['last_spph_id'])
 
-    def _test_LiveFSFilePruner(self, content_type, interval, expected_count=0):
+    def _test_LiveFSFilePruner(self, content_type, interval,
+                               keep_binary_files_days=_default,
+                               expected_count=0):
         # Garbo should (or should not, if `expected_count=1`) remove LiveFS
         # files of MIME type `content_type` that finished more than
-        # `interval` days ago.
+        # `interval` days ago.  If `keep_binary_files_days` is given, set
+        # that on the test LiveFS.
         now = datetime.now(UTC)
         switch_dbuser('testadmin')
         self.useFixture(FeatureFixture({LIVEFS_FEATURE_FLAG: u'on'}))
         store = IMasterStore(LiveFSFile)
 
+        livefs_kwargs = {}
+        if keep_binary_files_days is not _default:
+            livefs_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))
+            status=BuildStatus.FULLYBUILT, duration=timedelta(minutes=10),
+            **livefs_kwargs)
         db_lfa = self.factory.makeLibraryFileAlias(content_type=content_type)
         db_file = self.factory.makeLiveFSFile(
             livefsbuild=db_build, libraryfile=db_lfa)
@@ -1537,7 +1547,8 @@
         self.assertEqual(expected_count, store.find(LiveFSFile).count())
 
     def test_LiveFSFilePruner_old_binary_files(self):
-        # LiveFS binary files attached to builds over a day old are pruned.
+        # By default, LiveFS binary files attached to builds over a day old
+        # are pruned.
         self._test_LiveFSFilePruner('application/octet-stream', 1)
 
     def test_LiveFSFilePruner_old_text_files(self):
@@ -1550,6 +1561,25 @@
         self._test_LiveFSFilePruner(
             'application/octet-stream', 0, expected_count=1)
 
+    def test_LiveFSFilePruner_custom_interval_old_binary_files(self):
+        # If a custom retention interval is set, LiveFS binary files
+        # attached to builds over that interval old are pruned.
+        self._test_LiveFSFilePruner(
+            'application/octet-stream', 7, keep_binary_files_days=7)
+
+    def test_LiveFSFilePruner_custom_interval_recent_binary_files(self):
+        # If a custom retention interval is set, LiveFS binary files
+        # attached to builds less than that interval old are pruned.
+        self._test_LiveFSFilePruner(
+            'application/octet-stream', 6, keep_binary_files_days=7,
+            expected_count=1)
+
+    def test_LiveFSFilePruner_null_interval_disables_pruning(self):
+        # A null retention interval disables pruning.
+        self._test_LiveFSFilePruner(
+            'application/octet-stream', 100, keep_binary_files_days=None,
+            expected_count=1)
+
     def _test_SnapFilePruner(self, filename, job_status, interval,
                              expected_count=0):
         # Garbo should (or should not, if `expected_count=1`) remove snap
 
=== modified file 'lib/lp/soyuz/browser/livefs.py'
--- lib/lp/soyuz/browser/livefs.py	2018-07-16 00:51:23 +0000
+++ lib/lp/soyuz/browser/livefs.py	2019-06-12 12:20:38 +0000
@@ -1,4 +1,4 @@
-# Copyright 2014-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2014-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """LiveFS views."""
@@ -181,6 +181,7 @@
         'name',
         'require_virtualized',
         'relative_build_score',
+        'keep_binary_files_days',
         ])
     distro_series = Choice(
         vocabulary='BuildableDistroSeries', title=u'Distribution series')
@@ -282,13 +283,18 @@
 
     label = title
 
-    field_names = ['require_virtualized', 'relative_build_score']
+    field_names = [
+        'require_virtualized',
+        'relative_build_score',
+        'keep_binary_files_days',
+        ]
 
     @property
     def initial_values(self):
         return {
             'require_virtualized': self.context.require_virtualized,
             'relative_build_score': self.context.relative_build_score,
+            'keep_binary_files_days': self.context.keep_binary_files_days,
             }
 
 
 
=== modified file 'lib/lp/soyuz/interfaces/livefs.py'
--- lib/lp/soyuz/interfaces/livefs.py	2017-07-18 16:22:03 +0000
+++ lib/lp/soyuz/interfaces/livefs.py	2019-06-12 12:20:38 +0000
@@ -1,4 +1,4 @@
-# Copyright 2014-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2014-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Live filesystem interfaces."""
@@ -262,6 +262,13 @@
             "A delta to apply to all build scores for the live filesystem.  "
             "Builds with a higher score will build sooner.")))
 
+    keep_binary_files_days = exported(Int(
+        title=_("Binary file retention period"),
+        required=False, readonly=False,
+        description=_(
+            "Keep binary files attached to builds of this live filesystem "
+            "for at least this many days.  If unset, disable pruning.")))
+
 
 class ILiveFSAdminAttributes(Interface):
     """`ILiveFS` attributes that can be edited by admins.
 
=== modified file 'lib/lp/soyuz/model/livefs.py'
--- lib/lp/soyuz/model/livefs.py	2017-07-18 16:22:03 +0000
+++ lib/lp/soyuz/model/livefs.py	2019-06-12 12:20:38 +0000
@@ -1,4 +1,4 @@
-# Copyright 2014-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2014-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -6,6 +6,9 @@
     'LiveFS',
     ]
 
+from datetime import timedelta
+import math
+
 import pytz
 from storm.locals import (
     Bool,
@@ -17,6 +20,7 @@
     Reference,
     Store,
     Storm,
+    TimeDelta,
     Unicode,
     )
 from zope.component import getUtility
@@ -113,8 +117,12 @@
 
     relative_build_score = Int(name='relative_build_score', allow_none=False)
 
+    keep_binary_files_interval = TimeDelta(
+        name='keep_binary_files_interval', allow_none=True)
+
     def __init__(self, registrant, owner, distro_series, name,
-                 metadata, require_virtualized, date_created):
+                 metadata, require_virtualized, keep_binary_files_days,
+                 date_created):
         """Construct a `LiveFS`."""
         if not getFeatureFlag(LIVEFS_FEATURE_FLAG):
             raise LiveFSFeatureDisabled
@@ -128,6 +136,24 @@
         self.relative_build_score = 0
         self.date_created = date_created
         self.date_last_modified = date_created
+        self.keep_binary_files_days = keep_binary_files_days
+
+    @property
+    def keep_binary_files_days(self):
+        """See `ILiveFS`."""
+        # Rounding up preserves the "at least this many days" part of the
+        # contract, and makes the interface simpler.
+        if self.keep_binary_files_interval is not None:
+            return int(math.ceil(
+                self.keep_binary_files_interval.total_seconds() / 86400))
+        else:
+            return None
+
+    @keep_binary_files_days.setter
+    def keep_binary_files_days(self, days):
+        """See `ILiveFS`."""
+        self.keep_binary_files_interval = (
+            timedelta(days=days) if days is not None else None)
 
     def requestBuild(self, requester, archive, distro_arch_series, pocket,
                      unique_key=None, metadata_override=None, version=None):
@@ -230,7 +256,8 @@
     """See `ILiveFSSet`."""
 
     def new(self, registrant, owner, distro_series, name, metadata,
-            require_virtualized=True, date_created=DEFAULT):
+            require_virtualized=True, keep_binary_files_days=1,
+            date_created=DEFAULT):
         """See `ILiveFSSet`."""
         if not registrant.inTeam(owner):
             if owner.is_team:
@@ -248,7 +275,7 @@
         store = IMasterStore(LiveFS)
         livefs = LiveFS(
             registrant, owner, distro_series, name, metadata,
-            require_virtualized, date_created)
+            require_virtualized, keep_binary_files_days, date_created)
         store.add(livefs)
 
         return livefs
 
=== modified file 'lib/lp/soyuz/tests/test_livefs.py'
--- lib/lp/soyuz/tests/test_livefs.py	2019-01-25 11:47:20 +0000
+++ lib/lp/soyuz/tests/test_livefs.py	2019-06-12 12:20:38 +0000
@@ -1,4 +1,4 @@
-# Copyright 2014-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2014-2019 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test live filesystems."""
@@ -122,6 +122,26 @@
         with celebrity_logged_in("buildd_admin"):
             livefs.relative_build_score = 100
 
+    def test_keep_binary_files_days(self):
+        # Buildd admins can change the binary file retention period of a
+        # LiveFS, but ordinary users cannot.
+        livefs = self.factory.makeLiveFS()
+        self.assertEqual(1, livefs.keep_binary_files_days)
+        with person_logged_in(livefs.owner):
+            self.assertRaises(
+                Unauthorized, setattr, livefs, "keep_binary_files_days", 2)
+        with celebrity_logged_in("buildd_admin"):
+            livefs.keep_binary_files_days = 2
+        self.assertEqual(2, livefs.keep_binary_files_days)
+        self.assertEqual(
+            timedelta(days=2),
+            removeSecurityProxy(livefs).keep_binary_files_interval)
+        with celebrity_logged_in("buildd_admin"):
+            livefs.keep_binary_files_days = None
+        self.assertIsNone(livefs.keep_binary_files_days)
+        self.assertIsNone(
+            removeSecurityProxy(livefs).keep_binary_files_interval)
+
     def test_requestBuild(self):
         # requestBuild creates a new LiveFSBuild.
         livefs = self.factory.makeLiveFS()
 
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2019-05-06 14:22:34 +0000
+++ lib/lp/testing/factory.py	2019-06-12 12:20:38 +0000
@@ -4628,7 +4628,7 @@
 
     def makeLiveFS(self, registrant=None, owner=None, distroseries=None,
                    name=None, metadata=None, require_virtualized=True,
-                   date_created=DEFAULT):
+                   keep_binary_files_days=1, date_created=DEFAULT):
         """Make a new LiveFS."""
         if registrant is None:
             registrant = self.makePerson()
@@ -4642,7 +4642,9 @@
             metadata = {}
         livefs = getUtility(ILiveFSSet).new(
             registrant, owner, distroseries, name, metadata,
-            require_virtualized=require_virtualized, date_created=date_created)
+            require_virtualized=require_virtualized,
+            keep_binary_files_days=keep_binary_files_days,
+            date_created=date_created)
         IStore(livefs).flush()
         return livefs