launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/build-score-threshold-feature into lp:launchpad.

Commit message:
Add a feature flag to prevent dispatching any build under a given minimum score.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/build-score-threshold-feature/+merge/335861

Because this seems likely to be way better than having to maintain whitelists by hand via ops.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/build-score-threshold-feature into lp:launchpad.

=== modified file 'lib/lp/buildmaster/model/builder.py'
--- lib/lp/buildmaster/model/builder.py	2017-06-29 12:02:11 +0000
+++ lib/lp/buildmaster/model/builder.py	2018-01-08 21:15:45 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009,2011 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -73,6 +73,7 @@
     )
 from lp.services.database.sqlbase import SQLBase
 from lp.services.database.stormbase import StormBase
+from lp.services.features import getFeatureFlag
 from lp.services.propertycache import (
     cachedproperty,
     get_property_cache,
@@ -238,6 +239,8 @@
 
         :return: A candidate job.
         """
+        logger = self._getSlaveScannerLogger()
+
         job_type_conditions = []
         job_sources = specific_build_farm_job_sources()
         for job_type, job_source in job_sources.iteritems():
@@ -249,6 +252,17 @@
                         BuildFarmJob.job_type != job_type,
                         Exists(SQL(query))))
 
+        score_conditions = []
+        minimum_score_str = getFeatureFlag('buildmaster.minimum_score')
+        if minimum_score_str is not None:
+            try:
+                minimum_score = int(minimum_score_str)
+            except ValueError:
+                logger.error(
+                    'invalid buildmaster.minimum_score %r', minimum_score_str)
+            else:
+                score_conditions.append(BuildQueue.lastscore >= minimum_score)
+
         store = IStore(self.__class__)
         candidate_jobs = store.using(BuildQueue, BuildFarmJob).find(
             (BuildQueue.id,),
@@ -261,10 +275,9 @@
                 BuildQueue.processor == None),
             BuildQueue.virtualized == self.virtualized,
             BuildQueue.builder == None,
-            And(*job_type_conditions)
+            And(*(job_type_conditions + score_conditions))
             ).order_by(Desc(BuildQueue.lastscore), BuildQueue.id)
 
-        logger = self._getSlaveScannerLogger()
         # Only try the first handful of jobs. It's much easier on the
         # database, the chance of a large prefix of the queue being
         # bad candidates is negligible, and we want reasonably bounded

=== modified file 'lib/lp/buildmaster/tests/test_builder.py'
--- lib/lp/buildmaster/tests/test_builder.py	2018-01-02 10:54:31 +0000
+++ lib/lp/buildmaster/tests/test_builder.py	2018-01-08 21:15:45 +0000
@@ -1,8 +1,9 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test Builder features."""
 
+from fixtures import FakeLogger
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
@@ -20,6 +21,7 @@
 from lp.buildmaster.tests.mock_slaves import make_publisher
 from lp.services.database.interfaces import IStore
 from lp.services.database.sqlbase import flush_database_updates
+from lp.services.features.testing import FeatureFixture
 from lp.soyuz.enums import (
     ArchivePurpose,
     PackagePublishingStatus,
@@ -182,6 +184,45 @@
         # And the old_candidate is superseded:
         self.assertEqual(BuildStatus.SUPERSEDED, build.status)
 
+    def test_findBuildCandidate_honours_minimum_score(self):
+        # Sometimes there's an emergency that requires us to lock down the
+        # build farm except for certain whitelisted builds.  We do this by
+        # way of a feature flag to set a minimum score; if this is set,
+        # Builder._findBuildCandidate will ignore any build with a lower
+        # score.
+        bq1 = self.factory.makeBinaryPackageBuild().queueBuild()
+        bq1.manualScore(100000)
+        bq2 = self.factory.makeBinaryPackageBuild().queueBuild()
+        bq2.manualScore(99999)
+        builder1 = removeSecurityProxy(
+            self.factory.makeBuilder(
+                processors=[bq1.processor], virtualized=True))
+        builder2 = removeSecurityProxy(
+            self.factory.makeBuilder(
+                processors=[bq2.processor], virtualized=True))
+
+        # By default, each builder has the appropriate one of the two builds
+        # we just created as a candidate.
+        self.assertEqual(bq1, builder1._findBuildCandidate())
+        self.assertEqual(bq2, builder2._findBuildCandidate())
+
+        # If we set a minimum score, then only builds above that threshold
+        # are candidates.
+        with FeatureFixture({'buildmaster.minimum_score': '100000'}):
+            self.assertEqual(bq1, builder1._findBuildCandidate())
+            self.assertIsNone(builder2._findBuildCandidate())
+
+        # If we set an invalid minimum score, buildd-manager doesn't
+        # explode.
+        with FakeLogger() as logger:
+            with FeatureFixture({'buildmaster.minimum_score': 'nonsense'}):
+                self.assertEqual(bq1, builder1._findBuildCandidate())
+                self.assertEqual(bq2, builder2._findBuildCandidate())
+            self.assertEqual(
+                "invalid buildmaster.minimum_score u'nonsense'\n"
+                "invalid buildmaster.minimum_score u'nonsense'\n",
+                logger.output)
+
     def test_acquireBuildCandidate_marks_building(self):
         # acquireBuildCandidate() should call _findBuildCandidate and
         # mark the build as building.