launchpad-reviewers team mailing list archive

[Jürgen Gmach <mp+416956@xxxxxxxxxxxxxxxxxx>]

Jürgen Gmach has proposed merging ~jugmac00/launchpad:fix-automatic-snap-build-not-starting into launchpad:master.

Commit message:
Make automatic snap builds more resilient

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jugmac00/launchpad/+git/launchpad/+merge/416956
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~jugmac00/launchpad:fix-automatic-snap-build-not-starting into launchpad:master.

diff --git a/lib/lp/snappy/model/snap.py b/lib/lp/snappy/model/snap.py
index bad0393..1a2bf68 100644
--- a/lib/lp/snappy/model/snap.py
+++ b/lib/lp/snappy/model/snap.py
@@ -1739,9 +1739,27 @@ class SnapSet:
                 logger.debug(
                     "Scheduling builds of snap package %s/%s",
                     snap.owner.name, snap.name)
-            build_requests.append(snap.requestBuilds(
-                snap.owner, snap.auto_build_archive, snap.auto_build_pocket,
-                channels=snap.auto_build_channels))
+            try:
+                build_request = snap.requestBuilds(
+                    snap.owner, snap.auto_build_archive,
+                    snap.auto_build_pocket,
+                    channels=snap.auto_build_channels
+                )
+            except SnapBuildArchiveOwnerMismatch as e:
+                if logger is not None:
+                    logger.exception(
+                        "%s Snap owner: %s, Archive owner: %s" % (
+                            e,
+                            snap.owner.displayname,
+                            snap.auto_build_archive.owner.displayname,
+                        )
+                    )
+                continue
+            except Exception as e:
+                if logger is not None:
+                    logger.exception(e)
+                continue
+            build_requests.append(build_request)
         return build_requests
 
     def detachFromBranch(self, branch):
diff --git a/lib/lp/snappy/tests/test_snap.py b/lib/lp/snappy/tests/test_snap.py
index e3b9143..7d8d45e 100644
--- a/lib/lp/snappy/tests/test_snap.py
+++ b/lib/lp/snappy/tests/test_snap.py
@@ -2474,6 +2474,64 @@ class TestSnapSet(TestCaseWithFactory):
             expected_log_entries, logger.getLogBuffer().splitlines())
         self.assertFalse(snap.is_stale)
 
+    def test_makeAutoBuilds_skips_when_owner_mismatches(self):
+        # ISnapSet.makeAutoBuilds skips snap packages if
+        # snap.owner != snap.archive.owner
+        _, snap = self.makeAutoBuildableSnap(
+            is_stale=True,
+        )
+        # we need to create a snap with a private archive,
+        # and a different snap.owner and snap.auto_build_archive.owner
+        # to trigger the `SnapBuildArchiveOwnerMismatch` exception
+        # which finally skips the build
+        snap = removeSecurityProxy(snap)
+        snap.owner=self.factory.makePerson(name="other")
+        snap.auto_build_archive.private = True
+        logger = BufferLogger()
+        build_requests = getUtility(ISnapSet).makeAutoBuilds(logger=logger)
+        self.assertEqual([], build_requests)
+        self.assertEqual([
+            'DEBUG Scheduling builds of snap package %s/%s' % (
+                snap.owner.name, snap.name),
+            'ERROR Snap package builds against private archives are only '
+            'allowed if the snap package owner and the archive owner are '
+            'equal. Snap owner: Other, Archive owner: %s' % (
+                snap.auto_build_archive.owner.displayname,)],
+            logger.getLogBuffer().splitlines()
+        )
+
+    def test_makeAutoBuilds_skips_for_other_exceptions(self):
+        # scheduling builds need to be unaffected by one erroring
+        _, snap = self.makeAutoBuildableSnap(
+            is_stale=True,
+        )
+        # builds cannot be scheduled when the archive is disabled
+        snap = removeSecurityProxy(snap)
+        snap.auto_build_archive._enabled = False
+        logger = BufferLogger()
+        build_requests = getUtility(ISnapSet).makeAutoBuilds(logger=logger)
+        self.assertEqual([], build_requests)
+        self.assertEqual([
+            'DEBUG Scheduling builds of snap package %s/%s' % (
+                snap.owner.name, snap.name),
+            'ERROR %s is disabled.' % (
+                snap.auto_build_archive.displayname,)],
+            logger.getLogBuffer().splitlines()
+        )
+
+    def test_makeAutoBuilds_skips_and_no_logger_enabled(self):
+        # This is basically the same test case as
+        # `test_makeAutoBuilds_skips_when_if_owner_mismatches`
+        # but we particularly test with no logger enabled.
+        _, snap = self.makeAutoBuildableSnap(
+            is_stale=True,
+        )
+        snap = removeSecurityProxy(snap)
+        snap.owner=self.factory.makePerson(name="other")
+        snap.auto_build_archive.private = True
+        build_requests = getUtility(ISnapSet).makeAutoBuilds()
+        self.assertEqual([], build_requests)
+
     def test_makeAutoBuilds_skips_if_built_recently(self):
         # ISnapSet.makeAutoBuilds skips snap packages that have been built
         # recently.