Merge "libvirt: Use uuid for instance directory name"

5 files changed, 61 insertions, 27 deletions

diff --git a/nova/tests/fake_libvirt_utils.py b/nova/tests/fake_libvirt_utils.py
index caa5e72d1..38236cd21 100644
--- a/nova/tests/fake_libvirt_utils.py
+++ b/nova/tests/fake_libvirt_utils.py
@@ -18,6 +18,7 @@ import os
 import StringIO
 
 from nova.openstack.common import cfg
+from nova.virt.libvirt import utils as libvirt_utils
 
 
 CONF = cfg.CONF
@@ -195,9 +196,8 @@ def fetch_image(context, target, image_id, user_id, project_id):
     pass
 
 
-def get_instance_path(instance):
-    # TODO(mikal): we should really just call the real one here
-    return os.path.join(CONF.instances_path, instance['name'])
+def get_instance_path(instance, forceold=False):
+    return libvirt_utils.get_instance_path(instance, forceold=forceold)
 
 
 def pick_disk_driver_name(is_block_dev=False):
diff --git a/nova/tests/test_imagebackend.py b/nova/tests/test_imagebackend.py
index 87e51819d..bd88fcd60 100644
--- a/nova/tests/test_imagebackend.py
+++ b/nova/tests/test_imagebackend.py
@@ -19,16 +19,16 @@ import fixtures
 import os
 
 from nova.openstack.common import cfg
+from nova.openstack.common import uuidutils
 from nova import test
 from nova.tests import fake_libvirt_utils
 from nova.virt.libvirt import imagebackend
-from nova.virt.libvirt import utils as libvirt_utils
 
 CONF = cfg.CONF
 
 
 class _ImageTestCase(object):
-    INSTANCES_PATH = '/fake'
+    INSTANCES_PATH = '/instances_path'
 
     def mock_create_image(self, image):
         def create_image(fn, base, size, *args, **kwargs):
@@ -39,14 +39,19 @@ class _ImageTestCase(object):
         super(_ImageTestCase, self).setUp()
         self.flags(disable_process_locking=True,
                    instances_path=self.INSTANCES_PATH)
-        self.INSTANCE = {'name': 'instance'}
+        self.INSTANCE = {'name': 'instance',
+                         'uuid': uuidutils.generate_uuid()}
         self.NAME = 'fake.vm'
         self.TEMPLATE = 'template'
 
