Merge ~cjwatson/launchpad:refactor-updateBuild into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: 115222a6e4c30765a4ee17254bca1a14b269a566
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:refactor-updateBuild
Merge into: launchpad:master
Prerequisite: ~cjwatson/launchpad:rename-builder-interactor-slave
Diff against target: 844 lines (+262/-168)
8 files modified
lib/lp/buildmaster/interactor.py (+10/-27)
lib/lp/buildmaster/interfaces/buildfarmjobbehaviour.py (+3/-4)
lib/lp/buildmaster/model/buildfarmjobbehaviour.py (+66/-41)
lib/lp/buildmaster/tests/test_buildfarmjobbehaviour.py (+96/-32)
lib/lp/buildmaster/tests/test_interactor.py (+1/-16)
lib/lp/code/model/tests/test_recipebuilder.py (+6/-2)
lib/lp/oci/tests/test_ocirecipebuildbehaviour.py (+75/-27)
lib/lp/translations/tests/test_translationtemplatesbuildbehaviour.py (+5/-19)
Reviewer Review Type Date Requested Status
Ioana Lasc Approve
Review via email: mp+414070@code.launchpad.net

Commit message

Push more of BuilderInteractor.updateBuild down to behaviours

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Ioana Lasc (ilasc) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/buildmaster/interactor.py b/lib/lp/buildmaster/interactor.py
2index e8a1d7f..679d5c6 100644
3--- a/lib/lp/buildmaster/interactor.py
4+++ b/lib/lp/buildmaster/interactor.py
5@@ -1,4 +1,4 @@
6-# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
7+# Copyright 2009-2022 Canonical Ltd. This software is licensed under the
8 # GNU Affero General Public License version 3 (see the file LICENSE).
9
10 __all__ = [
11@@ -526,19 +526,6 @@ class BuilderInteractor:
12 return candidate
13
14 @staticmethod
15- def extractBuildStatus(worker_status):
16- """Read build status name.
17-
18- :param worker_status: build status dict from BuilderWorker.status.
19- :return: the unqualified status name, e.g. "OK".
20- """
21- status_string = worker_status['build_status']
22- lead_string = 'BuildStatus.'
23- assert status_string.startswith(lead_string), (
24- "Malformed status string: '%s'" % status_string)
25- return status_string[len(lead_string):]
26-
27- @staticmethod
28 def extractLogTail(worker_status):
29 """Extract the log tail from a builder status response.
30
31@@ -580,24 +567,20 @@ class BuilderInteractor:
32 # matches the DB, and this method isn't called unless the DB
33 # says there's a job.
34 builder_status = worker_status['builder_status']
35+ if builder_status not in (
36+ 'BuilderStatus.BUILDING', 'BuilderStatus.ABORTING',
37+ 'BuilderStatus.WAITING'):
38+ raise AssertionError("Unknown status %s" % builder_status)
39+ builder = builder_factory[vitals.name]
40+ behaviour = behaviour_factory(vitals.build_queue, builder, worker)
41 if builder_status in (
42 'BuilderStatus.BUILDING', 'BuilderStatus.ABORTING'):
43 logtail = cls.extractLogTail(worker_status)
44 if logtail is not None:
45 manager.addLogTail(vitals.build_queue.id, logtail)
46- vitals.build_queue.specific_build.updateStatus(
47- vitals.build_queue.specific_build.status,
48- worker_status=worker_status)
49- transaction.commit()
50- elif builder_status == 'BuilderStatus.WAITING':
51- # Build has finished. Delegate handling to the build itself.
52- builder = builder_factory[vitals.name]
53- behaviour = behaviour_factory(vitals.build_queue, builder, worker)
54- yield behaviour.handleStatus(
55- vitals.build_queue, cls.extractBuildStatus(worker_status),
56- worker_status)
57- else:
58- raise AssertionError("Unknown status %s" % builder_status)
59+ # Delegate the remaining handling to the build behaviour, which will
60+ # commit the transaction.
61+ yield behaviour.handleStatus(vitals.build_queue, worker_status)
62
63 @staticmethod
64 def _getWorkerScannerLogger():
65diff --git a/lib/lp/buildmaster/interfaces/buildfarmjobbehaviour.py b/lib/lp/buildmaster/interfaces/buildfarmjobbehaviour.py
66index 2d0eed4..c529327 100644
67--- a/lib/lp/buildmaster/interfaces/buildfarmjobbehaviour.py
68+++ b/lib/lp/buildmaster/interfaces/buildfarmjobbehaviour.py
69@@ -1,4 +1,4 @@
70-# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
71+# Copyright 2009-2022 Canonical Ltd. This software is licensed under the
72 # GNU Affero General Public License version 3 (see the file LICENSE).
73
74 """Interface for build farm job behaviours."""
75@@ -88,10 +88,9 @@ class IBuildFarmJobBehaviour(Interface):
76 def verifySuccessfulBuild():
77 """Check that we are allowed to collect this successful build."""
78
79- def handleStatus(bq, status, worker_status):
80- """Update the build from a WAITING worker result.
81+ def handleStatus(bq, worker_status):
82+ """Update the build from a worker's status response.
83
84 :param bq: The `BuildQueue` currently being processed.
85- :param status: The tail of the BuildStatus (eg. OK or PACKAGEFAIL).
86 :param worker_status: Worker status dict from `BuilderWorker.status`.
87 """
88diff --git a/lib/lp/buildmaster/model/buildfarmjobbehaviour.py b/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
89index 0315b52..a7537d3 100644
90--- a/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
91+++ b/lib/lp/buildmaster/model/buildfarmjobbehaviour.py
92@@ -1,4 +1,4 @@
93-# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
94+# Copyright 2009-2022 Canonical Ltd. This software is licensed under the
95 # GNU Affero General Public License version 3 (see the file LICENSE).
96
97 """Base and idle BuildFarmJobBehaviour classes."""
98@@ -33,6 +33,7 @@ from lp.services.config import config
99 from lp.services.helpers import filenameToContentType
100 from lp.services.librarian.interfaces import ILibraryFileAliasSet
101 from lp.services.librarian.utils import copy_and_close
102+from lp.services.propertycache import cachedproperty
103 from lp.services.statsd.interfaces.statsd_client import IStatsdClient
104 from lp.services.utils import sanitise_urls
105 from lp.services.webapp import canonical_url
106@@ -53,7 +54,10 @@ class BuildFarmJobBehaviourBase:
107 """Store a reference to the job_type with which we were created."""
108 self.build = build
109 self._builder = None
110- self._authserver = xmlrpc.Proxy(
111+
112+ @cachedproperty
113+ def _authserver(self):
114+ return xmlrpc.Proxy(
115 config.builddmaster.authentication_endpoint.encode('UTF-8'),
116 connectTimeout=config.builddmaster.authentication_timeout)
117
118@@ -255,6 +259,19 @@ class BuildFarmJobBehaviourBase:
119 "%s (%s) can not be built for pocket %s in %s: illegal status"
120 % (build.title, build.id, build.pocket.name, build.archive))
121
122+ @staticmethod
123+ def extractBuildStatus(worker_status):
124+ """Read build status name.
125+
126+ :param worker_status: build status dict from BuilderWorker.status.
127+ :return: the unqualified status name, e.g. "OK".
128+ """
129+ status_string = worker_status['build_status']
130+ lead_string = 'BuildStatus.'
131+ assert status_string.startswith(lead_string), (
132+ "Malformed status string: '%s'" % status_string)
133+ return status_string[len(lead_string):]
134+
135 # The list of build status values for which email notifications are
136 # allowed to be sent. It is up to each callback as to whether it will
137 # consider sending a notification but it won't do so if the status is not
138@@ -262,57 +279,65 @@ class BuildFarmJobBehaviourBase:
139 ALLOWED_STATUS_NOTIFICATIONS = ['PACKAGEFAIL', 'CHROOTFAIL']
140
141 @defer.inlineCallbacks
142- def handleStatus(self, bq, status, worker_status):
143+ def handleStatus(self, bq, worker_status):
144 """See `IBuildFarmJobBehaviour`."""
145 if bq != self.build.buildqueue_record:
146 raise AssertionError(
147 "%r != %r" % (bq, self.build.buildqueue_record))
148 from lp.buildmaster.manager import BUILDD_MANAGER_LOG_NAME
149 logger = logging.getLogger(BUILDD_MANAGER_LOG_NAME)
150- notify = status in self.ALLOWED_STATUS_NOTIFICATIONS
151- fail_status_map = {
152- 'PACKAGEFAIL': BuildStatus.FAILEDTOBUILD,
153- 'DEPFAIL': BuildStatus.MANUALDEPWAIT,
154- 'CHROOTFAIL': BuildStatus.CHROOTWAIT,
155- }
156- if self.build.status == BuildStatus.CANCELLING:
157- fail_status_map['ABORTED'] = BuildStatus.CANCELLED
158-
159- logger.info(
160- 'Processing finished job %s (%s) from builder %s: %s'
161- % (self.build.build_cookie, self.build.title,
162- self.build.buildqueue_record.builder.name, status))
163- build_status = None
164- if status == 'OK':
165- yield self.storeLogFromWorker()
166- # handleSuccess will sometimes perform write operations
167- # outside the database transaction, so a failure between
168- # here and the commit can cause duplicated results. For
169- # example, a BinaryPackageBuild will end up in the upload
170- # queue twice if notify() crashes.
171- build_status = yield self.handleSuccess(worker_status, logger)
172- elif status in fail_status_map:
173- # XXX wgrant: The builder should be set long before here, but
174- # currently isn't.
175- yield self.storeLogFromWorker()
176- build_status = fail_status_map[status]
177+ builder_status = worker_status["builder_status"]
178+
179+ if builder_status == "BuilderStatus.WAITING":
180+ # Build has finished.
181+ status = self.extractBuildStatus(worker_status)
182+ notify = status in self.ALLOWED_STATUS_NOTIFICATIONS
183+ fail_status_map = {
184+ 'PACKAGEFAIL': BuildStatus.FAILEDTOBUILD,
185+ 'DEPFAIL': BuildStatus.MANUALDEPWAIT,
186+ 'CHROOTFAIL': BuildStatus.CHROOTWAIT,
187+ }
188+ if self.build.status == BuildStatus.CANCELLING:
189+ fail_status_map['ABORTED'] = BuildStatus.CANCELLED
190+
191+ logger.info(
192+ 'Processing finished job %s (%s) from builder %s: %s'
193+ % (self.build.build_cookie, self.build.title,
194+ self.build.buildqueue_record.builder.name, status))
195+ build_status = None
196+ if status == 'OK':
197+ yield self.storeLogFromWorker()
198+ # handleSuccess will sometimes perform write operations
199+ # outside the database transaction, so a failure between
200+ # here and the commit can cause duplicated results. For
201+ # example, a BinaryPackageBuild will end up in the upload
202+ # queue twice if notify() crashes.
203+ build_status = yield self.handleSuccess(worker_status, logger)
204+ elif status in fail_status_map:
205+ yield self.storeLogFromWorker()
206+ build_status = fail_status_map[status]
207+ else:
208+ raise BuildDaemonError(
209+ "Build returned unexpected status: %r" % status)
210 else:
211- raise BuildDaemonError(
212- "Build returned unexpected status: %r" % status)
213+ # The build status remains unchanged.
214+ build_status = bq.specific_build.status
215
216- # Set the status and dequeue the build atomically. Setting the
217- # status to UPLOADING constitutes handoff to process-upload, so
218- # doing that before we've removed the BuildQueue causes races.
219+ # Set the status and (if the build has finished) dequeue the build
220+ # atomically. Setting the status to UPLOADING constitutes handoff to
221+ # process-upload, so doing that before we've removed the BuildQueue
222+ # causes races.
223
224 # XXX wgrant: The builder should be set long before here, but
225 # currently isn't.
226 self.build.updateStatus(
227- build_status,
228- builder=self.build.buildqueue_record.builder,
229- worker_status=worker_status)
230- if notify:
231- self.build.notify()
232- self.build.buildqueue_record.destroySelf()
233+ build_status, builder=bq.builder, worker_status=worker_status)
234+
235+ if builder_status == "BuilderStatus.WAITING":
236+ if notify:
237+ self.build.notify()
238+ self.build.buildqueue_record.destroySelf()
239+
240 transaction.commit()
241
242 @defer.inlineCallbacks
243diff --git a/lib/lp/buildmaster/tests/test_buildfarmjobbehaviour.py b/lib/lp/buildmaster/tests/test_buildfarmjobbehaviour.py
244index ad05908..f36750e 100644
245--- a/lib/lp/buildmaster/tests/test_buildfarmjobbehaviour.py
246+++ b/lib/lp/buildmaster/tests/test_buildfarmjobbehaviour.py
247@@ -1,4 +1,4 @@
248-# Copyright 2010-2020 Canonical Ltd. This software is licensed under the
249+# Copyright 2010-2022 Canonical Ltd. This software is licensed under the
250 # GNU Affero General Public License version 3 (see the file LICENSE).
251
252 """Unit tests for BuildFarmJobBehaviourBase."""
253@@ -30,6 +30,7 @@ from lp.buildmaster.interfaces.builder import BuildDaemonError
254 from lp.buildmaster.interfaces.buildfarmjobbehaviour import (
255 IBuildFarmJobBehaviour,
256 )
257+from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
258 from lp.buildmaster.interfaces.processor import IProcessorSet
259 from lp.buildmaster.model.buildfarmjobbehaviour import (
260 BuildFarmJobBehaviourBase,
261@@ -154,6 +155,22 @@ class TestBuildFarmJobBehaviourBase(TestCaseWithFactory):
262 behaviour.setBuilder(self.factory.makeBuilder(virtualized=False), None)
263 self.assertIs(False, behaviour.extraBuildArgs()["fast_cleanup"])
264
265+ def test_extractBuildStatus_baseline(self):
266+ # extractBuildStatus picks the name of the build status out of a
267+ # dict describing the worker's status.
268+ worker_status = {"build_status": "BuildStatus.BUILDING"}
269+ self.assertEqual(
270+ "BUILDING",
271+ BuildFarmJobBehaviourBase.extractBuildStatus(worker_status))
272+
273+ def test_extractBuildStatus_malformed(self):
274+ # extractBuildStatus errors out when the status string is not
275+ # of the form it expects.
276+ worker_status = {"build_status": "BUILDING"}
277+ self.assertRaises(
278+ AssertionError, BuildFarmJobBehaviourBase.extractBuildStatus,
279+ worker_status)
280+
281
282 class TestDispatchBuildToWorker(StatsMixin, TestCase):
283
284@@ -383,19 +400,44 @@ class TestHandleStatusMixin:
285 1, len(os.listdir(os.path.join(self.upload_root, result))))
286
287 @defer.inlineCallbacks
288- def test_handleStatus_OK_normal_file(self):
289+ def test_handleStatus_BUILDING(self):
290+ # If the builder is BUILDING (or any status other than WAITING),
291+ # then the behaviour calls updateStatus but doesn't do anything
292+ # else.
293+ initial_status = self.build.status
294+ bq_id = self.build.buildqueue_record.id
295+ worker_status = {"builder_status": "BuilderStatus.BUILDING"}
296+ removeSecurityProxy(self.build).updateStatus = FakeMethod()
297+ with dbuser(config.builddmaster.dbuser):
298+ yield self.behaviour.handleStatus(
299+ self.build.buildqueue_record, worker_status)
300+ self.assertEqual(None, self.build.log)
301+ self.assertEqual(0, len(os.listdir(self.upload_root)))
302+ self.assertEqual(
303+ [((initial_status,),
304+ {"builder": self.builder, "worker_status": worker_status})],
305+ removeSecurityProxy(self.build).updateStatus.calls)
306+ self.assertEqual(0, len(pop_notifications()), "Notifications received")
307+ self.assertEqual(
308+ self.build.buildqueue_record,
309+ getUtility(IBuildQueueSet).get(bq_id))
310+
311+ @defer.inlineCallbacks
312+ def test_handleStatus_WAITING_OK_normal_file(self):
313 # A filemap with plain filenames should not cause a problem.
314 # The call to handleStatus will attempt to get the file from
315 # the worker resulting in a URL error in this test case.
316 with dbuser(config.builddmaster.dbuser):
317 yield self.behaviour.handleStatus(
318- self.build.buildqueue_record, 'OK',
319- {'filemap': {'myfile.py': 'test_file_hash'}})
320+ self.build.buildqueue_record,
321+ {'builder_status': 'BuilderStatus.WAITING',
322+ 'build_status': 'BuildStatus.OK',
323+ 'filemap': {'myfile.py': 'test_file_hash'}})
324 self.assertEqual(BuildStatus.UPLOADING, self.build.status)
325 self.assertResultCount(1, "incoming")
326
327 @defer.inlineCallbacks
328- def test_handleStatus_OK_absolute_filepath(self):
329+ def test_handleStatus_WAITING_OK_absolute_filepath(self):
330 # A filemap that tries to write to files outside of the upload
331 # directory will not be collected.
332 with ExpectedException(
333@@ -403,11 +445,13 @@ class TestHandleStatusMixin:
334 "Build returned a file named '/tmp/myfile.py'."):
335 with dbuser(config.builddmaster.dbuser):
336 yield self.behaviour.handleStatus(
337- self.build.buildqueue_record, 'OK',
338- {'filemap': {'/tmp/myfile.py': 'test_file_hash'}})
339+ self.build.buildqueue_record,
340+ {'builder_status': 'BuilderStatus.WAITING',
341+ 'build_status': 'BuildStatus.OK',
342+ 'filemap': {'/tmp/myfile.py': 'test_file_hash'}})
343
344 @defer.inlineCallbacks
345- def test_handleStatus_OK_relative_filepath(self):
346+ def test_handleStatus_WAITING_OK_relative_filepath(self):
347 # A filemap that tries to write to files outside of
348 # the upload directory will not be collected.
349 with ExpectedException(
350@@ -415,21 +459,25 @@ class TestHandleStatusMixin:
351 "Build returned a file named '../myfile.py'."):
352 with dbuser(config.builddmaster.dbuser):
353 yield self.behaviour.handleStatus(
354- self.build.buildqueue_record, 'OK',
355- {'filemap': {'../myfile.py': 'test_file_hash'}})
356+ self.build.buildqueue_record,
357+ {'builder_status': 'BuilderStatus.WAITING',
358+ 'build_status': 'BuildStatus.OK',
359+ 'filemap': {'../myfile.py': 'test_file_hash'}})
360
361 @defer.inlineCallbacks
362- def test_handleStatus_OK_sets_build_log(self):
363+ def test_handleStatus_WAITING_OK_sets_build_log(self):
364 # The build log is set during handleStatus.
365 self.assertEqual(None, self.build.log)
366 with dbuser(config.builddmaster.dbuser):
367 yield self.behaviour.handleStatus(
368- self.build.buildqueue_record, 'OK',
369- {'filemap': {'myfile.py': 'test_file_hash'}})
370+ self.build.buildqueue_record,
371+ {'builder_status': 'BuilderStatus.WAITING',
372+ 'build_status': 'BuildStatus.OK',
373+ 'filemap': {'myfile.py': 'test_file_hash'}})
374 self.assertNotEqual(None, self.build.log)
375
376 @defer.inlineCallbacks
377- def _test_handleStatus_notifies(self, status):
378+ def _test_handleStatus_WAITING_notifies(self, status):
379 # An email notification is sent for a given build status if
380 # notifications are allowed for that status.
381 expected_notification = (
382@@ -437,7 +485,9 @@ class TestHandleStatusMixin:
383
384 with dbuser(config.builddmaster.dbuser):
385 yield self.behaviour.handleStatus(
386- self.build.buildqueue_record, status, {})
387+ self.build.buildqueue_record,
388+ {'builder_status': 'BuilderStatus.WAITING',
389+ 'build_status': 'BuildStatus.%s' % status})
390
391 if expected_notification:
392 self.assertNotEqual(
393@@ -446,26 +496,28 @@ class TestHandleStatusMixin:
394 self.assertEqual(
395 0, len(pop_notifications()), "Notifications received")
396
397- def test_handleStatus_DEPFAIL_notifies(self):
398- return self._test_handleStatus_notifies("DEPFAIL")
399+ def test_handleStatus_WAITING_DEPFAIL_notifies(self):
400+ return self._test_handleStatus_WAITING_notifies("DEPFAIL")
401
402- def test_handleStatus_CHROOTFAIL_notifies(self):
403- return self._test_handleStatus_notifies("CHROOTFAIL")
404+ def test_handleStatus_WAITING_CHROOTFAIL_notifies(self):
405+ return self._test_handleStatus_WAITING_notifies("CHROOTFAIL")
406
407- def test_handleStatus_PACKAGEFAIL_notifies(self):
408- return self._test_handleStatus_notifies("PACKAGEFAIL")
409+ def test_handleStatus_WAITING_PACKAGEFAIL_notifies(self):
410+ return self._test_handleStatus_WAITING_notifies("PACKAGEFAIL")
411
412 @defer.inlineCallbacks
413- def test_handleStatus_ABORTED_cancels_cancelling(self):
414+ def test_handleStatus_WAITING_ABORTED_cancels_cancelling(self):
415 with dbuser(config.builddmaster.dbuser):
416 self.build.updateStatus(BuildStatus.CANCELLING)
417 yield self.behaviour.handleStatus(
418- self.build.buildqueue_record, "ABORTED", {})
419+ self.build.buildqueue_record,
420+ {"builder_status": "BuilderStatus.WAITING",
421+ "build_status": "BuildStatus.ABORTED"})
422 self.assertEqual(0, len(pop_notifications()), "Notifications received")
423 self.assertEqual(BuildStatus.CANCELLED, self.build.status)
424
425 @defer.inlineCallbacks
426- def test_handleStatus_ABORTED_illegal_when_building(self):
427+ def test_handleStatus_WAITING_ABORTED_illegal_when_building(self):
428 self.builder.vm_host = "fake_vm_host"
429 self.behaviour = self.interactor.getBuildBehaviour(
430 self.build.buildqueue_record, self.builder, self.worker)
431@@ -475,16 +527,20 @@ class TestHandleStatusMixin:
432 BuildDaemonError,
433 "Build returned unexpected status: %r" % 'ABORTED'):
434 yield self.behaviour.handleStatus(
435- self.build.buildqueue_record, "ABORTED", {})
436+ self.build.buildqueue_record,
437+ {"builder_status": "BuilderStatus.WAITING",
438+ "build_status": "BuildStatus.ABORTED"})
439
440 @defer.inlineCallbacks
441- def test_handleStatus_ABORTED_cancelling_sets_build_log(self):
442+ def test_handleStatus_WAITING_ABORTED_cancelling_sets_build_log(self):
443 # If a build is intentionally cancelled, the build log is set.
444 self.assertEqual(None, self.build.log)
445 with dbuser(config.builddmaster.dbuser):
446 self.build.updateStatus(BuildStatus.CANCELLING)
447 yield self.behaviour.handleStatus(
448- self.build.buildqueue_record, "ABORTED", {})
449+ self.build.buildqueue_record,
450+ {"builder_status": "BuilderStatus.WAITING",
451+ "build_status": "BuildStatus.ABORTED"})
452 self.assertNotEqual(None, self.build.log)
453
454 @defer.inlineCallbacks
455@@ -493,8 +549,10 @@ class TestHandleStatusMixin:
456 self.assertEqual(None, self.build.date_finished)
457 with dbuser(config.builddmaster.dbuser):
458 yield self.behaviour.handleStatus(
459- self.build.buildqueue_record, 'OK',
460- {'filemap': {'myfile.py': 'test_file_hash'}})
461+ self.build.buildqueue_record,
462+ {'builder_status': 'BuilderStatus.WAITING',
463+ 'build_status': 'BuildStatus.OK',
464+ 'filemap': {'myfile.py': 'test_file_hash'}})
465 self.assertNotEqual(None, self.build.date_finished)
466
467 @defer.inlineCallbacks
468@@ -504,7 +562,9 @@ class TestHandleStatusMixin:
469 "Build returned unexpected status: %r" % 'GIVENBACK'):
470 with dbuser(config.builddmaster.dbuser):
471 yield self.behaviour.handleStatus(
472- self.build.buildqueue_record, "GIVENBACK", {})
473+ self.build.buildqueue_record,
474+ {"builder_status": "BuilderStatus.WAITING",
475+ "build_status": "BuildStatus.GIVENBACK"})
476
477 @defer.inlineCallbacks
478 def test_builderfail_collection(self):
479@@ -513,7 +573,9 @@ class TestHandleStatusMixin:
480 "Build returned unexpected status: %r" % 'BUILDERFAIL'):
481 with dbuser(config.builddmaster.dbuser):
482 yield self.behaviour.handleStatus(
483- self.build.buildqueue_record, "BUILDERFAIL", {})
484+ self.build.buildqueue_record,
485+ {"builder_status": "BuilderStatus.WAITING",
486+ "build_status": "BuildStatus.BUILDERFAIL"})
487
488 @defer.inlineCallbacks
489 def test_invalid_status_collection(self):
490@@ -522,4 +584,6 @@ class TestHandleStatusMixin:
491 "Build returned unexpected status: %r" % 'BORKED'):
492 with dbuser(config.builddmaster.dbuser):
493 yield self.behaviour.handleStatus(
494- self.build.buildqueue_record, "BORKED", {})
495+ self.build.buildqueue_record,
496+ {"builder_status": "BuilderStatus.WAITING",
497+ "build_status": "BuildStatus.BORKED"})
498diff --git a/lib/lp/buildmaster/tests/test_interactor.py b/lib/lp/buildmaster/tests/test_interactor.py
499index 6e4a790..9aa025c 100644
500--- a/lib/lp/buildmaster/tests/test_interactor.py
501+++ b/lib/lp/buildmaster/tests/test_interactor.py
502@@ -1,4 +1,4 @@
503-# Copyright 2009-2020 Canonical Ltd. This software is licensed under the
504+# Copyright 2009-2022 Canonical Ltd. This software is licensed under the
505 # GNU Affero General Public License version 3 (see the file LICENSE).
506
507 """Test BuilderInteractor features."""
508@@ -121,21 +121,6 @@ class TestBuilderInteractor(TestCase):
509 super().setUp()
510 self.addCleanup(shut_down_default_process_pool)
511
512- def test_extractBuildStatus_baseline(self):
513- # extractBuildStatus picks the name of the build status out of a
514- # dict describing the worker's status.
515- worker_status = {'build_status': 'BuildStatus.BUILDING'}
516- self.assertEqual(
517- 'BUILDING', BuilderInteractor.extractBuildStatus(worker_status))
518-
519- def test_extractBuildStatus_malformed(self):
520- # extractBuildStatus errors out when the status string is not
521- # of the form it expects.
522- worker_status = {'build_status': 'BUILDING'}
523- self.assertRaises(
524- AssertionError, BuilderInteractor.extractBuildStatus,
525- worker_status)
526-
527 def resumeWorkerHost(self, builder):
528 vitals = extract_vitals_from_db(builder)
529 return BuilderInteractor.resumeWorkerHost(
530diff --git a/lib/lp/code/model/tests/test_recipebuilder.py b/lib/lp/code/model/tests/test_recipebuilder.py
531index 9077a08..9c75d68 100644
532--- a/lib/lp/code/model/tests/test_recipebuilder.py
533+++ b/lib/lp/code/model/tests/test_recipebuilder.py
534@@ -1,4 +1,4 @@
535-# Copyright 2010-2020 Canonical Ltd. This software is licensed under the
536+# Copyright 2010-2022 Canonical Ltd. This software is licensed under the
537 # GNU Affero General Public License version 3 (see the file LICENSE).
538
539 """Test RecipeBuildBehaviour."""
540@@ -418,7 +418,11 @@ class TestBuildNotifications(TestCaseWithFactory):
541 self.queue_record, self.queue_record.builder, worker)
542
543 def assertDeferredNotifyCount(self, status, behaviour, expected_count):
544- d = behaviour.handleStatus(self.queue_record, status, {'filemap': {}})
545+ d = behaviour.handleStatus(
546+ self.queue_record,
547+ {'builder_status': 'BuilderStatus.WAITING',
548+ 'build_status': 'BuildStatus.%s' % status,
549+ 'filemap': {}})
550
551 def cb(result):
552 self.assertEqual(expected_count, len(pop_notifications()))
553diff --git a/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py b/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
554index 7804583..cb5438f 100644
555--- a/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
556+++ b/lib/lp/oci/tests/test_ocirecipebuildbehaviour.py
557@@ -1,4 +1,4 @@
558-# Copyright 2015-2021 Canonical Ltd. This software is licensed under the
559+# Copyright 2015-2022 Canonical Ltd. This software is licensed under the
560 # GNU Affero General Public License version 3 (see the file LICENSE).
561
562 """Tests for `OCIRecipeBuildBehaviour`."""
563@@ -55,6 +55,7 @@ from lp.buildmaster.interfaces.builder import (
564 from lp.buildmaster.interfaces.buildfarmjobbehaviour import (
565 IBuildFarmJobBehaviour,
566 )
567+from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
568 from lp.buildmaster.interfaces.processor import IProcessorSet
569 from lp.buildmaster.tests.builderproxy import (
570 InProcessProxyAuthAPIFixture,
571@@ -773,15 +774,40 @@ class TestHandleStatusForOCIRecipeBuild(MakeOCIBuildMixin,
572 1, len(os.listdir(os.path.join(self.upload_root, result))))
573
574 @defer.inlineCallbacks
575- def test_handleStatus_OK_normal_image(self):
576+ def test_handleStatus_BUILDING(self):
577+ # If the builder is BUILDING (or any status other than WAITING),
578+ # then the behaviour calls updateStatus but doesn't do anything
579+ # else.
580+ initial_status = self.build.status
581+ bq_id = self.build.buildqueue_record.id
582+ worker_status = {"builder_status": "BuilderStatus.BUILDING"}
583+ removeSecurityProxy(self.build).updateStatus = FakeMethod()
584+ with dbuser(config.builddmaster.dbuser):
585+ yield self.behaviour.handleStatus(
586+ self.build.buildqueue_record, worker_status)
587+ self.assertEqual(None, self.build.log)
588+ self.assertEqual(0, len(os.listdir(self.upload_root)))
589+ self.assertEqual(
590+ [((initial_status,),
591+ {"builder": self.builder, "worker_status": worker_status})],
592+ removeSecurityProxy(self.build).updateStatus.calls)
593+ self.assertEqual(0, len(pop_notifications()), "Notifications received")
594+ self.assertEqual(
595+ self.build.buildqueue_record,
596+ getUtility(IBuildQueueSet).get(bq_id))
597+
598+ @defer.inlineCallbacks
599+ def test_handleStatus_WAITING_OK_normal_image(self):
600 now = datetime.now()
601 mock_datetime = self.useFixture(MockPatch(
602 'lp.buildmaster.model.buildfarmjobbehaviour.datetime')).mock
603 mock_datetime.now = lambda: now
604 with dbuser(config.builddmaster.dbuser):
605 yield self.behaviour.handleStatus(
606- self.build.buildqueue_record, 'OK',
607- {'filemap': self.filemap})
608+ self.build.buildqueue_record,
609+ {'builder_status': 'BuilderStatus.WAITING',
610+ 'build_status': 'BuildStatus.OK',
611+ 'filemap': self.filemap})
612 self.assertEqual(
613 ['buildlog', 'manifest_hash', 'digests_hash', 'config_1_hash',
614 'layer_2_hash'],
615@@ -805,7 +831,7 @@ class TestHandleStatusForOCIRecipeBuild(MakeOCIBuildMixin,
616 self.assertEqual(contents, b'retrieved from librarian')
617
618 @defer.inlineCallbacks
619- def test_handleStatus_OK_reuse_from_other_build(self):
620+ def test_handleStatus_WAITING_OK_reuse_from_other_build(self):
621 """We should be able to reuse a layer file from a separate build."""
622 oci_file = self.factory.makeOCIFile(
623 layer_file_digest='digest_2',
624@@ -820,8 +846,10 @@ class TestHandleStatusForOCIRecipeBuild(MakeOCIBuildMixin,
625 mock_oci_datetime.now = lambda tzinfo=None: now
626 with dbuser(config.builddmaster.dbuser):
627 yield self.behaviour.handleStatus(
628- self.build.buildqueue_record, 'OK',
629- {'filemap': self.filemap})
630+ self.build.buildqueue_record,
631+ {'builder_status': 'BuilderStatus.WAITING',
632+ 'build_status': 'BuildStatus.OK',
633+ 'filemap': self.filemap})
634 self.assertEqual(
635 ['buildlog', 'manifest_hash', 'digests_hash', 'config_1_hash'],
636 self.worker._got_file_record)
637@@ -844,7 +872,7 @@ class TestHandleStatusForOCIRecipeBuild(MakeOCIBuildMixin,
638 self.assertEqual(now, oci_file.date_last_used)
639
640 @defer.inlineCallbacks
641- def test_handleStatus_OK_absolute_filepath(self):
642+ def test_handleStatus_WAITING_OK_absolute_filepath(self):
643
644 self._createTestFile(
645 'manifest.json',
646@@ -862,11 +890,13 @@ class TestHandleStatusForOCIRecipeBuild(MakeOCIBuildMixin,
647 "'/notvalid/config_file_1.json'."):
648 with dbuser(config.builddmaster.dbuser):
649 yield self.behaviour.handleStatus(
650- self.build.buildqueue_record, 'OK',
651- {'filemap': self.filemap})
652+ self.build.buildqueue_record,
653+ {'builder_status': 'BuilderStatus.WAITING',
654+ 'build_status': 'BuildStatus.OK',
655+ 'filemap': self.filemap})
656
657 @defer.inlineCallbacks
658- def test_handleStatus_OK_relative_filepath(self):
659+ def test_handleStatus_WAITING_OK_relative_filepath(self):
660
661 self._createTestFile(
662 'manifest.json',
663@@ -882,30 +912,36 @@ class TestHandleStatusForOCIRecipeBuild(MakeOCIBuildMixin,
664 "Build returned a file named '../config_file_1.json'."):
665 with dbuser(config.builddmaster.dbuser):
666 yield self.behaviour.handleStatus(
667- self.build.buildqueue_record, 'OK',
668- {'filemap': self.filemap})
669+ self.build.buildqueue_record,
670+ {'builder_status': 'BuilderStatus.WAITING',
671+ 'build_status': 'BuildStatus.OK',
672+ 'filemap': self.filemap})
673
674 @defer.inlineCallbacks
675- def test_handleStatus_OK_sets_build_log(self):
676+ def test_handleStatus_WAITING_OK_sets_build_log(self):
677 # The build log is set during handleStatus.
678 self.assertEqual(None, self.build.log)
679 with dbuser(config.builddmaster.dbuser):
680 yield self.behaviour.handleStatus(
681- self.build.buildqueue_record, 'OK',
682- {'filemap': self.filemap})
683+ self.build.buildqueue_record,
684+ {'builder_status': 'BuilderStatus.WAITING',
685+ 'build_status': 'BuildStatus.OK',
686+ 'filemap': self.filemap})
687 self.assertNotEqual(None, self.build.log)
688
689 @defer.inlineCallbacks
690- def test_handleStatus_ABORTED_cancels_cancelling(self):
691+ def test_handleStatus_WAITING_ABORTED_cancels_cancelling(self):
692 with dbuser(config.builddmaster.dbuser):
693 self.build.updateStatus(BuildStatus.CANCELLING)
694 yield self.behaviour.handleStatus(
695- self.build.buildqueue_record, "ABORTED", {})
696+ self.build.buildqueue_record,
697+ {"builder_status": "BuilderStatus.WAITING",
698+ "build_status": "BuildStatus.ABORTED"})
699 self.assertEqual(0, len(pop_notifications()), "Notifications received")
700 self.assertEqual(BuildStatus.CANCELLED, self.build.status)
701
702 @defer.inlineCallbacks
703- def test_handleStatus_ABORTED_illegal_when_building(self):
704+ def test_handleStatus_WAITING_ABORTED_illegal_when_building(self):
705 self.builder.vm_host = "fake_vm_host"
706 self.behaviour = self.interactor.getBuildBehaviour(
707 self.build.buildqueue_record, self.builder, self.worker)
708@@ -915,16 +951,20 @@ class TestHandleStatusForOCIRecipeBuild(MakeOCIBuildMixin,
709 BuildDaemonError,
710 "Build returned unexpected status: %r" % 'ABORTED'):
711 yield self.behaviour.handleStatus(
712- self.build.buildqueue_record, "ABORTED", {})
713+ self.build.buildqueue_record,
714+ {"builder_status": "BuilderStatus.WAITING",
715+ "build_status": "BuildStatus.ABORTED"})
716
717 @defer.inlineCallbacks
718- def test_handleStatus_ABORTED_cancelling_sets_build_log(self):
719+ def test_handleStatus_WAITING_ABORTED_cancelling_sets_build_log(self):
720 # If a build is intentionally cancelled, the build log is set.
721 self.assertEqual(None, self.build.log)
722 with dbuser(config.builddmaster.dbuser):
723 self.build.updateStatus(BuildStatus.CANCELLING)
724 yield self.behaviour.handleStatus(
725- self.build.buildqueue_record, "ABORTED", {})
726+ self.build.buildqueue_record,
727+ {"builder_status": "BuilderStatus.WAITING",
728+ "build_status": "BuildStatus.ABORTED"})
729 self.assertNotEqual(None, self.build.log)
730
731 @defer.inlineCallbacks
732@@ -933,8 +973,10 @@ class TestHandleStatusForOCIRecipeBuild(MakeOCIBuildMixin,
733 self.assertEqual(None, self.build.date_finished)
734 with dbuser(config.builddmaster.dbuser):
735 yield self.behaviour.handleStatus(
736- self.build.buildqueue_record, 'OK',
737- {'filemap': self.filemap})
738+ self.build.buildqueue_record,
739+ {'builder_status': 'BuilderStatus.WAITING',
740+ 'build_status': 'BuildStatus.OK',
741+ 'filemap': self.filemap})
742 self.assertNotEqual(None, self.build.date_finished)
743
744 @defer.inlineCallbacks
745@@ -944,7 +986,9 @@ class TestHandleStatusForOCIRecipeBuild(MakeOCIBuildMixin,
746 "Build returned unexpected status: %r" % 'GIVENBACK'):
747 with dbuser(config.builddmaster.dbuser):
748 yield self.behaviour.handleStatus(
749- self.build.buildqueue_record, "GIVENBACK", {})
750+ self.build.buildqueue_record,
751+ {"builder_status": "BuilderStatus.WAITING",
752+ "build_status": "BuildStatus.GIVENBACK"})
753
754 @defer.inlineCallbacks
755 def test_builderfail_collection(self):
756@@ -953,7 +997,9 @@ class TestHandleStatusForOCIRecipeBuild(MakeOCIBuildMixin,
757 "Build returned unexpected status: %r" % 'BUILDERFAIL'):
758 with dbuser(config.builddmaster.dbuser):
759 yield self.behaviour.handleStatus(
760- self.build.buildqueue_record, "BUILDERFAIL", {})
761+ self.build.buildqueue_record,
762+ {"builder_status": "BuilderStatus.WAITING",
763+ "build_status": "BuildStatus.BUILDERFAIL"})
764
765 @defer.inlineCallbacks
766 def test_invalid_status_collection(self):
767@@ -962,7 +1008,9 @@ class TestHandleStatusForOCIRecipeBuild(MakeOCIBuildMixin,
768 "Build returned unexpected status: %r" % 'BORKED'):
769 with dbuser(config.builddmaster.dbuser):
770 yield self.behaviour.handleStatus(
771- self.build.buildqueue_record, "BORKED", {})
772+ self.build.buildqueue_record,
773+ {"builder_status": "BuilderStatus.WAITING",
774+ "build_status": "BuildStatus.BORKED"})
775
776
777 class TestGetUploadMethodsForOCIRecipeBuild(
778diff --git a/lib/lp/translations/tests/test_translationtemplatesbuildbehaviour.py b/lib/lp/translations/tests/test_translationtemplatesbuildbehaviour.py
779index 634ed1a..5b532e5 100644
780--- a/lib/lp/translations/tests/test_translationtemplatesbuildbehaviour.py
781+++ b/lib/lp/translations/tests/test_translationtemplatesbuildbehaviour.py
782@@ -1,4 +1,4 @@
783-# Copyright 2010-2020 Canonical Ltd. This software is licensed under the
784+# Copyright 2010-2022 Canonical Ltd. This software is licensed under the
785 # GNU Affero General Public License version 3 (see the file LICENSE).
786
787 """Unit tests for TranslationTemplatesBuildBehaviour."""
788@@ -14,7 +14,6 @@ from zope.component import getUtility
789
790 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
791 from lp.buildmaster.enums import BuildStatus
792-from lp.buildmaster.interactor import BuilderInteractor
793 from lp.buildmaster.interfaces.buildfarmjobbehaviour import (
794 IBuildFarmJobBehaviour,
795 )
796@@ -164,11 +163,7 @@ class TestTranslationTemplatesBuildBehaviour(
797 d = worker.status()
798
799 def got_status(status):
800- return (
801- behaviour.handleStatus(
802- queue_item, BuilderInteractor.extractBuildStatus(status),
803- status),
804- worker.call_log)
805+ return behaviour.handleStatus(queue_item, status), worker.call_log
806
807 def build_updated(ignored):
808 self.assertEqual(BuildStatus.FULLYBUILT, behaviour.build.status)
809@@ -193,10 +188,7 @@ class TestTranslationTemplatesBuildBehaviour(
810
811 def got_status(status):
812 del status['filemap']
813- return behaviour.handleStatus(
814- queue_item,
815- BuilderInteractor.extractBuildStatus(status),
816- status),
817+ return behaviour.handleStatus(queue_item, status),
818
819 def build_updated(ignored):
820 self.assertEqual(BuildStatus.FAILEDTOBUILD, behaviour.build.status)
821@@ -222,10 +214,7 @@ class TestTranslationTemplatesBuildBehaviour(
822
823 def got_status(status):
824 del status['filemap']
825- return behaviour.handleStatus(
826- queue_item,
827- BuilderInteractor.extractBuildStatus(status),
828- status),
829+ return behaviour.handleStatus(queue_item, status),
830
831 def build_updated(ignored):
832 self.assertEqual(BuildStatus.FULLYBUILT, behaviour.build.status)
833@@ -257,10 +246,7 @@ class TestTranslationTemplatesBuildBehaviour(
834 d = worker.status()
835
836 def got_status(status):
837- return behaviour.handleStatus(
838- queue_item,
839- BuilderInteractor.extractBuildStatus(status),
840- status),
841+ return behaviour.handleStatus(queue_item, status),
842
843 def build_updated(ignored):
844 self.assertEqual(BuildStatus.FULLYBUILT, behaviour.build.status)

Subscribers

People subscribed via source and target branches

to status/vote changes: