launchpad-reviewers team mailing list archive

[Dimitri John Ledkov <mp+445277@xxxxxxxxxxxxxxxxxx>]

Dimitri John Ledkov has proposed merging ~xnox/launchpad:snapcraft-builds-allow-snapd into launchpad:master.

Commit message:
Allow to specify snapd channel in snap builds

snapcraft has a runtime dependency on snapd, and in rare cases when
there is incompatibility between the two, one may need to request both
snapcraft and snapd from specific channels to correctly complete
builds.

This was especially profound when we lost ability to rebuild core22
snap on riscv64 due to a regression in snapd, and unable to instruct
launchpad to temporarily use snapd from any other channel but stable.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~xnox/launchpad/+git/launchpad/+merge/445277
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~xnox/launchpad:snapcraft-builds-allow-snapd into launchpad:master.

diff --git a/lib/lp/snappy/interfaces/snap.py b/lib/lp/snappy/interfaces/snap.py
index 90e31fe..58a0148 100644
--- a/lib/lp/snappy/interfaces/snap.py
+++ b/lib/lp/snappy/interfaces/snap.py
@@ -470,7 +470,7 @@ class ISnapView(Interface):
                 "build."
             ),
             required=False,
-            extra_snap_names=["snapcraft"],
+            extra_snap_names=["snapcraft", "snapd"],
         ),
     )
     # Really ISnapBuild, patched in lp.snappy.interfaces.webservice.
@@ -513,7 +513,7 @@ class ISnapView(Interface):
                 "build."
             ),
             required=False,
-            extra_snap_names=["snapcraft"],
+            extra_snap_names=["snapcraft", "snapd"],
         ),
     )
     @export_factory_operation(ISnapBuildRequest, [])
@@ -1059,7 +1059,7 @@ class ISnapEditableAttributes(IHasOwner):
             title=_("Source snap channels for automatic builds"),
             required=False,
             readonly=False,
-            extra_snap_names=["snapcraft"],
+            extra_snap_names=["snapcraft", "snapd"],
             description_prefix=_(
                 "A dictionary mapping snap names to channels to use when "
                 "building this snap package."
diff --git a/lib/lp/snappy/interfaces/snapbuild.py b/lib/lp/snappy/interfaces/snapbuild.py
index 2429ecc..52019bf 100644
--- a/lib/lp/snappy/interfaces/snapbuild.py
+++ b/lib/lp/snappy/interfaces/snapbuild.py
@@ -221,7 +221,7 @@ class ISnapBuildView(IPackageBuildView, IPrivacy):
                 "A dictionary mapping snap names to channels to use for this "
                 "build."
             ),
-            extra_snap_names=["snapcraft"],
+            extra_snap_names=["snapcraft", "snapd"],
         )
     )
 
diff --git a/lib/lp/snappy/interfaces/snapjob.py b/lib/lp/snappy/interfaces/snapjob.py
index 64cefb7..f3f8458 100644
--- a/lib/lp/snappy/interfaces/snapjob.py
+++ b/lib/lp/snappy/interfaces/snapjob.py
@@ -75,7 +75,7 @@ class ISnapRequestBuildsJob(IRunnableJob):
         ),
         required=False,
         readonly=True,
-        extra_snap_names=["snapcraft"],
+        extra_snap_names=["snapcraft", "snapd"],
     )
 
     architectures = Set(
diff --git a/lib/lp/snappy/tests/test_snap.py b/lib/lp/snappy/tests/test_snap.py
index b500363..27ce3f6 100644
--- a/lib/lp/snappy/tests/test_snap.py
+++ b/lib/lp/snappy/tests/test_snap.py
@@ -441,9 +441,11 @@ class TestSnap(TestCaseWithFactory):
             snap.distro_series.main_archive,
             distroarchseries,
             PackagePublishingPocket.UPDATES,
-            channels={"snapcraft": "edge"},
+            channels={"snapcraft": "edge", "snapd": "edge"},
+        )
+        self.assertEqual(
+            {"snapcraft": "edge", "snapd": "edge"}, build.channels
         )
-        self.assertEqual({"snapcraft": "edge"}, build.channels)
 
     def test_requestBuild_rejects_repeats(self):
         # requestBuild refuses if there is already a pending build.
