openstack
/
nova
Code Issues Proposed changes

Delete disk metadata when detaching volume 

If a volume that was tagged with a device role tag is detached, this
patch deletes its metadata from the instance's device_metadata.

Change-Id: I94ffe4488173f40b31dd15832e5ed3f403b59e1f
Implements: blueprint virt-device-tagged-attach-detach
This commit is contained in:
Artom Lifshitz 2017-02-16 19:37:04 +00:00
parent 0ca64a8690
commit 7d428ac24a
8 changed files with 130 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
nova/api/openstack/compute/volumes.py
View File

@ -435,7 +435,8 @@ class VolumeAttachmentController(wsgi.Controller):
volume_id = id
instance = common.get_instance(self.compute_api, context, server_id)
instance = common.get_instance(self.compute_api, context, server_id,
expected_attrs=['device_metadata'])
if instance.vm_state in (vm_states.SHELVED,
vm_states.SHELVED_OFFLOADED):
_check_request_version(req, '2.20', 'detach_volume',

17
nova/compute/manager.py
View File

@ -4969,9 +4969,26 @@ class ComputeManager(manager.Manager):
phase=fields.NotificationPhase.END,
volume_id=volume_id)
if 'tag' in bdm and bdm.tag:
self._delete_disk_metadata(instance, bdm)
if destroy_bdm:
bdm.destroy()
def _delete_disk_metadata(self, instance, bdm):
for device in instance.device_metadata.devices:
if isinstance(device, objects.DiskMetadata):
if 'serial' in device:
if device.serial == bdm.volume_id:
instance.device_metadata.devices.remove(device)
instance.save()
break
else:
# NOTE(artom) We log the entire device object because all
# fields are nullable and may not be set
LOG.warning('Unable to determine whether to clean up '
'device metadata for disk %s', device,
instance=instance)
@wrap_exception()
@wrap_instance_fault
def detach_volume(self, context, volume_id, instance, attachment_id=None):

25
nova/tests/unit/api/openstack/compute/test_volumes.py
View File

@ -399,15 +399,22 @@ class VolumeAttachTestsV21(test.NoDBTestCase):
self.stubs.Set(compute_api.API,
'detach_volume',
fake_detach_volume)
result = self.attachments.delete(self.req, FAKE_UUID, FAKE_UUID_A)
# NOTE: on v2.1, http status code is set as wsgi_code of API
# method instead of status_int in a response object.
if isinstance(self.attachments,
volumes_v21.VolumeAttachmentController):
status_int = self.attachments.delete.wsgi_code
else:
status_int = result.status_int
self.assertEqual(202, status_int)
inst = fake_instance.fake_instance_obj(self.context,
**{'uuid': FAKE_UUID})
with mock.patch.object(common, 'get_instance',
return_value=inst) as mock_get_instance:
result = self.attachments.delete(self.req, FAKE_UUID, FAKE_UUID_A)
# NOTE: on v2.1, http status code is set as wsgi_code of API
# method instead of status_int in a response object.
if isinstance(self.attachments,
volumes_v21.VolumeAttachmentController):
status_int = self.attachments.delete.wsgi_code
else:
status_int = result.status_int
self.assertEqual(202, status_int)
mock_get_instance.assert_called_with(
self.attachments.compute_api, self.context, FAKE_UUID,
expected_attrs=['device_metadata'])
@mock.patch.object(common, 'get_instance')
def test_detach_vol_shelved_not_supported(self, mock_get_instance):

59
nova/tests/unit/compute/test_compute_mgr.py
View File

