Merge lp:~cjwatson/launchpad/snap-build-created-webhook into lp:launchpad

[Colin Watson]

Issue snap:build:0.1 webhook delivery when creating a snap build, and include a link to the build request in the payload.

I need this as part of converting build.snapcraft.io over to the new snap.requestBuilds interface; it needs to be able to issue a build request, keep a note of the reason it did so, and then retrieve that when it receives a webhook delivery indicating that the build has been created. (The alternatives are to have BSI's request-builds endpoint poll until the builds are created, which is obviously horrible, or to lose the build annotations.)

This requires https://code.launchpad.net/~cjwatson/launchpad/db-snap-build-build-request/+merge/352303.

[William Grant (wgrant)]

review: Approve (code)

=== modified file 'lib/lp/snappy/configure.zcml'
--- lib/lp/snappy/configure.zcml	2018-06-15 13:21:14 +0000
+++ lib/lp/snappy/configure.zcml	2018-08-03 14:22:02 +0000
@@ -63,6 +63,10 @@
     </class>
     <subscriber
         for="lp.snappy.interfaces.snapbuild.ISnapBuild
+             lazr.lifecycle.interfaces.IObjectCreatedEvent"
+        handler="lp.snappy.subscribers.snapbuild.snap_build_created" />
+    <subscriber
+        for="lp.snappy.interfaces.snapbuild.ISnapBuild
              lp.snappy.interfaces.snapbuild.ISnapBuildStatusChangedEvent"
         handler="lp.snappy.subscribers.snapbuild.snap_build_status_changed" />
 

=== modified file 'lib/lp/snappy/interfaces/snap.py'
--- lib/lp/snappy/interfaces/snap.py	2018-06-18 22:08:58 +0000
+++ lib/lp/snappy/interfaces/snap.py	2018-08-03 14:22:02 +0000
@@ -365,7 +365,7 @@
     @export_factory_operation(Interface, [])
     @operation_for_version("devel")
     def requestBuild(requester, archive, distro_arch_series, pocket,
-                     channels=None):
+                     channels=None, build_request=None):
         """Request that the snap package be built.
 
         :param requester: The person requesting the build.
@@ -374,6 +374,8 @@
         :param pocket: The pocket that should be targeted.
         :param channels: A dictionary mapping snap names to channels to use
             for this build.
+        :param build_request: The `ISnapBuildRequest` job being processed,
+            if any.
         :return: `ISnapBuild`.
         """
 
@@ -407,7 +409,7 @@
 
     def requestBuildsFromJob(requester, archive, pocket, channels=None,
                              allow_failures=False, fetch_snapcraft_yaml=True,
-                             logger=None):
+                             build_request=None, logger=None):
         """Synchronous part of `Snap.requestBuilds`.
 
         Request that the snap package be built for relevant architectures.
@@ -424,6 +426,8 @@
             appropriate branch or repository and use it to decide which
             builds to request; if False, fall back to building for all
             supported architectures.
+        :param build_request: The `ISnapBuildRequest` job being processed,
+            if any.
         :param logger: An optional logger.
         :return: A sequence of `ISnapBuild` instances.
         """

=== modified file 'lib/lp/snappy/interfaces/snapbuild.py'
--- lib/lp/snappy/interfaces/snapbuild.py	2018-03-22 16:48:16 +0000
+++ lib/lp/snappy/interfaces/snapbuild.py	2018-08-03 14:22:02 +0000
@@ -52,7 +52,10 @@
 from lp.registry.interfaces.pocket import PackagePublishingPocket
 from lp.services.database.constants import DEFAULT
 from lp.services.librarian.interfaces import ILibraryFileAlias
-from lp.snappy.interfaces.snap import ISnap
+from lp.snappy.interfaces.snap import (
+    ISnap,
+    ISnapBuildRequest,
+    )
 from lp.soyuz.interfaces.archive import IArchive
 from lp.soyuz.interfaces.distroarchseries import IDistroArchSeries
 
@@ -122,6 +125,11 @@
 class ISnapBuildView(IPackageBuild):
     """`ISnapBuild` attributes that require launchpad.View permission."""
 
