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