Merge lp:~julian-edwards/launchpad/config-ppa-for-slaves into lp:launchpad

[Julian Edwards]

= Summary =
Send a configured sources.list entry to the build slaves for recipe builds.

== Proposed fix ==
This branch adds a new config item builddmaster.bzr_builder_sources_list which
can optionally contain a sources.list entry that is set up on the build slaves
before starting recipe builds.

This will enable us to set up a PPA with a bzr-builder package in it that the
recipe builds can use.

We're doing this because the rate of change of development on bzr-builder is
too fast to make changes in Ubuntu with its SRU policy.

We'll set up two PPAs, one containing development package for testing which is
owned by the Launchpad team, and one for production owned by IS/buildd-admins.
The buildd admins will copy a package from the development PPA once it's
proven tested on dogfood.

This is why there's a config item: the PPA to use needs to be configured
differently on dogfood, development and production.

== Tests ==

bin/test -cvv test_recipebuilder

== Demo and Q/A ==

The Code team is testing this approach on dogfood right now with a hacked
change. We'll drop in this replacement branch and things should continue to
work :)

[Michael Nelson (michael.nelson)]

Great Julian!

As per IRC, you'll add a new test for the case where the config option is not set, and try s/AttributeError/KeyError for the substitution exception.

Thanks!

{{{
16:31 < noodles775> bigjools: given that you're change will also require updating lp-production-configs, should you be wrapping the actual config.buildmaster.bzr_builder_sources_list in a try/except?
16:31 < noodles775> wgrant: yep, trying now.
16:32 < wgrant> noodles775: Thanks.
16:32 < noodles775> bigjools: also, why not catch KeyError rather than StandardError when substituting the series?
16:33 < bigjools> noodles775: it returns None if it doesn't exist
16:33 < bigjools> I think!
16:33 < noodles775> bigjools: I just tried locally - AttributeError: No section key named bzr_builder_sources_list.
16:33 < bigjools> re. second point, I am shamefully copy & wasting the code in adapters/archivedependencies.py that does a similar thing
16:34 < bigjools> noodles775: crap, I'll fix that, thanks for noticing
16:34 < noodles775> np. Great.
16:38 < noodles775> bigjools: you could rework the second test (or add a separate test) so it shows the failure when the config option is not set?
16:39 < bigjools> noodles775: yep, I can add another test
}}}

review: Approve (code)

[Julian Edwards (julian-edwards)]

Thanks Michael!

Here's the partial diff for completeness. As we discussed, the KeyError
doesn't really catch everything, so I left the StandardError in.

=== modified file 'configs/development/launchpad-lazr.conf'
--- configs/development/launchpad-lazr.conf	2010-04-19 03:44:27 +0000
+++ configs/development/launchpad-lazr.conf	2010-04-29 14:57:34 +0000
@@ -16,6 +16,7 @@
 [builddmaster]
 root: /var/tmp/builddmaster/
 uploader: scripts/process-upload.py -Mvv
+bzr_builder_sources_list: None
 
 [buildsequencer]
 mailproblemsto: root

=== modified file 'lib/canonical/config/schema-lazr.conf'
--- lib/canonical/config/schema-lazr.conf	2010-04-19 03:44:27 +0000
+++ lib/canonical/config/schema-lazr.conf	2010-04-29 14:57:34 +0000
@@ -110,6 +110,12 @@
 # datatype: string
 root: none
 
+# Optional sources.list entry to send to build slaves when doing recipe
+# builds involving bzr-builder.  Use this form so that the series is set:
+# deb http://foo %(series)s main
+# datatype: string
+bzr_builder_sources_list: none
+
 
 [buildsequencer]
 # The name of the log file for the sequencer (passed to twistd)

=== modified file 'lib/lp/code/model/recipebuilder.py'
--- lib/lp/code/model/recipebuilder.py	2010-04-20 01:36:29 +0000
+++ lib/lp/code/model/recipebuilder.py	2010-04-29 14:57:34 +0000
@@ -8,9 +8,13 @@
     'RecipeBuildBehavior',
     ]
 
+import traceback
+
 from zope.component import adapts
 from zope.interface import implements
 
+from canonical.config import config
+
 from lp.archiveuploader.permission import check_upload_to_pocket
 from lp.buildmaster.interfaces.buildfarmjobbehavior import (
     IBuildFarmJobBehavior)
@@ -50,7 +54,7 @@
         """See `IBuildFarmJobBehavior`."""
         logger.info("startBuild(%s)", self.display_name)
 
-    def _extraBuildArgs(self, distroarchseries):
+    def _extraBuildArgs(self, distroarchseries, logger=None):
         """
         Return the extra arguments required by the slave for the given build.
         """
@@ -70,6 +74,27 @@
             self.build.sourcepackagename.name)
         args['archives'] = get_sources_list_for_building(self.build,
             distroarchseries, self.build.sourcepackagename.name)