+    build_request = Reference(
+        ISnapBuildRequest,
+        title=_("The build request that caused this build to be created."),
+        required=False, readonly=True)
+
     requester = exported(Reference(
         IPerson,
         title=_("The person who requested this build."),

=== modified file 'lib/lp/snappy/interfaces/snapjob.py'
--- lib/lp/snappy/interfaces/snapjob.py	2018-06-15 13:21:14 +0000
+++ lib/lp/snappy/interfaces/snapjob.py	2018-08-03 14:22:02 +0000
@@ -32,7 +32,10 @@
     IJobSource,
     IRunnableJob,
     )
-from lp.snappy.interfaces.snap import ISnap
+from lp.snappy.interfaces.snap import (
+    ISnap,
+    ISnapBuildRequest,
+    )
 from lp.snappy.interfaces.snapbuild import ISnapBuild
 from lp.soyuz.interfaces.archive import IArchive
 
@@ -78,6 +81,10 @@
         title=_("Error message resulting from running this job."),
         required=False, readonly=True)
 
+    build_request = Reference(
+        title=_("The build request corresponding to this job."),
+        schema=ISnapBuildRequest, required=True, readonly=True)
+
     builds = List(
         title=_("The builds created by this request."),
         value_type=Reference(schema=ISnapBuild), required=True, readonly=True)

=== modified file 'lib/lp/snappy/model/snap.py'
--- lib/lp/snappy/model/snap.py	2018-07-30 09:07:30 +0000
+++ lib/lp/snappy/model/snap.py	2018-08-03 14:22:02 +0000
@@ -14,6 +14,7 @@
 from operator import attrgetter
 from urlparse import urlsplit
 
+from lazr.lifecycle.event import ObjectCreatedEvent
 from pymacaroons import Macaroon
 import pytz
 from storm.expr import (
@@ -39,6 +40,7 @@
     getAdapter,
     getUtility,
     )
+from zope.event import notify
 from zope.interface import implementer
 from zope.security.interfaces import Unauthorized
 from zope.security.proxy import removeSecurityProxy
@@ -521,7 +523,7 @@
             raise SnapBuildArchiveOwnerMismatch()
 
     def requestBuild(self, requester, archive, distro_arch_series, pocket,
-                     channels=None):
+                     channels=None, build_request=None):
         """See `ISnap`."""
         self._checkRequestBuild(requester, archive)
         if distro_arch_series not in self.getAllowedArchitectures():
@@ -540,8 +542,9 @@
 
         build = getUtility(ISnapBuildSet).new(
             requester, self, archive, distro_arch_series, pocket,
-            channels=channels)
+            channels=channels, build_request=build_request)
         build.queueBuild()
+        notify(ObjectCreatedEvent(build, user=requester))
         return build
 
     def requestBuilds(self, requester, archive, pocket, channels=None):
@@ -553,7 +556,7 @@
 
     def requestBuildsFromJob(self, requester, archive, pocket, channels=None,
                              allow_failures=False, fetch_snapcraft_yaml=True,
-                             logger=None):
+                             build_request=None, logger=None):
         """See `ISnap`."""
         if fetch_snapcraft_yaml:
             try:
@@ -585,7 +588,7 @@
             try:
                 build = self.requestBuild(
                     requester, archive, supported_arches[arch], pocket,
-                    channels)
+                    channels, build_request=build_request)
                 if logger is not None:
                     logger.debug(
                         " - %s/%s/%s: Build requested.",

=== modified file 'lib/lp/snappy/model/snapbuild.py'
--- lib/lp/snappy/model/snapbuild.py	2018-03-22 16:48:16 +0000
+++ lib/lp/snappy/model/snapbuild.py	2018-08-03 14:22:02 +0000
@@ -128,6 +128,8 @@
     build_farm_job_id = Int(name='build_farm_job', allow_none=False)
     build_farm_job = Reference(build_farm_job_id, 'BuildFarmJob.id')
 
+    build_request_id = Int(name='build_request', allow_none=True)
+
     requester_id = Int(name='requester', allow_none=False)
     requester = Reference(requester_id, 'Person.id')
 
@@ -175,7 +177,7 @@
 
     def __init__(self, build_farm_job, requester, snap, archive,
                  distro_arch_series, pocket, channels, processor, virtualized,
-                 date_created):
+                 date_created, build_request=None):
         """Construct a `SnapBuild`."""
         super(SnapBuild, self).__init__()
         self.build_farm_job = build_farm_job
@@ -188,9 +190,17 @@
         self.processor = processor
         self.virtualized = virtualized
         self.date_created = date_created
