Skip to content

Commit

Permalink
Remove backward compatibility with pre-grizzly releases
Browse files Browse the repository at this point in the history
Method get_instance_path libvirt/utils.py contained code that
keeps compatibility with pre-grizzly releases, where instance-path
was instance.name. This patch removes extra param from that method,
and code, that was used for pre-grizzly.

Change-Id: I50f3003f82f50a4b8d61b2c558093ec849ba86e1
  • Loading branch information
timofei-durakov committed Aug 4, 2016
1 parent 7cf8ab7 commit 5ce0dbd
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 62 deletions.
5 changes: 2 additions & 3 deletions nova/tests/unit/virt/libvirt/fake_libvirt_utils.py
Expand Up @@ -151,9 +151,8 @@ def fetch_raw_image(context, target, image_id, max_size=0):
pass


def get_instance_path(instance, forceold=False, relative=False):
return libvirt_utils.get_instance_path(instance, forceold=forceold,
relative=relative)
def get_instance_path(instance, relative=False):
return libvirt_utils.get_instance_path(instance, relative=relative)


def get_instance_path_at_destination(instance, migrate_data=None):
Expand Down
8 changes: 4 additions & 4 deletions nova/tests/unit/virt/libvirt/test_driver.py
Expand Up @@ -16076,7 +16076,7 @@ def test_cleanup_resize_same_host(self):
mock_get_path.return_value = '/fake/inst'

drvr._cleanup_resize(ins_ref, _fake_network_info(self, 1))
mock_get_path.assert_called_once_with(ins_ref, forceold=True)
mock_get_path.assert_called_once_with(ins_ref)
mock_exec.assert_called_once_with('rm', '-rf', '/fake/inst_resize',
delay_on_retry=True, attempts=5)

Expand All @@ -16103,7 +16103,7 @@ def test_cleanup_resize_not_same_host(self):
mock_get_path.return_value = '/fake/inst'

drvr._cleanup_resize(ins_ref, fake_net)
mock_get_path.assert_called_once_with(ins_ref, forceold=True)
mock_get_path.assert_called_once_with(ins_ref)
mock_exec.assert_called_once_with('rm', '-rf', '/fake/inst_resize',
delay_on_retry=True, attempts=5)
mock_undef.assert_called_once_with(ins_ref)
Expand All @@ -16127,7 +16127,7 @@ def test_cleanup_resize_snap_backend(self):
mock_get_path.return_value = '/fake/inst'

