Merge lp:~cjwatson/launchpad/snap-build-channels into lp:launchpad

[Colin Watson]

Add the ability to select source channels when building snaps.

This will let us run snap builds where snapcraft is installed as a snap. It depends on https://code.launchpad.net/~cjwatson/launchpad-buildd/snapception/+merge/337126 and https://code.launchpad.net/~cjwatson/launchpad/db-snap-build-channels/+merge/337361.

We'll probably also want a feature flag to change the default, but that can be added a little later, once the basic mechanism works.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/snappy/interfaces/snap.py'
--- lib/lp/snappy/interfaces/snap.py	2017-08-22 11:36:30 +0000
+++ lib/lp/snappy/interfaces/snap.py	2018-03-22 16:49:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Snap package interfaces."""
@@ -63,6 +63,7 @@
     Bool,
     Choice,
     Datetime,
+    Dict,
     Int,
     List,
     Text,
@@ -281,17 +282,27 @@
     @operation_parameters(
         archive=Reference(schema=IArchive),
         distro_arch_series=Reference(schema=IDistroArchSeries),
-        pocket=Choice(vocabulary=PackagePublishingPocket))
+        pocket=Choice(vocabulary=PackagePublishingPocket),
+        channels=Dict(
+            title=_("Source snap channels to use for this build."),
+            description=_(
+                "A dictionary mapping snap names to channels to use for this "
+                "build.  Currently only 'core' and 'snapcraft' keys are "
+                "supported."),
+            key_type=TextLine(), required=False))
     # Really ISnapBuild, patched in lp.snappy.interfaces.webservice.
     @export_factory_operation(Interface, [])
     @operation_for_version("devel")
-    def requestBuild(requester, archive, distro_arch_series, pocket):
+    def requestBuild(requester, archive, distro_arch_series, pocket,
+                     channels=None):
         """Request that the snap package be built.
 
         :param requester: The person requesting the build.
         :param archive: The IArchive to associate the build with.
         :param distro_arch_series: The architecture to build for.
         :param pocket: The pocket that should be targeted.
+        :param channels: A dictionary mapping snap names to channels to use
+            for this build.
         :return: `ISnapBuild`.
         """
 
@@ -509,6 +520,14 @@
             "The package stream within the source distribution series to use "
             "when building the snap package.")))
 
+    auto_build_channels = exported(Dict(
+        title=_("Source snap channels for automatic builds"),
+        key_type=TextLine(), required=False, readonly=False,
+        description=_(
+            "A dictionary mapping snap names to channels to use when building "
+            "this snap package.  Currently only 'core' and 'snapcraft' keys "
+            "are supported.")))
+
     is_stale = Bool(
         title=_("Snap package is stale and is due to be rebuilt."),
         required=True, readonly=False)

=== modified file 'lib/lp/snappy/interfaces/snapbuild.py'
--- lib/lp/snappy/interfaces/snapbuild.py	2018-02-16 21:48:03 +0000
+++ lib/lp/snappy/interfaces/snapbuild.py	2018-03-22 16:49:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Snap package build interfaces."""
@@ -149,6 +149,14 @@
         title=_("The pocket for which to build."),
         vocabulary=PackagePublishingPocket, required=True, readonly=True))
 
+    channels = exported(Dict(
+        title=_("Source snap channels to use for this build."),
+        description=_(
+            "A dictionary mapping snap names to channels to use for this "
+            "build.  Currently only 'core' and 'snapcraft' keys are "
+            "supported."),
+        key_type=TextLine()))
+
     virtualized = Bool(
         title=_("If True, this build is virtualized."), readonly=True)
 

=== modified file 'lib/lp/snappy/model/snap.py'
--- lib/lp/snappy/model/snap.py	2017-11-10 11:23:27 +0000
+++ lib/lp/snappy/model/snap.py	2018-03-22 16:49:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -94,6 +94,7 @@
     )
 from lp.services.database.stormexpr import (
     Greatest,
+    IsDistinctFrom,
     NullsLast,
     )
 from lp.services.features import getFeatureFlag
@@ -189,6 +190,8 @@
 
     auto_build_pocket = DBEnum(enum=PackagePublishingPocket, allow_none=True)
 
+    auto_build_channels = JSON('auto_build_channels', allow_none=True)
+
     is_stale = Bool(name='is_stale', allow_none=False)
 
     require_virtualized = Bool(name='require_virtualized')
