Merge ~cjwatson/launchpad:charm-recipe-request-builds into launchpad:master

[Colin Watson]

Implement CharmRecipe.requestBuild

[Cristian Gonzalez (cristiangsp)]

Looks good!

review: Approve

diff --git a/lib/lp/charms/interfaces/charmrecipe.py b/lib/lp/charms/interfaces/charmrecipe.py
index 8b38153..d625615 100644
--- a/lib/lp/charms/interfaces/charmrecipe.py
+++ b/lib/lp/charms/interfaces/charmrecipe.py
@@ -12,6 +12,8 @@ __all__ = [
     "CHARM_RECIPE_ALLOW_CREATE",
     "CHARM_RECIPE_BUILD_DISTRIBUTION",
     "CHARM_RECIPE_PRIVATE_FEATURE_FLAG",
+    "CharmRecipeBuildAlreadyPending",
+    "CharmRecipeBuildDisallowedArchitecture",
     "CharmRecipeBuildRequestStatus",
     "CharmRecipeFeatureDisabled",
     "CharmRecipeNotOwner",
@@ -139,6 +141,25 @@ class BadCharmRecipeSearchContext(Exception):
     """The context is not valid for a charm recipe search."""
 
 
+@error_status(http_client.BAD_REQUEST)
+class CharmRecipeBuildAlreadyPending(Exception):
+    """A build was requested when an identical build was already pending."""
+
+    def __init__(self):
+        super(CharmRecipeBuildAlreadyPending, self).__init__(
+            "An identical build of this charm recipe is already pending.")
+
+
+@error_status(http_client.BAD_REQUEST)
+class CharmRecipeBuildDisallowedArchitecture(Exception):
+    """A build was requested for a disallowed architecture."""
+
+    def __init__(self, das):
+        super(CharmRecipeBuildDisallowedArchitecture, self).__init__(
+            "This charm recipe is not allowed to build for %s/%s." %
+            (das.distroseries.name, das.architecturetag))
+
+
 class CharmRecipeBuildRequestStatus(EnumeratedType):
     """The status of a request to build a charm recipe."""
 
@@ -233,6 +254,20 @@ class ICharmRecipeView(Interface):
     def visibleByUser(user):
         """Can the specified user see this charm recipe?"""
 
+    def requestBuild(build_request, distro_arch_series, channels=None):
+        """Request a single build of this charm recipe.
+
+        This method is for internal use; external callers should use
+        `requestBuilds` instead.
+
+        :param build_request: The `ICharmRecipeBuildRequest` 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: `ICharmRecipeBuild`.
+        """
+
     def requestBuilds(requester, channels=None, architectures=None):
         """Request that the charm recipe be built.
 
diff --git a/lib/lp/charms/model/charmrecipe.py b/lib/lp/charms/model/charmrecipe.py
index 7bb0d62..cbb078d 100644
--- a/lib/lp/charms/model/charmrecipe.py
+++ b/lib/lp/charms/model/charmrecipe.py
@@ -10,16 +10,23 @@ __all__ = [
     "CharmRecipe",
     ]
 
+from operator import itemgetter
+
+from lazr.lifecycle.event import ObjectCreatedEvent
 import pytz
 from storm.databases.postgres import JSON
 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
 
@@ -29,10 +36,13 @@ from lp.app.enums import (
     PUBLIC_INFORMATION_TYPES,
     )
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+from lp.buildmaster.enums import BuildStatus
 from lp.charms.interfaces.charmrecipe import (
     CHARM_RECIPE_ALLOW_CREATE,
     CHARM_RECIPE_BUILD_DISTRIBUTION,
     CHARM_RECIPE_PRIVATE_FEATURE_FLAG,
+    CharmRecipeBuildAlreadyPending,
+    CharmRecipeBuildDisallowedArchitecture,
     CharmRecipeBuildRequestStatus,
     CharmRecipeFeatureDisabled,
     CharmRecipeNotOwner,
@@ -44,9 +54,11 @@ from lp.charms.interfaces.charmrecipe import (
     ICharmRecipeSet,
     NoSourceForCharmRecipe,
     )
+from lp.charms.interfaces.charmrecipebuild import ICharmRecipeBuildSet
 from lp.charms.interfaces.charmrecipejob import (
     ICharmRecipeRequestBuildsJobSource,
     )
+from lp.charms.model.charmrecipebuild import CharmRecipeBuild
 from lp.code.model.gitcollection import GenericGitCollection
 from lp.code.model.gitrepository import GitRepository
 from lp.registry.errors import PrivatePersonLinkageError
@@ -55,11 +67,15 @@ 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 (
     IMasterStore,
@@ -68,10 +84,15 @@ from lp.services.database.interfaces import (
 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 charm_recipe_modified(recipe, event):
@@ -342,6 +363,48 @@ class CharmRecipe(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 `IOCIRecipe`."""
+        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 _checkRequestBuild(self, requester):
         """May `requester` request builds of this charm recipe?"""
         if not requester.inTeam(self.owner):
@@ -349,6 +412,44 @@ class CharmRecipe(StormBase):
                 "%s cannot create charm recipe builds owned by %s." %
                 (requester.display_name, self.owner.display_name))
 
