Merge ~twom/launchpad:oci-ocirecipebuild into launchpad:master

[Tom Wardill]

Implement more of OCIRecipeBuild

Using SnapBuild as a model, implement more of the required methods to meet the interfaces of IOCIRecipeBuild and IPackageBuild.

Ensure queueBuild works, for further work in to OCIRecipeBuildJob and BuildBehaviour

[Colin Watson (cjwatson)]

review: Approve

b8a2b9a... by Tom Wardill on 2020-02-12

Include virtualized tests

1e7cb5c... by Tom Wardill on 2020-02-12

Remove duplicate attributes

[Colin Watson (cjwatson)]

review: Approve

130aaf6... by Tom Wardill on 2020-02-13

Rename method, fix comment

9178c04... by Tom Wardill on 2020-02-13

Format imports

9a3042d... by Tom Wardill on 2020-02-13

Fix variable name

0427d6c... by Tom Wardill on 2020-02-13

Better filename tests

e923bac... by Tom Wardill on 2020-02-13

Better virtualized tests for ocirecipebuild

diff --git a/lib/lp/oci/interfaces/ocirecipe.py b/lib/lp/oci/interfaces/ocirecipe.py
index 340adfc..c6ce359 100644
--- a/lib/lp/oci/interfaces/ocirecipe.py
+++ b/lib/lp/oci/interfaces/ocirecipe.py
@@ -240,3 +240,6 @@ class IOCIRecipeSet(Interface):
 
     def findByOwner(owner):
         """Return all OCI Recipes with the given `owner`."""
+
+    def preloadDataForOCIRecipes(recipes, user):
+        """Load the data reloated to a list of OCI Recipes."""
diff --git a/lib/lp/oci/interfaces/ocirecipebuild.py b/lib/lp/oci/interfaces/ocirecipebuild.py
index 5ee6b46..95af3f6 100644
--- a/lib/lp/oci/interfaces/ocirecipebuild.py
+++ b/lib/lp/oci/interfaces/ocirecipebuild.py
@@ -7,13 +7,14 @@ from __future__ import absolute_import, print_function, unicode_literals
 
 __metaclass__ = type
 __all__ = [
+    'IOCIFile',
     'IOCIRecipeBuild',
     'IOCIRecipeBuildSet',
     ]
 
 from lazr.restful.fields import Reference
 from zope.interface import Interface
-from zope.schema import Text
+from zope.schema import TextLine
 
 from lp import _
 from lp.buildmaster.interfaces.buildfarmjob import ISpecificBuildFarmJobSource
@@ -21,11 +22,20 @@ from lp.buildmaster.interfaces.packagebuild import IPackageBuild
 from lp.oci.interfaces.ocirecipe import IOCIRecipe
 from lp.services.database.constants import DEFAULT
 from lp.services.fields import PublicPersonChoice
+from lp.services.librarian.interfaces import ILibraryFileAlias
 
 
 class IOCIRecipeBuildEdit(Interface):
+
     # XXX twom 2020-02-10 This will probably need cancel() implementing
-    pass
+
+    def addFile(lfa, layer_file_digest):
+        """Add an OCI file to this build.
+
+        :param lfa: An `ILibraryFileAlias`.
+        :param layer_file_digest: Digest for this file, used for image layers.
+        :return: An `IOCILayerFile`.
+        """
 
 
 class IOCIRecipeBuildView(IPackageBuild):
@@ -41,6 +51,21 @@ class IOCIRecipeBuildView(IPackageBuild):
         required=True,
         readonly=True)
 
+    def getByFileName():
+        """Retrieve a file by filename
+
+        :return: A result set of (`IOCIFile`, `ILibraryFileAlias`,
+            `ILibraryFileContent`).
+        """
+
+    def getLayerFileByDigest(layer_file_digest):
+        """Retrieve a layer file by the digest.
+
+        :param layer_file_digest: The digest to look up.
+        :raises NotFoundError: if no file exists with the given digest.
+        :return: The corresponding `ILibraryFileAlias`.
+        """
+
 
 class IOCIRecipeBuildAdmin(Interface):
     # XXX twom 2020-02-10 This will probably need rescore() implementing
@@ -61,3 +86,23 @@ class IOCIRecipeBuildSet(ISpecificBuildFarmJobSource):
 
     def preloadBuildsData(builds):
         """Load the data related to a list of OCI recipe builds."""
