Merge lp:~michael.nelson/launchpad/487009-db-more-soyuz-extraction into lp:launchpad/db-devel

[Michael Nelson]

[Michael Nelson (michael.nelson)]

= Summary =

This branch continues on from the prerequisite at:

https://code.edge.launchpad.net/~michael.nelson/launchpad/487009-db-generalise-ibuilder-1b/+merge/15477

and extracts most of the remaining soyuz-specific binary build logic from IBuilder and into the custom BinaryPackageBuildBehavior to which IBuilder delegates for binary builds, in particular leaving the IBuilder.startBuild() method looking like:

http://pastebin.ubuntu.com/332947/

with verifyBuildRequest() and dispatchBuildToSlave() now being delegated to the current build behavior.

I will have one further branch that:
 * extracts IBuilder.getBuildRecords()
 * extracts IBuilder.slaveStatus()
 * adds documentation for the IBuildFarmJobBehavior with examples for adding further behaviors.

== Pre-implementation notes ==

See the pre-implementation notes on bug 487009.

== Implementation details ==

== Tests ==

bin/test -vv -t doc/builder.txt -t doc/buildd-dispatching.txt

== Demo and Q/A ==

We will need to Q/A this on dogfood.

= Launchpad lint =

lib/lp/soyuz/model/builder.py
    23: [F0401] Unable to import 'lazr.delegates' (No module named delegates)

[Henning Eggers (henninge)]

Thank you for doing this important groundwork for our bridging-the-gap strategy. It will benefit all of us and most importantly Ubuntu.

review: Approve (code)

=== modified file 'lib/lp/buildmaster/interfaces/buildfarmjobbehavior.py'
--- lib/lp/buildmaster/interfaces/buildfarmjobbehavior.py	2009-12-02 08:50:30 +0000
+++ lib/lp/buildmaster/interfaces/buildfarmjobbehavior.py	2009-12-02 08:50:31 +0000
@@ -11,7 +11,7 @@
     'IBuildFarmJobBehavior',
     ]
 
-from zope.interface import Interface
+from zope.interface import Attribute, Interface
 
 
 class BuildBehaviorMismatch(Exception):
@@ -23,6 +23,9 @@
 
 class IBuildFarmJobBehavior(Interface):
 
+    status = Attribute(
+        "Generated status information for this particular job.")
+
     def setBuilder(builder):
         """Sets the associated builder reference for this instance."""
 
@@ -33,6 +36,18 @@
         :param build_queue_item: A BuildQueueItem to build.
         :param logger: A logger to be used to log diagnostic information.
         """
-    # A number of other methods to go here that can be customised for each of
-    # the different build types (branch build, recipe build, translation
-    # build etc.)
+
+    def dispatchBuildToSlave(build_queue_item, logger):
+        """Dispatch a specific build to the slave.
+
+        :param build_queue_item: The `BuildQueueItem` that will be built.
+        :logger: A logger to be used to log diagnostic information.
+        """
+
+    def verifyBuildRequest(build_queue_item, logger):
+        """Carry out any pre-build checks.
+
+        :param build_queue_item: The `BuildQueueItem` that is to be built.
+        :logger: A logger to be used to log diagnostic information.
+        """
+
 
=== modified file 'lib/lp/buildmaster/model/buildfarmjobbehavior.py'
--- lib/lp/buildmaster/model/buildfarmjobbehavior.py	2009-12-02 08:50:30 +0000
+++ lib/lp/buildmaster/model/buildfarmjobbehavior.py	2009-12-02 08:50:31 +0000
@@ -33,6 +33,10 @@
         """The builder should be set once and not changed."""
         self._builder = builder
 
+    def verifyBuildRequest(self, build_queue_item, logger):
+        """The default behavior is a no-op."""
+        pass
+
 
 class IdleBuildBehavior(BuildFarmJobBehaviorBase):
 
@@ -48,3 +52,13 @@
         """See `IBuildFarmJobBehavior`."""
         raise BuildBehaviorMismatch(
             "Builder was idle when asked to log the start of a build.")
+
+    def dispatchBuildToSlave(self, build_queue_item, logger):
+        """See `IBuildFarmJobBehavior`."""
+        raise BuildBehaviorMismatch(
+            "Builder was idle when asked to dispatch a build to the slave.")
+
+    @property
+    def status(self):
+        """See `IBuildFarmJobBehavior`."""
+        return "Idle"
 
=== modified file 'lib/lp/soyuz/interfaces/builder.py'
--- lib/lp/soyuz/interfaces/builder.py	2009-12-02 08:50:30 +0000
+++ lib/lp/soyuz/interfaces/builder.py	2009-12-02 08:50:31 +0000
@@ -154,19 +154,6 @@
             file.
         """
 