+    def requestBuild(self, build_request, distro_arch_series, channels=None):
+        """Request a single build of this charm recipe.
+
+        This method is for internal use; external callers should use
+        `requestBuilds` instead.
+
+        :param build_request: The `ICharmRecipeBuildRequest` 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: `ICharmRecipeBuild`.
+        """
+        self._checkRequestBuild(build_request.requester)
+        if not self._isArchitectureAllowed(distro_arch_series):
+            raise CharmRecipeBuildDisallowedArchitecture(distro_arch_series)
+
+        if not channels:
+            channels_clause = Or(
+                CharmRecipeBuild.channels == None,
+                CharmRecipeBuild.channels == {})
+        else:
+            channels_clause = CharmRecipeBuild.channels == channels
+        pending = IStore(self).find(
+            CharmRecipeBuild,
+            CharmRecipeBuild.recipe == self,
+            CharmRecipeBuild.processor == distro_arch_series.processor,
+            channels_clause,
+            CharmRecipeBuild.status == BuildStatus.NEEDSBUILD)
+        if pending.any() is not None:
+            raise CharmRecipeBuildAlreadyPending
+
+        build = getUtility(ICharmRecipeBuildSet).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 `ICharmRecipe`."""
         self._checkRequestBuild(requester)
diff --git a/lib/lp/charms/tests/test_charmrecipe.py b/lib/lp/charms/tests/test_charmrecipe.py
index bf5d0d0..cd53f3e 100644
--- a/lib/lp/charms/tests/test_charmrecipe.py
+++ b/lib/lp/charms/tests/test_charmrecipe.py
@@ -7,6 +7,7 @@ from __future__ import absolute_import, print_function, unicode_literals
 
 __metaclass__ = type
 
+from storm.locals import Store
 from testtools.matchers import (
     Equals,
     Is,
@@ -18,9 +19,21 @@ 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.charms.interfaces.charmrecipe import (
     CHARM_RECIPE_ALLOW_CREATE,
     CHARM_RECIPE_BUILD_DISTRIBUTION,
+    CharmRecipeBuildAlreadyPending,
+    CharmRecipeBuildDisallowedArchitecture,
     CharmRecipeBuildRequestStatus,
     CharmRecipeFeatureDisabled,
     CharmRecipePrivateFeatureDisabled,
@@ -28,6 +41,7 @@ from lp.charms.interfaces.charmrecipe import (
     ICharmRecipeSet,
     NoSourceForCharmRecipe,
     )
+from lp.charms.interfaces.charmrecipebuild import ICharmRecipeBuild
 from lp.charms.interfaces.charmrecipejob import (
     ICharmRecipeRequestBuildsJobSource,
     )
@@ -170,6 +184,140 @@ class TestCharmRecipe(TestCaseWithFactory):
                 current_series,
                 removeSecurityProxy(recipe)._default_distro_series)
 
+    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 CharmRecipeBuild.
+        recipe = self.factory.makeCharmRecipe()
+        das = self.makeBuildableDistroArchSeries()
+        build_request = self.factory.makeCharmRecipeBuildRequest(recipe=recipe)
+        build = recipe.requestBuild(build_request, das)
+        self.assertTrue(ICharmRecipeBuild.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.makeCharmRecipe()
+        das = self.makeBuildableDistroArchSeries()
+        build_request = self.factory.makeCharmRecipeBuildRequest(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.makeCharmRecipe()
+        das = self.makeBuildableDistroArchSeries()
+        build_request = self.factory.makeCharmRecipeBuildRequest(recipe=recipe)
+        build = recipe.requestBuild(
+            build_request, das, channels={"charmcraft": "edge"})
+        self.assertEqual({"charmcraft": "edge"}, build.channels)
+
+    def test_requestBuild_rejects_repeats(self):
+        # requestBuild refuses if there is already a pending build.
+        recipe = self.factory.makeCharmRecipe()
+        distro_series = self.factory.makeDistroSeries()
+        arches = [
+            self.makeBuildableDistroArchSeries(distroseries=distro_series)
+            for _ in range(2)]
+        build_request = self.factory.makeCharmRecipeBuildRequest(recipe=recipe)
+        old_build = recipe.requestBuild(build_request, arches[0])
+        self.assertRaises(
+            CharmRecipeBuildAlreadyPending, 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(
+            CharmRecipeBuildAlreadyPending, recipe.requestBuild,
+            build_request, arches[0], channels={})
+        recipe.requestBuild(
+            build_request, arches[0], channels={"core": "edge"})
+        self.assertRaises(
+            CharmRecipeBuildAlreadyPending, 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.makeCharmRecipe()
+        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.makeCharmRecipe(
+                require_virtualized=recipe_virt)
+            build_request = self.factory.makeCharmRecipeBuildRequest(
+                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 charm recipe iff the
+        # recipe has require_virtualized set to False.
+        recipe = self.factory.makeCharmRecipe()
+        distro_series = self.factory.makeDistroSeries()
+        das = self.makeBuildableDistroArchSeries(
+            distroseries=distro_series, supports_virtualized=False,
+            supports_nonvirtualized=True)
+        build_request = self.factory.makeCharmRecipeBuildRequest(recipe=recipe)
+        self.assertRaises(
+            CharmRecipeBuildDisallowedArchitecture, 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
         # CharmRecipeBuildRequest.
diff --git a/lib/lp/security.py b/lib/lp/security.py
index e11855f..6108048 100644
--- a/lib/lp/security.py
+++ b/lib/lp/security.py
@@ -69,6 +69,7 @@ from lp.charms.interfaces.charmrecipe import (
     ICharmRecipe,
     ICharmRecipeBuildRequest,
     )
+from lp.charms.interfaces.charmrecipebuild import ICharmRecipeBuild
 from lp.code.interfaces.branch import (
     IBranch,
     user_has_special_branch_access,
@@ -3668,3 +3669,32 @@ class ViewCharmRecipeBuildRequest(DelegatedAuthorization):
     def __init__(self, obj):
         super(ViewCharmRecipeBuildRequest, self).__init__(
             obj, obj.recipe, 'launchpad.View')
+
+
+class ViewCharmRecipeBuild(DelegatedAuthorization):
+    permission = 'launchpad.View'
+    usedfor = ICharmRecipeBuild
+
+    def iter_objects(self):
+        yield self.obj.recipe
+
+
+class EditCharmRecipeBuild(AdminByBuilddAdmin):
+    permission = 'launchpad.Edit'
+    usedfor = ICharmRecipeBuild
+
+    def checkAuthenticated(self, user):
+        """Check edit access for snap package builds.
+
+        Allow admins, buildd admins, and the owner of the charm recipe.
+        (Note that the requester of the build is required to be in the team
+        that owns the charm recipe.)
+        """
+        auth_recipe = EditCharmRecipe(self.obj.recipe)
+        if auth_recipe.checkAuthenticated(user):
+            return True
+        return super(EditCharmRecipeBuild, self).checkAuthenticated(user)
+
+
+class AdminCharmRecipeBuild(AdminByBuilddAdmin):
+    usedfor = ICharmRecipeBuild
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index e68c408..4c125a5 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -5158,7 +5158,10 @@ class BareLaunchpadObjectFactory(ObjectFactory):
         if recipe is None:
             recipe = self.makeCharmRecipe()
         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)