+        if build_request is not None:
+            self.build_request_id = build_request.id
         self.status = BuildStatus.NEEDSBUILD
 
     @property
+    def build_request(self):
+        """See `ISnapBuild`."""
+        if self.build_request_id is not None:
+            return self.snap.getBuildRequest(self.build_request_id)
+
+    @property
     def is_private(self):
         """See `IBuildFarmJob`."""
         return (
@@ -488,7 +498,7 @@
 class SnapBuildSet(SpecificBuildFarmJobSourceMixin):
 
     def new(self, requester, snap, archive, distro_arch_series, pocket,
-            channels=None, date_created=DEFAULT):
+            channels=None, date_created=DEFAULT, build_request=None):
         """See `ISnapBuildSet`."""
         store = IMasterStore(SnapBuild)
         build_farm_job = getUtility(IBuildFarmJobSource).new(
@@ -499,7 +509,7 @@
             pocket, channels, distro_arch_series.processor,
             not distro_arch_series.processor.supports_nonvirtualized
             or snap.require_virtualized or archive.require_virtualized,
-            date_created)
+            date_created, build_request=build_request)
         store.add(snapbuild)
         return snapbuild
 

=== modified file 'lib/lp/snappy/model/snapjob.py'
--- lib/lp/snappy/model/snapjob.py	2018-06-15 13:21:14 +0000
+++ lib/lp/snappy/model/snapjob.py	2018-08-03 14:22:02 +0000
@@ -243,6 +243,11 @@
         self.metadata["error_message"] = message
 
     @property
+    def build_request(self):
+        """See `ISnapRequestBuildsJob`."""
+        return self.snap.getBuildRequest(self.job.id)
+
+    @property
     def builds(self):
         """See `ISnapRequestBuildsJob`."""
         build_ids = self.metadata.get("builds")
@@ -272,7 +277,7 @@
         try:
             self.builds = self.snap.requestBuildsFromJob(
                 requester, archive, self.pocket, channels=self.channels,
-                logger=log)
+                build_request=self.build_request, logger=log)
             self.error_message = None
         except self.retry_error_types:
             raise

=== modified file 'lib/lp/snappy/subscribers/snapbuild.py'
--- lib/lp/snappy/subscribers/snapbuild.py	2017-03-20 00:03:52 +0000
+++ lib/lp/snappy/subscribers/snapbuild.py	2018-08-03 14:22:02 +0000
@@ -1,4 +1,4 @@
-# Copyright 2016-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2016-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Event subscribers for snap builds."""
@@ -19,21 +19,27 @@
 from lp.snappy.interfaces.snapbuildjob import ISnapStoreUploadJobSource
 
 
-def _trigger_snap_build_webhook(snapbuild):
+def _trigger_snap_build_webhook(snapbuild, action):
     if getFeatureFlag(SNAP_WEBHOOKS_FEATURE_FLAG):
         payload = {
             "snap_build": canonical_url(snapbuild, force_local_path=True),
-            "action": "status-changed",
+            "action": action,
             }
         payload.update(compose_webhook_payload(
-            ISnapBuild, snapbuild, ["snap", "status", "store_upload_status"]))
+            ISnapBuild, snapbuild,
+            ["snap", "build_request", "status", "store_upload_status"]))
         getUtility(IWebhookSet).trigger(
             snapbuild.snap, "snap:build:0.1", payload)
 
 
+def snap_build_created(snapbuild, event):
+    """Trigger events when a new snap package build is created."""
+    _trigger_snap_build_webhook(snapbuild, "created")
+
+
 def snap_build_status_changed(snapbuild, event):
     """Trigger events when snap package build statuses change."""
-    _trigger_snap_build_webhook(snapbuild)
+    _trigger_snap_build_webhook(snapbuild, "status-changed")
 
     if (snapbuild.snap.can_upload_to_store and snapbuild.snap.store_upload and
             snapbuild.status == BuildStatus.FULLYBUILT):
@@ -42,4 +48,4 @@
 
 def snap_build_store_upload_status_changed(snapbuild, event):
     """Trigger events when snap package build store upload statuses change."""
-    _trigger_snap_build_webhook(snapbuild)
+    _trigger_snap_build_webhook(snapbuild, "status-changed")

