launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #27953
[Merge] ~cjwatson/launchpad:refactor-updateBuild into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:refactor-updateBuild into launchpad:master with ~cjwatson/launchpad:rename-builder-interactor-slave as a prerequisite.
Commit message:
Push more of BuilderInteractor.updateBuild down to behaviours
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/414070
For upcoming work on CI jobs, we want to be able to do some build-type-specific work on non-terminal build states, namely incrementally fetching output files from builders. This requires `updateBuild` to call something build-type-specific in the `BUILDING` state. Since there's a small amount of code in common between `BUILDING`/`ABORTING` and `WAITING` already (the `updateStatus` call and the following commit), push this all down to `BuildFarmJobBehaviour.handleStatus`, which now handles everything except fetching the logtail.
No functional change is intended from this refactoring, although a number of tests need to change because `handleStatus` now requires a more complete version of launchpad-buildd's `status` response; previously some tests could get away with omitting some fields.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:refactor-updateBuild into launchpad:master.
diff --git a/lib/lp/buildmaster/interactor.py b/lib/lp/buildmaster/interactor.py
index e8a1d7f..679d5c6 100644
--- a/lib/lp/buildmaster/interactor.py
+++ b/lib/lp/buildmaster/interactor.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2022 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__all__ = [
@@ -526,19 +526,6 @@ class BuilderInteractor:
return candidate
@staticmethod
- def extractBuildStatus(worker_status):
- """Read build status name.
-
- :param worker_status: build status dict from BuilderWorker.status.
- :return: the unqualified status name, e.g. "OK".
- """
- status_string = worker_status['build_status']
- lead_string = 'BuildStatus.'
- assert status_string.startswith(lead_string), (
- "Malformed status string: '%s'" % status_string)
- return status_string[len(lead_string):]
-
- @staticmethod
def extractLogTail(worker_status):
"""Extract the log tail from a builder status response.
@@ -580,24 +567,20 @@ class BuilderInteractor:
# matches the DB, and this method isn't called unless the DB
# says there's a job.
builder_status = worker_status['builder_status']
+ if builder_status not in (
+ 'BuilderStatus.BUILDING', 'BuilderStatus.ABORTING',
+ 'BuilderStatus.WAITING'):
+ raise AssertionError("Unknown status %s" % builder_status)
+ builder = builder_factory[vitals.name]
+ behaviour = behaviour_factory(vitals.build_queue, builder, worker)
if builder_status in (
'BuilderStatus.BUILDING', 'BuilderStatus.ABORTING'):
logtail = cls.extractLogTail(worker_status)
if logtail is not None:
manager.addLogTail(vitals.build_queue.id, logtail)
- vitals.build_queue.specific_build.updateStatus(
- vitals.build_queue.specific_build.status,
- worker_status=worker_status)
- transaction.commit()
- elif builder_status == 'BuilderStatus.WAITING':
- # Build has finished. Delegate handling to the build itself.
- builder = builder_factory[vitals.name]
- behaviour = behaviour_factory(vitals.build_queue, builder, worker)
- yield behaviour.handleStatus(
- vitals.build_queue, cls.extractBuildStatus(worker_status),
- worker_status)
- else:
- raise AssertionError("Unknown status %s" % builder_status)
+ # Delegate the remaining handling to the build behaviour, which will
+ # commit the transaction.
+ yield behaviour.handleStatus(vitals.build_queue, worker_status)
@staticmethod
def _getWorkerScannerLogger():
diff --git a/lib/lp/buildmaster/interfaces/buildfarmjobbehaviour.py b/lib/lp/buildmaster/interfaces/buildfarmjobbehaviour.py
index 2d0eed4..c529327 100644
--- a/lib/lp/buildmaster/interfaces/buildfarmjobbehaviour.py
+++ b/lib/lp/buildmaster/interfaces/buildfarmjobbehaviour.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2022 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Interface for build farm job behaviours."""
@@ -88,10 +88,9 @@ class IBuildFarmJobBehaviour(Interface):
def verifySuccessfulBuild():
"""Check that we are allowed to collect this successful build."""
- def handleStatus(bq, status, worker_status):
- """Update the build from a WAITING worker result.
+ def handleStatus(bq, worker_status):
+ """Update the build from a worker's status response.
:param bq: The `BuildQueue` currently being processed.
- :param status: The tail of the BuildStatus (eg. OK or PACKAGEFAIL).
:param worker_status: Worker status dict from `BuilderWorker.status`.
"""
diff --git a/lib/lp/buildmaster/model/buildfarmjobbehaviour.py b/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
index 0315b52..a7537d3 100644
--- a/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
+++ b/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2022 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Base and idle BuildFarmJobBehaviour classes."""
@@ -33,6 +33,7 @@ from lp.services.config import config
from lp.services.helpers import filenameToContentType
from lp.services.librarian.interfaces import ILibraryFileAliasSet
from lp.services.librarian.utils import copy_and_close
+from lp.services.propertycache import cachedproperty
from lp.services.statsd.interfaces.statsd_client import IStatsdClient
from lp.services.utils import sanitise_urls
from lp.services.webapp import canonical_url
@@ -53,7 +54,10 @@ class BuildFarmJobBehaviourBase:
"""Store a reference to the job_type with which we were created."""
self.build = build
self._builder = None
- self._authserver = xmlrpc.Proxy(
+
+ @cachedproperty
+ def _authserver(self):
+ return xmlrpc.Proxy(
config.builddmaster.authentication_endpoint.encode('UTF-8'),
connectTimeout=config.builddmaster.authentication_timeout)
@@ -255,6 +259,19 @@ class BuildFarmJobBehaviourBase:
"%s (%s) can not be built for pocket %s in %s: illegal status"
% (build.title, build.id, build.pocket.name, build.archive))
+ @staticmethod
+ def extractBuildStatus(worker_status):
+ """Read build status name.
+
+ :param worker_status: build status dict from BuilderWorker.status.
+ :return: the unqualified status name, e.g. "OK".
+ """
+ status_string = worker_status['build_status']
+ lead_string = 'BuildStatus.'
+ assert status_string.startswith(lead_string), (
+ "Malformed status string: '%s'" % status_string)
+ return status_string[len(lead_string):]
+
# The list of build status values for which email notifications are
# allowed to be sent. It is up to each callback as to whether it will
# consider sending a notification but it won't do so if the status is not
@@ -262,57 +279,65 @@ class BuildFarmJobBehaviourBase:
ALLOWED_STATUS_NOTIFICATIONS = ['PACKAGEFAIL', 'CHROOTFAIL']
@defer.inlineCallbacks
- def handleStatus(self, bq, status, worker_status):
+ def handleStatus(self, bq, worker_status):
"""See `IBuildFarmJobBehaviour`."""
if bq != self.build.buildqueue_record:
raise AssertionError(
"%r != %r" % (bq, self.build.buildqueue_record))
from lp.buildmaster.manager import BUILDD_MANAGER_LOG_NAME
logger = logging.getLogger(BUILDD_MANAGER_LOG_NAME)
- notify = status in self.ALLOWED_STATUS_NOTIFICATIONS
- fail_status_map = {
- 'PACKAGEFAIL': BuildStatus.FAILEDTOBUILD,
- 'DEPFAIL': BuildStatus.MANUALDEPWAIT,
- 'CHROOTFAIL': BuildStatus.CHROOTWAIT,
- }
- if self.build.status == BuildStatus.CANCELLING:
- fail_status_map['ABORTED'] = BuildStatus.CANCELLED
-
- logger.info(
- 'Processing finished job %s (%s) from builder %s: %s'
- % (self.build.build_cookie, self.build.title,
- self.build.buildqueue_record.builder.name, status))
- build_status = None
- if status == 'OK':
- yield self.storeLogFromWorker()
- # handleSuccess will sometimes perform write operations
- # outside the database transaction, so a failure between
- # here and the commit can cause duplicated results. For
- # example, a BinaryPackageBuild will end up in the upload
- # queue twice if notify() crashes.
- build_status = yield self.handleSuccess(worker_status, logger)
- elif status in fail_status_map:
- # XXX wgrant: The builder should be set long before here, but
- # currently isn't.
- yield self.storeLogFromWorker()
- build_status = fail_status_map[status]
+ builder_status = worker_status["builder_status"]
+
+ if builder_status == "BuilderStatus.WAITING":
+ # Build has finished.
+ status = self.extractBuildStatus(worker_status)
+ notify = status in self.ALLOWED_STATUS_NOTIFICATIONS
+ fail_status_map = {
+ 'PACKAGEFAIL': BuildStatus.FAILEDTOBUILD,
+ 'DEPFAIL': BuildStatus.MANUALDEPWAIT,
+ 'CHROOTFAIL': BuildStatus.CHROOTWAIT,
+ }
+ if self.build.status == BuildStatus.CANCELLING:
+ fail_status_map['ABORTED'] = BuildStatus.CANCELLED
+
+ logger.info(
+ 'Processing finished job %s (%s) from builder %s: %s'
+ % (self.build.build_cookie, self.build.title,
+ self.build.buildqueue_record.builder.name, status))
+ build_status = None
+ if status == 'OK':
+ yield self.storeLogFromWorker()
+ # handleSuccess will sometimes perform write operations
+ # outside the database transaction, so a failure between
+ # here and the commit can cause duplicated results. For
+ # example, a BinaryPackageBuild will end up in the upload
+ # queue twice if notify() crashes.
+ build_status = yield self.handleSuccess(worker_status, logger)
+ elif status in fail_status_map:
+ yield self.storeLogFromWorker()
+ build_status = fail_status_map[status]
+ else:
+ raise BuildDaemonError(
+ "Build returned unexpected status: %r" % status)
else:
- raise BuildDaemonError(
- "Build returned unexpected status: %r" % status)
+ # The build status remains unchanged.
+ build_status = bq.specific_build.status
- # Set the status and dequeue the build atomically. Setting the
- # status to UPLOADING constitutes handoff to process-upload, so
- # doing that before we've removed the BuildQueue causes races.
+ # Set the status and (if the build has finished) dequeue the build
+ # atomically. Setting the status to UPLOADING constitutes handoff to
+ # process-upload, so doing that before we've removed the BuildQueue
+ # causes races.
# XXX wgrant: The builder should be set long before here, but
# currently isn't.
self.build.updateStatus(
- build_status,
- builder=self.build.buildqueue_record.builder,
- worker_status=worker_status)
- if notify:
- self.build.notify()
- self.build.buildqueue_record.destroySelf()
+ build_status, builder=bq.builder, worker_status=worker_status)
+
+ if builder_status == "BuilderStatus.WAITING":
+ if notify:
+ self.build.notify()
+ self.build.buildqueue_record.destroySelf()
+
transaction.commit()
@defer.inlineCallbacks
diff --git a/lib/lp/buildmaster/tests/test_buildfarmjobbehaviour.py b/lib/lp/buildmaster/tests/test_buildfarmjobbehaviour.py
index ad05908..f36750e 100644
--- a/lib/lp/buildmaster/tests/test_buildfarmjobbehaviour.py
+++ b/lib/lp/buildmaster/tests/test_buildfarmjobbehaviour.py
@@ -1,4 +1,4 @@
-# Copyright 2010-2020 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2022 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Unit tests for BuildFarmJobBehaviourBase."""
@@ -30,6 +30,7 @@ from lp.buildmaster.interfaces.builder import BuildDaemonError
from lp.buildmaster.interfaces.buildfarmjobbehaviour import (
IBuildFarmJobBehaviour,
)
+from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
from lp.buildmaster.interfaces.processor import IProcessorSet
from lp.buildmaster.model.buildfarmjobbehaviour import (
BuildFarmJobBehaviourBase,
@@ -154,6 +155,22 @@ class TestBuildFarmJobBehaviourBase(TestCaseWithFactory):
behaviour.setBuilder(self.factory.makeBuilder(virtualized=False), None)
self.assertIs(False, behaviour.extraBuildArgs()["fast_cleanup"])
+ def test_extractBuildStatus_baseline(self):
+ # extractBuildStatus picks the name of the build status out of a
+ # dict describing the worker's status.
+ worker_status = {"build_status": "BuildStatus.BUILDING"}
+ self.assertEqual(
+ "BUILDING",
+ BuildFarmJobBehaviourBase.extractBuildStatus(worker_status))
+
+ def test_extractBuildStatus_malformed(self):
+ # extractBuildStatus errors out when the status string is not
+ # of the form it expects.
+ worker_status = {"build_status": "BUILDING"}
+ self.assertRaises(
+ AssertionError, BuildFarmJobBehaviourBase.extractBuildStatus,
+ worker_status)
+
class TestDispatchBuildToWorker(StatsMixin, TestCase):
@@ -383,19 +400,44 @@ class TestHandleStatusMixin:
1, len(os.listdir(os.path.join(self.upload_root, result))))
@defer.inlineCallbacks
- def test_handleStatus_OK_normal_file(self):
+ def test_handleStatus_BUILDING(self):
+ # If the builder is BUILDING (or any status other than WAITING),
+ # then the behaviour calls updateStatus but doesn't do anything
+ # else.
+ initial_status = self.build.status
+ bq_id = self.build.buildqueue_record.id
+ worker_status = {"builder_status": "BuilderStatus.BUILDING"}
+ removeSecurityProxy(self.build).updateStatus = FakeMethod()
+ with dbuser(config.builddmaster.dbuser):
+ yield self.behaviour.handleStatus(
+ self.build.buildqueue_record, worker_status)
+ self.assertEqual(None, self.build.log)
+ self.assertEqual(0, len(os.listdir(self.upload_root)))
+ self.assertEqual(
+ [((initial_status,),
+ {"builder": self.builder, "worker_status": worker_status})],
+ removeSecurityProxy(self.build).updateStatus.calls)
+ self.assertEqual(0, len(pop_notifications()), "Notifications received")
+ self.assertEqual(
+ self.build.buildqueue_record,
+ getUtility(IBuildQueueSet).get(bq_id))
+
+ @defer.inlineCallbacks
+ def test_handleStatus_WAITING_OK_normal_file(self):
# A filemap with plain filenames should not cause a problem.
# The call to handleStatus will attempt to get the file from
# the worker resulting in a URL error in this test case.
with dbuser(config.builddmaster.dbuser):
yield self.behaviour.handleStatus(
- self.build.buildqueue_record, 'OK',
- {'filemap': {'myfile.py': 'test_file_hash'}})
+ self.build.buildqueue_record,
+ {'builder_status': 'BuilderStatus.WAITING',
+ 'build_status': 'BuildStatus.OK',
+ 'filemap': {'myfile.py': 'test_file_hash'}})
self.assertEqual(BuildStatus.UPLOADING, self.build.status)
self.assertResultCount(1, "incoming")
@defer.inlineCallbacks
- def test_handleStatus_OK_absolute_filepath(self):
+ def test_handleStatus_WAITING_OK_absolute_filepath(self):
# A filemap that tries to write to files outside of the upload
# directory will not be collected.
with ExpectedException(
@@ -403,11 +445,13 @@ class TestHandleStatusMixin:
"Build returned a file named '/tmp/myfile.py'."):
with dbuser(config.builddmaster.dbuser):
yield self.behaviour.handleStatus(
- self.build.buildqueue_record, 'OK',
- {'filemap': {'/tmp/myfile.py': 'test_file_hash'}})
+ self.build.buildqueue_record,
+ {'builder_status': 'BuilderStatus.WAITING',
+ 'build_status': 'BuildStatus.OK',
+ 'filemap': {'/tmp/myfile.py': 'test_file_hash'}})
@defer.inlineCallbacks
- def test_handleStatus_OK_relative_filepath(self):
+ def test_handleStatus_WAITING_OK_relative_filepath(self):
# A filemap that tries to write to files outside of
# the upload directory will not be collected.
with ExpectedException(
@@ -415,21 +459,25 @@ class TestHandleStatusMixin:
"Build returned a file named '../myfile.py'."):
with dbuser(config.builddmaster.dbuser):
yield self.behaviour.handleStatus(
- self.build.buildqueue_record, 'OK',
- {'filemap': {'../myfile.py': 'test_file_hash'}})
+ self.build.buildqueue_record,
+ {'builder_status': 'BuilderStatus.WAITING',
+ 'build_status': 'BuildStatus.OK',
+ 'filemap': {'../myfile.py': 'test_file_hash'}})
@defer.inlineCallbacks
- def test_handleStatus_OK_sets_build_log(self):
+ def test_handleStatus_WAITING_OK_sets_build_log(self):
# The build log is set during handleStatus.
self.assertEqual(None, self.build.log)
with dbuser(config.builddmaster.dbuser):
yield self.behaviour.handleStatus(
- self.build.buildqueue_record, 'OK',
- {'filemap': {'myfile.py': 'test_file_hash'}})
+ self.build.buildqueue_record,
+ {'builder_status': 'BuilderStatus.WAITING',
+ 'build_status': 'BuildStatus.OK',
+ 'filemap': {'myfile.py': 'test_file_hash'}})
self.assertNotEqual(None, self.build.log)
@defer.inlineCallbacks
- def _test_handleStatus_notifies(self, status):
+ def _test_handleStatus_WAITING_notifies(self, status):
# An email notification is sent for a given build status if
# notifications are allowed for that status.
expected_notification = (
@@ -437,7 +485,9 @@ class TestHandleStatusMixin:
with dbuser(config.builddmaster.dbuser):
yield self.behaviour.handleStatus(
- self.build.buildqueue_record, status, {})
+ self.build.buildqueue_record,
+ {'builder_status': 'BuilderStatus.WAITING',
+ 'build_status': 'BuildStatus.%s' % status})
if expected_notification:
self.assertNotEqual(
@@ -446,26 +496,28 @@ class TestHandleStatusMixin:
self.assertEqual(
0, len(pop_notifications()), "Notifications received")
- def test_handleStatus_DEPFAIL_notifies(self):
- return self._test_handleStatus_notifies("DEPFAIL")
+ def test_handleStatus_WAITING_DEPFAIL_notifies(self):
+ return self._test_handleStatus_WAITING_notifies("DEPFAIL")
- def test_handleStatus_CHROOTFAIL_notifies(self):
- return self._test_handleStatus_notifies("CHROOTFAIL")
+ def test_handleStatus_WAITING_CHROOTFAIL_notifies(self):
+ return self._test_handleStatus_WAITING_notifies("CHROOTFAIL")
- def test_handleStatus_PACKAGEFAIL_notifies(self):
- return self._test_handleStatus_notifies("PACKAGEFAIL")
+ def test_handleStatus_WAITING_PACKAGEFAIL_notifies(self):
+ return self._test_handleStatus_WAITING_notifies("PACKAGEFAIL")
@defer.inlineCallbacks
- def test_handleStatus_ABORTED_cancels_cancelling(self):
+ def test_handleStatus_WAITING_ABORTED_cancels_cancelling(self):
with dbuser(config.builddmaster.dbuser):
self.build.updateStatus(BuildStatus.CANCELLING)
yield self.behaviour.handleStatus(
- self.build.buildqueue_record, "ABORTED", {})
+ self.build.buildqueue_record,
+ {"builder_status": "BuilderStatus.WAITING",
+ "build_status": "BuildStatus.ABORTED"})
self.assertEqual(0, len(pop_notifications()), "Notifications received")
self.assertEqual(BuildStatus.CANCELLED, self.build.status)
@defer.inlineCallbacks
- def test_handleStatus_ABORTED_illegal_when_building(self):
+ def test_handleStatus_WAITING_ABORTED_illegal_when_building(self):
self.builder.vm_host = "fake_vm_host"
self.behaviour = self.interactor.getBuildBehaviour(
self.build.buildqueue_record, self.builder, self.worker)
@@ -475,16 +527,20 @@ class TestHandleStatusMixin:
BuildDaemonError,
"Build returned unexpected status: %r" % 'ABORTED'):
yield self.behaviour.handleStatus(
- self.build.buildqueue_record, "ABORTED", {})
+ self.build.buildqueue_record,
+ {"builder_status": "BuilderStatus.WAITING",
+ "build_status": "BuildStatus.ABORTED"})
@defer.inlineCallbacks
- def test_handleStatus_ABORTED_cancelling_sets_build_log(self):
+ def test_handleStatus_WAITING_ABORTED_cancelling_sets_build_log(self):
# If a build is intentionally cancelled, the build log is set.
self.assertEqual(None, self.build.log)
with dbuser(config.builddmaster.dbuser):
self.build.updateStatus(BuildStatus.CANCELLING)
yield self.behaviour.handleStatus(
- self.build.buildqueue_record, "ABORTED", {})
+ self.build.buildqueue_record,
+ {"builder_status": "BuilderStatus.WAITING",
+ "build_status": "BuildStatus.ABORTED"})
self.assertNotEqual(None, self.build.log)
@defer.inlineCallbacks
@@ -493,8 +549,10 @@ class TestHandleStatusMixin:
self.assertEqual(None, self.build.date_finished)
with dbuser(config.builddmaster.dbuser):
yield self.behaviour.handleStatus(
- self.build.buildqueue_record, 'OK',
- {'filemap': {'myfile.py': 'test_file_hash'}})
+ self.build.buildqueue_record,
+ {'builder_status': 'BuilderStatus.WAITING',
+ 'build_status': 'BuildStatus.OK',
+ 'filemap': {'myfile.py': 'test_file_hash'}})
self.assertNotEqual(None, self.build.date_finished)
@defer.inlineCallbacks
@@ -504,7 +562,9 @@ class TestHandleStatusMixin:
"Build returned unexpected status: %r" % 'GIVENBACK'):
with dbuser(config.builddmaster.dbuser):
yield self.behaviour.handleStatus(
- self.build.buildqueue_record, "GIVENBACK", {})
+ self.build.buildqueue_record,
+ {"builder_status": "BuilderStatus.WAITING",
+ "build_status": "BuildStatus.GIVENBACK"})
@defer.inlineCallbacks
def test_builderfail_collection(self):
@@ -513,7 +573,9 @@ class TestHandleStatusMixin:
"Build returned unexpected status: %r" % 'BUILDERFAIL'):
with dbuser(config.builddmaster.dbuser):
yield self.behaviour.handleStatus(
- self.build.buildqueue_record, "BUILDERFAIL", {})
+ self.build.buildqueue_record,
+ {"builder_status": "BuilderStatus.WAITING",
+ "build_status": "BuildStatus.BUILDERFAIL"})
@defer.inlineCallbacks
def test_invalid_status_collection(self):
@@ -522,4 +584,6 @@ class TestHandleStatusMixin:
"Build returned unexpected status: %r" % 'BORKED'):
with dbuser(config.builddmaster.dbuser):
yield self.behaviour.handleStatus(
- self.build.buildqueue_record, "BORKED", {})
+ self.build.buildqueue_record,
+ {"builder_status": "BuilderStatus.WAITING",
+ "build_status": "BuildStatus.BORKED"})
diff --git a/lib/lp/buildmaster/tests/test_interactor.py b/lib/lp/buildmaster/tests/test_interactor.py
index 6e4a790..9aa025c 100644
--- a/lib/lp/buildmaster/tests/test_interactor.py
+++ b/lib/lp/buildmaster/tests/test_interactor.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2022 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test BuilderInteractor features."""
@@ -121,21 +121,6 @@ class TestBuilderInteractor(TestCase):
super().setUp()
self.addCleanup(shut_down_default_process_pool)
- def test_extractBuildStatus_baseline(self):
- # extractBuildStatus picks the name of the build status out of a
- # dict describing the worker's status.
- worker_status = {'build_status': 'BuildStatus.BUILDING'}
- self.assertEqual(
- 'BUILDING', BuilderInteractor.extractBuildStatus(worker_status))
-
- def test_extractBuildStatus_malformed(self):
- # extractBuildStatus errors out when the status string is not
- # of the form it expects.
- worker_status = {'build_status': 'BUILDING'}
- self.assertRaises(
- AssertionError, BuilderInteractor.extractBuildStatus,
- worker_status)
-
def resumeWorkerHost(self, builder):
vitals = extract_vitals_from_db(builder)
return BuilderInteractor.resumeWorkerHost(
diff --git a/lib/lp/code/model/tests/test_recipebuilder.py b/lib/lp/code/model/tests/test_recipebuilder.py
index 9077a08..9c75d68 100644
--- a/lib/lp/code/model/tests/test_recipebuilder.py
+++ b/lib/lp/code/model/tests/test_recipebuilder.py
@@ -1,4 +1,4 @@
-# Copyright 2010-2020 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2022 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test RecipeBuildBehaviour."""
@@ -418,7 +418,11 @@ class TestBuildNotifications(TestCaseWithFactory):
self.queue_record, self.queue_record.builder, worker)
def assertDeferredNotifyCount(self, status, behaviour, expected_count):
- d = behaviour.handleStatus(self.queue_record, status, {'filemap': {}})
+ d = behaviour.handleStatus(
+ self.queue_record,
+ {'builder_status': 'BuilderStatus.WAITING',
+ 'build_status': 'BuildStatus.%s' % status,
+ 'filemap': {}})
def cb(result):
self.assertEqual(expected_count, len(pop_notifications()))
diff --git a/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py b/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
index 7804583..cb5438f 100644
--- a/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
+++ b/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
@@ -1,4 +1,4 @@
-# Copyright 2015-2021 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2022 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for `OCIRecipeBuildBehaviour`."""
@@ -55,6 +55,7 @@ from lp.buildmaster.interfaces.builder import (
from lp.buildmaster.interfaces.buildfarmjobbehaviour import (
IBuildFarmJobBehaviour,
)
+from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
from lp.buildmaster.interfaces.processor import IProcessorSet
from lp.buildmaster.tests.builderproxy import (
InProcessProxyAuthAPIFixture,
@@ -773,15 +774,40 @@ class TestHandleStatusForOCIRecipeBuild(MakeOCIBuildMixin,
1, len(os.listdir(os.path.join(self.upload_root, result))))
@defer.inlineCallbacks
- def test_handleStatus_OK_normal_image(self):
+ def test_handleStatus_BUILDING(self):
+ # If the builder is BUILDING (or any status other than WAITING),
+ # then the behaviour calls updateStatus but doesn't do anything
+ # else.
+ initial_status = self.build.status
+ bq_id = self.build.buildqueue_record.id
+ worker_status = {"builder_status": "BuilderStatus.BUILDING"}
+ removeSecurityProxy(self.build).updateStatus = FakeMethod()
+ with dbuser(config.builddmaster.dbuser):
+ yield self.behaviour.handleStatus(
+ self.build.buildqueue_record, worker_status)
+ self.assertEqual(None, self.build.log)
+ self.assertEqual(0, len(os.listdir(self.upload_root)))
+ self.assertEqual(
+ [((initial_status,),
+ {"builder": self.builder, "worker_status": worker_status})],
+ removeSecurityProxy(self.build).updateStatus.calls)
+ self.assertEqual(0, len(pop_notifications()), "Notifications received")
+ self.assertEqual(
+ self.build.buildqueue_record,
+ getUtility(IBuildQueueSet).get(bq_id))
+
+ @defer.inlineCallbacks
+ def test_handleStatus_WAITING_OK_normal_image(self):
now = datetime.now()
mock_datetime = self.useFixture(MockPatch(
'lp.buildmaster.model.buildfarmjobbehaviour.datetime')).mock
mock_datetime.now = lambda: now
with dbuser(config.builddmaster.dbuser):
yield self.behaviour.handleStatus(
- self.build.buildqueue_record, 'OK',
- {'filemap': self.filemap})
+ self.build.buildqueue_record,
+ {'builder_status': 'BuilderStatus.WAITING',
+ 'build_status': 'BuildStatus.OK',
+ 'filemap': self.filemap})
self.assertEqual(
['buildlog', 'manifest_hash', 'digests_hash', 'config_1_hash',
'layer_2_hash'],
@@ -805,7 +831,7 @@ class TestHandleStatusForOCIRecipeBuild(MakeOCIBuildMixin,
self.assertEqual(contents, b'retrieved from librarian')
@defer.inlineCallbacks
- def test_handleStatus_OK_reuse_from_other_build(self):
+ def test_handleStatus_WAITING_OK_reuse_from_other_build(self):
"""We should be able to reuse a layer file from a separate build."""
oci_file = self.factory.makeOCIFile(
layer_file_digest='digest_2',
@@ -820,8 +846,10 @@ class TestHandleStatusForOCIRecipeBuild(MakeOCIBuildMixin,
mock_oci_datetime.now = lambda tzinfo=None: now
with dbuser(config.builddmaster.dbuser):
yield self.behaviour.handleStatus(
- self.build.buildqueue_record, 'OK',
- {'filemap': self.filemap})
+ self.build.buildqueue_record,
+ {'builder_status': 'BuilderStatus.WAITING',
+ 'build_status': 'BuildStatus.OK',
+ 'filemap': self.filemap})
self.assertEqual(
['buildlog', 'manifest_hash', 'digests_hash', 'config_1_hash'],
self.worker._got_file_record)
@@ -844,7 +872,7 @@ class TestHandleStatusForOCIRecipeBuild(MakeOCIBuildMixin,
self.assertEqual(now, oci_file.date_last_used)
@defer.inlineCallbacks
- def test_handleStatus_OK_absolute_filepath(self):
+ def test_handleStatus_WAITING_OK_absolute_filepath(self):
self._createTestFile(
'manifest.json',
@@ -862,11 +890,13 @@ class TestHandleStatusForOCIRecipeBuild(MakeOCIBuildMixin,
"'/notvalid/config_file_1.json'."):
with dbuser(config.builddmaster.dbuser):
yield self.behaviour.handleStatus(
- self.build.buildqueue_record, 'OK',
- {'filemap': self.filemap})
+ self.build.buildqueue_record,
+ {'builder_status': 'BuilderStatus.WAITING',
+ 'build_status': 'BuildStatus.OK',
+ 'filemap': self.filemap})
@defer.inlineCallbacks
- def test_handleStatus_OK_relative_filepath(self):
+ def test_handleStatus_WAITING_OK_relative_filepath(self):
self._createTestFile(
'manifest.json',
@@ -882,30 +912,36 @@ class TestHandleStatusForOCIRecipeBuild(MakeOCIBuildMixin,
"Build returned a file named '../config_file_1.json'."):
with dbuser(config.builddmaster.dbuser):
yield self.behaviour.handleStatus(
- self.build.buildqueue_record, 'OK',
- {'filemap': self.filemap})
+ self.build.buildqueue_record,
+ {'builder_status': 'BuilderStatus.WAITING',
+ 'build_status': 'BuildStatus.OK',
+ 'filemap': self.filemap})
@defer.inlineCallbacks
- def test_handleStatus_OK_sets_build_log(self):
+ def test_handleStatus_WAITING_OK_sets_build_log(self):
# The build log is set during handleStatus.
self.assertEqual(None, self.build.log)
with dbuser(config.builddmaster.dbuser):
yield self.behaviour.handleStatus(
- self.build.buildqueue_record, 'OK',
- {'filemap': self.filemap})
+ self.build.buildqueue_record,
+ {'builder_status': 'BuilderStatus.WAITING',
+ 'build_status': 'BuildStatus.OK',
+ 'filemap': self.filemap})
self.assertNotEqual(None, self.build.log)
@defer.inlineCallbacks
- def test_handleStatus_ABORTED_cancels_cancelling(self):
+ def test_handleStatus_WAITING_ABORTED_cancels_cancelling(self):
with dbuser(config.builddmaster.dbuser):
self.build.updateStatus(BuildStatus.CANCELLING)
yield self.behaviour.handleStatus(
- self.build.buildqueue_record, "ABORTED", {})
+ self.build.buildqueue_record,
+ {"builder_status": "BuilderStatus.WAITING",
+ "build_status": "BuildStatus.ABORTED"})
self.assertEqual(0, len(pop_notifications()), "Notifications received")
self.assertEqual(BuildStatus.CANCELLED, self.build.status)
@defer.inlineCallbacks
- def test_handleStatus_ABORTED_illegal_when_building(self):
+ def test_handleStatus_WAITING_ABORTED_illegal_when_building(self):
self.builder.vm_host = "fake_vm_host"
self.behaviour = self.interactor.getBuildBehaviour(
self.build.buildqueue_record, self.builder, self.worker)
@@ -915,16 +951,20 @@ class TestHandleStatusForOCIRecipeBuild(MakeOCIBuildMixin,
BuildDaemonError,
"Build returned unexpected status: %r" % 'ABORTED'):
yield self.behaviour.handleStatus(
- self.build.buildqueue_record, "ABORTED", {})
+ self.build.buildqueue_record,
+ {"builder_status": "BuilderStatus.WAITING",
+ "build_status": "BuildStatus.ABORTED"})
@defer.inlineCallbacks
- def test_handleStatus_ABORTED_cancelling_sets_build_log(self):
+ def test_handleStatus_WAITING_ABORTED_cancelling_sets_build_log(self):
# If a build is intentionally cancelled, the build log is set.
self.assertEqual(None, self.build.log)
with dbuser(config.builddmaster.dbuser):
self.build.updateStatus(BuildStatus.CANCELLING)
yield self.behaviour.handleStatus(
- self.build.buildqueue_record, "ABORTED", {})
+ self.build.buildqueue_record,
+ {"builder_status": "BuilderStatus.WAITING",
+ "build_status": "BuildStatus.ABORTED"})
self.assertNotEqual(None, self.build.log)
@defer.inlineCallbacks
@@ -933,8 +973,10 @@ class TestHandleStatusForOCIRecipeBuild(MakeOCIBuildMixin,
self.assertEqual(None, self.build.date_finished)
with dbuser(config.builddmaster.dbuser):
yield self.behaviour.handleStatus(
- self.build.buildqueue_record, 'OK',
- {'filemap': self.filemap})
+ self.build.buildqueue_record,
+ {'builder_status': 'BuilderStatus.WAITING',
+ 'build_status': 'BuildStatus.OK',
+ 'filemap': self.filemap})
self.assertNotEqual(None, self.build.date_finished)
@defer.inlineCallbacks
@@ -944,7 +986,9 @@ class TestHandleStatusForOCIRecipeBuild(MakeOCIBuildMixin,
"Build returned unexpected status: %r" % 'GIVENBACK'):
with dbuser(config.builddmaster.dbuser):
yield self.behaviour.handleStatus(
- self.build.buildqueue_record, "GIVENBACK", {})
+ self.build.buildqueue_record,
+ {"builder_status": "BuilderStatus.WAITING",
+ "build_status": "BuildStatus.GIVENBACK"})
@defer.inlineCallbacks
def test_builderfail_collection(self):
@@ -953,7 +997,9 @@ class TestHandleStatusForOCIRecipeBuild(MakeOCIBuildMixin,
"Build returned unexpected status: %r" % 'BUILDERFAIL'):
with dbuser(config.builddmaster.dbuser):
yield self.behaviour.handleStatus(
- self.build.buildqueue_record, "BUILDERFAIL", {})
+ self.build.buildqueue_record,
+ {"builder_status": "BuilderStatus.WAITING",
+ "build_status": "BuildStatus.BUILDERFAIL"})
@defer.inlineCallbacks
def test_invalid_status_collection(self):
@@ -962,7 +1008,9 @@ class TestHandleStatusForOCIRecipeBuild(MakeOCIBuildMixin,
"Build returned unexpected status: %r" % 'BORKED'):
with dbuser(config.builddmaster.dbuser):
yield self.behaviour.handleStatus(
- self.build.buildqueue_record, "BORKED", {})
+ self.build.buildqueue_record,
+ {"builder_status": "BuilderStatus.WAITING",
+ "build_status": "BuildStatus.BORKED"})
class TestGetUploadMethodsForOCIRecipeBuild(
diff --git a/lib/lp/translations/tests/test_translationtemplatesbuildbehaviour.py b/lib/lp/translations/tests/test_translationtemplatesbuildbehaviour.py
index 0e446ec..93f9325 100644
--- a/lib/lp/translations/tests/test_translationtemplatesbuildbehaviour.py
+++ b/lib/lp/translations/tests/test_translationtemplatesbuildbehaviour.py
@@ -1,4 +1,4 @@
-# Copyright 2010-2020 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2022 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Unit tests for TranslationTemplatesBuildBehaviour."""
@@ -14,7 +14,6 @@ from zope.component import getUtility
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
from lp.buildmaster.enums import BuildStatus
-from lp.buildmaster.interactor import BuilderInteractor
from lp.buildmaster.interfaces.buildfarmjobbehaviour import (
IBuildFarmJobBehaviour,
)
@@ -164,11 +163,7 @@ class TestTranslationTemplatesBuildBehaviour(
d = worker.status()
def got_status(status):
- return (
- behaviour.handleStatus(
- queue_item, BuilderInteractor.extractBuildStatus(status),
- status),
- worker.call_log)
+ return behaviour.handleStatus(queue_item, status), worker.call_log
def build_updated(ignored):
self.assertEqual(BuildStatus.FULLYBUILT, behaviour.build.status)
@@ -193,10 +188,7 @@ class TestTranslationTemplatesBuildBehaviour(
def got_status(status):
del status['filemap']
- return behaviour.handleStatus(
- queue_item,
- BuilderInteractor.extractBuildStatus(status),
- status),
+ return behaviour.handleStatus(queue_item, status),
def build_updated(ignored):
self.assertEqual(BuildStatus.FAILEDTOBUILD, behaviour.build.status)
@@ -222,10 +214,7 @@ class TestTranslationTemplatesBuildBehaviour(
def got_status(status):
del status['filemap']
- return behaviour.handleStatus(
- queue_item,
- BuilderInteractor.extractBuildStatus(status),
- status),
+ return behaviour.handleStatus(queue_item, status),
def build_updated(ignored):
self.assertEqual(BuildStatus.FULLYBUILT, behaviour.build.status)
@@ -257,10 +246,7 @@ class TestTranslationTemplatesBuildBehaviour(
d = worker.status()
def got_status(status):
- return behaviour.handleStatus(
- queue_item,
- BuilderInteractor.extractBuildStatus(status),
- status),
+ return behaviour.handleStatus(queue_item, status),
def build_updated(ignored):
self.assertEqual(BuildStatus.FULLYBUILT, behaviour.build.status)
