Fix issue with not removing rbd rescue disk

Currently when instance that use RBD as backend
is rescued and next unrescued, rescue image is
not removed, this cause issue when the same
instance is rescued again it's use old rescue
image not new one.

Change-Id: Idf4086303baa4b936c90be89552ad8deb45cef3a
Closes-Bug: #1475652
This commit is contained in:
Bartek Zurawski 2016-05-10 17:31:19 +02:00 committed by melanie witt
parent 6228068b1b
commit c12d388070
4 changed files with 93 additions and 29 deletions

View File

@ -341,12 +341,15 @@ class RbdTestCase(test.NoDBTestCase):
def test_cleanup_volumes(self, mock_client, mock_rados, mock_rbd):
instance = objects.Instance(id=1, uuid=uuids.instance,
task_state=None)
# this is duplicated from nova/virt/libvirt/driver.py
filter_fn = lambda disk: disk.startswith(instance.uuid)
rbd = mock_rbd.RBD.return_value
rbd.list.return_value = ['%s_test' % uuids.instance, '111_test']
client = mock_client.return_value
self.driver.cleanup_volumes(instance)
self.driver.cleanup_volumes(filter_fn)
rbd.remove.assert_called_once_with(client.ioctx,
'%s_test' % uuids.instance)
client.__enter__.assert_called_once_with()
@ -359,6 +362,8 @@ class RbdTestCase(test.NoDBTestCase):
mock_client, mock_rados, mock_rbd):
instance = objects.Instance(id=1, uuid=uuids.instance,
task_state=None)
# this is duplicated from nova/virt/libvirt/driver.py
filter_fn = lambda disk: disk.startswith(instance.uuid)
setattr(mock_rbd, exception_name, test.TestingException)
rbd = mock_rbd.RBD.return_value
@ -367,7 +372,7 @@ class RbdTestCase(test.NoDBTestCase):
client = mock_client.return_value
with mock.patch('eventlet.greenthread.sleep'):
self.driver.cleanup_volumes(instance)
self.driver.cleanup_volumes(filter_fn)
rbd.remove.assert_any_call(client.ioctx, '%s_test' % uuids.instance)
# NOTE(danms): 10 retries + 1 final attempt to propagate = 11
self.assertEqual(11, len(rbd.remove.call_args_list))
@ -390,6 +395,8 @@ class RbdTestCase(test.NoDBTestCase):
mock_rados, mock_rbd):
instance = objects.Instance(id=1, uuid=uuids.instance,
task_state=None)
# this is duplicated from nova/virt/libvirt/driver.py
filter_fn = lambda disk: disk.startswith(instance.uuid)
setattr(mock_rbd, 'ImageHasSnapshots', test.TestingException)
rbd = mock_rbd.RBD.return_value
@ -400,7 +407,7 @@ class RbdTestCase(test.NoDBTestCase):
proxy.list_snaps.return_value = [
{'name': libvirt_utils.RESIZE_SNAPSHOT_NAME}]
client = mock_client.return_value
self.driver.cleanup_volumes(instance)
self.driver.cleanup_volumes(filter_fn)
remove_call = mock.call(client.ioctx, '%s_test' % uuids.instance)
rbd.remove.assert_has_calls([remove_call, remove_call])
@ -416,13 +423,16 @@ class RbdTestCase(test.NoDBTestCase):
mock_rbd):
instance = objects.Instance(id=1, uuid=uuids.instance,
task_state=task_states.RESIZE_REVERTING)
# this is duplicated from nova/virt/libvirt/driver.py
filter_fn = lambda disk: (disk.startswith(instance.uuid) and
disk.endswith('disk.local'))
rbd = mock_rbd.RBD.return_value
rbd.list.return_value = ['%s_test' % uuids.instance, '111_test',
'%s_test_disk.local' % uuids.instance]
client = mock_client.return_value
self.driver.cleanup_volumes(instance)
self.driver.cleanup_volumes(filter_fn)
rbd.remove.assert_called_once_with(
client.ioctx,
'%s_test_disk.local' % uuids.instance)