drvr._cleanup_resize(ins_ref, _fake_network_info(self, 1))
mock_get_path.assert_called_once_with(ins_ref, forceold=True)
mock_get_path.assert_called_once_with(ins_ref)
mock_exec.assert_called_once_with('rm', '-rf', '/fake/inst_resize',
delay_on_retry=True, attempts=5)
mock_remove.assert_called_once_with(
Expand All @@ -16151,7 +16151,7 @@ def test_cleanup_resize_snap_backend_image_does_not_exist(self):
mock_get_path.return_value = '/fake/inst'

drvr._cleanup_resize(ins_ref, _fake_network_info(self, 1))
mock_get_path.assert_called_once_with(ins_ref, forceold=True)
mock_get_path.assert_called_once_with(ins_ref)
mock_exec.assert_called_once_with('rm', '-rf', '/fake/inst_resize',
delay_on_retry=True, attempts=5)
self.assertFalse(mock_remove.called)
Expand Down
36 changes: 0 additions & 36 deletions nova/tests/unit/virt/libvirt/test_imagebackend.py
Expand Up @@ -77,8 +77,6 @@ def setUp(self):
self.TEMPLATE = 'template'
self.CONTEXT = context.get_admin_context()

self.OLD_STYLE_INSTANCE_PATH = \
fake_libvirt_utils.get_instance_path(self.INSTANCE, forceold=True)
self.PATH = os.path.join(
fake_libvirt_utils.get_instance_path(self.INSTANCE), self.NAME)

Expand Down Expand Up @@ -215,8 +213,6 @@ def prepare_mocks(self):

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)
Expand All @@ -234,8 +230,6 @@ def test_cache(self):

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)
Expand All @@ -248,8 +242,6 @@ def test_cache_image_exists(self):

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)
Expand All @@ -266,8 +258,6 @@ def test_cache_base_dir_exists(self):

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)
Expand Down Expand Up @@ -371,8 +361,6 @@ def prepare_mocks(self):

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.DISK_INFO_PATH).AndReturn(False)
os.path.exists(CONF.instances_path).AndReturn(True)
os.path.exists(self.TEMPLATE_DIR).AndReturn(False)
Expand All @@ -391,8 +379,6 @@ def test_cache(self):

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.DISK_INFO_PATH).AndReturn(False)
os.path.exists(self.INSTANCES_PATH).AndReturn(True)
os.path.exists(self.TEMPLATE_DIR).AndReturn(True)
Expand All @@ -407,8 +393,6 @@ def test_cache_image_exists(self):

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.DISK_INFO_PATH).AndReturn(False)
os.path.exists(self.INSTANCES_PATH).AndReturn(True)
os.path.exists(self.TEMPLATE_DIR).AndReturn(True)
Expand All @@ -426,8 +410,6 @@ def test_cache_base_dir_exists(self):

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.DISK_INFO_PATH).AndReturn(False)
os.path.exists(self.INSTANCES_PATH).AndReturn(True)
os.path.exists(self.TEMPLATE_DIR).AndReturn(True)
Expand Down Expand Up @@ -459,8 +441,6 @@ def test_create_image_with_size(self):
self.mox.StubOutWithMock(os.path, 'exists')
self.mox.StubOutWithMock(imagebackend.Image,
'verify_base_size')
if self.OLD_STYLE_INSTANCE_PATH:
os.path.exists(self.OLD_STYLE_INSTANCE_PATH).AndReturn(False)
os.path.exists(self.DISK_INFO_PATH).AndReturn(False)
os.path.exists(self.INSTANCES_PATH).AndReturn(True)
os.path.exists(self.TEMPLATE_PATH).AndReturn(False)
Expand All @@ -482,8 +462,6 @@ def test_create_image_too_small(self):
fn = self.prepare_mocks()
self.mox.StubOutWithMock(os.path, 'exists')
self.mox.StubOutWithMock(imagebackend.Qcow2, 'get_disk_size')
if self.OLD_STYLE_INSTANCE_PATH:
os.path.exists(self.OLD_STYLE_INSTANCE_PATH).AndReturn(False)
os.path.exists(self.DISK_INFO_PATH).AndReturn(False)
os.path.exists(self.INSTANCES_PATH).AndReturn(True)
os.path.exists(self.TEMPLATE_PATH).AndReturn(True)
Expand All @@ -504,8 +482,6 @@ def test_generate_resized_backing_files(self):
'get_disk_backing_file')
self.mox.StubOutWithMock(imagebackend.Image,
'verify_base_size')
if self.OLD_STYLE_INSTANCE_PATH:
os.path.exists(self.OLD_STYLE_INSTANCE_PATH).AndReturn(False)
os.path.exists(self.DISK_INFO_PATH).AndReturn(False)
os.path.exists(CONF.instances_path).AndReturn(True)
os.path.exists(self.TEMPLATE_PATH).AndReturn(False)
Expand Down Expand Up @@ -537,8 +513,6 @@ def test_qcow2_exists_and_has_no_backing_file(self):
'get_disk_backing_file')
self.mox.StubOutWithMock(imagebackend.Image,
'verify_base_size')
if self.OLD_STYLE_INSTANCE_PATH:
os.path.exists(self.OLD_STYLE_INSTANCE_PATH).AndReturn(False)
os.path.exists(self.DISK_INFO_PATH).AndReturn(False)
os.path.exists(self.INSTANCES_PATH).AndReturn(True)