@ -2493,6 +2493,28 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
mock_volume_api.terminate_connection.assert_called_once_with(
self.context, uuids.volume_id, connector)
def test_delete_disk_metadata(self):
bdm = objects.BlockDeviceMapping(volume_id=uuids.volume_id, tag='foo')
instance = fake_instance.fake_instance_obj(self.context)
instance.device_metadata = objects.InstanceDeviceMetadata(
devices=[objects.DiskMetadata(serial=uuids.volume_id,
tag='foo')])
instance.save = mock.Mock()
self.compute._delete_disk_metadata(instance, bdm)
self.assertEqual(0, len(instance.device_metadata.devices))
instance.save.assert_called_once_with()
def test_delete_disk_metadata_no_serial(self):
bdm = objects.BlockDeviceMapping(tag='foo')
instance = fake_instance.fake_instance_obj(self.context)
instance.device_metadata = objects.InstanceDeviceMetadata(
devices=[objects.DiskMetadata(tag='foo')])
self.compute._delete_disk_metadata(instance, bdm)
# NOTE(artom) This looks weird because we haven't deleted anything, but
# it's normal behaviour for when DiskMetadata doesn't have serial set
# and we can't find it based on BlockDeviceMapping's volume_id.
self.assertEqual(1, len(instance.device_metadata.devices))
def test_detach_volume(self):
# TODO(lyarwood): Test DriverVolumeBlockDevice.detach in
# ../virt/test_block_device.py
@ -2503,6 +2525,43 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
# ../virt/test_block_device.py
self._test_detach_volume(destroy_bdm=False)
@mock.patch('nova.compute.manager.ComputeManager.'
'_notify_about_instance_usage')
@mock.patch.object(driver_bdm_volume, 'detach')
@mock.patch('nova.compute.utils.notify_about_volume_attach_detach')
@mock.patch('nova.compute.manager.ComputeManager._delete_disk_metadata')
def test_detach_untagged_volume_metadata_not_deleted(
self, mock_delete_metadata, _, __, ___):
inst_obj = mock.Mock()
fake_bdm = fake_block_device.FakeDbBlockDeviceDict(
{'source_type': 'volume', 'destination_type': 'volume',
'volume_id': uuids.volume, 'device_name': '/dev/vdb',
'connection_info': '{"test": "test"}'})
bdm = objects.BlockDeviceMapping(context=self.context, **fake_bdm)
self.compute._detach_volume(self.context, bdm, inst_obj,
destroy_bdm=False,
attachment_id=uuids.attachment)
self.assertFalse(mock_delete_metadata.called)
@mock.patch('nova.compute.manager.ComputeManager.'
'_notify_about_instance_usage')
@mock.patch.object(driver_bdm_volume, 'detach')
@mock.patch('nova.compute.utils.notify_about_volume_attach_detach')
@mock.patch('nova.compute.manager.ComputeManager._delete_disk_metadata')
def test_detach_tagged_volume(self, mock_delete_metadata, _, __, ___):
inst_obj = mock.Mock()
fake_bdm = fake_block_device.FakeDbBlockDeviceDict(
{'source_type': 'volume', 'destination_type': 'volume',
'volume_id': uuids.volume, 'device_name': '/dev/vdb',
'connection_info': '{"test": "test"}', 'tag': 'foo'})
bdm = objects.BlockDeviceMapping(context=self.context, **fake_bdm)
self.compute._detach_volume(self.context, bdm, inst_obj,
destroy_bdm=False,
attachment_id=uuids.attachment)
mock_delete_metadata.assert_called_once_with(inst_obj, bdm)
@mock.patch.object(driver_bdm_volume, 'detach')
@mock.patch('nova.compute.manager.ComputeManager.'
'_notify_about_instance_usage')

12
nova/tests/unit/virt/hyperv/test_block_device_manager.py
View File

@ -68,8 +68,10 @@ class BlockDeviceManagerTestCase(test_base.HyperVBaseTestCase):
'ephemerals': [ephemeral],
}
bdm = self._bdm_mock(device_name=mock.sentinel.dev0, tag='taggy')
eph = self._bdm_mock(device_name=mock.sentinel.dev1, tag='ephy')
bdm = self._bdm_mock(device_name=mock.sentinel.dev0, tag='taggy',
volume_id=mock.sentinel.uuid1)
eph = self._bdm_mock(device_name=mock.sentinel.dev1, tag='ephy',
volume_id=mock.sentinel.uuid2)
mock_get_by_inst_uuid.return_value = [
bdm, eph, self._bdm_mock(device_name=mock.sentinel.dev2, tag=None),
]
@ -83,8 +85,10 @@ class BlockDeviceManagerTestCase(test_base.HyperVBaseTestCase):
mock_get_device_bus.assert_has_calls(
[mock.call(root_disk), mock.call(ephemeral)], any_order=True)
mock_DiskMetadata.assert_has_calls(
[mock.call(bus=mock_get_device_bus.return_value, tags=[bdm.tag]),
mock.call(bus=mock_get_device_bus.return_value, tags=[eph.tag])],
[mock.call(bus=mock_get_device_bus.return_value,
serial=bdm.volume_id, tags=[bdm.tag]),
mock.call(bus=mock_get_device_bus.return_value,
serial=eph.volume_id, tags=[eph.tag])],
any_order=True)
self.assertEqual([mock_DiskMetadata.return_value] * 2, bdm_metadata)

29
nova/tests/unit/virt/libvirt/test_driver.py
View File

