Merge "Remove database usage from libvirt imagecache module"

7 files changed, 92 insertions, 71 deletions

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 136a35ffb..6d19af6c7 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -2998,7 +2998,5 @@ class ComputeManager(manager.SchedulerDependentManager):
         if FLAGS.image_cache_manager_interval == 0:
             return
 
-        try:
-            self.driver.manage_image_cache(context)
-        except NotImplementedError:
-            pass
+        all_instances = self.db.instance_get_all(context)
+        self.driver.manage_image_cache(context, all_instances)
diff --git a/nova/tests/test_imagecache.py b/nova/tests/test_imagecache.py
index e1df3ab19..f2c33e623 100644
--- a/nova/tests/test_imagecache.py
+++ b/nova/tests/test_imagecache.py
@@ -25,9 +25,11 @@ import time
 
 from nova import test
 
+from nova.compute import manager as compute_manager
 from nova.compute import vm_states
 from nova import db
 from nova import flags
+from nova.openstack.common import importutils
 from nova.openstack.common import log
 from nova import utils
 from nova.virt.libvirt import imagecache
@@ -144,30 +146,29 @@ class ImageCacheManagerTestCase(test.TestCase):
         self.assertFalse(unexpected in image_cache_manager.originals)
 
     def test_list_running_instances(self):
-        self.stubs.Set(db, 'instance_get_all',
-                       lambda x: [{'image_ref': '1',
-                                   'host': FLAGS.host,
-                                   'name': 'inst-1',
-                                   'uuid': '123',
-                                   'vm_state': '',
-                                   'task_state': ''},
-                                  {'image_ref': '2',
-                                   'host': FLAGS.host,
-                                   'name': 'inst-2',
-                                   'uuid': '456',
-                                   'vm_state': '',
-                                   'task_state': ''},
-                                  {'image_ref': '2',
-                                   'host': 'remotehost',
-                                   'name': 'inst-3',
-                                   'uuid': '789',
-                                   'vm_state': '',
-                                   'task_state': ''}])
+        all_instances = [{'image_ref': '1',
+                          'host': FLAGS.host,
+                          'name': 'inst-1',
+                          'uuid': '123',
+                          'vm_state': '',
+                          'task_state': ''},
+                         {'image_ref': '2',
+                          'host': FLAGS.host,
+                          'name': 'inst-2',
+                          'uuid': '456',
+                          'vm_state': '',
+                          'task_state': ''},
+                         {'image_ref': '2',
+                          'host': 'remotehost',
+                          'name': 'inst-3',
+                          'uuid': '789',
+                          'vm_state': '',
+                          'task_state': ''}]
 
         image_cache_manager = imagecache.ImageCacheManager()
 
         # The argument here should be a context, but it's mocked out
