launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #24629
[Merge] ~cjwatson/launchpad:oci-recipe-webhook-build-request into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:oci-recipe-webhook-build-request into launchpad:master.
Commit message:
Add build_request to OCI recipe webhook payloads
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1847444 in Launchpad itself: "Support OCI image building"
https://bugs.launchpad.net/launchpad/+bug/1847444
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/383026
This entailed keeping track of the originating build request in OCIRecipeBuild, and adding a few missing tests.
DB patch: https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/383021
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:oci-recipe-webhook-build-request into launchpad:master.
diff --git a/lib/lp/oci/interfaces/ocirecipebuild.py b/lib/lp/oci/interfaces/ocirecipebuild.py
index a1fbfc0..a1f3e46 100644
--- a/lib/lp/oci/interfaces/ocirecipebuild.py
+++ b/lib/lp/oci/interfaces/ocirecipebuild.py
@@ -36,7 +36,10 @@ from zope.schema import (
from lp import _
from lp.buildmaster.interfaces.buildfarmjob import ISpecificBuildFarmJobSource
from lp.buildmaster.interfaces.packagebuild import IPackageBuild
-from lp.oci.interfaces.ocirecipe import IOCIRecipe
+from lp.oci.interfaces.ocirecipe import (
+ IOCIRecipe,
+ IOCIRecipeBuildRequest,
+ )
from lp.services.database.constants import DEFAULT
from lp.services.fields import PublicPersonChoice
from lp.services.librarian.interfaces import ILibraryFileAlias
@@ -78,6 +81,11 @@ class OCIRecipeBuildRegistryUploadStatus(EnumeratedType):
class IOCIRecipeBuildView(IPackageBuild):
"""`IOCIRecipeBuild` attributes that require launchpad.View permission."""
+ build_request = Reference(
+ IOCIRecipeBuildRequest,
+ title=_("The build request that caused this build to be created."),
+ required=False, readonly=True)
+
requester = PublicPersonChoice(
title=_("Requester"),
description=_("The person who requested this OCI recipe build."),
diff --git a/lib/lp/oci/model/ocirecipe.py b/lib/lp/oci/model/ocirecipe.py
index 1f3cf55..269370e 100644
--- a/lib/lp/oci/model/ocirecipe.py
+++ b/lib/lp/oci/model/ocirecipe.py
@@ -321,13 +321,13 @@ class OCIRecipe(Storm, WebhookTargetMixin):
pending_processors = {i.processor for i in pending}
return len(pending_processors) == len(processors)
- def requestBuild(self, requester, distro_arch_series):
+ def requestBuild(self, requester, distro_arch_series, build_request=None):
self._checkRequestBuild(requester)
if self._hasPendingBuilds([distro_arch_series]):
raise OCIRecipeBuildAlreadyPending
build = getUtility(IOCIRecipeBuildSet).new(
- requester, self, distro_arch_series)
+ requester, self, distro_arch_series, build_request=build_request)
build.queueBuild()
notify(ObjectCreatedEvent(build, user=requester))
return build
@@ -335,14 +335,15 @@ class OCIRecipe(Storm, WebhookTargetMixin):
def getBuildRequest(self, job_id):
return OCIRecipeBuildRequest(self, job_id)
- def requestBuildsFromJob(self, requester):
+ def requestBuildsFromJob(self, requester, build_request=None):
self._checkRequestBuild(requester)
distro_arch_series_to_build = set(self.getAllowedArchitectures())
builds = []
for das in distro_arch_series_to_build:
try:
- builds.append(self.requestBuild(requester, das))
+ builds.append(self.requestBuild(
+ requester, das, build_request=build_request))
except OCIRecipeBuildAlreadyPending:
pass
diff --git a/lib/lp/oci/model/ocirecipebuild.py b/lib/lp/oci/model/ocirecipebuild.py
index 55a2f34..6b8e023 100644
--- a/lib/lp/oci/model/ocirecipebuild.py
+++ b/lib/lp/oci/model/ocirecipebuild.py
@@ -107,6 +107,8 @@ class OCIRecipeBuild(PackageBuildMixin, Storm):
id = Int(name='id', primary=True)
+ build_request_id = Int(name='build_request', allow_none=True)
+
requester_id = Int(name='requester', allow_none=False)
requester = Reference(requester_id, 'Person.id')
@@ -148,8 +150,8 @@ class OCIRecipeBuild(PackageBuildMixin, Storm):
pocket = PackagePublishingPocket.UPDATES
def __init__(self, build_farm_job, requester, recipe,
- processor, virtualized, date_created):
-
+ processor, virtualized, date_created, build_request=None):
+ """Construct an `OCIRecipeBuild`."""
self.requester = requester
self.recipe = recipe
self.processor = processor
@@ -157,6 +159,14 @@ class OCIRecipeBuild(PackageBuildMixin, Storm):
self.date_created = date_created
self.status = BuildStatus.NEEDSBUILD
self.build_farm_job = build_farm_job
+ if build_request is not None:
+ self.build_request_id = build_request.id
+
+ @property
+ def build_request(self):
+ """See `IOCIRecipeBuild`."""
+ if self.build_request_id is not None:
+ return self.recipe.getBuildRequest(self.build_request_id)
def __repr__(self):
return "<OCIRecipeBuild ~%s/%s/+oci/%s/+recipe/%s/+build/%d>" % (
@@ -421,9 +431,8 @@ class OCIRecipeBuildSet(SpecificBuildFarmJobSourceMixin):
"""See `IOCIRecipeBuildSet`."""
def new(self, requester, recipe, distro_arch_series,
- date_created=DEFAULT):
+ date_created=DEFAULT, build_request=None):
"""See `IOCIRecipeBuildSet`."""
-
virtualized = (
not distro_arch_series.processor.supports_nonvirtualized
or recipe.require_virtualized)
@@ -433,7 +442,7 @@ class OCIRecipeBuildSet(SpecificBuildFarmJobSourceMixin):
OCIRecipeBuild.job_type, BuildStatus.NEEDSBUILD, date_created)
ocirecipebuild = OCIRecipeBuild(
build_farm_job, requester, recipe, distro_arch_series.processor,
- virtualized, date_created)
+ virtualized, date_created, build_request=build_request)
store.add(ocirecipebuild)
return ocirecipebuild
diff --git a/lib/lp/oci/model/ocirecipejob.py b/lib/lp/oci/model/ocirecipejob.py
index 529d7c1..66bcd29 100644
--- a/lib/lp/oci/model/ocirecipejob.py
+++ b/lib/lp/oci/model/ocirecipejob.py
@@ -241,7 +241,8 @@ class OCIRecipeRequestBuildsJob(OCIRecipeJobDerived):
"Skipping %r because the requester has been deleted." % self)
return
try:
- self.builds = self.recipe.requestBuildsFromJob(requester)
+ self.builds = self.recipe.requestBuildsFromJob(
+ requester, build_request=self.build_request)
self.error_message = None
except self.retry_error_types:
raise
diff --git a/lib/lp/oci/subscribers/ocirecipebuild.py b/lib/lp/oci/subscribers/ocirecipebuild.py
index 2f3fc01..242d761 100644
--- a/lib/lp/oci/subscribers/ocirecipebuild.py
+++ b/lib/lp/oci/subscribers/ocirecipebuild.py
@@ -27,7 +27,7 @@ def _trigger_oci_recipe_build_webhook(build, action):
}
payload.update(compose_webhook_payload(
IOCIRecipeBuild, build,
- ["recipe", "status", "registry_upload_status"]))
+ ["recipe", "build_request", "status", "registry_upload_status"]))
getUtility(IWebhookSet).trigger(
build.recipe, "oci-recipe:build:0.1", payload)
@@ -36,6 +36,7 @@ def oci_recipe_build_created(build, event):
"""Trigger events when a new OCI recipe build is created."""
_trigger_oci_recipe_build_webhook(build, "created")
+
def oci_recipe_build_modified(build, event):
"""Trigger events when OCI recipe build statuses change."""
if event.edited_fields is not None:
diff --git a/lib/lp/oci/tests/test_ocirecipe.py b/lib/lp/oci/tests/test_ocirecipe.py
index 965096b..600cde8 100644
--- a/lib/lp/oci/tests/test_ocirecipe.py
+++ b/lib/lp/oci/tests/test_ocirecipe.py
@@ -16,6 +16,7 @@ from storm.exceptions import LostObjectError
from testtools.matchers import (
ContainsDict,
Equals,
+ Is,
MatchesDict,
MatchesSetwise,
MatchesStructure,
@@ -203,6 +204,7 @@ class TestOCIRecipe(OCIConfigHelperMixin, TestCaseWithFactory):
canonical_url(build, force_local_path=True)),
"action": Equals("created"),
"recipe": Equals(canonical_url(recipe, force_local_path=True)),
+ "build_request": Is(None),
"status": Equals("Needs building"),
'registry_upload_status': Equals('Unscheduled'),
}
@@ -230,12 +232,14 @@ class TestOCIRecipe(OCIConfigHelperMixin, TestCaseWithFactory):
distro = ocirecipe.oci_project.distribution
series = self.factory.makeDistroSeries(
distribution=distro, status=SeriesStatus.CURRENT)
-
arch_series_386 = self.getDistroArchSeries(series, "386", "386")
arch_series_hppa = self.getDistroArchSeries(series, "hppa", "hppa")
+ job = getUtility(IOCIRecipeRequestBuildsJobSource).create(
+ ocirecipe, owner)
- with person_logged_in(owner):
- builds = ocirecipe.requestBuildsFromJob(owner)
+ with person_logged_in(job.requester):
+ builds = ocirecipe.requestBuildsFromJob(
+ job.requester, build_request=job.build_request)
self.assertThat(builds, MatchesSetwise(
MatchesStructure(
recipe=Equals(ocirecipe),
@@ -251,27 +255,79 @@ class TestOCIRecipe(OCIConfigHelperMixin, TestCaseWithFactory):
distribution=ocirecipe.oci_project.distribution,
status=SeriesStatus.CURRENT)
another_user = self.factory.makePerson()
- with person_logged_in(another_user):
+ job = getUtility(IOCIRecipeRequestBuildsJobSource).create(
+ ocirecipe, another_user)
+ with person_logged_in(job.requester):
self.assertRaises(
OCIRecipeNotOwner,
- ocirecipe.requestBuildsFromJob, another_user)
+ ocirecipe.requestBuildsFromJob,
+ job.requester, build_request=job.build_request)
def test_requestBuildsFromJob_with_pending_jobs(self):
ocirecipe = removeSecurityProxy(self.factory.makeOCIRecipe())
distro = ocirecipe.oci_project.distribution
series = self.factory.makeDistroSeries(
distribution=distro, status=SeriesStatus.CURRENT)
-
arch_series_386 = self.getDistroArchSeries(series, "386", "386")
-
self.factory.makeOCIRecipeBuild(
recipe=ocirecipe, status=BuildStatus.NEEDSBUILD,
distro_arch_series=arch_series_386)
+ job = getUtility(IOCIRecipeRequestBuildsJobSource).create(
+ ocirecipe, ocirecipe.owner)
- with person_logged_in(ocirecipe.owner):
+ with person_logged_in(job.requester):
self.assertRaises(
OCIRecipeBuildAlreadyPending,
- ocirecipe.requestBuildsFromJob, ocirecipe.owner)
+ ocirecipe.requestBuildsFromJob,
+ job.requester, build_request=job.build_request)
+
+ def test_requestBuildsFromJob_triggers_webhooks(self):
+ # requestBuildsFromJob triggers webhooks, and the payload includes a
+ # link to the build request.
+ self.useFixture(FeatureFixture({
+ OCI_RECIPE_WEBHOOKS_FEATURE_FLAG: "on",
+ OCI_RECIPE_ALLOW_CREATE: "on",
+ }))
+ recipe = removeSecurityProxy(self.factory.makeOCIRecipe(
+ require_virtualized=False))
+ distro = recipe.oci_project.distribution
+ series = self.factory.makeDistroSeries(
+ distribution=distro, status=SeriesStatus.CURRENT)
+ self.getDistroArchSeries(series, "386", "386")
+ self.getDistroArchSeries(series, "hppa", "hppa")
+ job = getUtility(IOCIRecipeRequestBuildsJobSource).create(
+ recipe, recipe.owner)
+
+ logger = self.useFixture(FakeLogger())
+ hook = self.factory.makeWebhook(
+ target=job.recipe, event_types=["oci-recipe:build:0.1"])
+ with person_logged_in(job.requester):
+ builds = recipe.requestBuildsFromJob(
+ job.requester, build_request=job.build_request)
+ self.assertEqual(2, len(builds))
+ payload_matchers = [
+ MatchesDict({
+ "recipe_build": Equals(canonical_url(
+ build, force_local_path=True)),
+ "action": Equals("created"),
+ "recipe": Equals(canonical_url(
+ job.recipe, force_local_path=True)),
+ "build_request": Equals(canonical_url(
+ job.build_request, force_local_path=True)),
+ "status": Equals("Needs building"),
+ "registry_upload_status": Equals("Unscheduled"),
+ })
+ for build in builds]
+ self.assertThat(hook.deliveries, MatchesSetwise(*(
+ MatchesStructure(
+ event_type=Equals("oci-recipe:build:0.1"),
+ payload=payload_matcher)
+ for payload_matcher in payload_matchers)))
+ self.assertThat(
+ logger.output,
+ LogsScheduledWebhooks([
+ (hook, "oci-recipe:build:0.1", payload_matcher)
+ for payload_matcher in payload_matchers]))
def test_destroySelf(self):
oci_recipe = self.factory.makeOCIRecipe()
diff --git a/lib/lp/oci/tests/test_ocirecipebuild.py b/lib/lp/oci/tests/test_ocirecipebuild.py
index 0579a8b..84a69f4 100644
--- a/lib/lp/oci/tests/test_ocirecipebuild.py
+++ b/lib/lp/oci/tests/test_ocirecipebuild.py
@@ -12,6 +12,7 @@ import six
from testtools.matchers import (
ContainsDict,
Equals,
+ Is,
MatchesDict,
MatchesStructure,
)
@@ -181,6 +182,7 @@ class TestOCIRecipeBuild(TestCaseWithFactory):
"action": Equals("status-changed"),
"recipe": Equals(
canonical_url(self.build.recipe, force_local_path=True)),
+ "build_request": Is(None),
"status": Equals("Successfully built"),
'registry_upload_status': Equals("Unscheduled"),
}
diff --git a/lib/lp/oci/tests/test_ocirecipebuildjob.py b/lib/lp/oci/tests/test_ocirecipebuildjob.py
index f4628e0..c0926a4 100644
--- a/lib/lp/oci/tests/test_ocirecipebuildjob.py
+++ b/lib/lp/oci/tests/test_ocirecipebuildjob.py
@@ -10,6 +10,7 @@ __metaclass__ = type
from fixtures import FakeLogger
from testtools.matchers import (
Equals,
+ Is,
MatchesDict,
MatchesListwise,
MatchesStructure,
@@ -25,7 +26,6 @@ from lp.oci.interfaces.ocirecipe import (
from lp.oci.interfaces.ocirecipebuildjob import (
IOCIRecipeBuildJob,
IOCIRegistryUploadJob,
- IOCIRegistryUploadJobSource,
)
from lp.oci.interfaces.ociregistryclient import IOCIRegistryClient
from lp.oci.model.ocirecipebuildjob import (
@@ -37,6 +37,8 @@ from lp.oci.model.ocirecipebuildjob import (
from lp.services.config import config
from lp.services.features.testing import FeatureFixture
from lp.services.job.runner import JobRunner
+from lp.services.webapp import canonical_url
+from lp.services.webhooks.testing import LogsScheduledWebhooks
from lp.testing import TestCaseWithFactory
from lp.testing.dbuser import dbuser
from lp.testing.fakemethod import FakeMethod
@@ -46,8 +48,6 @@ from lp.testing.layers import (
LaunchpadZopelessLayer,
)
from lp.testing.mail_helpers import pop_notifications
-from lp.services.webapp import canonical_url
-from lp.services.webhooks.testing import LogsScheduledWebhooks
def run_isolated_jobs(jobs):
@@ -134,6 +134,7 @@ class TestOCIRegistryUploadJob(TestCaseWithFactory):
"action": Equals("created"),
"recipe": Equals(
canonical_url(ocibuild.recipe, force_local_path=True)),
+ "build_request": Is(None),
"status": Equals("Successfully built"),
"registry_upload_status": Equals("Pending")}]
expected_payloads += [{
@@ -142,6 +143,7 @@ class TestOCIRegistryUploadJob(TestCaseWithFactory):
"action": Equals("status-changed"),
"recipe": Equals(
canonical_url(ocibuild.recipe, force_local_path=True)),
+ "build_request": Is(None),
"status": Equals("Successfully built"),
"registry_upload_status": Equals(expected),
} for expected in expected_registry_upload_statuses]