Expand Down Expand Up @@ -581,7 +555,6 @@ def setUp(self):
self.flags(enabled=False, group='ephemeral_storage_encryption')
self.INSTANCE['ephemeral_key_uuid'] = None
self.LV = '%s_%s' % (self.INSTANCE['uuid'], self.NAME)
self.OLD_STYLE_INSTANCE_PATH = None
self.PATH = os.path.join('/dev', self.VG, self.LV)
self.disk = imagebackend.disk
self.utils = imagebackend.utils
Expand Down Expand Up @@ -647,8 +620,6 @@ def _create_image_resize(self, sparse):

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)
Expand All @@ -667,8 +638,6 @@ def test_cache(self):

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)
Expand All @@ -681,8 +650,6 @@ def test_cache_image_exists(self):

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)
Expand Down Expand Up @@ -793,7 +760,6 @@ def setUp(self):
group='key_manager')
self.flags(images_volume_group=self.VG, group='libvirt')
self.LV = '%s_%s' % (self.INSTANCE['uuid'], self.NAME)
self.OLD_STYLE_INSTANCE_PATH = None
self.LV_PATH = os.path.join('/dev', self.VG, self.LV)
self.PATH = os.path.join('/dev/mapper',
imagebackend.dmcrypt.volume_name(self.LV))
Expand Down Expand Up @@ -1592,8 +1558,6 @@ def prepare_mocks(self):

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)
Expand Down
9 changes: 1 addition & 8 deletions nova/virt/libvirt/driver.py
Expand Up @@ -1114,14 +1114,7 @@ def get_volume_connector(self, instance):
host=CONF.host)

def _cleanup_resize(self, instance, network_info):
# NOTE(wangpan): we get the pre-grizzly instance path firstly,
# so the backup dir of pre-grizzly instance can
# be deleted correctly with grizzly or later nova.
pre_grizzly_name = libvirt_utils.get_instance_path(instance,
forceold=True)
target = pre_grizzly_name + '_resize'
if not os.path.exists(target):
target = libvirt_utils.get_instance_path(instance) + '_resize'
target = libvirt_utils.get_instance_path(instance) + '_resize'

if os.path.exists(target):
# Deletion can fail over NFS, so retry the deletion as required.
Expand Down
13 changes: 2 additions & 11 deletions nova/virt/libvirt/utils.py
Expand Up @@ -429,25 +429,16 @@ def fetch_raw_image(context, target, image_id):
images.fetch(context, image_id, target)


def get_instance_path(instance, forceold=False, relative=False):
def get_instance_path(instance, relative=False):
"""Determine the correct path for instance storage.
This method determines the directory name for instance storage, while
handling the fact that we changed the naming style to something more
unique in the grizzly release.
This method determines the directory name for instance storage.
:param instance: the instance we want a path for
:param forceold: force the use of the pre-grizzly format
:param relative: if True, just the relative path is returned
:returns: a path to store information about that instance
"""
pre_grizzly_name = os.path.join(CONF.instances_path, instance.name)
if forceold or os.path.exists(pre_grizzly_name):
if relative:
return instance.name
return pre_grizzly_name

if relative:
return instance.uuid
return os.path.join(CONF.instances_path, instance.uuid)
Expand Down
11 changes: 11 additions & 0 deletions releasenotes/notes/instance-path-2efca507456d8a70.yaml
@@ -0,0 +1,11 @@
---
upgrade:
- Prior to Grizzly release default instance directory names were based on
instance.id field, for example directory for instance could be named
``instance-00000008``. In Grizzly this mechanism was changed,
instance.uuid is used as an instance directory name, e.g. path to instance:
``/opt/stack/data/nova/instances/34198248-5541-4d52-a0b4-a6635a7802dd/``.
In Newton backward compatibility is dropped. For instances that haven't
been restarted since Folsom and earlier maintanance should be scheduled
before upgrade(stop, rename directory to instance.uuid, then start) so Nova
will start using new paths for instances.

0 comments on commit 5ce0dbd

Please sign in to comment.