Merge lp:~blake-rouse/maas/cache-cluster-boot-images into lp:~maas-committers/maas/trunk

[Blake Rouse]

Cache the list of boot images on the cluster. Instead of reading from the disk on every request of the boot images, it is only updated when the contents of the disk have changed from an import.

[Jason Hobbs (jason-hobbs)]

lgtm.

review: Approve

[MAAS Lander (maas-lander)]

The attempt to merge lp:~blake-rouse/maas/cache-cluster-boot-images into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Hit http://security.ubuntu.com trusty-security Release.gpg
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Hit http://security.ubuntu.com trusty-security Release
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Get:1 http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg [933 B]
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Get:2 http://nova.clouds.archive.ubuntu.com trusty-updates Release [62.0 kB]
Hit http://security.ubuntu.com trusty-security/main Sources
Hit http://security.ubuntu.com trusty-security/universe Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Hit http://security.ubuntu.com trusty-security/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Hit http://security.ubuntu.com trusty-security/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Get:3 http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources [148 kB]
Get:4 http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources [94.7 kB]
Get:5 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [384 kB]
Get:6 http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [227 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Fetched 917 kB in 2s (321 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 authbind bind9 bind9utils build-essential bzr-builddeb curl daemontools debhelper dh-apport distro-info dnsutils firefox freeipmi-tools gjs ipython isc-dhcp-common libjs-raphael libjs-yui3-full libjs-yui3-min libpq-dev make pep8 postgresql pyflakes python-amqplib python-bzrlib python-celery python-convoy python-crochet python-cssselect python-curtin python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-extras python-fixtures python-flake8 python-formencode python-hivex python-httplib2 python-jinja2 python-jsonschema python-lockfile python-lxml python-mimeparse python-mock python-netaddr python-netifaces python-nose python-oauth python-oops python-oops-amqp python-oops-datedir-repo python-oops-twisted python-oops-wsgi python-openssl python-paramiko python-pexpect python-pip python-pocket-lin...

=== modified file 'src/maasserver/clusterrpc/tests/test_boot_images.py'
--- src/maasserver/clusterrpc/tests/test_boot_images.py	2014-10-30 13:39:50 +0000
+++ src/maasserver/clusterrpc/tests/test_boot_images.py	2014-12-16 18:59:29 +0000
@@ -144,6 +144,7 @@
         resource_dir = self.make_dir()
         self.tftproot = os.path.join(resource_dir, 'current')
         os.mkdir(self.tftproot)
+        self.patch(boot_images, 'CACHED_BOOT_IMAGES', None)
         self.patch(boot_images, 'BOOT_RESOURCES_STORAGE', resource_dir)
 
     def test_returns_boot_images(self):
@@ -172,6 +173,7 @@
         resource_dir = self.make_dir()
         self.tftproot = os.path.join(resource_dir, 'current')
         os.mkdir(self.tftproot)
+        self.patch(boot_images, 'CACHED_BOOT_IMAGES', None)
         self.patch(boot_images, 'BOOT_RESOURCES_STORAGE', resource_dir)
 
     def test_returns_boot_images_for_one_cluster(self):
@@ -261,6 +263,7 @@
         resource_dir = self.make_dir()
         self.tftproot = os.path.join(resource_dir, 'current')
         os.mkdir(self.tftproot)
+        self.patch(boot_images, 'CACHED_BOOT_IMAGES', None)
         self.patch(boot_images, 'BOOT_RESOURCES_STORAGE', resource_dir)
 
     def make_boot_images(self):

=== modified file 'src/provisioningserver/rpc/boot_images.py'
--- src/provisioningserver/rpc/boot_images.py	2014-10-30 13:17:51 +0000
+++ src/provisioningserver/rpc/boot_images.py	2014-12-16 18:59:29 +0000
@@ -31,9 +31,28 @@
 from twisted.internet.threads import deferToThread
 
 
+CACHED_BOOT_IMAGES = None
+
+
 def list_boot_images():
-    """List the boot images that exist on the cluster."""
-    return tftppath.list_boot_images(
+    """List the boot images that exist on the cluster.
+
+    This return value of this function is cached. This helps reduce the amount
+    of IO, as this function is called often. To update the cache call
+    `reload_boot_images`.
+    """
+    global CACHED_BOOT_IMAGES
+    if CACHED_BOOT_IMAGES is None:
+        CACHED_BOOT_IMAGES = tftppath.list_boot_images(
+            os.path.join(BOOT_RESOURCES_STORAGE, 'current'))
+    return CACHED_BOOT_IMAGES
+
+
+def reload_boot_images():
+    """Update the cached boot images so `list_boot_images` returns the
+    most up-to-date boot images list."""
+    global CACHED_BOOT_IMAGES
+    CACHED_BOOT_IMAGES = tftppath.list_boot_images(
         os.path.join(BOOT_RESOURCES_STORAGE, 'current'))
 
 
@@ -67,6 +86,10 @@
     with environment_variables(variables):
         boot_resources.import_images(sources)
 
+    # Update the boot images cache so `list_boot_images` returns the
+    # correct information.
+    reload_boot_images()
+
 
 def import_boot_images(sources, http_proxy=None, https_proxy=None):
     """Imports the boot images from the given sources."""

=== modified file 'src/provisioningserver/rpc/tests/test_boot_images.py'
--- src/provisioningserver/rpc/tests/test_boot_images.py	2014-10-30 13:17:51 +0000
+++ src/provisioningserver/rpc/tests/test_boot_images.py	2014-12-16 18:59:29 +0000
@@ -23,7 +23,10 @@
     MockNotCalled,
     )
 from maastesting.testcase import MAASTwistedRunTest
-from mock import sentinel
+from mock import (
+    ANY,
+    sentinel,
+    )
 from provisioningserver import concurrency
 from provisioningserver.boot import tftppath
 from provisioningserver.config import BOOT_RESOURCES_STORAGE
@@ -35,6 +38,7 @@
     import_boot_images,
     is_import_boot_images_running,
     list_boot_images,
+    reload_boot_images,
     )
 from provisioningserver.testing.config import BootSourcesFixture
 from provisioningserver.testing.testcase import PservTestCase
@@ -61,6 +65,7 @@
 class TestListBootImages(PservTestCase):
 
     def test__calls_list_boot_images_with_boot_resource_storage(self):
+        self.patch(boot_images, 'CACHED_BOOT_IMAGES', None)
         mock_list_boot_images = self.patch(tftppath, 'list_boot_images')
         list_boot_images()
         self.assertThat(
@@ -68,6 +73,36 @@
             MockCalledOnceWith(
                 os.path.join(BOOT_RESOURCES_STORAGE, "current")))
 
+    def test__calls_list_boot_images_when_cache_is_None(self):
+        self.patch(boot_images, 'CACHED_BOOT_IMAGES', None)
+        mock_list_boot_images = self.patch(tftppath, 'list_boot_images')
+        list_boot_images()
+        self.assertThat(
+            mock_list_boot_images,
+            MockCalledOnceWith(ANY))
+
+    def test__doesnt_call_list_boot_images_when_cache_is_not_None(self):
+        fake_boot_images = [factory.make_name('image') for _ in range(3)]
+        self.patch(boot_images, 'CACHED_BOOT_IMAGES', fake_boot_images)
+        mock_list_boot_images = self.patch(tftppath, 'list_boot_images')
+        self.expectThat(list_boot_images(), Equals(fake_boot_images))
+        self.expectThat(
+            mock_list_boot_images,
+            MockNotCalled())
+
+
+class TestReloadBootImages(PservTestCase):
+
+    def test__sets_CACHED_BOOT_IMAGES(self):
+        self.patch(
+            boot_images, 'CACHED_BOOT_IMAGES', factory.make_name('old_cache'))
+        fake_boot_images = [factory.make_name('image') for _ in range(3)]
+        mock_list_boot_images = self.patch(tftppath, 'list_boot_images')
+        mock_list_boot_images.return_value = fake_boot_images
+        reload_boot_images()
+        self.assertEquals(
+            boot_images.CACHED_BOOT_IMAGES, fake_boot_images)
+
 
 class TestGetHostsFromSources(PservTestCase):
 
@@ -141,6 +176,13 @@
         _run_import(sources=sources)
         self.assertThat(fake, MockCalledOnceWith(sources))
 
+    def test__run_import_calls_reload_boot_images(self):
+        fake_reload = self.patch(boot_images, 'reload_boot_images')
+        self.patch(boot_resources, 'import_images')
+        sources, _ = make_sources()
+        _run_import(sources=sources)
+        self.assertThat(fake_reload, MockCalledOnceWith())
+
 
 class TestImportBootImages(PservTestCase):
 

=== modified file 'src/provisioningserver/rpc/tests/test_clusterservice.py'
--- src/provisioningserver/rpc/tests/test_clusterservice.py	2014-12-12 00:56:22 +0000
+++ src/provisioningserver/rpc/tests/test_clusterservice.py	2014-12-16 18:59:29 +0000
@@ -227,6 +227,7 @@
 
     @inlineCallbacks
     def test_list_boot_images_can_be_called(self):
+        self.patch(boot_images, 'CACHED_BOOT_IMAGES', None)
         list_boot_images = self.patch(tftppath, "list_boot_images")
         list_boot_images.return_value = []
 
@@ -256,6 +257,7 @@
             os.makedirs(os.path.join(current_dir, *options))
             make_osystem(self, options[0], purposes)
         self.patch(boot_images, 'BOOT_RESOURCES_STORAGE', tftpdir)
+        self.patch(boot_images, 'CACHED_BOOT_IMAGES', None)
 
         expected_images = [
             {