Merge lp:~blake-rouse/maas/fix-cluster-image-syncing into lp:~maas-committers/maas/trunk
- fix-cluster-image-syncing
- Merge into trunk
Proposed by
Blake Rouse
Status: | Merged |
---|---|
Approved by: | Blake Rouse |
Approved revision: | no longer in the source branch. |
Merged at revision: | 2960 |
Proposed branch: | lp:~blake-rouse/maas/fix-cluster-image-syncing |
Merge into: | lp:~maas-committers/maas/trunk |
Diff against target: |
529 lines (+65/-217) 10 files modified
src/maasserver/rpc/bootsources.py (+0/-46) src/maasserver/rpc/regionservice.py (+3/-7) src/maasserver/rpc/tests/test_bootsources.py (+0/-42) src/maasserver/rpc/tests/test_regionservice.py (+9/-49) src/provisioningserver/import_images/boot_resources.py (+6/-8) src/provisioningserver/import_images/tests/test_boot_resources.py (+1/-2) src/provisioningserver/pserv_services/image_download_service.py (+7/-19) src/provisioningserver/pserv_services/tests/test_image_download_service.py (+3/-40) src/provisioningserver/rpc/boot_images.py (+6/-3) src/provisioningserver/rpc/tests/test_boot_images.py (+30/-1) |
To merge this branch: | bzr merge lp:~blake-rouse/maas/fix-cluster-image-syncing |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gavin Panella (community) | Approve | ||
Review via email: mp+234192@code.launchpad.net |
Commit message
Fix GetBootSources* RPC to return simplestreams endpoint on the region. Move the service_lock into rpc import images, so other RPC calls will block from running the import at the same time. Cleanup log messages from the import on the cluster side.
Description of the change
To post a comment you must log in.
Revision history for this message
Blake Rouse (blake-rouse) : | # |
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === removed file 'src/maasserver/rpc/bootsources.py' |
2 | --- src/maasserver/rpc/bootsources.py 2014-08-26 17:32:40 +0000 |
3 | +++ src/maasserver/rpc/bootsources.py 1970-01-01 00:00:00 +0000 |
4 | @@ -1,46 +0,0 @@ |
5 | -# Copyright 2014 Canonical Ltd. This software is licensed under the |
6 | -# GNU Affero General Public License version 3 (see the file LICENSE). |
7 | - |
8 | -"""RPC helpers relating to boot sources.""" |
9 | - |
10 | -from __future__ import ( |
11 | - absolute_import, |
12 | - print_function, |
13 | - unicode_literals, |
14 | - ) |
15 | - |
16 | -str = None |
17 | - |
18 | -__metaclass__ = type |
19 | -__all__ = [ |
20 | - "get_boot_sources", |
21 | -] |
22 | - |
23 | -from maasserver.models import BootSource |
24 | -from maasserver.utils.async import transactional |
25 | -from provisioningserver.utils.twisted import synchronous |
26 | - |
27 | - |
28 | -@synchronous |
29 | -@transactional |
30 | -def get_boot_sources(uuid, remove_os=False): |
31 | - """Obtain boot sources and selections from the database. |
32 | - |
33 | - Returns them as a structure suitable for returning in the response |
34 | - for :py:class:`~provisioningserver.rpc.region.GetBootSources`. |
35 | - |
36 | - :param remove_os: Remove the os field from selections. This is |
37 | - used for v1 of get_boot_sources, as it should not include the os field. |
38 | - For v2 of get_boot_sources, the os field is included. |
39 | - """ |
40 | - # No longer is uuid used for this, as its now global. The uuid is just |
41 | - # ignored. |
42 | - sources = [ |
43 | - source.to_dict() |
44 | - for source in BootSource.objects.all() |
45 | - ] |
46 | - for source in sources: |
47 | - if remove_os: |
48 | - for selection in source['selections']: |
49 | - del selection['os'] |
50 | - return sources |
51 | |
52 | === modified file 'src/maasserver/rpc/regionservice.py' |
53 | --- src/maasserver/rpc/regionservice.py 2014-09-05 08:10:06 +0000 |
54 | +++ src/maasserver/rpc/regionservice.py 2014-09-11 13:00:18 +0000 |
55 | @@ -30,8 +30,8 @@ |
56 | eventloop, |
57 | locks, |
58 | ) |
59 | +from maasserver.bootresources import get_simplestream_endpoint |
60 | from maasserver.rpc import ( |
61 | - bootsources, |
62 | configuration, |
63 | events, |
64 | leases, |
65 | @@ -133,9 +133,7 @@ |
66 | Implementation of |
67 | :py:class:`~provisioningserver.rpc.region.GetBootSources`. |
68 | """ |
69 | - d = deferToThread(bootsources.get_boot_sources, uuid, remove_os=True) |
70 | - d.addCallback(lambda sources: {b"sources": sources}) |
71 | - return d |
72 | + return {b"sources": [get_simplestream_endpoint()]} |
73 | |
74 | @region.GetBootSourcesV2.responder |
75 | def get_boot_sources_v2(self, uuid): |
76 | @@ -144,9 +142,7 @@ |
77 | Implementation of |
78 | :py:class:`~provisioningserver.rpc.region.GetBootSources`. |
79 | """ |
80 | - d = deferToThread(bootsources.get_boot_sources, uuid) |
81 | - d.addCallback(lambda sources: {b"sources": sources}) |
82 | - return d |
83 | + return {b"sources": [get_simplestream_endpoint()]} |
84 | |
85 | @region.GetArchiveMirrors.responder |
86 | def get_archive_mirrors(self): |
87 | |
88 | === removed file 'src/maasserver/rpc/tests/test_bootsources.py' |
89 | --- src/maasserver/rpc/tests/test_bootsources.py 2014-09-05 16:38:32 +0000 |
90 | +++ src/maasserver/rpc/tests/test_bootsources.py 1970-01-01 00:00:00 +0000 |
91 | @@ -1,42 +0,0 @@ |
92 | -# Copyright 2014 Canonical Ltd. This software is licensed under the |
93 | -# GNU Affero General Public License version 3 (see the file LICENSE). |
94 | - |
95 | -"""Tests for :py:module:`~maasserver.rpc.bootsources`.""" |
96 | - |
97 | -from __future__ import ( |
98 | - absolute_import, |
99 | - print_function, |
100 | - unicode_literals, |
101 | - ) |
102 | - |
103 | -str = None |
104 | - |
105 | -__metaclass__ = type |
106 | -__all__ = [] |
107 | - |
108 | -from maasserver.rpc.bootsources import get_boot_sources |
109 | -from maasserver.testing.factory import factory |
110 | -from maasserver.testing.testcase import MAASServerTestCase |
111 | - |
112 | - |
113 | -class TestGetBootSources(MAASServerTestCase): |
114 | - |
115 | - def test_returns_boot_sources_and_selections(self): |
116 | - keyring = factory.make_bytes() |
117 | - nodegroup = factory.make_NodeGroup() |
118 | - source = factory.make_BootSource(keyring_data=keyring) |
119 | - factory.make_BootSourceSelection(source) |
120 | - |
121 | - expected = source.to_dict() |
122 | - self.assertEqual([expected], get_boot_sources(nodegroup.uuid)) |
123 | - |
124 | - def test_removes_os_from_boot_source_selections(self): |
125 | - keyring = factory.make_bytes() |
126 | - nodegroup = factory.make_NodeGroup() |
127 | - source = factory.make_BootSource(keyring_data=keyring) |
128 | - factory.make_BootSourceSelection(source) |
129 | - |
130 | - expected = source.to_dict() |
131 | - del expected['selections'][0]['os'] |
132 | - self.assertEqual( |
133 | - [expected], get_boot_sources(nodegroup.uuid, remove_os=True)) |
134 | |
135 | === modified file 'src/maasserver/rpc/tests/test_regionservice.py' |
136 | --- src/maasserver/rpc/tests/test_regionservice.py 2014-09-08 20:07:07 +0000 |
137 | +++ src/maasserver/rpc/tests/test_regionservice.py 2014-09-11 13:00:18 +0000 |
138 | @@ -31,6 +31,7 @@ |
139 | eventloop, |
140 | locks, |
141 | ) |
142 | +from maasserver.bootresources import get_simplestream_endpoint |
143 | from maasserver.enum import ( |
144 | NODE_STATUS, |
145 | POWER_STATE, |
146 | @@ -331,37 +332,18 @@ |
147 | self.assertIsNot(responder, None) |
148 | |
149 | @wait_for_reactor |
150 | - def test_get_boot_sources_can_be_called(self): |
151 | + def test_get_boot_sources_returns_simplestreams_endpoint(self): |
152 | uuid = factory.make_name("uuid") |
153 | |
154 | d = call_responder(Region(), GetBootSources, {b"uuid": uuid}) |
155 | |
156 | def check(response): |
157 | - self.assertEqual({b"sources": []}, response) |
158 | + self.assertEqual( |
159 | + {b"sources": [get_simplestream_endpoint()]}, |
160 | + response) |
161 | |
162 | return d.addCallback(check) |
163 | |
164 | - @transactional |
165 | - def make_boot_source_selection(self, keyring): |
166 | - nodegroup = factory.make_NodeGroup() |
167 | - boot_source = factory.make_BootSource(keyring_data=keyring) |
168 | - factory.make_BootSourceSelection(boot_source) |
169 | - return nodegroup.uuid, boot_source.to_dict() |
170 | - |
171 | - @wait_for_reactor |
172 | - @inlineCallbacks |
173 | - def test_get_boot_sources_with_real_cluster(self): |
174 | - keyring = factory.make_bytes() |
175 | - |
176 | - uuid, boot_source = yield deferToThread( |
177 | - self.make_boot_source_selection, keyring) |
178 | - del boot_source['selections'][0]['os'] |
179 | - |
180 | - response = yield call_responder( |
181 | - Region(), GetBootSources, {b"uuid": uuid}) |
182 | - |
183 | - self.assertEqual({b"sources": [boot_source]}, response) |
184 | - |
185 | |
186 | class TestRegionProtocol_GetBootSourcesV2(TransactionTestCase): |
187 | |
188 | @@ -371,40 +353,18 @@ |
189 | self.assertIsNot(responder, None) |
190 | |
191 | @wait_for_reactor |
192 | - def test_get_boot_sources_v2_can_be_called(self): |
193 | + def test_get_boot_sources_v2_returns_simplestreams_endpoint(self): |
194 | uuid = factory.make_name("uuid") |
195 | |
196 | d = call_responder(Region(), GetBootSourcesV2, {b"uuid": uuid}) |
197 | |
198 | def check(response): |
199 | - self.assertEqual({b"sources": []}, response) |
200 | + self.assertEqual( |
201 | + {b"sources": [get_simplestream_endpoint()]}, |
202 | + response) |
203 | |
204 | return d.addCallback(check) |
205 | |
206 | - @transactional |
207 | - def make_boot_source_selection(self, keyring): |
208 | - nodegroup = factory.make_NodeGroup() |
209 | - boot_source = factory.make_BootSource(keyring_data=keyring) |
210 | - factory.make_BootSourceSelection(boot_source) |
211 | - return nodegroup.uuid, boot_source.to_dict() |
212 | - |
213 | - @wait_for_reactor |
214 | - @inlineCallbacks |
215 | - def test_get_boot_sources_v2_with_real_cluster(self): |
216 | - keyring = factory.make_bytes() |
217 | - |
218 | - uuid, boot_source = yield deferToThread( |
219 | - self.make_boot_source_selection, keyring) |
220 | - |
221 | - # keyring_data contains the b64decoded representation since AMP |
222 | - # is fine with bytes. |
223 | - boot_source["keyring_data"] = keyring |
224 | - |
225 | - response = yield call_responder( |
226 | - Region(), GetBootSourcesV2, {b"uuid": uuid}) |
227 | - |
228 | - self.assertEqual({b"sources": [boot_source]}, response) |
229 | - |
230 | |
231 | class TestRegionProtocol_GetArchiveMirrors(MAASTestCase): |
232 | |
233 | |
234 | === modified file 'src/provisioningserver/import_images/boot_resources.py' |
235 | --- src/provisioningserver/import_images/boot_resources.py 2014-09-08 06:37:02 +0000 |
236 | +++ src/provisioningserver/import_images/boot_resources.py 2014-09-11 13:00:18 +0000 |
237 | @@ -222,7 +222,7 @@ |
238 | :param config: An iterable of dicts representing the sources from |
239 | which boot images will be downloaded. |
240 | """ |
241 | - maaslog.info("Importing boot resources.") |
242 | + maaslog.info("Started importing boot images.") |
243 | if len(sources) == 0: |
244 | maaslog.warn("Can't import: no Simplestreams sources selected.") |
245 | return |
246 | @@ -236,15 +236,13 @@ |
247 | image_descriptions = download_all_image_descriptions(sources) |
248 | if image_descriptions.is_empty(): |
249 | maaslog.warn( |
250 | - "No boot resources found. " |
251 | - "Check sources specification and connectivity.") |
252 | + "Finished importing boot images, no boot images available.") |
253 | return |
254 | |
255 | storage = provisioningserver.config.BOOT_RESOURCES_STORAGE |
256 | meta_file_content = image_descriptions.dump_json() |
257 | if meta_contains(storage, meta_file_content): |
258 | - # The current maas.meta already contains the new config. No need |
259 | - # to rewrite anything. |
260 | + maaslog.info("Finished importing boot images, no new images.") |
261 | return |
262 | |
263 | product_mapping = map_products(image_descriptions) |
264 | @@ -252,7 +250,7 @@ |
265 | snapshot_path = download_all_boot_resources( |
266 | sources, storage, product_mapping) |
267 | |
268 | - maaslog.info("Writing metadata and iSCSI targets.") |
269 | + maaslog.info("Writing boot image metadata and iSCSI targets.") |
270 | write_snapshot_metadata(snapshot_path, meta_file_content) |
271 | write_targets_conf(snapshot_path) |
272 | |
273 | @@ -261,9 +259,9 @@ |
274 | |
275 | # If we got here, all went well. This is now truly the "current" snapshot. |
276 | update_current_symlink(storage, snapshot_path) |
277 | - maaslog.info("Updating iSCSI targets.") |
278 | + maaslog.info("Updating boot image iSCSI targets.") |
279 | update_targets_conf(snapshot_path) |
280 | - maaslog.info("Import done.") |
281 | + maaslog.info("Finished importing boot images.") |
282 | |
283 | |
284 | def main(args): |
285 | |
286 | === modified file 'src/provisioningserver/import_images/tests/test_boot_resources.py' |
287 | --- src/provisioningserver/import_images/tests/test_boot_resources.py 2014-09-08 04:08:54 +0000 |
288 | +++ src/provisioningserver/import_images/tests/test_boot_resources.py 2014-09-11 13:00:18 +0000 |
289 | @@ -366,8 +366,7 @@ |
290 | self.assertThat( |
291 | boot_resources.maaslog.warn, |
292 | MockAnyCall( |
293 | - "No boot resources found. " |
294 | - "Check sources specification and connectivity.")) |
295 | + "Finished importing boot images, no boot images available.")) |
296 | |
297 | def test_raises_ioerror_when_no_sources_file_found(self): |
298 | self.patch_maaslog() |
299 | |
300 | === modified file 'src/provisioningserver/pserv_services/image_download_service.py' |
301 | --- src/provisioningserver/pserv_services/image_download_service.py 2014-08-27 06:53:37 +0000 |
302 | +++ src/provisioningserver/pserv_services/image_download_service.py 2014-09-11 13:00:18 +0000 |
303 | @@ -33,7 +33,6 @@ |
304 | ) |
305 | from twisted.application.internet import TimerService |
306 | from twisted.internet.defer import ( |
307 | - DeferredLock, |
308 | inlineCallbacks, |
309 | returnValue, |
310 | ) |
311 | @@ -42,7 +41,6 @@ |
312 | |
313 | |
314 | maaslog = get_maas_logger("boot_image_download_service") |
315 | -service_lock = DeferredLock() |
316 | |
317 | |
318 | class PeriodicImageDownloadService(TimerService, object): |
319 | @@ -117,21 +115,11 @@ |
320 | """Check the time the last image refresh happened and initiate a new |
321 | one if older than 15 minutes. |
322 | """ |
323 | - # Use a DeferredLock to prevent simultaneous downloads. |
324 | - if service_lock.locked: |
325 | - # Don't want to block on lock release. |
326 | + last_modified = tftppath.maas_meta_last_modified() |
327 | + if last_modified is None: |
328 | + yield self._start_download() |
329 | return |
330 | - yield service_lock.acquire() |
331 | - try: |
332 | - last_modified = tftppath.maas_meta_last_modified() |
333 | - if last_modified is None: |
334 | - # Don't auto-refresh if the user has never manually initiated |
335 | - # a download. |
336 | - return |
337 | - |
338 | - age_in_seconds = self.clock.seconds() - last_modified |
339 | - if age_in_seconds >= timedelta(minutes=15).total_seconds(): |
340 | - yield self._start_download() |
341 | - |
342 | - finally: |
343 | - service_lock.release() |
344 | + |
345 | + age_in_seconds = self.clock.seconds() - last_modified |
346 | + if age_in_seconds >= timedelta(minutes=15).total_seconds(): |
347 | + yield self._start_download() |
348 | |
349 | === modified file 'src/provisioningserver/pserv_services/tests/test_image_download_service.py' |
350 | --- src/provisioningserver/pserv_services/tests/test_image_download_service.py 2014-09-01 08:40:13 +0000 |
351 | +++ src/provisioningserver/pserv_services/tests/test_image_download_service.py 2014-09-11 13:00:18 +0000 |
352 | @@ -34,7 +34,6 @@ |
353 | from provisioningserver.boot import tftppath |
354 | from provisioningserver.pserv_services.image_download_service import ( |
355 | PeriodicImageDownloadService, |
356 | - service_lock, |
357 | ) |
358 | from provisioningserver.rpc import boot_images |
359 | from provisioningserver.rpc.boot_images import _run_import |
360 | @@ -45,7 +44,6 @@ |
361 | ) |
362 | from provisioningserver.rpc.testing import TwistedLoggerFixture |
363 | from provisioningserver.testing.testcase import PservTestCase |
364 | -from provisioningserver.utils.twisted import pause |
365 | from testtools.deferredruntest import extract_result |
366 | from twisted.application.internet import TimerService |
367 | from twisted.internet import defer |
368 | @@ -99,7 +97,7 @@ |
369 | clock.advance(service.check_interval) |
370 | self.assertEqual(3, len(get_mock_calls(maas_meta_last_modified))) |
371 | |
372 | - def test_no_download_if_no_meta_file(self): |
373 | + def test_initiates_download_if_no_meta_file(self): |
374 | clock = Clock() |
375 | service = PeriodicImageDownloadService( |
376 | sentinel.service, clock, sentinel.uuid) |
377 | @@ -108,7 +106,7 @@ |
378 | tftppath, |
379 | 'maas_meta_last_modified').return_value = None |
380 | service.startService() |
381 | - self.assertThat(_start_download, MockNotCalled()) |
382 | + self.assertThat(_start_download, MockCalledOnceWith()) |
383 | |
384 | def test_initiates_download_if_15_minutes_has_passed(self): |
385 | clock = Clock() |
386 | @@ -165,7 +163,7 @@ |
387 | def test_no_download_if_no_rpc_connections(self): |
388 | rpc_client = Mock() |
389 | failure = NoConnectionsAvailable() |
390 | - rpc_client.getClient.return_value.side_effect = failure |
391 | + rpc_client.getClient.side_effect = failure |
392 | |
393 | deferToThread = self.patch(boot_images, 'deferToThread') |
394 | service = PeriodicImageDownloadService( |
395 | @@ -173,41 +171,6 @@ |
396 | service.startService() |
397 | self.assertThat(deferToThread, MockNotCalled()) |
398 | |
399 | - @defer.inlineCallbacks |
400 | - def test_does_not_run_if_lock_taken(self): |
401 | - maas_meta_last_modified = self.patch( |
402 | - tftppath, 'maas_meta_last_modified') |
403 | - yield service_lock.acquire() |
404 | - self.addCleanup(service_lock.release) |
405 | - service = PeriodicImageDownloadService( |
406 | - sentinel.rpc, Clock(), sentinel.uuid) |
407 | - service.startService() |
408 | - self.assertThat(maas_meta_last_modified, MockNotCalled()) |
409 | - |
410 | - def test_takes_lock_when_running(self): |
411 | - clock = Clock() |
412 | - service = PeriodicImageDownloadService( |
413 | - sentinel.rpc, clock, sentinel.uuid) |
414 | - |
415 | - # Patch the download func so it's just a Deferred that waits for |
416 | - # one second. |
417 | - _start_download = self.patch(service, '_start_download') |
418 | - _start_download.return_value = pause(1, clock) |
419 | - |
420 | - # Set conditions for a required download: |
421 | - one_week_ago = clock.seconds() - timedelta(minutes=15).total_seconds() |
422 | - self.patch( |
423 | - tftppath, |
424 | - 'maas_meta_last_modified').return_value = one_week_ago |
425 | - |
426 | - # Lock is acquired for the first download after startup. |
427 | - service.startService() |
428 | - self.assertTrue(service_lock.locked) |
429 | - |
430 | - # Lock is released once the download is done. |
431 | - clock.advance(1) |
432 | - self.assertFalse(service_lock.locked) |
433 | - |
434 | def test_logs_other_errors(self): |
435 | service = PeriodicImageDownloadService( |
436 | sentinel.rpc, Clock(), sentinel.uuid) |
437 | |
438 | === modified file 'src/provisioningserver/rpc/boot_images.py' |
439 | --- src/provisioningserver/rpc/boot_images.py 2014-09-02 06:21:04 +0000 |
440 | +++ src/provisioningserver/rpc/boot_images.py 2014-09-11 13:00:18 +0000 |
441 | @@ -24,9 +24,12 @@ |
442 | from provisioningserver.import_images import boot_resources |
443 | from provisioningserver.utils.env import environment_variables |
444 | from provisioningserver.utils.twisted import synchronous |
445 | -from twisted.internet.defer import inlineCallbacks |
446 | +from twisted.internet.defer import DeferredLock |
447 | from twisted.internet.threads import deferToThread |
448 | |
449 | +# Lock is used so more than one import is not running at the same time. |
450 | +import_lock = DeferredLock() |
451 | + |
452 | |
453 | def list_boot_images(): |
454 | """List the boot images that exist on the cluster.""" |
455 | @@ -47,7 +50,7 @@ |
456 | boot_resources.import_images(sources) |
457 | |
458 | |
459 | -@inlineCallbacks |
460 | def import_boot_images(sources): |
461 | """Imports the boot images from the given sources.""" |
462 | - yield deferToThread(_run_import, sources) |
463 | + if not import_lock.locked: |
464 | + return import_lock.run(deferToThread, _run_import, sources) |
465 | |
466 | === modified file 'src/provisioningserver/rpc/tests/test_boot_images.py' |
467 | --- src/provisioningserver/rpc/tests/test_boot_images.py 2014-09-08 20:25:42 +0000 |
468 | +++ src/provisioningserver/rpc/tests/test_boot_images.py 2014-09-11 13:00:18 +0000 |
469 | @@ -17,7 +17,10 @@ |
470 | import os |
471 | |
472 | from maastesting.factory import factory |
473 | -from maastesting.matchers import MockCalledOnceWith |
474 | +from maastesting.matchers import ( |
475 | + MockCalledOnceWith, |
476 | + MockNotCalled, |
477 | + ) |
478 | from maastesting.testcase import MAASTwistedRunTest |
479 | from mock import sentinel |
480 | from provisioningserver.boot import tftppath |
481 | @@ -27,11 +30,14 @@ |
482 | from provisioningserver.rpc.boot_images import ( |
483 | _run_import, |
484 | import_boot_images, |
485 | + import_lock, |
486 | list_boot_images, |
487 | ) |
488 | from provisioningserver.testing.config import BootSourcesFixture |
489 | from provisioningserver.testing.testcase import PservTestCase |
490 | +from provisioningserver.utils.twisted import pause |
491 | from twisted.internet import defer |
492 | +from twisted.internet.task import Clock |
493 | |
494 | |
495 | class TestListBootImages(PservTestCase): |
496 | @@ -113,6 +119,16 @@ |
497 | run_tests_with = MAASTwistedRunTest.make_factory(timeout=5) |
498 | |
499 | @defer.inlineCallbacks |
500 | + def test__does_not_run_if_lock_taken(self): |
501 | + yield import_lock.acquire() |
502 | + self.addCleanup(import_lock.release) |
503 | + deferToThread = self.patch(boot_images, 'deferToThread') |
504 | + deferToThread.return_value = defer.succeed(None) |
505 | + yield import_boot_images(sentinel.sources) |
506 | + self.assertThat( |
507 | + deferToThread, MockNotCalled()) |
508 | + |
509 | + @defer.inlineCallbacks |
510 | def test__calls__run_import_using_deferToThread(self): |
511 | deferToThread = self.patch(boot_images, 'deferToThread') |
512 | deferToThread.return_value = defer.succeed(None) |
513 | @@ -120,3 +136,16 @@ |
514 | self.assertThat( |
515 | deferToThread, MockCalledOnceWith( |
516 | _run_import, sentinel.sources)) |
517 | + |
518 | + def test__takes_lock_when_running(self): |
519 | + clock = Clock() |
520 | + deferToThread = self.patch(boot_images, 'deferToThread') |
521 | + deferToThread.return_value = pause(1, clock) |
522 | + |
523 | + # Lock is acquired when import is started. |
524 | + import_boot_images(sentinel.sources) |
525 | + self.assertTrue(import_lock.locked) |
526 | + |
527 | + # Lock is released once the download is done. |
528 | + clock.advance(1) |
529 | + self.assertFalse(import_lock.locked) |
This is cool stuff :) It looks good, but I'm teetering on the edge of Needs Fixing for a couple of things, but Approve has it. However, please address (or put me right on) my comments before landing.