@@ -209,9 +212,10 @@
     def __init__(self, registrant, owner, distro_series, name,
                  description=None, branch=None, git_ref=None, auto_build=False,
                  auto_build_archive=None, auto_build_pocket=None,
-                 require_virtualized=True, date_created=DEFAULT,
-                 private=False, store_upload=False, store_series=None,
-                 store_name=None, store_secrets=None, store_channels=None):
+                 auto_build_channels=None, require_virtualized=True,
+                 date_created=DEFAULT, private=False, store_upload=False,
+                 store_series=None, store_name=None, store_secrets=None,
+                 store_channels=None):
         """Construct a `Snap`."""
         super(Snap, self).__init__()
         self.registrant = registrant
@@ -224,6 +228,7 @@
         self.auto_build = auto_build
         self.auto_build_archive = auto_build_archive
         self.auto_build_pocket = auto_build_pocket
+        self.auto_build_channels = auto_build_channels
         self.require_virtualized = require_virtualized
         self.date_created = date_created
         self.date_last_modified = date_created
@@ -439,7 +444,8 @@
             return False
         return True
 
-    def requestBuild(self, requester, archive, distro_arch_series, pocket):
+    def requestBuild(self, requester, archive, distro_arch_series, pocket,
+                     channels=None):
         """See `ISnap`."""
         if not requester.inTeam(self.owner):
             raise SnapNotOwner(
@@ -459,12 +465,14 @@
             SnapBuild.archive_id == archive.id,
             SnapBuild.distro_arch_series_id == distro_arch_series.id,
             SnapBuild.pocket == pocket,
+            SnapBuild.channels == channels,
             SnapBuild.status == BuildStatus.NEEDSBUILD)
         if pending.any() is not None:
             raise SnapBuildAlreadyPending
 
         build = getUtility(ISnapBuildSet).new(
-            requester, self, archive, distro_arch_series, pocket)
+            requester, self, archive, distro_arch_series, pocket,
+            channels=channels)
         build.queueBuild()
         return build
 
@@ -484,7 +492,7 @@
             try:
                 build = self.requestBuild(
                     self.owner, self.auto_build_archive, arch,
-                    self.auto_build_pocket)
+                    self.auto_build_pocket, self.auto_build_channels)
                 if logger is not None:
                     logger.debug(
                         " - %s/%s/%s: Build requested.",
@@ -659,9 +667,10 @@
             branch=None, git_repository=None, git_repository_url=None,
             git_path=None, git_ref=None, auto_build=False,
             auto_build_archive=None, auto_build_pocket=None,
-            require_virtualized=True, processors=None, date_created=DEFAULT,
-            private=False, store_upload=False, store_series=None,
-            store_name=None, store_secrets=None, store_channels=None):
+            auto_build_channels=None, require_virtualized=True,
+            processors=None, date_created=DEFAULT, private=False,
+            store_upload=False, store_series=None, store_name=None,
+            store_secrets=None, store_channels=None):
         """See `ISnapSet`."""
         if not registrant.inTeam(owner):
             if owner.is_team:
@@ -702,6 +711,7 @@
             branch=branch, git_ref=git_ref, auto_build=auto_build,
             auto_build_archive=auto_build_archive,
             auto_build_pocket=auto_build_pocket,
+            auto_build_channels=auto_build_channels,
             require_virtualized=require_virtualized, date_created=date_created,
             private=private, store_upload=store_upload,
             store_series=store_series, store_name=store_name,
@@ -917,6 +927,8 @@
                     SnapBuild.snap_id == Snap.id,
                     SnapBuild.archive_id == Snap.auto_build_archive_id,
                     SnapBuild.pocket == Snap.auto_build_pocket,
+                    Not(IsDistinctFrom(
+                        SnapBuild.channels, Snap.auto_build_channels)),
                     # We only want Snaps that haven't had an automatic
                     # SnapBuild dispatched for them recently.
                     SnapBuild.date_created >= threshold_date)),

=== modified file 'lib/lp/snappy/model/snapbuild.py'
--- lib/lp/snappy/model/snapbuild.py	2018-02-16 21:48:03 +0000
+++ lib/lp/snappy/model/snapbuild.py	2018-03-22 16:49:25 +0000
@@ -22,6 +22,7 @@
     DateTime,
     Desc,
     Int,
+    JSON,
     Reference,
     Select,
     SQL,
@@ -142,6 +143,8 @@
 
     pocket = DBEnum(enum=PackagePublishingPocket, allow_none=False)
 
+    channels = JSON('channels', allow_none=True)
+
     processor_id = Int(name='processor', allow_none=False)
     processor = Reference(processor_id, 'Processor.id')
     virtualized = Bool(name='virtualized')
