From cc4d766a0476211288d5b67f3647a721159185d3 Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Mon, 22 Oct 2012 10:25:21 -0700 Subject: Remove database usage from libvirt imagecache module This removes the database instance list operation from the libvirt imagecache module and pushes it up into compute/manager.py. This is a step in the proposed strategy to move db usage out of the virt drivers up into compute/manager.py in support of bp:no-db-compute. Change-Id: I2ea39f14304ed4b44c7ac9650703a5f780411c18 --- nova/compute/manager.py | 6 +- nova/tests/test_imagecache.py | 135 +++++++++++++++++++++++----------------- nova/virt/driver.py | 2 +- nova/virt/fake.py | 4 ++ nova/virt/libvirt/driver.py | 4 +- nova/virt/libvirt/imagecache.py | 10 ++- nova/virt/powervm/driver.py | 2 +- 7 files changed, 92 insertions(+), 71 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 515737992..712a7aa62 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -2975,7 +2975,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 fcbc8f688..d9218f629 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -2767,9 +2767,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. -- cgit