Merge ~blake-rouse/maas:fix-1754493 into maas:master

Proposed by Blake Rouse
Status: Merged
Approved by: Blake Rouse
Approved revision: 5b917d8b9d56202b56194b325668abef07d20abb
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~blake-rouse/maas:fix-1754493
Merge into: maas:master
Diff against target: 59 lines (+22/-7)
2 files modified
src/provisioningserver/rpc/boot_images.py (+7/-1)
src/provisioningserver/rpc/tests/test_boot_images.py (+15/-6)
Reviewer Review Type Date Requested Status
Mike Pontillo (community) Approve
MAAS Lander Approve
Review via email: mp+341678@code.launchpad.net

Commit message

Fixes LP: #1754493 - Call import boot images again if trigger for import when an import is already running.

This fixes an issue where the rack could still be performing an update with old data when the region triggers the rack to import again. This allows up to 1 import to run after the current import is finished. This will ensure that the rack controller will get always match the regiond images.

To post a comment you must log in.
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b fix-1754493 lp:~blake-rouse/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 5b917d8b9d56202b56194b325668abef07d20abb

review: Approve
Revision history for this message
Blake Rouse (blake-rouse) wrote :
Revision history for this message
Mike Pontillo (mpontillo) wrote :

Interesting and subtle problem. Fix looks good!

review: Approve
Revision history for this message
MAAS Lander (maas-lander) wrote :
Revision history for this message
MAAS Lander (maas-lander) wrote :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/src/provisioningserver/rpc/boot_images.py b/src/provisioningserver/rpc/boot_images.py
index 317d01f..e1ebdf4 100644
--- a/src/provisioningserver/rpc/boot_images.py
+++ b/src/provisioningserver/rpc/boot_images.py
@@ -129,7 +129,13 @@ def _run_import(sources, http_proxy=None, https_proxy=None):
129def import_boot_images(sources, http_proxy=None, https_proxy=None):129def import_boot_images(sources, http_proxy=None, https_proxy=None):
130 """Imports the boot images from the given sources."""130 """Imports the boot images from the given sources."""
131 lock = concurrency.boot_images131 lock = concurrency.boot_images
132 if not lock.locked:132 # This checks if any other defer is already waiting. If nothing is waiting
133 # then add the _import again. If its already waiting nothing is added.
134 #
135 # This is important to how this functions. If the rackd is already
136 # importing images and the regiond triggers another import then after the
137 # original import another will be fired.
138 if not lock.waiting:
133 return lock.run(139 return lock.run(
134 _import_boot_images, sources, http_proxy=http_proxy,140 _import_boot_images, sources, http_proxy=http_proxy,
135 https_proxy=https_proxy)141 https_proxy=https_proxy)
diff --git a/src/provisioningserver/rpc/tests/test_boot_images.py b/src/provisioningserver/rpc/tests/test_boot_images.py
index 5cbc734..0d45b79 100644
--- a/src/provisioningserver/rpc/tests/test_boot_images.py
+++ b/src/provisioningserver/rpc/tests/test_boot_images.py
@@ -272,20 +272,29 @@ class TestImportBootImages(MAASTestCase):
272 run_tests_with = MAASTwistedRunTest.make_factory(timeout=5)272 run_tests_with = MAASTwistedRunTest.make_factory(timeout=5)
273273
274 @defer.inlineCallbacks274 @defer.inlineCallbacks
275 def test__does_not_run_if_lock_taken(self):275 def test__add_to_waiting_if_lock_already_held(self):
276 yield concurrency.boot_images.acquire()276 yield concurrency.boot_images.acquire()
277 self.addCleanup(concurrency.boot_images.release)
278 deferToThread = self.patch(boot_images, 'deferToThread')277 deferToThread = self.patch(boot_images, 'deferToThread')
279 deferToThread.return_value = defer.succeed(None)278 deferToThread.return_value = defer.succeed(None)
280 yield import_boot_images(sentinel.sources)279 d = import_boot_images(sentinel.sources)
280 self.assertEqual(1, len(concurrency.boot_images.waiting))
281 concurrency.boot_images.release()
282 yield d
281 self.assertThat(283 self.assertThat(
282 deferToThread, MockNotCalled())284 deferToThread, MockCalledOnceWith(
285 _run_import, sentinel.sources,
286 http_proxy=None, https_proxy=None))
283287
284 @defer.inlineCallbacks288 @defer.inlineCallbacks
285 def test__calls__run_import_using_deferToThread(self):289 def test__never_more_than_one_waiting(self):
290 yield concurrency.boot_images.acquire()
286 deferToThread = self.patch(boot_images, 'deferToThread')291 deferToThread = self.patch(boot_images, 'deferToThread')
287 deferToThread.return_value = defer.succeed(None)292 deferToThread.return_value = defer.succeed(None)
288 yield import_boot_images(sentinel.sources)293 d = import_boot_images(sentinel.sources)
294 self.assertIsNone(import_boot_images(sentinel.sources))
295 self.assertEqual(1, len(concurrency.boot_images.waiting))
296 concurrency.boot_images.release()
297 yield d
289 self.assertThat(298 self.assertThat(
290 deferToThread, MockCalledOnceWith(299 deferToThread, MockCalledOnceWith(
291 _run_import, sentinel.sources,300 _run_import, sentinel.sources,

Subscribers

People subscribed via source and target branches