+
+
+class IOCIFile(Interface):
+    """A link between an OCI recipe build and a file in the librarian."""
+
+    build = Reference(
+        IOCIRecipeBuild,
+        title=_("The OCI recipe build producing this file."),
+        required=True, readonly=True)
+
+    library_file = Reference(
+        ILibraryFileAlias, title=_("A file in the librarian."),
+        required=True, readonly=True)
+
+    layer_file_digest = TextLine(
+        title=_("Content-addressable hash of the file''s contents, "
+                "used for reassembling image layers when pushing "
+                "a build to a registry. This hash is in an opaque format "
+                "generated by the OCI build tool."),
+        required=False, readonly=True)
diff --git a/lib/lp/oci/model/ocirecipe.py b/lib/lp/oci/model/ocirecipe.py
index 0d4d20f..8a01b2d 100644
--- a/lib/lp/oci/model/ocirecipe.py
+++ b/lib/lp/oci/model/ocirecipe.py
@@ -30,11 +30,14 @@ from storm.locals import (
 from zope.component import getUtility
 from zope.event import notify
 from zope.interface import implementer
+from zope.security.proxy import removeSecurityProxy
 
 from lp.buildmaster.enums import BuildStatus
 from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
 from lp.buildmaster.model.buildfarmjob import BuildFarmJob
 from lp.buildmaster.model.buildqueue import BuildQueue
+from lp.code.model.gitcollection import GenericGitCollection
+from lp.code.model.gitrepository import GitRepository
 from lp.oci.interfaces.ocirecipe import (
     DuplicateOCIRecipeName,
     IOCIRecipe,
@@ -46,6 +49,8 @@ from lp.oci.interfaces.ocirecipe import (
     )
 from lp.oci.interfaces.ocirecipebuild import IOCIRecipeBuildSet
 from lp.oci.model.ocirecipebuild import OCIRecipeBuild
+from lp.registry.interfaces.person import IPersonSet
+from lp.services.database.bulk import load_related
 from lp.services.database.constants import DEFAULT
 from lp.services.database.decoratedresultset import DecoratedResultSet
 from lp.services.database.interfaces import (
@@ -293,3 +298,15 @@ class OCIRecipeSet:
     def findByOwner(self, owner):
         """See `IOCIRecipe`."""
         return IStore(OCIRecipe).find(OCIRecipe, OCIRecipe.owner == owner)
+
+    def preloadDataForOCIRecipes(self, recipes, user=None):
+        """See `IOCIRecipeSet`."""
+        recipes = [removeSecurityProxy(recipe) for recipe in recipes]
+
+        person_ids = set()
+        for recipe in recipes:
+            person_ids.add(recipe.registrant_id)
+            person_ids.add(recipe.owner_id)
+
+        list(getUtility(IPersonSet).getPrecachedPersonsFromIDs(
+            person_ids, need_validity=True))
diff --git a/lib/lp/oci/model/ocirecipebuild.py b/lib/lp/oci/model/ocirecipebuild.py
index d069800..307fbd2 100644
--- a/lib/lp/oci/model/ocirecipebuild.py
+++ b/lib/lp/oci/model/ocirecipebuild.py
@@ -7,6 +7,7 @@ from __future__ import absolute_import, print_function, unicode_literals
 
 __metaclass__ = type
 __all__ = [
+    'OCIFile',
     'OCIRecipeBuild',
     'OCIRecipeBuildSet',
     ]
@@ -28,6 +29,7 @@ from storm.store import EmptyResultSet
 from zope.component import getUtility
 from zope.interface import implementer
 
+from lp.app.errors import NotFoundError
 from lp.buildmaster.enums import (
     BuildFarmJobType,
     BuildStatus,
@@ -35,10 +37,14 @@ from lp.buildmaster.enums import (
 from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJobSource
 from lp.buildmaster.model.buildfarmjob import SpecificBuildFarmJobSourceMixin
 from lp.buildmaster.model.packagebuild import PackageBuildMixin
+from lp.oci.interfaces.ocirecipe import IOCIRecipeSet
 from lp.oci.interfaces.ocirecipebuild import (
+    IOCIFile,
     IOCIRecipeBuild,
     IOCIRecipeBuildSet,
     )
+from lp.registry.model.person import Person
+from lp.services.database.bulk import load_related
 from lp.services.database.constants import DEFAULT
 from lp.services.database.decoratedresultset import DecoratedResultSet
 from lp.services.database.enumcol import DBEnum
@@ -46,6 +52,33 @@ from lp.services.database.interfaces import (
     IMasterStore,
     IStore,
     )
+from lp.services.librarian.model import (
+    LibraryFileAlias,
+    LibraryFileContent,
+    )
+
+
+@implementer(IOCIFile)
+class OCIFile(Storm):
+
+    __storm_table__ = 'OCIFile'
+
+    id = Int(name='id', primary=True)
+
+    build_id = Int(name='build', allow_none=False)
+    build = Reference(build_id, 'OCIRecipeBuild.id')
+
+    library_file_id = Int(name='library_file', allow_none=False)
+    library_file = Reference(library_file_id, 'LibraryFileAlias.id')
+
+    layer_file_digest = Unicode(name='layer_file_digest', allow_none=True)
+
+    def __init__(self, build, library_file, layer_file_digest=None):
+        """Construct a `OCIFile`."""
+        super(OCIFile, self).__init__()
+        self.build = build
+        self.library_file = library_file
+        self.layer_file_digest = layer_file_digest
 
 
 @implementer(IOCIRecipeBuild)
@@ -93,6 +126,11 @@ class OCIRecipeBuild(PackageBuildMixin, Storm):
     build_farm_job_id = Int(name='build_farm_job', allow_none=False)
     build_farm_job = Reference(build_farm_job_id, 'BuildFarmJob.id')
 
+    # Stub attributes to match the IPackageBuild interface that we
+    # are not using in this implementation at this time.
+    pocket = None
+    distro_series = None
+
     def __init__(self, build_farm_job, requester, recipe,
                  processor, virtualized, date_created):
 
@@ -130,6 +168,34 @@ class OCIRecipeBuild(PackageBuildMixin, Storm):
         durations.sort()
         return durations[len(durations) // 2]
 
+    def getByFileName(self, filename):
+        result = Store.of(self).find(
+            (OCIFile, LibraryFileAlias, LibraryFileContent),
+            OCIFile.build == self.id,
+            LibraryFileAlias.id == OCIFile.library_file_id,
+            LibraryFileContent.id == LibraryFileAlias.contentID,
+            LibraryFileAlias.filename == filename).one()
+        if result is not None:
+            return result
+        raise NotFoundError(filename)
+
+    def getLayerFileByDigest(self, layer_file_digest):
+        file_object = Store.of(self).find(
+            (OCIFile, LibraryFileAlias, LibraryFileContent),
+            OCIFile.build == self.id,
+            LibraryFileAlias.id == OCIFile.library_file_id,
+            LibraryFileContent.id == LibraryFileAlias.contentID,
+            OCIFile.layer_file_digest == layer_file_digest).one()
+        if file_object is not None:
+            return file_object
+        raise NotFoundError(layer_file_digest)
+
+    def addFile(self, lfa, layer_file_digest=None):
+        oci_file = OCIFile(
+            build=self, library_file=lfa, layer_file_digest=layer_file_digest)
+        IMasterStore(OCIFile).add(oci_file)
+        return oci_file
+
     @property
     def archive(self):
         # XXX twom 2019-12-05 This may need to change when an OCIProject
@@ -142,11 +208,6 @@ class OCIRecipeBuild(PackageBuildMixin, Storm):
         # pillar isn't just a distribution
         return self.recipe.oci_project.distribution
 
-    # Stub attributes to match the IPackageBuild interface that we
-    # will not use in this implementation.
-    pocket = None
-    distro_series = None
-
 
 @implementer(IOCIRecipeBuildSet)
 class OCIRecipeBuildSet(SpecificBuildFarmJobSourceMixin):
@@ -171,7 +232,15 @@ class OCIRecipeBuildSet(SpecificBuildFarmJobSourceMixin):
 
     def preloadBuildsData(self, builds):
         """See `IOCIRecipeBuildSet`."""
-        # XXX twom 2019-12-02 Currently a no-op skeleton, to be filled in
+        # Circular import.
+        from lp.oci.model.ocirecipe import OCIRecipe
+        load_related(Person, builds, ["requester_id"])
+        lfas = load_related(LibraryFileAlias, builds, ["log_id"])
+        load_related(LibraryFileContent, lfas, ["contentID"])
+        recipes = load_related(OCIRecipe, builds, ["recipe_id"])
+        getUtility(IOCIRecipeSet).preloadDataForOCIRecipes(recipes)
+        # XXX twom 2019-12-05 This needs to be extended to include
+        # OCIRecipeBuildJob when that exists.
         return
 
     def getByID(self, build_id):
diff --git a/lib/lp/oci/tests/test_ocirecipebuild.py b/lib/lp/oci/tests/test_ocirecipebuild.py
new file mode 100644
index 0000000..3d51de2
--- /dev/null
+++ b/lib/lp/oci/tests/test_ocirecipebuild.py
@@ -0,0 +1,168 @@
+# Copyright 2019 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for OCI image building recipe functionality."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+from datetime import timedelta
+
+import six
+from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
+
+from lp.app.errors import NotFoundError
+from lp.buildmaster.enums import BuildStatus
+from lp.buildmaster.interfaces.buildqueue import IBuildQueue
+from lp.buildmaster.interfaces.packagebuild import IPackageBuild
+from lp.oci.interfaces.ocirecipebuild import (
+    IOCIRecipeBuild,
+    IOCIRecipeBuildSet,
+    )
+from lp.oci.model.ocirecipebuild import OCIRecipeBuildSet
+from lp.testing import (
+    admin_logged_in,
+    TestCaseWithFactory,
+    )
+from lp.testing.layers import (
+    DatabaseFunctionalLayer,
+    LaunchpadZopelessLayer,
+    )
+
+
+class TestOCIRecipeBuild(TestCaseWithFactory):
+
+    layer = LaunchpadZopelessLayer
+
+    def setUp(self):
+        super(TestOCIRecipeBuild, self).setUp()
+        self.build = self.factory.makeOCIRecipeBuild()
+
+    def test_implements_interface(self):
+        with admin_logged_in():
+            self.assertProvides(self.build, IOCIRecipeBuild)
+            self.assertProvides(self.build, IPackageBuild)
+
+    def test_addFile(self):
+        lfa = self.factory.makeLibraryFileAlias()
+        self.build.addFile(lfa)
+        _, result_lfa, _ = self.build.getByFileName(lfa.filename)
+        self.assertEqual(result_lfa, lfa)
+
+    def test_getByFileName(self):
+        files = [self.factory.makeOCIFile(build=self.build) for x in range(3)]
+        result, _, _ = self.build.getByFileName(
+            files[0].library_file.filename)
+        self.assertEqual(result, files[0])
+
+    def test_getByFileName_missing(self):
+        self.assertRaises(
+            NotFoundError,
+            self.build.getByFileName,
+            "missing")
+
+    def test_getLayerFileByDigest(self):
+        files = [self.factory.makeOCIFile(
+                    build=self.build, layer_file_digest=six.text_type(x))
+                 for x in range(3)]
+        result, _, _ = self.build.getLayerFileByDigest(
+            files[0].layer_file_digest)
+        self.assertEqual(result, files[0])
+
+    def test_getLayerFileByDigest_missing(self):
+        [self.factory.makeOCIFile(
+            build=self.build, layer_file_digest=six.text_type(x))
+         for x in range(3)]
+        self.assertRaises(
+            NotFoundError,
+            self.build.getLayerFileByDigest,
+            'missing')
+
+    def test_estimateDuration(self):
+        # Without previous builds, the default time estimate is 30m.
+        self.assertEqual(1800, self.build.estimateDuration().seconds)
+
+    def test_estimateDuration_with_history(self):
+        # Previous successful builds of the same OCI recipe are used for
+        # estimates.
+        oci_build = self.factory.makeOCIRecipeBuild(
+            status=BuildStatus.FULLYBUILT, duration=timedelta(seconds=335))
+        for i in range(3):
+            self.factory.makeOCIRecipeBuild(
+                requester=oci_build.requester, recipe=oci_build.recipe,
+                status=BuildStatus.FAILEDTOBUILD,
+                duration=timedelta(seconds=20))
+        self.assertEqual(335, oci_build.estimateDuration().seconds)
+
+    def test_queueBuild(self):
+        # OCIRecipeBuild can create the queue entry for itself.
+        bq = self.build.queueBuild()
+        self.assertProvides(bq, IBuildQueue)
+        self.assertEqual(
+            self.build.build_farm_job, removeSecurityProxy(bq)._build_farm_job)
+        self.assertEqual(self.build, bq.specific_build)
+        self.assertEqual(self.build.virtualized, bq.virtualized)
+        self.assertIsNotNone(bq.processor)
+        self.assertEqual(bq, self.build.buildqueue_record)
+
+
+class TestOCIRecipeBuildSet(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_implements_interface(self):
+        target = OCIRecipeBuildSet()
+        with admin_logged_in():
+            self.assertProvides(target, IOCIRecipeBuildSet)
+
+    def test_new(self):
+        requester = self.factory.makePerson()
+        recipe = self.factory.makeOCIRecipe()
+        distro_arch_series = self.factory.makeDistroArchSeries()
+        target = getUtility(IOCIRecipeBuildSet).new(
+            requester, recipe, distro_arch_series)
+        with admin_logged_in():
+            self.assertProvides(target, IOCIRecipeBuild)
+
+    def test_getByID(self):
+        builds = [self.factory.makeOCIRecipeBuild() for x in range(3)]
+        result = getUtility(IOCIRecipeBuildSet).getByID(builds[1].id)
+        self.assertEqual(result, builds[1])
+
+    def test_getByBuildFarmJob(self):
+        builds = [self.factory.makeOCIRecipeBuild() for x in range(3)]
+        result = getUtility(IOCIRecipeBuildSet).getByBuildFarmJob(
+            builds[1].build_farm_job)
+        self.assertEqual(result, builds[1])
+
+    def test_getByBuildFarmJobs(self):
+        builds = [self.factory.makeOCIRecipeBuild() for x in range(3)]
+        self.assertContentEqual(
+            builds,
+            getUtility(IOCIRecipeBuildSet).getByBuildFarmJobs(
+                [build.build_farm_job for build in builds]))
+
+    def test_getByBuildFarmJobs_empty(self):
+        self.assertContentEqual(
+            [], getUtility(IOCIRecipeBuildSet).getByBuildFarmJobs([]))
+
+    def test_virtualized_recipe_requires(self):
+        recipe = self.factory.makeOCIRecipe(require_virtualized=True)
+        target = self.factory.makeOCIRecipeBuild(recipe=recipe)
+        self.assertTrue(target.virtualized)
+
+    def test_virtualized_processor_requires(self):
+        distro_arch_series = self.factory.makeDistroArchSeries()
+        distro_arch_series.processor.supports_nonvirtualized = False
+        recipe = self.factory.makeOCIRecipe(require_virtualized=False)
+        target = self.factory.makeOCIRecipeBuild(
+            distro_arch_series=distro_arch_series, recipe=recipe)
+        self.assertTrue(target.virtualized)
+
+    def test_virtualized_no_support(self):
+        recipe = self.factory.makeOCIRecipe(require_virtualized=False)
+        distro_arch_series = self.factory.makeDistroArchSeries()
+        distro_arch_series.processor.supports_nonvirtualized = True
+        target = self.factory.makeOCIRecipeBuild(
+            recipe=recipe, distro_arch_series=distro_arch_series)
+        self.assertFalse(target.virtualized)
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index d0ecd18..f0788d8 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -160,6 +160,7 @@ from lp.hardwaredb.interfaces.hwdb import (
 from lp.oci.interfaces.ocirecipe import IOCIRecipeSet
 from lp.oci.interfaces.ocirecipebuild import IOCIRecipeBuildSet
 from lp.oci.model.ocirecipe import OCIRecipeArch
+from lp.oci.model.ocirecipebuild import OCIFile
 from lp.registry.enums import (
     BranchSharingPolicy,
     BugSharingPolicy,
@@ -4979,7 +4980,9 @@ class BareLaunchpadObjectFactory(ObjectFactory):
         return OCIRecipeArch(recipe, processor)
 
     def makeOCIRecipeBuild(self, requester=None, recipe=None,
-                           distro_arch_series=None, date_created=DEFAULT):
+                           distro_arch_series=None, date_created=DEFAULT,
+                           status=BuildStatus.NEEDSBUILD, builder=None,
+                           duration=None):
         """Make a new OCIRecipeBuild."""
         if requester is None:
             requester = self.makePerson()
@@ -4987,9 +4990,29 @@ class BareLaunchpadObjectFactory(ObjectFactory):
             distro_arch_series = self.makeDistroArchSeries()
         if recipe is None:
             recipe = self.makeOCIRecipe()
-
-        return getUtility(IOCIRecipeBuildSet).new(
+        oci_build = getUtility(IOCIRecipeBuildSet).new(
             requester, recipe, distro_arch_series, date_created)
+        if duration is not None:
+            removeSecurityProxy(oci_build).updateStatus(
+                BuildStatus.BUILDING, builder=builder,
+                date_started=oci_build.date_created)
+            removeSecurityProxy(oci_build).updateStatus(
+                status, builder=builder,
+                date_finished=oci_build.date_started + duration)
+        else:
+            removeSecurityProxy(oci_build).updateStatus(
+                status, builder=builder)
+        return oci_build
+
+    def makeOCIFile(self, build=None, library_file=None,
+                    layer_file_digest=None):
+        """Make a new OCIFile."""
+        if build is None:
+            build = self.makeOCIRecipeBuild()
+        if library_file is None:
+            library_file = self.makeLibraryFileAlias()
+        return OCIFile(build=build, library_file=library_file,
+                       layer_file_digest=layer_file_digest)
 
 
 # Some factory methods return simple Python types. We don't add