@@ -171,7 +174,7 @@
     failure_count = Int(name='failure_count', allow_none=False)
 
     def __init__(self, build_farm_job, requester, snap, archive,
-                 distro_arch_series, pocket, processor, virtualized,
+                 distro_arch_series, pocket, channels, processor, virtualized,
                  date_created):
         """Construct a `SnapBuild`."""
         super(SnapBuild, self).__init__()
@@ -181,6 +184,7 @@
         self.archive = archive
         self.distro_arch_series = distro_arch_series
         self.pocket = pocket
+        self.channels = channels
         self.processor = processor
         self.virtualized = virtualized
         self.date_created = date_created
@@ -484,7 +488,7 @@
 class SnapBuildSet(SpecificBuildFarmJobSourceMixin):
 
     def new(self, requester, snap, archive, distro_arch_series, pocket,
-            date_created=DEFAULT):
+            channels=None, date_created=DEFAULT):
         """See `ISnapBuildSet`."""
         store = IMasterStore(SnapBuild)
         build_farm_job = getUtility(IBuildFarmJobSource).new(
@@ -492,7 +496,7 @@
             archive)
         snapbuild = SnapBuild(
             build_farm_job, requester, snap, archive, distro_arch_series,
-            pocket, distro_arch_series.processor,
+            pocket, channels, distro_arch_series.processor,
             not distro_arch_series.processor.supports_nonvirtualized
             or snap.require_virtualized or archive.require_virtualized,
             date_created)