@@ -737,7 +739,7 @@ class TestSnap(TestCaseWithFactory):
                 snap.owner.teamowner,
                 snap.distro_series.main_archive,
                 PackagePublishingPocket.UPDATES,
-                channels={"snapcraft": "edge"},
+                channels={"snapcraft": "edge", "snapd": "edge"},
             )
         self.assertThat(
             request,
@@ -751,7 +753,9 @@ class TestSnap(TestCaseWithFactory):
                 requester=Equals(snap.owner.teamowner),
                 archive=Equals(snap.distro_series.main_archive),
                 pocket=Equals(PackagePublishingPocket.UPDATES),
-                channels=MatchesDict({"snapcraft": Equals("edge")}),
+                channels=MatchesDict(
+                    {"snapcraft": Equals("edge"), "snapd": Equals("edge")}
+                ),
                 architectures=Is(None),
             ),
         )
@@ -765,7 +769,7 @@ class TestSnap(TestCaseWithFactory):
                 requester=Equals(snap.owner.teamowner),
                 archive=Equals(snap.distro_series.main_archive),
                 pocket=Equals(PackagePublishingPocket.UPDATES),
-                channels=Equals({"snapcraft": "edge"}),
+                channels=Equals({"snapcraft": "edge", "snapd": "edge"}),
                 architectures=Is(None),
             ),
         )
@@ -780,7 +784,7 @@ class TestSnap(TestCaseWithFactory):
                 snap.owner.teamowner,
                 archive,
                 PackagePublishingPocket.UPDATES,
-                channels={"snapcraft": "edge"},
+                channels={"snapcraft": "edge", "snapd": "edge"},
             )
         self.assertThat(
             request,
@@ -794,7 +798,9 @@ class TestSnap(TestCaseWithFactory):
                 requester=Equals(snap.owner.teamowner),
                 archive=Equals(archive),
                 pocket=Equals(PackagePublishingPocket.UPDATES),
-                channels=MatchesDict({"snapcraft": Equals("edge")}),
+                channels=MatchesDict(
+                    {"snapcraft": Equals("edge"), "snapd": Equals("edge")}
+                ),
                 architectures=Is(None),
             ),
         )
@@ -808,7 +814,7 @@ class TestSnap(TestCaseWithFactory):
                 requester=Equals(snap.owner.teamowner),
                 archive=Equals(archive),
                 pocket=Equals(PackagePublishingPocket.UPDATES),
-                channels=Equals({"snapcraft": "edge"}),
+                channels=Equals({"snapcraft": "edge", "snapd": "edge"}),
                 architectures=Is(None),
             ),
         )
@@ -823,7 +829,7 @@ class TestSnap(TestCaseWithFactory):
                 snap.owner.teamowner,
                 snap.distro_series.main_archive,
                 PackagePublishingPocket.UPDATES,
-                channels={"snapcraft": "edge"},
+                channels={"snapcraft": "edge", "snapd": "edge"},
                 architectures={"amd64", "i386"},
             )
         self.assertThat(
@@ -838,7 +844,9 @@ class TestSnap(TestCaseWithFactory):
                 requester=Equals(snap.owner.teamowner),
                 archive=Equals(snap.distro_series.main_archive),
                 pocket=Equals(PackagePublishingPocket.UPDATES),
-                channels=MatchesDict({"snapcraft": Equals("edge")}),
+                channels=MatchesDict(
+                    {"snapcraft": Equals("edge"), "snapd": Equals("edge")}
+                ),
                 architectures=MatchesSetwise(Equals("amd64"), Equals("i386")),
             ),
         )
@@ -852,7 +860,7 @@ class TestSnap(TestCaseWithFactory):
                 requester=Equals(snap.owner.teamowner),
                 archive=Equals(snap.distro_series.main_archive),
                 pocket=Equals(PackagePublishingPocket.UPDATES),
-                channels=Equals({"snapcraft": "edge"}),
+                channels=Equals({"snapcraft": "edge", "snapd": "edge"}),
                 architectures=MatchesSetwise(Equals("amd64"), Equals("i386")),
             ),
         )
@@ -958,7 +966,7 @@ class TestSnap(TestCaseWithFactory):
             snap.owner.teamowner,
             distro.main_archive,
             PackagePublishingPocket.RELEASE,