+
+        # config.builddmaster.bzr_builder_sources_list can contain a
+        # sources.list entry for an archive that will contain a
+        # bzr-builder package that needs to be used to build this
+        # recipe.
+        try:
+            extra_archive = config.builddmaster.bzr_builder_sources_list
+        except AttributeError:
+            extra_archive = None
+
+        if extra_archive is not None:
+            try:
+                sources_line = extra_archive % (
+                    {'series': self.build.distroseries.name})
+                args['archives'].append(sources_line)
+            except StandardError:
+                # Someone messed up the config, don't add it.
+                logger.error(
+                    "Exception processing bzr_builder_sources_list:\n%s"
+                    % traceback.format_exc())
+
         args['distroseries_name'] = self.build.distroseries.name
         return args
 
@@ -101,7 +126,7 @@
         logger.debug(
             "Initiating build %s on %s" % (buildid, self._builder.url))
 
-        args = self._extraBuildArgs(distroarchseries)
+        args = self._extraBuildArgs(distroarchseries, logger)
         status, info = self._builder.slave.build(
             cookie, "sourcepackagerecipe", chroot_sha1, {}, args)
         message = """%s (%s):

=== modified file 'lib/lp/code/tests/test_recipebuilder.py'
--- lib/lp/code/tests/test_recipebuilder.py	2010-04-20 23:22:24 +0000
+++ lib/lp/code/tests/test_recipebuilder.py	2010-04-29 14:57:34 +0000
@@ -12,6 +12,7 @@
 
 from zope.security.proxy import removeSecurityProxy
 
+from canonical.config import config
 from canonical.testing import LaunchpadFunctionalLayer
 from canonical.launchpad.scripts.logger import BufferLogger
 
@@ -100,8 +101,19 @@
 
     def test__extraBuildArgs(self):
         # _extraBuildArgs will return a sane set of additional arguments
+        bzr_builder_config = """
+            [builddmaster]
+            bzr_builder_sources_list = deb http://foo %(series)s main
+            """
+        config.push("bzr_builder_config", bzr_builder_config)
+        self.addCleanup(config.pop, "bzr_builder_config")
+
         job = self.makeJob()
         distroarchseries = job.build.distroseries.architectures[0]
+        expected_archives = get_sources_list_for_building(
+            job.build, distroarchseries, job.build.sourcepackagename.name)
+        expected_archives.append(
+            "deb http://foo %s main" % job.build.distroseries.name)
         self.assertEqual({
            'author_email': u'[e-mail]',
            'suite': u'mydistro',
@@ -111,11 +123,54 @@
            'ogrecomponent': 'universe',
            'recipe_text': '# bzr-builder format 0.2 deb-version 1.0\n'
                           'lp://dev/~joe/someapp/pkg\n',
-           'archives': get_sources_list_for_building(job.build,
-                distroarchseries, job.build.sourcepackagename.name),
+           'archives': expected_archives,
            'distroseries_name': job.build.distroseries.name,
             }, job._extraBuildArgs(distroarchseries))
 
+    def test_extraBuildArgs_withBadConfigForBzrBuilderPPA(self):
+        # Ensure _extraBuildArgs doesn't blow up with a badly formatted
+        # bzr_builder_sources_list in the config.
+        bzr_builder_config = """
+            [builddmaster]
+            bzr_builder_sources_list = deb http://foo %(series) main
+            """
+        # (note the missing 's' in %(series)
+
+        config.push("bzr_builder_config", bzr_builder_config)
+        self.addCleanup(config.pop, "bzr_builder_config")
+
+        job = self.makeJob()
+        distroarchseries = job.build.distroseries.architectures[0]
+        expected_archives = get_sources_list_for_building(
+            job.build, distroarchseries, job.build.sourcepackagename.name)
+        logger = BufferLogger()
+        self.assertEqual({
+           'author_email': u'[e-mail]',
+           'suite': u'mydistro',
+           'author_name': u'Joe User',
+           'package_name': u'apackage',
+           'archive_purpose': 'PPA',
+           'ogrecomponent': 'universe',
+           'recipe_text': '# bzr-builder format 0.2 deb-version 1.0\n'
+                          'lp://dev/~joe/someapp/pkg\n',
+           'archives': expected_archives,
+           'distroseries_name': job.build.distroseries.name,
+            }, job._extraBuildArgs(distroarchseries, logger))
+        self.assertIn(
+            "Exception processing bzr_builder_sources_list:",
+            logger.buffer.getvalue())
+
+    def test_extraBuildArgs_withNoBZrBuilderConfigSet(self):
+        # Ensure _extraBuildArgs doesn't blow up when
+        # bzr_builder_sources_list isn't set.
+        job = self.makeJob()
+        distroarchseries = job.build.distroseries.architectures[0]
+        args = job._extraBuildArgs(distroarchseries)
+        expected_archives = get_sources_list_for_building(
+            job.build, distroarchseries, job.build.sourcepackagename.name)
+        self.assertEqual(args["archives"], expected_archives)
+
+
     def test_dispatchBuildToSlave(self):
         # Ensure dispatchBuildToSlave will make the right calls to the slave
         job = self.makeJob()