=== modified file 'lib/lp/snappy/model/snapbuildbehaviour.py'
--- lib/lp/snappy/model/snapbuildbehaviour.py	2018-03-01 17:36:31 +0000
+++ lib/lp/snappy/model/snapbuildbehaviour.py	2018-03-22 16:49:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """An `IBuildFarmJobBehaviour` for `SnapBuild`.
@@ -109,6 +109,8 @@
                 logger=logger))
         args["archive_private"] = build.archive.private
         args["build_url"] = canonical_url(build)
+        if build.channels is not None:
+            args["channels"] = build.channels
         if build.snap.branch is not None:
             args["branch"] = build.snap.branch.bzr_identity
         elif build.snap.git_ref is not None:

=== modified file 'lib/lp/snappy/tests/test_snap.py'
--- lib/lp/snappy/tests/test_snap.py	2018-01-23 18:54:30 +0000
+++ lib/lp/snappy/tests/test_snap.py	2018-03-22 16:49:25 +0000
@@ -54,6 +54,7 @@
     ONE_DAY_AGO,
     UTC_NOW,
     )
+from lp.services.database.interfaces import IStore
 from lp.services.database.sqlbase import flush_database_caches
 from lp.services.features.testing import (
     FeatureFixture,
@@ -188,6 +189,7 @@
         self.assertEqual(snap.distro_series.main_archive, build.archive)
         self.assertEqual(distroarchseries, build.distro_arch_series)
         self.assertEqual(PackagePublishingPocket.UPDATES, build.pocket)
+        self.assertIsNone(build.channels)
         self.assertEqual(BuildStatus.NEEDSBUILD, build.status)
         store = Store.of(build)
         store.flush()
@@ -232,6 +234,18 @@
         queue_record.score()
         self.assertEqual(2610, queue_record.lastscore)
 
+    def test_requestBuild_channels(self):
+        # requestBuild can select non-default channels.
+        processor = self.factory.makeProcessor(supports_virtualized=True)
+        distroarchseries = self.makeBuildableDistroArchSeries(
+            processor=processor)
+        snap = self.factory.makeSnap(
+            distroseries=distroarchseries.distroseries, processors=[processor])
+        build = snap.requestBuild(
+            snap.owner, snap.distro_series.main_archive, distroarchseries,
+            PackagePublishingPocket.UPDATES, channels={"snapcraft": "edge"})
+        self.assertEqual({"snapcraft": "edge"}, build.channels)
+
     def test_requestBuild_rejects_repeats(self):
         # requestBuild refuses if there is already a pending build.
         distroseries = self.factory.makeDistroSeries()
@@ -355,10 +369,36 @@
         with person_logged_in(snap.owner):
             builds = snap.requestAutoBuilds()
         self.assertThat(builds, MatchesSetwise(
+            *(MatchesStructure(
+                requester=Equals(snap.owner), snap=Equals(snap),
+                archive=Equals(archive), distro_arch_series=Equals(das),
+                pocket=Equals(PackagePublishingPocket.PROPOSED),
+                channels=Is(None))
+              for das in dases[:2])))
+
+    def test_requestAutoBuilds_channels(self):
+        # requestAutoBuilds honours Snap.auto_build_channels.
+        distroseries = self.factory.makeDistroSeries()
+        dases = []
+        for _ in range(3):
+            processor = self.factory.makeProcessor(supports_virtualized=True)
+            dases.append(self.makeBuildableDistroArchSeries(
+                distroseries=distroseries, processor=processor))
+        archive = self.factory.makeArchive()
+        snap = self.factory.makeSnap(
+            distroseries=distroseries,
+            processors=[das.processor for das in dases[:2]],
+            auto_build_archive=archive,
+            auto_build_pocket=PackagePublishingPocket.PROPOSED,
+            auto_build_channels={"snapcraft": "edge"})
+        with person_logged_in(snap.owner):
+            builds = snap.requestAutoBuilds()
+        self.assertThat(builds, MatchesSetwise(
             *(MatchesStructure.byEquality(
                 requester=snap.owner, snap=snap, archive=archive,
                 distro_arch_series=das,
-                pocket=PackagePublishingPocket.PROPOSED)
+                pocket=PackagePublishingPocket.PROPOSED,
+                channels={"snapcraft": "edge"})
               for das in dases[:2])))
 
     def test_getBuilds(self):
@@ -610,6 +650,7 @@
         self.assertFalse(snap.auto_build)
         self.assertIsNone(snap.auto_build_archive)
         self.assertIsNone(snap.auto_build_pocket)
+        self.assertIsNone(snap.auto_build_channels)
         self.assertTrue(snap.require_virtualized)
         self.assertFalse(snap.private)
 
@@ -630,6 +671,7 @@
         self.assertFalse(snap.auto_build)
         self.assertIsNone(snap.auto_build_archive)
         self.assertIsNone(snap.auto_build_pocket)
+        self.assertIsNone(snap.auto_build_channels)
         self.assertTrue(snap.require_virtualized)
         self.assertFalse(snap.private)
 
@@ -1008,6 +1050,56 @@
         self.assertEqual([], builds)
         self.assertEqual([], logger.getLogBuffer().splitlines())
 
+    def test_makeAutoBuilds_skips_if_built_recently_matching_channels(self):
+        # ISnapSet.makeAutoBuilds only considers recently-requested builds
+        # to match a snap if they match its auto_build_channels.
+        das1, snap1 = self.makeAutoBuildableSnap(is_stale=True)
+        das2, snap2 = self.makeAutoBuildableSnap(
+            is_stale=True, auto_build_channels={"snapcraft": "edge"})
+        # Create some builds with mismatched channels.
+        self.factory.makeSnapBuild(
+            requester=snap1.owner, snap=snap1,
+            archive=snap1.auto_build_archive, distroarchseries=das1,
+            channels={"snapcraft": "edge"})
+        self.factory.makeSnapBuild(
+            requester=snap2.owner, snap=snap2,
+            archive=snap2.auto_build_archive, distroarchseries=das2,
+            channels={"snapcraft": "stable"})
+
+        logger = BufferLogger()
+        builds = getUtility(ISnapSet).makeAutoBuilds(logger=logger)
+        self.assertThat(builds, MatchesSetwise(
+            MatchesStructure(
+                requester=Equals(snap1.owner), snap=Equals(snap1),
+                distro_arch_series=Equals(das1), channels=Is(None),
+                status=Equals(BuildStatus.NEEDSBUILD)),
+            MatchesStructure.byEquality(
+                requester=snap2.owner, snap=snap2, distro_arch_series=das2,
+                channels={"snapcraft": "edge"}, status=BuildStatus.NEEDSBUILD),
+            ))
+        log_entries = logger.getLogBuffer().splitlines()
+        self.assertEqual(4, len(log_entries))
+        for das, snap in (das1, snap1), (das2, snap2):
+            self.assertIn(
+                "DEBUG Scheduling builds of snap package %s/%s" % (
+                    snap.owner.name, snap.name),
+                log_entries)
+            self.assertIn(
+                "DEBUG  - %s/%s/%s: Build requested." % (
+                    snap.owner.name, snap.name, das.architecturetag),
+                log_entries)
+            self.assertFalse(snap.is_stale)
+
+        # Mark the two snaps stale and try again.  There are now matching
+        # builds so we don't try to request more.
+        for snap in snap1, snap2:
+            removeSecurityProxy(snap).is_stale = True
+            IStore(snap).flush()
+        logger = BufferLogger()
+        builds = getUtility(ISnapSet).makeAutoBuilds(logger=logger)
+        self.assertEqual([], builds)
+        self.assertEqual([], logger.getLogBuffer().splitlines())
+
     def test_makeAutoBuilds_skips_non_stale_snaps(self):
         # ISnapSet.makeAutoBuilds skips snap packages that are not stale.
         das, snap = self.makeAutoBuildableSnap(is_stale=False)

=== modified file 'lib/lp/snappy/tests/test_snapbuild.py'
--- lib/lp/snappy/tests/test_snapbuild.py	2018-01-23 10:59:44 +0000
+++ lib/lp/snappy/tests/test_snapbuild.py	2018-03-22 16:49:25 +0000
@@ -597,6 +597,7 @@
             self.assertEqual(
                 db_build.distro_arch_series.architecturetag, build["arch_tag"])
             self.assertEqual("Updates", build["pocket"])
+            self.assertIsNone(build["channels"])
             self.assertIsNone(build["score"])
             self.assertFalse(build["can_be_rescored"])
             self.assertFalse(build["can_be_cancelled"])

=== modified file 'lib/lp/snappy/tests/test_snapbuildbehaviour.py'
--- lib/lp/snappy/tests/test_snapbuildbehaviour.py	2018-03-01 17:36:31 +0000
+++ lib/lp/snappy/tests/test_snapbuildbehaviour.py	2018-03-22 16:49:25 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test snap package build behaviour."""
@@ -402,6 +402,17 @@
             ]))
 
     @defer.inlineCallbacks