-        image_cache_manager._list_running_instances(None)
+        image_cache_manager._list_running_instances(None, all_instances)
 
         self.assertEqual(len(image_cache_manager.used_images), 2)
         self.assertTrue(image_cache_manager.used_images['1'] ==
@@ -180,16 +181,15 @@ class ImageCacheManagerTestCase(test.TestCase):
         self.assertEqual(image_cache_manager.image_popularity['2'], 2)
 
     def test_list_resizing_instances(self):
-        self.stubs.Set(db, 'instance_get_all',
-                       lambda x: [{'image_ref': '1',
-                                   'host': FLAGS.host,
-                                   'name': 'inst-1',
-                                   'uuid': '123',
-                                   'vm_state': vm_states.RESIZED,
-                                   'task_state': None}])
+        all_instances = [{'image_ref': '1',
+                          'host': FLAGS.host,
+                          'name': 'inst-1',
+                          'uuid': '123',
+                          'vm_state': vm_states.RESIZED,
+                          'task_state': None}]
 
         image_cache_manager = imagecache.ImageCacheManager()
-        image_cache_manager._list_running_instances(None)
+        image_cache_manager._list_running_instances(None, all_instances)
 
         self.assertEqual(len(image_cache_manager.used_images), 1)
         self.assertTrue(image_cache_manager.used_images['1'] ==
@@ -764,19 +764,18 @@ class ImageCacheManagerTestCase(test.TestCase):
         self.stubs.Set(os.path, 'isfile', lambda x: isfile(x))
 
         # Fake the database call which lists running instances
-        self.stubs.Set(db, 'instance_get_all',
-                       lambda x: [{'image_ref': '1',
-                                   'host': FLAGS.host,
-                                   'name': 'instance-1',
-                                   'uuid': '123',
-                                   'vm_state': '',
-                                   'task_state': ''},
-                                  {'image_ref': '1',
-                                   'host': FLAGS.host,
-                                   'name': 'instance-2',
-                                   'uuid': '456',
-                                   'vm_state': '',
-                                   'task_state': ''}])
+        all_instances = [{'image_ref': '1',
+                          'host': FLAGS.host,
+                          'name': 'instance-1',
+                          'uuid': '123',
+                          'vm_state': '',
+                          'task_state': ''},
+                         {'image_ref': '1',
+                          'host': FLAGS.host,
+                          'name': 'instance-2',
+                          'uuid': '456',
+                          'vm_state': '',
+                          'task_state': ''}]
 
         image_cache_manager = imagecache.ImageCacheManager()
 
@@ -819,7 +818,7 @@ class ImageCacheManagerTestCase(test.TestCase):
 
         # And finally we can make the call we're actually testing...
         # The argument here should be a context, but it is mocked out
-        image_cache_manager.verify_base_images(None)
+        image_cache_manager.verify_base_images(None, all_instances)
 
         # Verify
         active = [fq_path(hashed_1), fq_path('%s_5368709120' % hashed_1)]
@@ -837,7 +836,7 @@ class ImageCacheManagerTestCase(test.TestCase):
     def test_verify_base_images_no_base(self):
         self.flags(instances_path='/tmp/no/such/dir/name/please')
         image_cache_manager = imagecache.ImageCacheManager()
-        image_cache_manager.verify_base_images(None)
+        image_cache_manager.verify_base_images(None, [])
 
     def test_is_valid_info_file(self):
         hashed = 'e97222e91fc4241f49a7f520d1dcf446751129b3'
@@ -865,19 +864,18 @@ class ImageCacheManagerTestCase(test.TestCase):
             os.mkdir(os.path.join(tmpdir, '_base'))
 
             # Fake the database call which lists running instances
-            self.stubs.Set(db, 'instance_get_all',
-                           lambda x: [{'image_ref': '1',
-                                       'host': FLAGS.host,
-                                       'name': 'instance-1',
-                                       'uuid': '123',
-                                       'vm_state': '',
-                                       'task_state': ''},
-                                      {'image_ref': '1',
-                                       'host': FLAGS.host,
-                                       'name': 'instance-2',
-                                       'uuid': '456',
-                                       'vm_state': '',
-                                       'task_state': ''}])
+            all_instances = [{'image_ref': '1',
+                              'host': FLAGS.host,
+                              'name': 'instance-1',
+                              'uuid': '123',
+                              'vm_state': '',
+                              'task_state': ''},
+                             {'image_ref': '1',
+                              'host': FLAGS.host,
+                              'name': 'instance-2',
+                              'uuid': '456',
+                              'vm_state': '',
+                              'task_state': ''}]
 
             def touch(filename):
                 f = open(filename, 'w')
@@ -894,7 +892,30 @@ class ImageCacheManagerTestCase(test.TestCase):
             os.utime(base_filename + '.info', (old, old))
 
             image_cache_manager = imagecache.ImageCacheManager()
-            image_cache_manager.verify_base_images(None)
+            image_cache_manager.verify_base_images(None, all_instances)
 
             self.assertTrue(os.path.exists(base_filename))
             self.assertTrue(os.path.exists(base_filename + '.info'))
+
+    def test_compute_manager(self):
+        was = {'called': False}
+
+        def fake_get_all(context):
+            was['called'] = True
+            return [{'image_ref': '1',
+                     'host': FLAGS.host,
+                     'name': 'instance-1',
+                     'uuid': '123',
+                     'vm_state': '',
+                     'task_state': ''},
+                    {'image_ref': '1',
+                     'host': FLAGS.host,
+                     'name': 'instance-2',
+                     'uuid': '456',
+                     'vm_state': '',
+                     'task_state': ''}]
+
+        self.stubs.Set(db, 'instance_get_all', fake_get_all)
+        compute = importutils.import_object(FLAGS.compute_manager)
+        compute._run_image_cache_manager_pass(None)
+        self.assertTrue(was['called'])
diff --git a/nova/virt/driver.py b/nova/virt/driver.py
index 8a7c39fa8..941d46e5e 100644
--- a/nova/virt/driver.py
+++ b/nova/virt/driver.py
@@ -641,7 +641,7 @@ class ComputeDriver(object):
         # TODO(tr3buchet): update all subclasses and remove this
         return True
 
-    def manage_image_cache(self, context):
+    def manage_image_cache(self, context, all_instances):
         """
         Manage the driver's local image cache.
 
diff --git a/nova/virt/fake.py b/nova/virt/fake.py
index 0996988cd..3527ac19b 100644
--- a/nova/virt/fake.py
+++ b/nova/virt/fake.py
@@ -46,6 +46,10 @@ class FakeInstance(object):
 
 
 class FakeDriver(driver.ComputeDriver):
+    capabilities = {
+        "has_imagecache": True,
+        }
+
     """Fake hypervisor driver"""
 
     def __init__(self, read_only=False):
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index fd241c38d..34d667c16 100644
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -2753,9 +2753,9 @@ class LibvirtDriver(driver.ComputeDriver):
         out, err = utils.execute('env', 'LANG=C', 'uptime')
         return out
 
-    def manage_image_cache(self, context):
+    def manage_image_cache(self, context, all_instances):
         """Manage the local cache of images."""
-        self.image_cache_manager.verify_base_images(context)
+        self.image_cache_manager.verify_base_images(context, all_instances)
 
     def _cleanup_remote_migration(self, dest, inst_base, inst_base_resize):
         """Used only for cleanup in case migrate_disk_and_power_off fails"""
diff --git a/nova/virt/libvirt/imagecache.py b/nova/virt/libvirt/imagecache.py
index ba350749a..dd4635123 100644
--- a/nova/virt/libvirt/imagecache.py
+++ b/nova/virt/libvirt/imagecache.py
@@ -29,7 +29,6 @@ import time
 
 from nova.compute import task_states
 from nova.compute import vm_states
-from nova import db
 from nova import flags
 from nova.openstack.common import cfg
 from nova.openstack.common import log as logging
@@ -127,14 +126,13 @@ class ImageCacheManager(object):
                                                                 ent))):
                 self._store_image(base_dir, ent, original=False)
 
-    def _list_running_instances(self, context):
+    def _list_running_instances(self, context, all_instances):
         """List running instances (on all compute nodes)."""
         self.used_images = {}
         self.image_popularity = {}
         self.instance_names = set()
 
-        instances = db.instance_get_all(context)
-        for instance in instances:
+        for instance in all_instances:
             self.instance_names.add(instance['name'])
 
             resize_states = [task_states.RESIZE_PREP,
@@ -357,7 +355,7 @@ class ImageCacheManager(object):
                     virtutils.chown(base_file, os.getuid())
                     os.utime(base_file, None)
 
-    def verify_base_images(self, context):
+    def verify_base_images(self, context, all_instances):
         """Verify that base images are in a reasonable state."""
 
         # NOTE(mikal): The new scheme for base images is as follows -- an
@@ -379,7 +377,7 @@ class ImageCacheManager(object):
 
         LOG.debug(_('Verify base images'))
         self._list_base_images(base_dir)
-        self._list_running_instances(context)
+        self._list_running_instances(context, all_instances)
 
         # Determine what images are on disk because they're in use
         for img in self.used_images:
diff --git a/nova/virt/powervm/driver.py b/nova/virt/powervm/driver.py
index 3b65b5c45..5fc73b4a9 100644
--- a/nova/virt/powervm/driver.py
+++ b/nova/virt/powervm/driver.py
@@ -169,7 +169,7 @@ class PowerVMDriver(driver.ComputeDriver):
         """
         return False
 
-    def manage_image_cache(self, context):
+    def manage_image_cache(self, context, all_instances):
         """
         Manage the driver's local image cache.