launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #31560
[Merge] ~ruinedyourlife/launchpad:implement-craftrecipe-requestbuild into launchpad:master
Quentin Debhi has proposed merging ~ruinedyourlife/launchpad:implement-craftrecipe-requestbuild into launchpad:master with ~ruinedyourlife/launchpad:handle-craft-recipes-in-personmerge as a prerequisite.
Commit message:
Implement CraftRecipe.requestBuild
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~ruinedyourlife/launchpad/+git/launchpad/+merge/473929
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~ruinedyourlife/launchpad:implement-craftrecipe-requestbuild into launchpad:master.
diff --git a/lib/lp/crafts/interfaces/craftrecipe.py b/lib/lp/crafts/interfaces/craftrecipe.py
index da16d0b..9595363 100644
--- a/lib/lp/crafts/interfaces/craftrecipe.py
+++ b/lib/lp/crafts/interfaces/craftrecipe.py
@@ -8,6 +8,8 @@ __all__ = [
"BadCraftRecipeSearchContext",
"CRAFT_RECIPE_ALLOW_CREATE",
"CRAFT_RECIPE_PRIVATE_FEATURE_FLAG",
+ "CraftRecipeBuildAlreadyPending",
+ "CraftRecipeBuildDisallowedArchitecture",
"CraftRecipeBuildRequestStatus",
"CraftRecipeFeatureDisabled",
"CraftRecipeNotOwner",
@@ -128,6 +130,27 @@ class BadCraftRecipeSearchContext(Exception):
"""The context is not valid for a craft recipe search."""
+@error_status(http.client.BAD_REQUEST)
+class CraftRecipeBuildAlreadyPending(Exception):
+ """A build was requested when an identical build was already pending."""
+
+ def __init__(self):
+ super().__init__(
+ "An identical build of this craft recipe is already pending."
+ )
+
+
+@error_status(http.client.BAD_REQUEST)
+class CraftRecipeBuildDisallowedArchitecture(Exception):
+ """A build was requested for a disallowed architecture."""
+
+ def __init__(self, das):
+ super().__init__(
+ "This craft recipe is not allowed to build for %s/%s."
+ % (das.distroseries.name, das.architecturetag)
+ )
+
+
class CraftRecipeBuildRequestStatus(EnumeratedType):
"""The status of a request to build a craft recipe."""
@@ -258,6 +281,20 @@ class ICraftRecipeView(Interface):
def visibleByUser(user):
"""Can the specified user see this craft recipe?"""
+ def requestBuild(build_request, distro_arch_series, channels=None):
+ """Request a single build of this craft recipe.
+
+ This method is for internal use; external callers should use
+ `requestBuilds` instead.
+
+ :param build_request: The `ICraftRecipeBuildRequest` job being
+ processed.
+ :param distro_arch_series: The architecture to build for.
+ :param channels: A dictionary mapping snap names to channels to use
+ for this build.
+ :return: `ICraftRecipeBuild`.
+ """
+
def requestBuilds(requester, channels=None, architectures=None):
"""Request that the craft recipe be built.
diff --git a/lib/lp/crafts/model/craftrecipe.py b/lib/lp/crafts/model/craftrecipe.py
index f95b377..7a1dc0d 100644
--- a/lib/lp/crafts/model/craftrecipe.py
+++ b/lib/lp/crafts/model/craftrecipe.py
@@ -8,10 +8,22 @@ __all__ = [
]
from datetime import timezone
+from operator import itemgetter
+from lazr.lifecycle.event import ObjectCreatedEvent
from storm.databases.postgres import JSON
-from storm.locals import Bool, DateTime, Int, Reference, Unicode
+from storm.locals import (
+ Bool,
+ DateTime,
+ Int,
+ Join,
+ Or,
+ Reference,
+ Store,
+ Unicode,
+)
from zope.component import getUtility
+from zope.event import notify
from zope.interface import implementer
from zope.security.proxy import removeSecurityProxy
@@ -20,12 +32,15 @@ from lp.app.enums import (
PUBLIC_INFORMATION_TYPES,
InformationType,
)
+from lp.buildmaster.enums import BuildStatus
from lp.code.model.gitcollection import GenericGitCollection
from lp.code.model.gitrepository import GitRepository
from lp.code.model.reciperegistry import recipe_registry
from lp.crafts.interfaces.craftrecipe import (
CRAFT_RECIPE_ALLOW_CREATE,
CRAFT_RECIPE_PRIVATE_FEATURE_FLAG,
+ CraftRecipeBuildAlreadyPending,
+ CraftRecipeBuildDisallowedArchitecture,
CraftRecipeBuildRequestStatus,
CraftRecipeFeatureDisabled,
CraftRecipeNotOwner,
@@ -37,19 +52,27 @@ from lp.crafts.interfaces.craftrecipe import (
ICraftRecipeSet,
NoSourceForCraftRecipe,
)
+from lp.crafts.interfaces.craftrecipebuild import ICraftRecipeBuildSet
from lp.crafts.interfaces.craftrecipejob import (
ICraftRecipeRequestBuildsJobSource,
)
+from lp.crafts.model.craftrecipebuild import CraftRecipeBuild
from lp.registry.errors import PrivatePersonLinkageError
from lp.registry.interfaces.person import IPersonSet, validate_public_person
+from lp.registry.model.distribution import Distribution
+from lp.registry.model.distroseries import DistroSeries
+from lp.registry.model.series import ACTIVE_STATUSES
from lp.services.database.bulk import load_related
from lp.services.database.constants import DEFAULT, UTC_NOW
+from lp.services.database.decoratedresultset import DecoratedResultSet
from lp.services.database.enumcol import DBEnum
from lp.services.database.interfaces import IPrimaryStore, IStore
from lp.services.database.stormbase import StormBase
from lp.services.features import getFeatureFlag
from lp.services.job.interfaces.job import JobStatus
+from lp.services.librarian.model import LibraryFileAlias
from lp.services.propertycache import cachedproperty, get_property_cache
+from lp.soyuz.model.distroarchseries import DistroArchSeries, PocketChroot
def craft_recipe_modified(recipe, event):
@@ -247,6 +270,52 @@ class CraftRecipe(StormBase):
# more privacy infrastructure.
return False
+ def _isBuildableArchitectureAllowed(self, das):
+ """Check whether we may build for a buildable `DistroArchSeries`.
+
+ The caller is assumed to have already checked that a suitable chroot
+ is available (either directly or via
+ `DistroSeries.buildable_architectures`).
+ """
+ return das.enabled and (
+ das.processor.supports_virtualized or not self.require_virtualized
+ )
+
+ def _isArchitectureAllowed(self, das):
+ """Check whether we may build for a `DistroArchSeries`."""
+ return (
+ das.getChroot() is not None
+ and self._isBuildableArchitectureAllowed(das)
+ )
+
+ def getAllowedArchitectures(self):
+ """See `ICraftRecipe`."""
+ store = Store.of(self)
+ origin = [
+ DistroArchSeries,
+ Join(
+ DistroSeries, DistroArchSeries.distroseries == DistroSeries.id
+ ),
+ Join(Distribution, DistroSeries.distribution == Distribution.id),
+ Join(
+ PocketChroot,
+ PocketChroot.distroarchseries == DistroArchSeries.id,
+ ),
+ Join(LibraryFileAlias, PocketChroot.chroot == LibraryFileAlias.id),
+ ]
+ # Preload DistroSeries and Distribution, since we'll need those in
+ # determine_architectures_to_build.
+ results = store.using(*origin).find(
+ (DistroArchSeries, DistroSeries, Distribution),
+ DistroSeries.status.is_in(ACTIVE_STATUSES),
+ )
+ all_buildable_dases = DecoratedResultSet(results, itemgetter(0))
+ return [
+ das
+ for das in all_buildable_dases
+ if self._isBuildableArchitectureAllowed(das)
+ ]
+
def destroySelf(self):
"""See `ICraftRecipe`."""
IStore(CraftRecipe).remove(self)
@@ -259,6 +328,47 @@ class CraftRecipe(StormBase):
% (requester.display_name, self.owner.display_name)
)
+ def requestBuild(self, build_request, distro_arch_series, channels=None):
+ """Request a single build of this craft recipe.
+
+ This method is for internal use; external callers should use
+ `requestBuilds` instead.
+
+ :param build_request: The `ICraftRecipeBuildRequest` job being
+ processed.
+ :param distro_arch_series: The architecture to build for.
+ :param channels: A dictionary mapping snap names to channels to use
+ for this build.
+ :return: `ICraftRecipeBuild`.
+ """
+ self._checkRequestBuild(build_request.requester)
+ if not self._isArchitectureAllowed(distro_arch_series):
+ raise CraftRecipeBuildDisallowedArchitecture(distro_arch_series)
+
+ if not channels:
+ channels_clause = Or(
+ CraftRecipeBuild.channels == None,
+ CraftRecipeBuild.channels == {},
+ )
+ else:
+ channels_clause = CraftRecipeBuild.channels == channels
+ pending = IStore(self).find(
+ CraftRecipeBuild,
+ CraftRecipeBuild.recipe == self,
+ CraftRecipeBuild.processor == distro_arch_series.processor,
+ channels_clause,
+ CraftRecipeBuild.status == BuildStatus.NEEDSBUILD,
+ )
+ if pending.any() is not None:
+ raise CraftRecipeBuildAlreadyPending
+
+ build = getUtility(ICraftRecipeBuildSet).new(
+ build_request, self, distro_arch_series, channels=channels
+ )
+ build.queueBuild()
+ notify(ObjectCreatedEvent(build, user=build_request.requester))
+ return build
+
def requestBuilds(self, requester, channels=None, architectures=None):
"""See `ICraftRecipe`."""
self._checkRequestBuild(requester)
diff --git a/lib/lp/crafts/security.py b/lib/lp/crafts/security.py
index 20920c9..26a7053 100644
--- a/lib/lp/crafts/security.py
+++ b/lib/lp/crafts/security.py
@@ -10,6 +10,8 @@ from lp.crafts.interfaces.craftrecipe import (
ICraftRecipe,
ICraftRecipeBuildRequest,
)
+from lp.crafts.interfaces.craftrecipebuild import ICraftRecipeBuild
+from lp.security import AdminByBuilddAdmin
class ViewCraftRecipe(AuthorizationBase):
@@ -60,3 +62,32 @@ class ViewCraftRecipeBuildRequest(DelegatedAuthorization):
def __init__(self, obj):
super().__init__(obj, obj.recipe, "launchpad.View")
+
+
+class ViewCraftRecipeBuild(DelegatedAuthorization):
+ permission = "launchpad.View"
+ usedfor = ICraftRecipeBuild
+
+ def iter_objects(self):
+ yield self.obj.recipe
+
+
+class EditCraftRecipeBuild(AdminByBuilddAdmin):
+ permission = "launchpad.Edit"
+ usedfor = ICraftRecipeBuild
+
+ def checkAuthenticated(self, user):
+ """Check edit access for craft recipe builds.
+
+ Allow admins, buildd admins, and the owner of the craft recipe.
+ (Note that the requester of the build is required to be in the team
+ that owns the craft recipe.)
+ """
+ auth_recipe = EditCraftRecipe(self.obj.recipe)
+ if auth_recipe.checkAuthenticated(user):
+ return True
+ return super().checkAuthenticated(user)
+
+
+class AdminCraftRecipeBuild(AdminByBuilddAdmin):
+ usedfor = ICraftRecipeBuild
diff --git a/lib/lp/crafts/tests/test_craftrecipe.py b/lib/lp/crafts/tests/test_craftrecipe.py
index d6cf3ac..1c0bc7b 100644
--- a/lib/lp/crafts/tests/test_craftrecipe.py
+++ b/lib/lp/crafts/tests/test_craftrecipe.py
@@ -3,6 +3,7 @@
"""Test craft recipes."""
+from storm.locals import Store
from testtools.matchers import (
Equals,
Is,
@@ -14,8 +15,17 @@ from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
from lp.app.enums import InformationType
+from lp.buildmaster.enums import BuildQueueStatus, BuildStatus
+from lp.buildmaster.interfaces.buildqueue import IBuildQueue
+from lp.buildmaster.interfaces.processor import (
+ IProcessorSet,
+ ProcessorNotFound,
+)
+from lp.buildmaster.model.buildqueue import BuildQueue
from lp.crafts.interfaces.craftrecipe import (
CRAFT_RECIPE_ALLOW_CREATE,
+ CraftRecipeBuildAlreadyPending,
+ CraftRecipeBuildDisallowedArchitecture,
CraftRecipeBuildRequestStatus,
CraftRecipeFeatureDisabled,
CraftRecipePrivateFeatureDisabled,
@@ -23,6 +33,7 @@ from lp.crafts.interfaces.craftrecipe import (
ICraftRecipeSet,
NoSourceForCraftRecipe,
)
+from lp.crafts.interfaces.craftrecipebuild import ICraftRecipeBuild
from lp.crafts.interfaces.craftrecipejob import (
ICraftRecipeRequestBuildsJobSource,
)
@@ -111,6 +122,175 @@ class TestCraftRecipe(TestCaseWithFactory):
getUtility(ICraftRecipeSet).getByName(owner, project, "condemned")
)
+ def makeBuildableDistroArchSeries(
+ self,
+ architecturetag=None,
+ processor=None,
+ supports_virtualized=True,
+ supports_nonvirtualized=True,
+ **kwargs,
+ ):
+ if architecturetag is None:
+ architecturetag = self.factory.getUniqueUnicode("arch")
+ if processor is None:
+ try:
+ processor = getUtility(IProcessorSet).getByName(
+ architecturetag
+ )
+ except ProcessorNotFound:
+ processor = self.factory.makeProcessor(
+ name=architecturetag,
+ supports_virtualized=supports_virtualized,
+ supports_nonvirtualized=supports_nonvirtualized,
+ )
+ das = self.factory.makeDistroArchSeries(
+ architecturetag=architecturetag, processor=processor, **kwargs
+ )
+ fake_chroot = self.factory.makeLibraryFileAlias(
+ filename="fake_chroot.tar.gz", db_only=True
+ )
+ das.addOrUpdateChroot(fake_chroot)
+ return das
+
+ def test_requestBuild(self):
+ # requestBuild creates a new CraftRecipeBuild.
+ recipe = self.factory.makeCraftRecipe()
+ das = self.makeBuildableDistroArchSeries()
+ build_request = self.factory.makeCraftRecipeBuildRequest(recipe=recipe)
+ build = recipe.requestBuild(build_request, das)
+ self.assertTrue(ICraftRecipeBuild.providedBy(build))
+ self.assertThat(
+ build,
+ MatchesStructure(
+ requester=Equals(recipe.owner.teamowner),
+ distro_arch_series=Equals(das),
+ channels=Is(None),
+ status=Equals(BuildStatus.NEEDSBUILD),
+ ),
+ )
+ store = Store.of(build)
+ store.flush()
+ build_queue = store.find(
+ BuildQueue,
+ BuildQueue._build_farm_job_id
+ == removeSecurityProxy(build).build_farm_job_id,
+ ).one()
+ self.assertProvides(build_queue, IBuildQueue)
+ self.assertEqual(recipe.require_virtualized, build_queue.virtualized)
+ self.assertEqual(BuildQueueStatus.WAITING, build_queue.status)
+
+ def test_requestBuild_score(self):
+ # Build requests have a relatively low queue score (2510).
+ recipe = self.factory.makeCraftRecipe()
+ das = self.makeBuildableDistroArchSeries()
+ build_request = self.factory.makeCraftRecipeBuildRequest(recipe=recipe)
+ build = recipe.requestBuild(build_request, das)
+ queue_record = build.buildqueue_record
+ queue_record.score()
+ self.assertEqual(2510, queue_record.lastscore)
+
+ def test_requestBuild_channels(self):
+ # requestBuild can select non-default channels.
+ recipe = self.factory.makeCraftRecipe()
+ das = self.makeBuildableDistroArchSeries()
+ build_request = self.factory.makeCraftRecipeBuildRequest(recipe=recipe)
+ build = recipe.requestBuild(
+ build_request, das, channels={"craftcraft": "edge"}
+ )
+ self.assertEqual({"craftcraft": "edge"}, build.channels)
+
+ def test_requestBuild_rejects_repeats(self):
+ # requestBuild refuses if there is already a pending build.
+ recipe = self.factory.makeCraftRecipe()
+ distro_series = self.factory.makeDistroSeries()
+ arches = [
+ self.makeBuildableDistroArchSeries(distroseries=distro_series)
+ for _ in range(2)
+ ]
+ build_request = self.factory.makeCraftRecipeBuildRequest(recipe=recipe)
+ old_build = recipe.requestBuild(build_request, arches[0])
+ self.assertRaises(
+ CraftRecipeBuildAlreadyPending,
+ recipe.requestBuild,
+ build_request,
+ arches[0],
+ )
+ # We can build for a different distroarchseries.
+ recipe.requestBuild(build_request, arches[1])
+ # channels=None and channels={} are treated as equivalent, but
+ # anything else allows a new build.
+ self.assertRaises(
+ CraftRecipeBuildAlreadyPending,
+ recipe.requestBuild,
+ build_request,
+ arches[0],
+ channels={},
+ )
+ recipe.requestBuild(
+ build_request, arches[0], channels={"core": "edge"}
+ )
+ self.assertRaises(
+ CraftRecipeBuildAlreadyPending,
+ recipe.requestBuild,
+ build_request,
+ arches[0],
+ channels={"core": "edge"},
+ )
+ # Changing the status of the old build allows a new build.
+ old_build.updateStatus(BuildStatus.BUILDING)
+ old_build.updateStatus(BuildStatus.FULLYBUILT)
+ recipe.requestBuild(build_request, arches[0])
+
+ def test_requestBuild_virtualization(self):
+ # New builds are virtualized if any of the processor or recipe
+ # require it.
+ recipe = self.factory.makeCraftRecipe()
+ distro_series = self.factory.makeDistroSeries()
+ dases = {}
+ for proc_nonvirt in True, False:
+ das = self.makeBuildableDistroArchSeries(
+ distroseries=distro_series,
+ supports_virtualized=True,
+ supports_nonvirtualized=proc_nonvirt,
+ )
+ dases[proc_nonvirt] = das
+ for proc_nonvirt, recipe_virt, build_virt in (
+ (True, False, False),
+ (True, True, True),
+ (False, False, True),
+ (False, True, True),
+ ):
+ das = dases[proc_nonvirt]
+ recipe = self.factory.makeCraftRecipe(
+ require_virtualized=recipe_virt
+ )
+ build_request = self.factory.makeCraftRecipeBuildRequest(
+ recipe=recipe
+ )
+ build = recipe.requestBuild(build_request, das)
+ self.assertEqual(build_virt, build.virtualized)
+
+ def test_requestBuild_nonvirtualized(self):
+ # A non-virtualized processor can build a craft recipe iff the
+ # recipe has require_virtualized set to False.
+ recipe = self.factory.makeCraftRecipe()
+ distro_series = self.factory.makeDistroSeries()
+ das = self.makeBuildableDistroArchSeries(
+ distroseries=distro_series,
+ supports_virtualized=False,
+ supports_nonvirtualized=True,
+ )
+ build_request = self.factory.makeCraftRecipeBuildRequest(recipe=recipe)
+ self.assertRaises(
+ CraftRecipeBuildDisallowedArchitecture,
+ recipe.requestBuild,
+ build_request,
+ das,
+ )
+ with admin_logged_in():
+ recipe.require_virtualized = False
+ recipe.requestBuild(build_request, das)
+
def test_requestBuilds(self):
# requestBuilds schedules a job and returns a corresponding
# CraftRecipeBuildRequest.
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index 9dece89..909b220 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -6976,6 +6976,10 @@ class LaunchpadObjectFactory(ObjectFactory):
recipe = self.makeCraftRecipe()
if requester is None:
requester = recipe.owner.teamowner
+ if recipe.owner.is_team:
+ requester = recipe.owner.teamowner
+ else:
+ requester = recipe.owner
return recipe.requestBuilds(
requester, channels=channels, architectures=architectures
)