View File

@ -11601,16 +11601,48 @@ class LibvirtConnTestCase(test.NoDBTestCase):
drvr.destroy(self.context, instance, [])
mock_save.assert_called_once_with()
@mock.patch.object(rbd_utils, 'RBDDriver')
def test_cleanup_rbd(self, mock_driver):
driver = mock_driver.return_value
driver.cleanup_volumes = mock.Mock()
fake_instance = {'uuid': '875a8070-d0b9-4949-8b31-104d125c9a64'}
@mock.patch.object(rbd_utils.RBDDriver, '_destroy_volume')
@mock.patch.object(rbd_utils.RBDDriver, '_disconnect_from_rados')
@mock.patch.object(rbd_utils.RBDDriver, '_connect_to_rados')
@mock.patch.object(rbd_utils, 'rbd')
@mock.patch.object(rbd_utils, 'rados')
def test_cleanup_rbd(self, mock_rados, mock_rbd, mock_connect,
mock_disconnect, mock_destroy_volume):
mock_connect.return_value = mock.MagicMock(), mock.MagicMock()
instance = objects.Instance(**self.test_instance)
all_volumes = [uuids.other_instance + '_disk',
uuids.other_instance + '_disk.swap',
instance.uuid + '_disk',
instance.uuid + '_disk.swap']
mock_rbd.RBD.return_value.list.return_value = all_volumes
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
drvr._cleanup_rbd(fake_instance)
drvr._cleanup_rbd(instance)
calls = [mock.call(mock.ANY, instance.uuid + '_disk'),
mock.call(mock.ANY, instance.uuid + '_disk.swap')]
mock_destroy_volume.assert_has_calls(calls)
self.assertEqual(2, mock_destroy_volume.call_count)
driver.cleanup_volumes.assert_called_once_with(fake_instance)
@mock.patch.object(rbd_utils.RBDDriver, '_destroy_volume')
@mock.patch.object(rbd_utils.RBDDriver, '_disconnect_from_rados')
@mock.patch.object(rbd_utils.RBDDriver, '_connect_to_rados')
@mock.patch.object(rbd_utils, 'rbd')
@mock.patch.object(rbd_utils, 'rados')
def test_cleanup_rbd_resize_reverting(self, mock_rados, mock_rbd,
mock_connect, mock_disconnect,
mock_destroy_volume):
mock_connect.return_value = mock.MagicMock(), mock.MagicMock()
instance = objects.Instance(**self.test_instance)
instance.task_state = task_states.RESIZE_REVERTING
all_volumes = [uuids.other_instance + '_disk',
uuids.other_instance + '_disk.local',
instance.uuid + '_disk',
instance.uuid + '_disk.local']
mock_rbd.RBD.return_value.list.return_value = all_volumes
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
drvr._cleanup_rbd(instance)
mock_destroy_volume.assert_called_once_with(
mock.ANY, instance.uuid + '_disk.local')
@mock.patch.object(objects.Instance, 'save')
def test_destroy_undefines_no_undefine_flags(self, mock_save):
@ -16391,7 +16423,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
@mock.patch.object(libvirt_utils, 'get_instance_path')
@mock.patch.object(libvirt_utils, 'load_file')
@mock.patch.object(host.Host, "get_domain")
def test_unrescue(self, mock_get_domain, mock_load_file,
def _test_unrescue(self, instance, mock_get_domain, mock_load_file,
mock_get_instance_path):
dummyxml = ("<domain type='kvm'><name>instance-0000000a</name>"
"<devices>"
@ -16401,7 +16433,6 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
"</devices></domain>")
mock_get_instance_path.return_value = '/path'
instance = objects.Instance(uuid=uuids.instance, id=1)
fake_dom = FakeVirtDomain(fake_xml=dummyxml)
mock_get_domain.return_value = fake_dom
mock_load_file.return_value = "fake_unrescue_xml"
@ -16446,6 +16477,29 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
self.assertEqual(rescue_file, mock_del.call_args_list[1][0][0])
mock_remove_volumes.assert_called_once_with(['lvm.rescue'])
def test_unrescue(self):
instance = objects.Instance(uuid=uuids.instance, id=1)
self._test_unrescue(instance)
@mock.patch.object(rbd_utils.RBDDriver, '_destroy_volume')
@mock.patch.object(rbd_utils.RBDDriver, '_disconnect_from_rados')
@mock.patch.object(rbd_utils.RBDDriver, '_connect_to_rados')
@mock.patch.object(rbd_utils, 'rbd')
@mock.patch.object(rbd_utils, 'rados')
def test_unrescue_rbd(self, mock_rados, mock_rbd, mock_connect,
mock_disconnect, mock_destroy_volume):
self.flags(images_type='rbd', group='libvirt')
mock_connect.return_value = mock.MagicMock(), mock.MagicMock()
instance = objects.Instance(uuid=uuids.instance, id=1)
all_volumes = [uuids.other_instance + '_disk',
uuids.other_instance + '_disk.rescue',
instance.uuid + '_disk',
instance.uuid + '_disk.rescue']
mock_rbd.RBD.return_value.list.return_value = all_volumes
self._test_unrescue(instance)
mock_destroy_volume.assert_called_once_with(
mock.ANY, instance.uuid + '_disk.rescue')
@mock.patch('shutil.rmtree')
@mock.patch('nova.utils.execute')
@mock.patch('os.path.exists')

View File

@ -994,7 +994,16 @@ class LibvirtDriver(driver.ComputeDriver):
rbd_user=CONF.libvirt.rbd_user)
def _cleanup_rbd(self, instance):
LibvirtDriver._get_rbd_driver().cleanup_volumes(instance)
# NOTE(nic): On revert_resize, the cleanup steps for the root
# volume are handled with an "rbd snap rollback" command,
# and none of this is needed (and is, in fact, harmful) so
# filter out non-ephemerals from the list
if instance.task_state == task_states.RESIZE_REVERTING:
filter_fn = lambda disk: (disk.startswith(instance.uuid) and
disk.endswith('disk.local'))
else:
filter_fn = lambda disk: disk.startswith(instance.uuid)
LibvirtDriver._get_rbd_driver().cleanup_volumes(filter_fn)
def _cleanup_lvm(self, instance, block_device_info):
"""Delete all LVM disks for given instance object."""
@ -2546,6 +2555,10 @@ class LibvirtDriver(driver.ComputeDriver):
# cleanup rescue volume
lvm.remove_volumes([lvmdisk for lvmdisk in self._lvm_disks(instance)
if lvmdisk.endswith('.rescue')])
if CONF.libvirt.images_type == 'rbd':
filter_fn = lambda disk: (disk.startswith(instance.uuid) and
disk.endswith('.rescue'))
LibvirtDriver._get_rbd_driver().cleanup_volumes(filter_fn)
def poll_rebooting_instances(self, timeout, instances):
pass

View File

@ -30,7 +30,6 @@ from oslo_service import loopingcall
from oslo_utils import excutils
from oslo_utils import units
from nova.compute import task_states
from nova import exception
from nova.i18n import _
from nova.i18n import _LE
@ -358,22 +357,10 @@ class RBDDriver(object):
except loopingcall.LoopingCallDone:
pass
def cleanup_volumes(self, instance):
def cleanup_volumes(self, filter_fn):
with RADOSClient(self, self.pool) as client:
def belongs_to_instance(disk):
# NOTE(nic): On revert_resize, the cleanup steps for the root
# volume are handled with an "rbd snap rollback" command,
# and none of this is needed (and is, in fact, harmful) so
# filter out non-ephemerals from the list
if instance.task_state == task_states.RESIZE_REVERTING:
return (disk.startswith(instance.uuid) and
disk.endswith('disk.local'))
else:
return disk.startswith(instance.uuid)
volumes = RbdProxy().list(client.ioctx)
for volume in filter(belongs_to_instance, volumes):
for volume in filter(filter_fn, volumes):
self._destroy_volume(client, volume)
def get_pool_info(self):