+        self.OLD_STYLE_INSTANCE_PATH = \
+            fake_libvirt_utils.get_instance_path(self.INSTANCE, forceold=True)
         self.PATH = os.path.join(
-            libvirt_utils.get_instance_path(self.INSTANCE), self.NAME)
-        self.TEMPLATE_DIR = os.path.join(CONF.instances_path,
-                                         '_base')
+            fake_libvirt_utils.get_instance_path(self.INSTANCE), self.NAME)
+
+        # TODO(mikal): rename template_dir to base_dir and template_path
+        # to cached_image_path. This will be less confusing.
+        self.TEMPLATE_DIR = os.path.join(CONF.instances_path, '_base')
         self.TEMPLATE_PATH = os.path.join(self.TEMPLATE_DIR, 'template')
 
         self.useFixture(fixtures.MonkeyPatch(
@@ -55,6 +60,8 @@ class _ImageTestCase(object):
 
     def test_cache(self):
         self.mox.StubOutWithMock(os.path, 'exists')
+        if self.OLD_STYLE_INSTANCE_PATH:
+            os.path.exists(self.OLD_STYLE_INSTANCE_PATH).AndReturn(False)
         os.path.exists(self.TEMPLATE_DIR).AndReturn(False)
         os.path.exists(self.PATH).AndReturn(False)
         os.path.exists(self.TEMPLATE_PATH).AndReturn(False)
@@ -72,6 +79,8 @@ class _ImageTestCase(object):
 
     def test_cache_image_exists(self):
         self.mox.StubOutWithMock(os.path, 'exists')
+        if self.OLD_STYLE_INSTANCE_PATH:
+            os.path.exists(self.OLD_STYLE_INSTANCE_PATH).AndReturn(False)
         os.path.exists(self.TEMPLATE_DIR).AndReturn(True)
         os.path.exists(self.PATH).AndReturn(True)
         os.path.exists(self.TEMPLATE_PATH).AndReturn(True)
@@ -84,6 +93,8 @@ class _ImageTestCase(object):
 
     def test_cache_base_dir_exists(self):
         self.mox.StubOutWithMock(os.path, 'exists')
+        if self.OLD_STYLE_INSTANCE_PATH:
+            os.path.exists(self.OLD_STYLE_INSTANCE_PATH).AndReturn(False)
         os.path.exists(self.TEMPLATE_DIR).AndReturn(True)
         os.path.exists(self.PATH).AndReturn(False)
         os.path.exists(self.TEMPLATE_PATH).AndReturn(False)
@@ -100,6 +111,8 @@ class _ImageTestCase(object):
 
     def test_cache_template_exists(self):
         self.mox.StubOutWithMock(os.path, 'exists')
+        if self.OLD_STYLE_INSTANCE_PATH:
+            os.path.exists(self.OLD_STYLE_INSTANCE_PATH).AndReturn(False)
         os.path.exists(self.TEMPLATE_DIR).AndReturn(True)
         os.path.exists(self.PATH).AndReturn(False)
         os.path.exists(self.TEMPLATE_PATH).AndReturn(True)
@@ -197,6 +210,11 @@ class Qcow2TestCase(_ImageTestCase, test.TestCase):
     def test_create_image_with_size(self):
         fn = self.prepare_mocks()
         fn(target=self.TEMPLATE_PATH)
+        self.mox.StubOutWithMock(os.path, 'exists')
+        if self.OLD_STYLE_INSTANCE_PATH:
+            os.path.exists(self.OLD_STYLE_INSTANCE_PATH).AndReturn(False)
+        os.path.exists(self.TEMPLATE_PATH).AndReturn(False)
+        os.path.exists(self.PATH).AndReturn(False)
         imagebackend.libvirt_utils.create_cow_image(self.TEMPLATE_PATH,
                                                     self.PATH)
         imagebackend.disk.extend(self.PATH, self.SIZE)
@@ -218,6 +236,7 @@ class LvmTestCase(_ImageTestCase, test.TestCase):
         super(LvmTestCase, self).setUp()
         self.flags(libvirt_images_volume_group=self.VG)
         self.LV = '%s_%s' % (self.INSTANCE['name'], self.NAME)
+        self.OLD_STYLE_INSTANCE_PATH = None
         self.PATH = os.path.join('/dev', self.VG, self.LV)
 
         self.disk = imagebackend.disk
@@ -344,7 +363,8 @@ class LvmTestCase(_ImageTestCase, test.TestCase):
 
 
 class BackendTestCase(test.TestCase):
-    INSTANCE = {'name': 'fake-instance'}
+    INSTANCE = {'name': 'fake-instance',
+                'uuid': uuidutils.generate_uuid()}
     NAME = 'fake-name.suffix'
 
     def get_image(self, use_cow, image_type):
diff --git a/nova/tests/test_imagecache.py b/nova/tests/test_imagecache.py
index 611519514..389612c64 100644
--- a/nova/tests/test_imagecache.py
+++ b/nova/tests/test_imagecache.py
@@ -47,10 +47,10 @@ class ImageCacheManagerTestCase(test.TestCase):
 
     def setUp(self):
         super(ImageCacheManagerTestCase, self).setUp()
-        self.stock_instance_names = {'instance-00000001': '123',
-                                     'instance-00000002': '456',
-                                     'instance-00000003': '789',
-                                     'banana-42-hamster': '444'}
+        self.stock_instance_names = set(['instance-00000001',
+                                         'instance-00000002',
+                                         'instance-00000003',
+                                         'banana-42-hamster'])
 
     def test_read_stored_checksum_missing(self):
         self.stubs.Set(os.path, 'exists', lambda x: False)
@@ -181,6 +181,9 @@ class ImageCacheManagerTestCase(test.TestCase):
         self.assertTrue(image_cache_manager.used_images['2'] ==
                         (1, 1, ['inst-2', 'inst-3']))
 
+        self.assertTrue('inst-1' in image_cache_manager.instance_names)
+        self.assertTrue('123' in image_cache_manager.instance_names)
+
         self.assertEqual(len(image_cache_manager.image_popularity), 2)
         self.assertEqual(image_cache_manager.image_popularity['1'], 1)
         self.assertEqual(image_cache_manager.image_popularity['2'], 2)
@@ -200,7 +203,7 @@ class ImageCacheManagerTestCase(test.TestCase):
         self.assertTrue(image_cache_manager.used_images['1'] ==
                         (1, 0, ['inst-1']))
         self.assertTrue(image_cache_manager.instance_names ==
-                        set(['inst-1', 'inst-1_resize']))
+                        set(['inst-1', '123', 'inst-1_resize', '123_resize']))
 
         self.assertEqual(len(image_cache_manager.image_popularity), 1)
         self.assertEqual(image_cache_manager.image_popularity['1'], 1)
diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py
index 9e5b2900d..8ac74aabd 100644
--- a/nova/tests/test_libvirt.py
+++ b/nova/tests/test_libvirt.py
@@ -43,6 +43,7 @@ from nova.openstack.common import fileutils
 from nova.openstack.common import importutils
 from nova.openstack.common import jsonutils
 from nova.openstack.common import log as logging
+from nova.openstack.common import uuidutils
 from nova import test
 from nova.tests import fake_libvirt_utils
 from nova.tests import fake_network
@@ -135,7 +136,8 @@ class FakeVirtDomain(object):
 class CacheConcurrencyTestCase(test.TestCase):
     def setUp(self):
         super(CacheConcurrencyTestCase, self).setUp()
-        self.flags(instances_path='nova.compute.manager')
+
+        self.flags(instances_path=self.useFixture(fixtures.TempDir()).path)
 
         # utils.synchronized() will create the lock_path for us if it
         # doesn't already exist. It will also delete it when it's done,
@@ -165,19 +167,18 @@ class CacheConcurrencyTestCase(test.TestCase):
             fake_libvirt_utils))
 
     def tearDown(self):