@ -1560,31 +1560,38 @@ class LibvirtConnTestCase(test.NoDBTestCase,
fake_block_device.FakeDbBlockDeviceDict(
{'id': 1,
'source_type': 'volume', 'destination_type': 'volume',
'device_name': '/dev/sda', 'tag': "db"}),
'device_name': '/dev/sda', 'tag': "db",
'volume_id': uuids.volume_1}),
fake_block_device.FakeDbBlockDeviceDict(
{'id': 2,
'source_type': 'volume', 'destination_type': 'volume',
'device_name': '/dev/hda', 'tag': "nfvfunc1"}),
'device_name': '/dev/hda', 'tag': "nfvfunc1",
'volume_id': uuids.volume_2}),
fake_block_device.FakeDbBlockDeviceDict(
{'id': 3,
'source_type': 'volume', 'destination_type': 'volume',
'device_name': '/dev/sdb', 'tag': "nfvfunc2"}),
'device_name': '/dev/sdb', 'tag': "nfvfunc2",
'volume_id': uuids.volume_3}),
fake_block_device.FakeDbBlockDeviceDict(
{'id': 4,
'source_type': 'volume', 'destination_type': 'volume',
'device_name': '/dev/hdb'}),
'device_name': '/dev/hdb',
'volume_id': uuids.volume_4}),
fake_block_device.FakeDbBlockDeviceDict(
{'id': 5,
'source_type': 'volume', 'destination_type': 'volume',
'device_name': '/dev/vda', 'tag': "nfvfunc3"}),
'device_name': '/dev/vda', 'tag': "nfvfunc3",
'volume_id': uuids.volume_5}),
fake_block_device.FakeDbBlockDeviceDict(
{'id': 6,
'source_type': 'volume', 'destination_type': 'volume',
'device_name': '/dev/vdb', 'tag': "nfvfunc4"}),
'device_name': '/dev/vdb', 'tag': "nfvfunc4",
'volume_id': uuids.volume_6}),
fake_block_device.FakeDbBlockDeviceDict(
{'id': 7,
'source_type': 'volume', 'destination_type': 'volume',
'device_name': '/dev/vdc', 'tag': "nfvfunc5"}),
'device_name': '/dev/vdc', 'tag': "nfvfunc5",
'volume_id': uuids.volume_7}),
]
)
vif = obj_vif.VirtualInterface(context=self.context)
@ -1637,8 +1644,10 @@ class LibvirtConnTestCase(test.NoDBTestCase,
self.assertIsInstance(metadata[0].bus,
objects.SCSIDeviceBus)
self.assertEqual(['db'], metadata[0].tags)
self.assertEqual(uuids.volume_1, metadata[0].serial)
self.assertFalse(metadata[0].bus.obj_attr_is_set('address'))
self.assertEqual(['nfvfunc1'], metadata[1].tags)
self.assertEqual(uuids.volume_2, metadata[1].serial)
self.assertIsInstance(metadata[1],
objects.DiskMetadata)
self.assertIsInstance(metadata[1].bus,
@ -1650,19 +1659,25 @@ class LibvirtConnTestCase(test.NoDBTestCase,
self.assertIsInstance(metadata[2].bus,
objects.USBDeviceBus)
self.assertEqual(['nfvfunc2'], metadata[2].tags)
self.assertEqual(uuids.volume_3, metadata[2].serial)
self.assertFalse(metadata[2].bus.obj_attr_is_set('address'))
self.assertIsInstance(metadata[3],
objects.DiskMetadata)
self.assertIsInstance(metadata[3].bus,
objects.PCIDeviceBus)
self.assertEqual(['nfvfunc3'], metadata[3].tags)
# NOTE(artom) We're not checking volume 4 because it's not tagged
# and only tagged devices appear in the metadata
self.assertEqual(uuids.volume_5, metadata[3].serial)
self.assertEqual('0000:00:09.0', metadata[3].bus.address)
self.assertIsInstance(metadata[4],
objects.DiskMetadata)
self.assertEqual(['nfvfunc4'], metadata[4].tags)
self.assertEqual(uuids.volume_6, metadata[4].serial)
self.assertIsInstance(metadata[5],
objects.DiskMetadata)
self.assertEqual(['nfvfunc5'], metadata[5].tags)
self.assertEqual(uuids.volume_7, metadata[5].serial)
self.assertIsInstance(metadata[6],
objects.NetworkInterfaceMetadata)
self.assertIsInstance(metadata[6].bus,

1
nova/virt/hyperv/block_device_manager.py
View File

@ -85,6 +85,7 @@ class BlockDeviceInfoManager(object):
if bdm_obj and 'tag' in bdm_obj and bdm_obj.tag:
bus = self._get_device_bus(bdm)
device = objects.DiskMetadata(bus=bus,
serial=bdm_obj.volume_id,
tags=[bdm_obj.tag])
bdm_metadata.append(device)

5
nova/virt/libvirt/driver.py
View File

@ -7855,6 +7855,11 @@ class LibvirtDriver(driver.ComputeDriver):
continue
bus = self._prepare_device_bus(dev)
device = objects.DiskMetadata(tags=[bdm.tag])
# NOTE(artom) Setting the serial (which corresponds to
# volume_id in BlockDeviceMapping) in DiskMetadata allows us to
# find the disks's BlockDeviceMapping object when we detach the
# volume and want to clean up its metadata.
device.serial = bdm.volume_id
if bus:
device.bus = bus
devices.append(device)