=== modified file 'lib/lp/snappy/tests/test_snap.py'
--- lib/lp/snappy/tests/test_snap.py	2018-07-30 09:07:30 +0000
+++ lib/lp/snappy/tests/test_snap.py	2018-08-03 14:22:02 +0000
@@ -76,6 +76,7 @@
 from lp.services.propertycache import clear_property_cache
 from lp.services.timeout import default_timeout
 from lp.services.webapp.interfaces import OAuthPermission
+from lp.services.webapp.publisher import canonical_url
 from lp.snappy.interfaces.snap import (
     BadSnapSearchContext,
     CannotFetchSnapcraftYaml,
@@ -368,6 +369,38 @@
             snap.owner, snap.distro_series.main_archive, distroarchseries,
             PackagePublishingPocket.UPDATES)
 
+    def test_requestBuild_triggers_webhooks(self):
+        # Requesting a build triggers webhooks.
+        processor = self.factory.makeProcessor(supports_virtualized=True)
+        distroarchseries = self.makeBuildableDistroArchSeries(
+            processor=processor)
+        snap = self.factory.makeSnap(
+            distroseries=distroarchseries.distroseries, processors=[processor])
+        hook = self.factory.makeWebhook(
+            target=snap, event_types=["snap:build:0.1"])
+        build = snap.requestBuild(
+            snap.owner, snap.distro_series.main_archive, distroarchseries,
+            PackagePublishingPocket.UPDATES)
+        expected_payload = {
+            "snap_build": Equals(canonical_url(build, force_local_path=True)),
+            "action": Equals("created"),
+            "snap": Equals(canonical_url(snap, force_local_path=True)),
+            "build_request": Is(None),
+            "status": Equals("Needs building"),
+            "store_upload_status": Equals("Unscheduled"),
+            }
+        with person_logged_in(snap.owner):
+            delivery = hook.deliveries.one()
+            self.assertThat(
+                delivery, MatchesStructure(
+                    event_type=Equals("snap:build:0.1"),
+                    payload=MatchesDict(expected_payload)))
+            with dbuser(config.IWebhookDeliveryJobSource.dbuser):
+                self.assertEqual(
+                    "<WebhookDeliveryJob for webhook %d on %r>" % (
+                        hook.id, hook.target),
+                    repr(delivery))
+
     def test_requestBuilds(self):
         # requestBuilds schedules a job and returns a corresponding
         # SnapBuildRequest.
@@ -437,7 +470,8 @@
         with person_logged_in(job.requester):
             builds = job.snap.requestBuildsFromJob(
                 job.requester, job.archive, job.pocket,
-                removeSecurityProxy(job.channels))
+                removeSecurityProxy(job.channels),
+                build_request=job.build_request)
         self.assertRequestedBuildsMatch(builds, job, ["sparc"])
 
     def test_requestBuildsFromJob_no_explicit_architectures(self):
@@ -449,10 +483,11 @@
         with person_logged_in(job.requester):
             builds = job.snap.requestBuildsFromJob(
                 job.requester, job.archive, job.pocket,
-                removeSecurityProxy(job.channels))
+                removeSecurityProxy(job.channels),
+                build_request=job.build_request)
         self.assertRequestedBuildsMatch(builds, job, ["mips64el", "riscv64"])
 
-    def test_requestBuilds_unsupported_remote(self):
+    def test_requestBuildsFromJob_unsupported_remote(self):
         # If the snap is based on an external Git repository from which we
         # don't support fetching blobs, requestBuildsFromJob falls back to
         # requesting builds for all configured architectures.
@@ -463,9 +498,43 @@
         with person_logged_in(job.requester):
             builds = job.snap.requestBuildsFromJob(
                 job.requester, job.archive, job.pocket,
-                removeSecurityProxy(job.channels))
+                removeSecurityProxy(job.channels),
+                build_request=job.build_request)
         self.assertRequestedBuildsMatch(builds, job, ["mips64el", "riscv64"])
 