-        # Make sure the lock_path for this test is cleaned up
-        if os.path.exists(self.lock_path):
-            shutil.rmtree(self.lock_path)
-
         super(CacheConcurrencyTestCase, self).tearDown()
 
     def test_same_fname_concurrency(self):
         # Ensures that the same fname cache runs at a sequentially.
+        uuid = uuidutils.generate_uuid()
+
         backend = imagebackend.Backend(False)
         wait1 = eventlet.event.Event()
         done1 = eventlet.event.Event()
         sig1 = eventlet.event.Event()
-        thr1 = eventlet.spawn(backend.image({'name': 'instance'},
+        thr1 = eventlet.spawn(backend.image({'name': 'instance',
+                                             'uuid': uuid},
                                             'name').cache,
                 _concurrency, 'fname', None,
                 signal=sig1, wait=wait1, done=done1)
@@ -188,7 +189,8 @@ class CacheConcurrencyTestCase(test.TestCase):
         wait2 = eventlet.event.Event()
         done2 = eventlet.event.Event()
         sig2 = eventlet.event.Event()
-        thr2 = eventlet.spawn(backend.image({'name': 'instance'},
+        thr2 = eventlet.spawn(backend.image({'name': 'instance',
+                                             'uuid': uuid},
                                             'name').cache,
                 _concurrency, 'fname', None,
                 signal=sig2, wait=wait2, done=done2)
@@ -209,11 +211,14 @@ class CacheConcurrencyTestCase(test.TestCase):
 
     def test_different_fname_concurrency(self):
         # Ensures that two different fname caches are concurrent.
+        uuid = uuidutils.generate_uuid()
+
         backend = imagebackend.Backend(False)
         wait1 = eventlet.event.Event()
         done1 = eventlet.event.Event()
         sig1 = eventlet.event.Event()
-        thr1 = eventlet.spawn(backend.image({'name': 'instance'},
+        thr1 = eventlet.spawn(backend.image({'name': 'instance',
+                                             'uuid': uuid},
                                             'name').cache,
                 _concurrency, 'fname2', None,
                 signal=sig1, wait=wait1, done=done1)
@@ -224,7 +229,8 @@ class CacheConcurrencyTestCase(test.TestCase):
         wait2 = eventlet.event.Event()
         done2 = eventlet.event.Event()
         sig2 = eventlet.event.Event()
-        thr2 = eventlet.spawn(backend.image({'name': 'instance'},
+        thr2 = eventlet.spawn(backend.image({'name': 'instance',
+                                             'uuid': uuid},
                                             'name').cache,
                 _concurrency, 'fname1', None,
                 signal=sig2, wait=wait2, done=done2)
@@ -2357,8 +2363,8 @@ class LibvirtConnTestCase(test.TestCase):
             ret = conn.pre_live_migration(c, inst_ref, vol, nw_info,
                                           migrate_data)
             self.assertEqual(ret, None)
-            self.assertTrue(os.path.exists('%s/%s/' %
-                           (tmpdir, inst_ref['name'])))
+            self.assertTrue(os.path.exists('%s/%s/' % (tmpdir,
+                                                       inst_ref['uuid'])))
         db.instance_destroy(self.context, inst_ref['uuid'])
 
     def test_pre_block_migration_works_correctly(self):
@@ -2394,7 +2400,7 @@ class LibvirtConnTestCase(test.TestCase):
                                      dummyjson)
 
             self.assertTrue(os.path.exists('%s/%s/' %
-                                           (tmpdir, instance_ref['name'])))
+                                           (tmpdir, instance_ref['uuid'])))
 
         db.instance_destroy(self.context, instance_ref['uuid'])
 
diff --git a/nova/tests/test_virt_drivers.py b/nova/tests/test_virt_drivers.py
index a94fdc3c5..5a46beffb 100644
--- a/nova/tests/test_virt_drivers.py
+++ b/nova/tests/test_virt_drivers.py
@@ -184,11 +184,16 @@ class VirtDriverLoaderTestCase(_FakeDriverBackendTestCase, test.TestCase):
 class _VirtDriverTestCase(_FakeDriverBackendTestCase):
     def setUp(self):
         super(_VirtDriverTestCase, self).setUp()
+
+        self.flags(instances_path=self.useFixture(fixtures.TempDir()).path)
         self.connection = importutils.import_object(self.driver_module,
                                                     fake.FakeVirtAPI())
         self.ctxt = test_utils.get_test_admin_context()
         self.image_service = fake_image.FakeImageService()
 
+    def tearDown(self):
+        super(_VirtDriverTestCase, self).tearDown()
+
     def _get_running_instance(self):
         instance_ref = test_utils.get_test_instance()
         network_info = test_utils.get_test_network_info()