-    def cachePrivateSourceOnSlave(logger, build_queue_item):
-        """Ask the slave to download source files for a private build.
-
-        The slave will cache the files for the source in build_queue_item
-        to its local disk in preparation for a private build.  Private builds
-        will always take the source files from the archive rather than the
-        librarian since the archive has more granular access to each
-        archive's files.
-
-        :param logger: A logger used for providing debug information.
-        :param build_queue_item: The `IBuildQueue` being built.
-        """
-
     def checkCanBuildForDistroArchSeries(distro_arch_series):
         """Check that the slave can compile for the given distro_arch_release.
 
 
=== modified file 'lib/lp/soyuz/model/binarypackagebuildbehavior.py'
--- lib/lp/soyuz/model/binarypackagebuildbehavior.py	2009-12-02 08:50:30 +0000
+++ lib/lp/soyuz/model/binarypackagebuildbehavior.py	2009-12-02 08:50:31 +0000
@@ -11,11 +11,20 @@
     'BinaryPackageBuildBehavior',
     ]
 
+import socket
+import xmlrpclib
+
+from canonical.launchpad.webapp import urlappend
 from lp.buildmaster.interfaces.buildfarmjobbehavior import (
     IBuildFarmJobBehavior)
 from lp.buildmaster.model.buildfarmjobbehavior import (
     BuildFarmJobBehaviorBase)
+from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.soyuz.adapters.archivedependencies import (
+    get_primary_current_component, get_sources_list_for_building)
+from lp.soyuz.interfaces.archive import ArchivePurpose
 from lp.soyuz.interfaces.build import IBuildSet
+from lp.soyuz.interfaces.builder import BuildSlaveFailure, CannotBuild
 
 from zope.component import getUtility
 from zope.interface import implements
@@ -33,3 +42,200 @@
 
         logger.info("startBuild(%s, %s, %s, %s)", self._builder.url,
                     spr.name, spr.version, build.pocket.title)
+
+    @property
+    def status(self):
+        """See `IBuildFarmJobBehavior`."""
+        build = getUtility(IBuildSet).getByQueueEntry(
+            self._builder.currentjob)
+        msg = 'Building %s' % build.title
+        archive = build.archive
+        if not archive.owner.private and (archive.is_ppa or archive.is_copy):
+            return '%s [%s/%s]' % (msg, archive.owner.name, archive.name)
+        else:
+            return msg
+
+    def dispatchBuildToSlave(self, build_queue_item, logger):
+        """See `IBuildFarmJobBehavior`."""
+
+        # Start the binary package build on the slave builder. First
+        # we send the chroot.
+        build = getUtility(IBuildSet).getByQueueEntry(build_queue_item)
+        chroot = build.distroarchseries.getChroot()
+        self._builder.cacheFileOnSlave(logger, chroot)
+
+        # Build filemap structure with the files required in this build
+        # and send them to the slave.
+        # If the build is private we tell the slave to get the files from the
+        # archive instead of the librarian because the slaves cannot
+        # access the restricted librarian.
+        private = build.archive.private
+        if private:
+            self._cachePrivateSourceOnSlave(build_queue_item, logger)
+        filemap = {}
+        for source_file in build.sourcepackagerelease.files:
+            lfa = source_file.libraryfile
+            filemap[lfa.filename] = lfa.content.sha1
+            if not private:
+                self._builder.cacheFileOnSlave(
+                    logger, source_file.libraryfile)
+
+        # Generate a string which can be used to cross-check when obtaining
+        # results so we know we are referring to the right database object in
+        # subsequent runs.
+        buildid = "%s-%s" % (build.id, build_queue_item.id)
+        chroot_sha1 = chroot.content.sha1
+        logger.debug(
+            "Initiating build %s on %s" % (buildid, self._builder.url))
+
+        try:
+            args = self._extraBuildArgs(build)
+            status, info = self._builder.slave.build(
+                buildid, "debian", chroot_sha1, filemap, args)
+            message = """%s (%s):
+            ***** RESULT *****
+            %s
+            %s
+            %s: %s
+            ******************
+            """ % (
+                self._builder.name,
+                self._builder.url,
+                filemap,
+                args,
+                status,
+                info,
+                )
+            logger.info(message)
+        except xmlrpclib.Fault, info:
+            # Mark builder as 'failed'.
+            logger.debug(
+                "Disabling builder: %s" % self._builder.url, exc_info=1)
+            self._builder.failbuilder(
+                "Exception (%s) when setting up to new job" % info)
+            raise BuildSlaveFailure
+        except socket.error, info:
+            error_message = "Exception (%s) when setting up new job" % info
+            self._builder.handleTimeout(logger, error_message)
+            raise BuildSlaveFailure
+
+    def verifyBuildRequest(self, build_queue_item, logger):
+        """Assert some pre-build checks.
+
+        The build request is checked:
+         * Virtualized builds can't build on a non-virtual builder
+         * Ensure that we have a chroot
+         * Ensure that the build pocket allows builds for the current
+           distroseries state.
+        """
+        build = getUtility(IBuildSet).getByQueueEntry(build_queue_item)
+        assert not (not self._builder.virtualized and build.is_virtualized), (
+            "Attempt to build non-virtual item on a virtual builder.")
+
+        # Assert that we are not silently building SECURITY jobs.
+        # See findBuildCandidates. Once we start building SECURITY
+        # correctly from EMBARGOED archive this assertion can be removed.
+        # XXX Julian 2007-12-18 spec=security-in-soyuz: This is being
+        # addressed in the work on the blueprint:
+        # https://blueprints.launchpad.net/soyuz/+spec/security-in-soyuz
+        target_pocket = build.pocket
+        assert target_pocket != PackagePublishingPocket.SECURITY, (
+            "Soyuz is not yet capable of building SECURITY uploads.")
+
+        # Ensure build has the needed chroot
+        chroot = build.distroarchseries.getChroot()
+        if chroot is None:
+            raise CannotBuild(
+                "Missing CHROOT for %s/%s/%s" % (
+                    build.distroseries.distribution.name,
+                    build.distroseries.name,
+                    build.distroarchseries.architecturetag)
+                )
+
+        # The main distribution has policies to prevent uploads to some
+        # pockets (e.g. security) during different parts of the distribution
+        # series lifecycle. These do not apply to PPA builds nor any archive
+        # that allows release pocket updates.
+        if (build.archive.purpose != ArchivePurpose.PPA and
+            not build.archive.allowUpdatesToReleasePocket()):
+            # XXX Robert Collins 2007-05-26: not an explicit CannotBuild
+            # exception yet because the callers have not been audited
+            assert build.distroseries.canUploadToPocket(build.pocket), (
+                "%s (%s) can not be built for pocket %s: invalid pocket due "
+                "to the series status of %s."
+                % (build.title, build.id, build.pocket.name,
+                   build.distroseries.name))
+
+    def _cachePrivateSourceOnSlave(self, build_queue_item, logger):
+        """Ask the slave to download source files for a private build.
+
+        The slave will cache the files for the source in build_queue_item
+        to its local disk in preparation for a private build.  Private builds
+        will always take the source files from the archive rather than the
+        librarian since the archive has more granular access to each
+        archive's files.
+
+        :param build_queue_item: The `IBuildQueue` being built.
+        :param logger: A logger used for providing debug information.
+        """
+        # The URL to the file in the archive consists of these parts:
+        # archive_url / makePoolPath() / filename
+        # Once this is constructed we add the http basic auth info.
+
+        # Avoid circular imports.
+        from lp.soyuz.model.publishing import makePoolPath
+
+        build = getUtility(IBuildSet).getByQueueEntry(build_queue_item)
+        archive = build.archive
+        archive_url = archive.archive_url
+        component_name = build.current_component.name
+        for source_file in build.sourcepackagerelease.files:
+            file_name = source_file.libraryfile.filename
+            sha1 = source_file.libraryfile.content.sha1
+            source_name = build.sourcepackagerelease.sourcepackagename.name
+            poolpath = makePoolPath(source_name, component_name)
+            url = urlappend(archive_url, poolpath)
+            url = urlappend(url, file_name)
+            logger.debug("Asking builder on %s to ensure it has file %s "
+                         "(%s, %s)" % (
+                            self._builder.url, file_name, url, sha1))
+            self._builder._sendFileToSlave(
+                url, sha1, "buildd", archive.buildd_secret)
+
+    def _extraBuildArgs(self, build):
+        """
+        Return the extra arguments required by the slave for the given build.
+        """
+        # Build extra arguments.
+        args = {}
+        # turn 'arch_indep' ON only if build is archindep or if
+        # the specific architecture is the nominatedarchindep for
+        # this distroseries (in case it requires any archindep source)
+        args['arch_indep'] = build.distroarchseries.isNominatedArchIndep
+
+        suite = build.distroarchseries.distroseries.name
+        if build.pocket != PackagePublishingPocket.RELEASE:
+            suite += "-%s" % (build.pocket.name.lower())
+        args['suite'] = suite
+
+        archive_purpose = build.archive.purpose
+        if (archive_purpose == ArchivePurpose.PPA and
+            not build.archive.require_virtualized):
+            # If we're building a non-virtual PPA, override the purpose
+            # to PRIMARY and use the primary component override.
+            # This ensures that the package mangling tools will run over
+            # the built packages.
+            args['archive_purpose'] = ArchivePurpose.PRIMARY.name
+            args["ogrecomponent"] = (
+                get_primary_current_component(build))
+        else:
+            args['archive_purpose'] = archive_purpose.name
+            args["ogrecomponent"] = (
+                build.current_component.name)
+
+        args['archives'] = get_sources_list_for_building(build)
+
+        # Let the build slave know whether this is a build in a private
+        # archive.
+        args['archive_private'] = build.archive.private
+        return args
 
=== modified file 'lib/lp/soyuz/model/builder.py'
--- lib/lp/soyuz/model/builder.py	2009-12-02 08:50:30 +0000
+++ lib/lp/soyuz/model/builder.py	2009-12-02 08:50:31 +0000
@@ -38,8 +38,6 @@
 from lp.buildmaster.master import BuilddMaster
 from lp.buildmaster.model.buildfarmjobbehavior import IdleBuildBehavior
 from canonical.database.sqlbase import SQLBase, sqlvalues
-from lp.soyuz.adapters.archivedependencies import (
-    get_primary_current_component, get_sources_list_for_building)
 from lp.soyuz.model.buildqueue import BuildQueue
 from lp.registry.interfaces.person import validate_public_person
 from lp.registry.interfaces.pocket import PackagePublishingPocket
@@ -117,6 +115,7 @@
 
         return (stdout, stderr, resume_process.returncode)
 
+
 class Builder(SQLBase):
 
     implements(IBuilder, IHasBuildRecords)
@@ -195,30 +194,6 @@
                                    url, libraryfilealias.content.sha1))
         self._sendFileToSlave(url, libraryfilealias.content.sha1)
 
-    def cachePrivateSourceOnSlave(self, logger, build_queue_item):
-        """See `IBuilder`."""
-        # The URL to the file in the archive consists of these parts:
-        # archive_url / makePoolPath() / filename
-        # Once this is constructed we add the http basic auth info.
-
-        # Avoid circular imports.
-        from lp.soyuz.model.publishing import makePoolPath
-
-        build = getUtility(IBuildSet).getByQueueEntry(build_queue_item)
-        archive = build.archive
-        archive_url = archive.archive_url
-        component_name = build.current_component.name
-        for source_file in build.sourcepackagerelease.files:
-            file_name = source_file.libraryfile.filename
-            sha1 = source_file.libraryfile.content.sha1
-            source_name = build.sourcepackagerelease.sourcepackagename.name
-            poolpath = makePoolPath(source_name, component_name)
-            url = urlappend(archive_url, poolpath)
-            url = urlappend(url, file_name)
-            logger.debug("Asking builder on %s to ensure it has file %s "
-                         "(%s, %s)" % (self.url, file_name, url, sha1))
-            self._sendFileToSlave(url, sha1, "buildd", archive.buildd_secret)
-
     def _sendFileToSlave(self, url, sha1, username="", password=""):
         """Helper to send the file at 'url' with 'sha1' to this builder."""
         if not self.builderok:
@@ -308,99 +283,6 @@
         """See IBuilder."""
         self.slave = proxy
 
-    def _verifyBuildRequest(self, build_queue_item, logger):
-        """Assert some pre-build checks.
-
-        The build request is checked:
-         * Virtualized builds can't build on a non-virtual builder
-         * Ensure that we have a chroot
-         * Ensure that the build pocket allows builds for the current
-           distroseries state.
-        """
-        build = getUtility(IBuildSet).getByQueueEntry(build_queue_item)
-        assert not (not self.virtualized and build.is_virtualized), (
-            "Attempt to build non-virtual item on a virtual builder.")
-
-        # Assert that we are not silently building SECURITY jobs.
-        # See findBuildCandidates. Once we start building SECURITY
-        # correctly from EMBARGOED archive this assertion can be removed.
-        # XXX Julian 2007-12-18 spec=security-in-soyuz: This is being
-        # addressed in the work on the blueprint:
-        # https://blueprints.launchpad.net/soyuz/+spec/security-in-soyuz
-        target_pocket = build.pocket
-        assert target_pocket != PackagePublishingPocket.SECURITY, (
-            "Soyuz is not yet capable of building SECURITY uploads.")
-
-        # Ensure build has the needed chroot
-        build = getUtility(IBuildSet).getByQueueEntry(build_queue_item)
-        chroot = build.distroarchseries.getChroot()
-        if chroot is None:
-            raise CannotBuild(
-                "Missing CHROOT for %s/%s/%s" % (
-                    build.distroseries.distribution.name,
-                    build.distroseries.name,
-                    build.distroarchseries.architecturetag)
-                )
-
-        # The main distribution has policies to prevent uploads to some
-        # pockets (e.g. security) during different parts of the distribution
-        # series lifecycle. These do not apply to PPA builds nor any archive
-        # that allows release pocket updates.
-        if (build.archive.purpose != ArchivePurpose.PPA and
-            not build.archive.allowUpdatesToReleasePocket()):
-            # XXX Robert Collins 2007-05-26: not an explicit CannotBuild
-            # exception yet because the callers have not been audited
-            assert build.distroseries.canUploadToPocket(build.pocket), (
-                "%s (%s) can not be built for pocket %s: invalid pocket due "
-                "to the series status of %s."
-                % (build.title, build.id, build.pocket.name,
-                   build.distroseries.name))
-
-    def _dispatchBuildToSlave(self, build_queue_item, args, buildid, logger):
-        """Start the build on the slave builder."""
-        # Send chroot.
-        build = getUtility(IBuildSet).getByQueueEntry(build_queue_item)
-        chroot = build.distroarchseries.getChroot()
-        self.cacheFileOnSlave(logger, chroot)
-
-        # Build filemap structure with the files required in this build
-        # and send them to the slave.
-        # If the build is private we tell the slave to get the files from the
-        # archive instead of the librarian because the slaves cannot
-        # access the restricted librarian.
-        private = build.archive.private
-        if private:
-            self.cachePrivateSourceOnSlave(logger, build_queue_item)
-        filemap = {}
-        for source_file in build.sourcepackagerelease.files:
-            lfa = source_file.libraryfile
-            filemap[lfa.filename] = lfa.content.sha1
-            if not private:
-                self.cacheFileOnSlave(logger, source_file.libraryfile)
-
-        chroot_sha1 = chroot.content.sha1
-        try:
-            status, info = self.slave.build(
-                buildid, "debian", chroot_sha1, filemap, args)
-            message = """%s (%s):
-            ***** RESULT *****
-            %s
-            %s
-            %s: %s
-            ******************
-            """ % (self.name, self.url, filemap, args, status, info)
-            logger.info(message)
-        except xmlrpclib.Fault, info:
-            # Mark builder as 'failed'.
-            logger.debug("Disabling builder: %s" % self.url, exc_info=1)
-            self.failbuilder(
-                "Exception (%s) when setting up to new job" % info)
-            raise BuildSlaveFailure
-        except socket.error, info:
-            error_message = "Exception (%s) when setting up new job" % info
-            self.handleTimeout(logger, error_message)
-            raise BuildSlaveFailure
-
     def startBuild(self, build_queue_item, logger):
         """See IBuilder."""
         # Set the build behavior depending on the provided build queue item.
@@ -408,55 +290,15 @@
         self.logStartBuild(build_queue_item, logger)
 
         # Make sure the request is valid; an exception is raised if it's not.
-        self._verifyBuildRequest(build_queue_item, logger)
+        self.verifyBuildRequest(build_queue_item, logger)
 
         # If we are building a virtual build, resume the virtual machine.
         if self.virtualized:
             self.resumeSlaveHost()
 
-        # Build extra arguments.
-        args = {}
-        # turn 'arch_indep' ON only if build is archindep or if
-        # the specific architecture is the nominatedarchindep for
-        # this distroseries (in case it requires any archindep source)
-        build = getUtility(IBuildSet).getByQueueEntry(build_queue_item)
-        args['arch_indep'] = build.distroarchseries.isNominatedArchIndep
-
-        suite = build.distroarchseries.distroseries.name
-        if build.pocket != PackagePublishingPocket.RELEASE:
-            suite += "-%s" % (build.pocket.name.lower())
-        args['suite'] = suite
-
-        archive_purpose = build.archive.purpose
-        if (archive_purpose == ArchivePurpose.PPA and
-            not build.archive.require_virtualized):
-            # If we're building a non-virtual PPA, override the purpose
-            # to PRIMARY and use the primary component override.
-            # This ensures that the package mangling tools will run over
-            # the built packages.
-            args['archive_purpose'] = ArchivePurpose.PRIMARY.name
-            args["ogrecomponent"] = (
-                get_primary_current_component(build))
-        else:
-            args['archive_purpose'] = archive_purpose.name
-            args["ogrecomponent"] = (
-                build.current_component.name)
-
-        args['archives'] = get_sources_list_for_building(build)
-
-        # Let the build slave know whether this is a build in a private
-        # archive.
-        args['archive_private'] = build.archive.private
-
-        # Generate a string which can be used to cross-check when obtaining
-        # results so we know we are referring to the right database object in
-        # subsequent runs.
-        buildid = "%s-%s" % (build.id, build_queue_item.id)
-        logger.debug("Initiating build %s on %s" % (buildid, self.url))
-
         # Do it.
         build_queue_item.markAsBuilding(self)
-        self._dispatchBuildToSlave(build_queue_item, args, buildid, logger)
+        self.dispatchBuildToSlave(build_queue_item, logger)
 
     # XXX cprov 2009-06-24: This code does not belong to the content
     # class domain. Here we cannot make sensible decisions about what
@@ -469,19 +311,10 @@
             if self.failnotes is not None:
                 return self.failnotes
             return 'Disabled'
-        # Cache the 'currentjob', so we don't have to hit the database
-        # more than once.
-        currentjob = self.currentjob
-        if currentjob is None:
-            return 'Idle'
 
-        build = getUtility(IBuildSet).getByQueueEntry(currentjob)
-        msg = 'Building %s' % build.title
-        archive = build.archive
-        if not archive.owner.private and (archive.is_ppa or archive.is_copy):
-            return '%s [%s/%s]' % (msg, archive.owner.name, archive.name)
-        else:
-            return msg
+        # If the builder is OK then we delegate the status
+        # to our current behavior.
+        return self.current_build_behavior.status
 
     def failbuilder(self, reason):
         """See IBuilder"""
@@ -722,7 +555,7 @@
         logger = self._getSlaveScannerLogger()
         try:
             self.startBuild(candidate, logger)
-        except (BuildSlaveFailure, CannotBuild), err:
+        except (BuildSlaveFailure, CannotBuild, BuildBehaviorMismatch), err:
             logger.warn('Could not build: %s' % err)
 
     def handleTimeout(self, logger, error_message):