+    def test_requestBuildsFromJob_triggers_webhooks(self):
+        # requestBuildsFromJob triggers webhooks, and the payload includes a
+        # link to the build request.
+        self.useFixture(GitHostingFixture(blob=dedent("""\
+            architectures:
+              - build-on: avr
+              - build-on: riscv64
+            """)))
+        job = self.makeRequestBuildsJob(["avr", "riscv64", "sparc"])
+        hook = self.factory.makeWebhook(
+            target=job.snap, event_types=["snap:build:0.1"])
+        with person_logged_in(job.requester):
+            builds = job.snap.requestBuildsFromJob(
+                job.requester, job.archive, job.pocket,
+                removeSecurityProxy(job.channels),
+                build_request=job.build_request)
+            self.assertEqual(2, len(builds))
+            self.assertThat(hook.deliveries, MatchesSetwise(*(
+                MatchesStructure(
+                    event_type=Equals("snap:build:0.1"),
+                    payload=MatchesDict({
+                        "snap_build": Equals(canonical_url(
+                            build, force_local_path=True)),
+                        "action": Equals("created"),
+                        "snap": Equals(canonical_url(
+                            job.snap, force_local_path=True)),
+                        "build_request": Equals(canonical_url(
+                            job.build_request, force_local_path=True)),
+                        "status": Equals("Needs building"),
+                        "store_upload_status": Equals("Unscheduled"),
+                        }))
+                for build in builds)))
+
     def test_requestAutoBuilds(self):
         # requestAutoBuilds creates new builds for all configured
         # architectures with appropriate parameters.

=== modified file 'lib/lp/snappy/tests/test_snapbuild.py'
--- lib/lp/snappy/tests/test_snapbuild.py	2018-02-08 12:47:44 +0000
+++ lib/lp/snappy/tests/test_snapbuild.py	2018-08-03 14:22:02 +0000
@@ -20,6 +20,7 @@
 import pytz
 from testtools.matchers import (
     Equals,
+    Is,
     MatchesDict,
     MatchesStructure,
     )
@@ -241,6 +242,7 @@
             "action": Equals("status-changed"),
             "snap": Equals(
                 canonical_url(self.build.snap, force_local_path=True)),
+            "build_request": Is(None),
             "status": Equals("Successfully built"),
             "store_upload_status": Equals("Unscheduled"),
             }
@@ -516,6 +518,7 @@
             "action": Equals("status-changed"),
             "snap": Equals(
                 canonical_url(self.build.snap, force_local_path=True)),
+            "build_request": Is(None),
             "status": Equals("Successfully built"),
             "store_upload_status": Equals("Pending"),
             }

=== modified file 'lib/lp/snappy/tests/test_snapbuildjob.py'
--- lib/lp/snappy/tests/test_snapbuildjob.py	2018-02-19 17:48:22 +0000
+++ lib/lp/snappy/tests/test_snapbuildjob.py	2018-08-03 14:22:02 +0000
@@ -1,4 +1,4 @@
-# Copyright 2016-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2016-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for snap build jobs."""
@@ -12,6 +12,7 @@
 from fixtures import FakeLogger
 from testtools.matchers import (
     Equals,
+    Is,
     MatchesDict,
     MatchesListwise,
     MatchesStructure,
@@ -125,6 +126,7 @@
             "action": Equals("status-changed"),
             "snap": Equals(
                 canonical_url(snapbuild.snap, force_local_path=True)),
+            "build_request": Is(None),
             "status": Equals("Successfully built"),
             "store_upload_status": Equals(expected),
             } for expected in expected_store_upload_statuses]

=== modified file 'lib/lp/snappy/tests/test_snapjob.py'
--- lib/lp/snappy/tests/test_snapjob.py	2018-06-15 12:54:27 +0000
+++ lib/lp/snappy/tests/test_snapjob.py	2018-08-03 14:22:02 +0000
@@ -113,13 +113,14 @@
             job=MatchesStructure.byEquality(status=JobStatus.COMPLETED),
             error_message=Is(None),
             builds=AfterPreprocessing(set, MatchesSetwise(*[
-                MatchesStructure.byEquality(
-                    requester=snap.registrant,
-                    snap=snap,
-                    archive=distroseries.main_archive,
-                    distro_arch_series=distroseries[arch],
-                    pocket=PackagePublishingPocket.RELEASE,
-                    channels={"core": "stable"})
+                MatchesStructure(
+                    build_request=MatchesStructure.byEquality(id=job.job.id),
+                    requester=Equals(snap.registrant),
+                    snap=Equals(snap),
+                    archive=Equals(distroseries.main_archive),
+                    distro_arch_series=Equals(distroseries[arch]),
+                    pocket=Equals(PackagePublishingPocket.RELEASE),
+                    channels=Equals({"core": "stable"}))
                 for arch in ("avr2001", "x32")]))))
 
     def test_run_failed(self):