Merge lp:~ltrager/maas/lp1630394 into lp:~maas-committers/maas/trunk

[Lee Trager]

Always wait to cache_boot_sources before importing resources.

On first run MAAS sets the default boot source in the ImportResourcesService. This causes a signal which refreshes the boot source cache. To prevent the cache from being updated twice MAAS just let the signal handle updating the cache and wasn't waiting for it to complete. This worked as MAAS starts downloading the actual files later on in the process. The way bootloaders are picked are by looking at what bootloaders are available in BootSourceCache. As this happened earlier in the process they were missed. This modifies the ImportResourcesService to always wait on refreshing the cache but modifying the signal to only refresh the cache if a boot source already exists.

[Blake Rouse (blake-rouse)]

Looks good.

review: Approve

=== modified file 'src/maasserver/bootresources.py'
--- src/maasserver/bootresources.py	2016-10-04 13:59:47 +0000
+++ src/maasserver/bootresources.py	2016-10-05 21:35:18 +0000
@@ -1295,9 +1295,10 @@
     create_gnupg_home()
 
     # Ensure that boot sources exist.
-    if not ensure_boot_source_definition():
-        # Nothing was created so cache the boot sources before import.
-        cache_boot_sources()
+    ensure_boot_source_definition()
+
+    # Cache the boot sources before import.
+    cache_boot_sources()
 
     # FIXME: This modifies the environment of the entire process, which is Not
     # Cool. We should integrate with simplestreams in a more Pythonic manner.

=== modified file 'src/maasserver/models/signals/bootsources.py'
--- src/maasserver/models/signals/bootsources.py	2016-06-22 17:03:02 +0000
+++ src/maasserver/models/signals/bootsources.py	2016-10-05 21:35:18 +0000
@@ -9,6 +9,7 @@
 
 from django.db.models.signals import (
     post_delete,
+    post_init,
     post_save,
 )
 from maasserver.bootsources import cache_boot_sources
@@ -21,15 +22,27 @@
 signals = SignalsManager()
 
 
+def is_first_run(sender, instance, *args, **kwargs):
+    """Store whether or not this is the first BootSource added to MAAS.
+
+    On first run the ImportResourceService sets the default BootSource then
+    caches the stream's contents as normal. Setting the default BootSource
+    triggers this signal. This prevents updating the cache twice.
+    """
+    instance.__is_first_run = BootSource.objects.count() == 0
+
+
 def update_boot_source_cache(sender, instance, *args, **kwargs):
     """Update the `BootSourceCache` using the updated source.
 
     This only begins after a successful commit to the database, and is then
     run in a thread. Nothing waits for its completion.
     """
-    post_commit_do(reactor.callLater, 0, cache_boot_sources)
-
-
+    if not isinstance(instance, BootSource) or not instance.__is_first_run:
+        post_commit_do(reactor.callLater, 0, cache_boot_sources)
+
+
+signals.watch(post_init, is_first_run, BootSource)
 signals.watch(post_save, update_boot_source_cache, BootSource)
 signals.watch(post_delete, update_boot_source_cache, BootSource)
 signals.watch_config(update_boot_source_cache, "enable_http_proxy")

=== modified file 'src/maasserver/models/signals/tests/test_bootsources.py'
--- src/maasserver/models/signals/tests/test_bootsources.py	2016-05-25 20:32:57 +0000
+++ src/maasserver/models/signals/tests/test_bootsources.py	2016-10-05 21:35:18 +0000
@@ -10,20 +10,30 @@
 from maasserver.models.config import Config
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
-from maastesting.matchers import MockCalledOnceWith
+from maastesting.matchers import (
+    MockCalledOnceWith,
+    MockNotCalled,
+)
 from twisted.internet import reactor
 
 
 class TestBootSourceSignals(MAASServerTestCase):
     """Tests for the `BootSource` model's signals."""
 
+    def test_doesnt_update_on_initial_BootSource_create(self):
+        post_commit_do = self.patch(signals.bootsources, "post_commit_do")
+        factory.make_BootSource(keyring_data=factory.make_bytes())
+        self.assertThat(post_commit_do, MockNotCalled())
+
     def test_arranges_for_update_on_BootSource_create(self):
+        factory.make_BootSource(keyring_data=factory.make_bytes())
         post_commit_do = self.patch(signals.bootsources, "post_commit_do")
         factory.make_BootSource(keyring_data=factory.make_bytes())
         self.assertThat(post_commit_do, MockCalledOnceWith(
             reactor.callLater, 0, cache_boot_sources))
 
     def test_arranges_for_update_on_BootSource_update(self):
+        factory.make_BootSource(keyring_data=factory.make_bytes())
         self.patch(signals.bootsources, "post_commit_do")
         boot_source = factory.make_BootSource(
             keyring_data=factory.make_bytes())
@@ -34,6 +44,7 @@
             reactor.callLater, 0, cache_boot_sources))
 
     def test_arranges_for_update_on_BootSource_delete(self):
+        factory.make_BootSource(keyring_data=factory.make_bytes())
         self.patch(signals.bootsources, "post_commit_do")
         boot_source = factory.make_BootSource(
             keyring_data=factory.make_bytes())