-            {"snapcraft": "edge"},
+            {"snapcraft": "edge", "snapd": "edge"},
         )
 
     def assertRequestedBuildsMatch(
@@ -1480,7 +1488,7 @@ class TestSnap(TestCaseWithFactory):
             processors=[das.processor for das in dases[:2]],
             auto_build_archive=archive,
             auto_build_pocket=PackagePublishingPocket.PROPOSED,
-            auto_build_channels={"snapcraft": "edge"},
+            auto_build_channels={"snapcraft": "edge", "snapd": "edge"},
         )
         with person_logged_in(snap.owner):
             builds = snap.requestAutoBuilds()
@@ -1494,7 +1502,7 @@ class TestSnap(TestCaseWithFactory):
                         archive=archive,
                         distro_arch_series=das,
                         pocket=PackagePublishingPocket.PROPOSED,
-                        channels={"snapcraft": "edge"},
+                        channels={"snapcraft": "edge", "snapd": "edge"},
                     )
                     for das in dases[:2]
                 )
@@ -3307,7 +3315,8 @@ class TestSnapSet(TestCaseWithFactory):
         # 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"}
+            is_stale=True,
+            auto_build_channels={"snapcraft": "edge", "snapd": "edge"},
         )
         # Create some builds with mismatched channels.
         self.factory.makeSnapBuild(
@@ -3315,7 +3324,7 @@ class TestSnapSet(TestCaseWithFactory):
             snap=snap1,
             archive=snap1.auto_build_archive,
             distroarchseries=das1,
-            channels={"snapcraft": "edge"},
+            channels={"snapcraft": "edge", "snapd": "edge"},
         )
         self.factory.makeSnapBuild(
             requester=snap2.owner,
@@ -3344,7 +3353,7 @@ class TestSnapSet(TestCaseWithFactory):
                     requester=snap2.owner,
                     archive=snap2.auto_build_archive,
                     pocket=snap2.auto_build_pocket,
-                    channels={"snapcraft": "edge"},
+                    channels={"snapcraft": "edge", "snapd": "edge"},
                 ),
             ),
         )
@@ -5225,7 +5234,7 @@ class TestSnapWebservice(TestCaseWithFactory):
             "requestBuilds",
             archive=archive_url,
             pocket="Updates",
-            channels={"snapcraft": "edge"},
+            channels={"snapcraft": "edge", "snapd": "edge"},
         )
         self.assertEqual(201, response.status)
         build_request_url = response.getHeader("Location")
@@ -5297,7 +5306,9 @@ class TestSnapWebservice(TestCaseWithFactory):
                                 ),
                                 "arch_tag": Equals(processor.name),
                                 "pocket": Equals("Updates"),
-                                "channels": Equals({"snapcraft": "edge"}),
+                                "channels": Equals(
+                                    {"snapcraft": "edge", "snapd": "edge"}
+                                ),
                             }
                         )
                         for processor in processors
diff --git a/lib/lp/snappy/tests/test_snapbuildbehaviour.py b/lib/lp/snappy/tests/test_snapbuildbehaviour.py
index 6508601..b1d888b 100644
--- a/lib/lp/snappy/tests/test_snapbuildbehaviour.py
+++ b/lib/lp/snappy/tests/test_snapbuildbehaviour.py
@@ -772,7 +772,7 @@ class TestAsyncSnapBuildBehaviour(StatsMixin, TestSnapBuildBehaviourBase):
     @defer.inlineCallbacks
     def test_extraBuildArgs_channels(self):
         # If the build needs particular channels, extraBuildArgs sends them.
-        job = self.makeJob(channels={"snapcraft": "edge"})
+        job = self.makeJob(channels={"snapcraft": "edge", "snapd": "edge"})
         (
             expected_archives,
             expected_trusted_keys,
@@ -782,7 +782,9 @@ class TestAsyncSnapBuildBehaviour(StatsMixin, TestSnapBuildBehaviourBase):
         with dbuser(config.builddmaster.dbuser):
             args = yield job.extraBuildArgs()
         self.assertFalse(isProxy(args["channels"]))
-        self.assertEqual({"snapcraft": "edge"}, args["channels"])
+        self.assertEqual(
+            {"snapcraft": "edge", "snapd": "edge"}, args["channels"]
+        )
 
     @defer.inlineCallbacks
     def test_extraBuildArgs_channels_apt(self):