+    def test_extraBuildArgs_channels(self):
+        # If the build needs particular channels, _extraBuildArgs sends
+        # them.
+        job = self.makeJob(channels={"snapcraft": "edge"})
+        expected_archives, expected_trusted_keys = (
+            yield get_sources_list_for_building(
+                job.build, job.build.distro_arch_series, None))
+        args = yield job._extraBuildArgs()
+        self.assertEqual({"snapcraft": "edge"}, args["channels"])
+
+    @defer.inlineCallbacks
     def test_composeBuildRequest_proxy_url_set(self):
         job = self.makeJob()
         build_request = yield job.composeBuildRequest(None)

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2018-02-24 09:11:39 +0000
+++ lib/lp/testing/factory.py	2018-03-22 16:49:25 +0000
@@ -4670,7 +4670,8 @@
     def makeSnap(self, registrant=None, owner=None, distroseries=None,
                  name=None, branch=None, git_ref=None, auto_build=False,
                  auto_build_archive=None, auto_build_pocket=None,
-                 is_stale=None, require_virtualized=True, processors=None,
+                 auto_build_channels=None, is_stale=None,
+                 require_virtualized=True, processors=None,
                  date_created=DEFAULT, private=False, store_upload=False,
                  store_series=None, store_name=None, store_secrets=None,
                  store_channels=None):
@@ -4696,7 +4697,8 @@
             require_virtualized=require_virtualized, processors=processors,
             date_created=date_created, branch=branch, git_ref=git_ref,
             auto_build=auto_build, auto_build_archive=auto_build_archive,
-            auto_build_pocket=auto_build_pocket, private=private,
+            auto_build_pocket=auto_build_pocket,
+            auto_build_channels=auto_build_channels, private=private,
             store_upload=store_upload, store_series=store_series,
             store_name=store_name, store_secrets=store_secrets,
             store_channels=store_channels)
@@ -4707,8 +4709,9 @@
 
     def makeSnapBuild(self, requester=None, registrant=None, snap=None,
                       archive=None, distroarchseries=None, pocket=None,
-                      date_created=DEFAULT, status=BuildStatus.NEEDSBUILD,
-                      builder=None, duration=None, **kwargs):
+                      channels=None, date_created=DEFAULT,
+                      status=BuildStatus.NEEDSBUILD, builder=None,
+                      duration=None, **kwargs):
         """Make a new SnapBuild."""
         if requester is None:
             requester = self.makePerson()
@@ -4736,7 +4739,7 @@
             pocket = PackagePublishingPocket.UPDATES
         snapbuild = getUtility(ISnapBuildSet).new(
             requester, snap, archive, distroarchseries, pocket,
-            date_created=date_created)
+            channels=channels, date_created=date_created)
         if duration is not None:
             removeSecurityProxy(snapbuild).updateStatus(
                 BuildStatus.BUILDING, builder=builder,