openstack
/
nova
Code Issues Proposed changes

libvirt: fix native luks encryption failure to find volume_id 

Not all volume types put a 'volume_id' entry in the
connection_info['data'] dict. This change uses a new
utility method to look up the volume_id in the connection_info
data dict and if not found there, uses the 'serial' value
from the connection_info, which we know at least gets set
when the DriverVolumeBlockDevice code attaches the volume.

This also has to update pre_live_migration since the connection_info
dict doesn't have a 'volume_id' key in it. It's unclear what
this code was expecting, or if it ever really worked, but since
an attached volume represented by a BlockDeviceMapping here has
a volume_id attribute, we can just use that. As that code path
was never tested, this updates related unit tests and refactors
the tests to actually use the type of DriverVolumeBlockDevice
objects that the ComputeManager would be sending down to the
driver pre_live_migration method. The hard-to-read squashed
dicts in the tests are also re-formatted so a human can actually
read them.

Change-Id: Ie02d298cd92d5b5ebcbbcd2b0e8be01f197bfafb
Closes-Bug: #1746609
This commit is contained in:
Matt Riedemann 2018-01-31 19:06:46 -05:00
parent c1442d3c8c
commit cafe3d066e
5 changed files with 158 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -7127,6 +7127,40 @@ class LibvirtConnTestCase(test.NoDBTestCase,
mock_encryptor.attach_volume.assert_called_once_with(self.context,
**encryption)
@mock.patch.object(key_manager, 'API')
@mock.patch('os_brick.encryptors.get_encryption_metadata')
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor')
def test_attach_encryptor_encrypted_native_luks_serial(self,
mock_get_encryptor, mock_get_metadata, mock_get_key_mgr):
"""Uses native luks encryption with a provider encryptor and the
connection_info has a serial but not volume_id in the 'data'
sub-dict.
"""
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
mock_encryptor = mock.MagicMock()
mock_get_encryptor.return_value = mock_encryptor
encryption = {'provider': 'luks', 'control_location': 'front-end',
'encryption_key_id': uuids.encryption_key_id}
connection_info = {'serial': uuids.serial, 'data': {}}
# Mock out the key manager
key = u'3734363537333734'
key_encoded = binascii.unhexlify(key)
mock_key = mock.Mock()
mock_key_mgr = mock.Mock()
mock_get_key_mgr.return_value = mock_key_mgr
mock_key_mgr.get.return_value = mock_key
mock_key.get_encoded.return_value = key_encoded
with mock.patch.object(drvr, '_use_native_luks', return_value=True):
with mock.patch.object(drvr._host, 'create_secret') as crt_scrt:
drvr._attach_encryptor(self.context, connection_info,
encryption, allow_native_luks=True)
mock_get_metadata.assert_not_called()
mock_get_encryptor.assert_not_called()
crt_scrt.assert_called_once_with(
'volume', uuids.serial, password=key)
@mock.patch('os_brick.encryptors.get_encryption_metadata')
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._get_volume_encryptor')
def test_detach_encryptor_connection_info_incomplete(self,
@ -10473,15 +10507,42 @@ class LibvirtConnTestCase(test.NoDBTestCase,
target_ret=None, src_supports_native_luks=True,
dest_supports_native_luks=True, allow_native_luks=True):
# Creating testdata
vol = {'block_device_mapping': [
{'connection_info': {'serial': '12345', u'data':
{'device_path':
u'/dev/disk/by-path/ip-1.2.3.4:3260-iqn.abc.12345.opst-lun-X'}},
'mount_device': '/dev/sda'},
{'connection_info': {'serial': '67890', u'data':
{'device_path':
u'/dev/disk/by-path/ip-1.2.3.4:3260-iqn.cde.67890.opst-lun-Z'}},
'mount_device': '/dev/sdb'}]}
c = context.get_admin_context()
instance = objects.Instance(root_device_name='/dev/vda',
**self.test_instance)
bdms = objects.BlockDeviceMappingList(objects=[
fake_block_device.fake_bdm_object(c, {
'connection_info': jsonutils.dumps({
'serial': '12345',
'data': {
'device_path': '/dev/disk/by-path/ip-1.2.3.4:3260'
'-iqn.abc.12345.opst-lun-X'
}
}),
'device_name': '/dev/sda',
'volume_id': uuids.volume1,
'source_type': 'volume',
'destination_type': 'volume'
}),
fake_block_device.fake_bdm_object(c, {
'connection_info': jsonutils.dumps({
'serial': '67890',
'data': {
'device_path': '/dev/disk/by-path/ip-1.2.3.4:3260'
'-iqn.cde.67890.opst-lun-Z'
}
}),
'device_name': '/dev/sdb',
'volume_id': uuids.volume2,
'source_type': 'volume',
'destination_type': 'volume'
})
])
# We go through get_block_device_info to simulate what the
# ComputeManager sends to the driver (make sure we're using the
# correct type of BDM objects since there are many of them and
# they are super confusing).
block_device_info = driver.get_block_device_info(instance, bdms)
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
@ -10496,16 +10557,11 @@ class LibvirtConnTestCase(test.NoDBTestCase,
self.stubs.Set(drvr, '_is_native_luks_available',
lambda: dest_supports_native_luks)
instance = objects.Instance(**self.test_instance)
c = context.get_admin_context()
nw_info = FakeNetworkInfo()
# Creating mocks
self.mox.StubOutWithMock(driver, "block_device_info_get_mapping")
driver.block_device_info_get_mapping(vol
).AndReturn(vol['block_device_mapping'])
self.mox.StubOutWithMock(drvr, "_connect_volume")
for v in vol['block_device_mapping']:
for v in block_device_info['block_device_mapping']:
drvr._connect_volume(c, v['connection_info'], instance,
allow_native_luks=allow_native_luks)
self.mox.StubOutWithMock(drvr, 'plug_vifs')
@ -10526,14 +10582,14 @@ class LibvirtConnTestCase(test.NoDBTestCase,
migrate_data.src_supports_native_luks = True
result = drvr.pre_live_migration(
c, instance, vol, nw_info, None,
c, instance, block_device_info, nw_info, None,
migrate_data=migrate_data)
if not target_ret:
target_ret = self._generate_target_ret()
self.assertEqual(
target_ret,
result.to_legacy_dict(
pre_migration_result=True)['pre_live_migration_result'],
target_ret)
pre_migration_result=True)['pre_live_migration_result'])
@mock.patch.object(os, 'mkdir')
@mock.patch('nova.virt.libvirt.utils.get_instance_path_at_destination')
@ -10617,15 +10673,42 @@ class LibvirtConnTestCase(test.NoDBTestCase,
# Creating testdata, using temp dir.
with utils.tempdir() as tmpdir:
self.flags(instances_path=tmpdir)
vol = {'block_device_mapping': [
{'connection_info': {'serial': '12345', u'data':
{'device_path':
u'/dev/disk/by-path/ip-1.2.3.4:3260-iqn.abc.12345.opst-lun-X'}},
'mount_device': '/dev/sda'},
{'connection_info': {'serial': '67890', u'data':
{'device_path':
u'/dev/disk/by-path/ip-1.2.3.4:3260-iqn.cde.67890.opst-lun-Z'}},
'mount_device': '/dev/sdb'}]}
c = context.get_admin_context()
inst_ref = objects.Instance(root_device_name='/dev/vda',
**self.test_instance)
bdms = objects.BlockDeviceMappingList(objects=[
fake_block_device.fake_bdm_object(c, {
'connection_info': jsonutils.dumps({
'serial': '12345',
'data': {
'device_path': '/dev/disk/by-path/ip-1.2.3.4:3260'
'-iqn.abc.12345.opst-lun-X'
}
}),
'device_name': '/dev/sda',
'volume_id': uuids.volume1,
'source_type': 'volume',
'destination_type': 'volume'
}),
fake_block_device.fake_bdm_object(c, {
'connection_info': jsonutils.dumps({
'serial': '67890',
'data': {
'device_path': '/dev/disk/by-path/ip-1.2.3.4:3260'
'-iqn.cde.67890.opst-lun-Z'
}
}),
'device_name': '/dev/sdb',
'volume_id': uuids.volume2,
'source_type': 'volume',
'destination_type': 'volume'
})
])
# We go through get_block_device_info to simulate what the
# ComputeManager sends to the driver (make sure we're using the
# correct type of BDM objects since there are many of them and
# they are super confusing).
block_device_info = driver.get_block_device_info(inst_ref, bdms)
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
@ -10638,12 +10721,10 @@ class LibvirtConnTestCase(test.NoDBTestCase,
class FakeNetworkInfo(object):
def fixed_ips(self):
return ["test_ip_addr"]
inst_ref = objects.Instance(**self.test_instance)
c = context.get_admin_context()
nw_info = FakeNetworkInfo()
# Creating mocks
self.mox.StubOutWithMock(drvr, "_connect_volume")
for v in vol['block_device_mapping']:
for v in block_device_info['block_device_mapping']:
drvr._connect_volume(c, v['connection_info'], inst_ref,
allow_native_luks=True)
self.mox.StubOutWithMock(drvr, 'plug_vifs')
@ -10661,9 +10742,9 @@ class LibvirtConnTestCase(test.NoDBTestCase,
filename='foo',
src_supports_native_luks=True,
)
ret = drvr.pre_live_migration(c, inst_ref, vol, nw_info, None,
migrate_data)
target_ret = {
ret = drvr.pre_live_migration(
c, inst_ref, block_device_info, nw_info, None, migrate_data)
expected_result = {
'graphics_listen_addrs': {'spice': None,
'vnc': None},
'target_connect_addr': None,
@ -10682,8 +10763,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
'dev': 'sdb',
'type': 'disk'}}}}
self.assertEqual(
ret.to_legacy_dict(True)['pre_live_migration_result'],
target_ret)
expected_result,
ret.to_legacy_dict(True)['pre_live_migration_result'])
self.assertTrue(os.path.exists('%s/%s/' % (tmpdir,
inst_ref['name'])))

25
nova/tests/unit/virt/test_block_device.py
View File

@ -1274,3 +1274,28 @@ class TestDriverBlockDeviceNewFlow(TestDriverBlockDevice):
self.assertRaises(exception.MultiattachNotSupportedByVirtDriver,
test_bdm.attach, self.context, instance,
self.volume_api, self.virt_driver)
class TestGetVolumeId(test.NoDBTestCase):
def test_get_volume_id_none_found(self):
self.assertIsNone(driver_block_device.get_volume_id(None))
self.assertIsNone(driver_block_device.get_volume_id({}))
self.assertIsNone(driver_block_device.get_volume_id({'data': {}}))
def test_get_volume_id_found_volume_id_no_serial(self):
self.assertEqual(uuids.volume_id,
driver_block_device.get_volume_id(
{'data': {'volume_id': uuids.volume_id}}))
def test_get_volume_id_found_no_volume_id_serial(self):
self.assertEqual(uuids.serial,
driver_block_device.get_volume_id(
{'serial': uuids.serial}))
def test_get_volume_id_found_both(self):
# volume_id is taken over serial
self.assertEqual(uuids.volume_id,
driver_block_device.get_volume_id(
{'serial': uuids.serial,
'data': {'volume_id': uuids.volume_id}}))

10
nova/virt/block_device.py
View File

@ -880,3 +880,13 @@ def is_block_device_mapping(bdm):
return (bdm.source_type in ('image', 'volume', 'snapshot', 'blank')
and bdm.destination_type == 'volume'
and is_implemented(bdm))
def get_volume_id(connection_info):
if connection_info:
# Check for volume_id in 'data' and if not there, fallback to
# the 'serial' that the DriverVolumeBlockDevice adds during attach.
volume_id = connection_info.get('data', {}).get('volume_id')
if not volume_id:
volume_id = connection_info.get('serial')
return volume_id

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

@ -1274,8 +1274,8 @@ class LibvirtDriver(driver.ComputeDriver):
"""Get the encryption metadata dict if it is not provided
"""
encryption = {}
if connection_info.get('data', {}).get('volume_id'):
volume_id = connection_info['data']['volume_id']
volume_id = driver_block_device.get_volume_id(connection_info)
if volume_id:
encryption = encryptors.get_encryption_metadata(context,
self._volume_api, volume_id, connection_info)
return encryption
@ -1323,7 +1323,7 @@ class LibvirtDriver(driver.ComputeDriver):
# NOTE(lyarwood): Store the passphrase as a libvirt secret locally
# on the compute node. This secret is used later when generating
# the volume config.
volume_id = connection_info.get('data', {}).get('volume_id')
volume_id = driver_block_device.get_volume_id(connection_info)
self._host.create_secret('volume', volume_id, password=passphrase)
elif encryption:
encryptor = self._get_volume_encryptor(connection_info,
@ -1340,7 +1340,7 @@ class LibvirtDriver(driver.ComputeDriver):
If native LUKS decryption is enabled then delete previously created
Libvirt volume secret from the host.
"""
volume_id = connection_info.get('data', {}).get('volume_id')
volume_id = driver_block_device.get_volume_id(connection_info)
if volume_id and self._host.find_secret('volume', volume_id):
return self._host.delete_secret('volume', volume_id)
if encryption is None:
@ -7427,10 +7427,7 @@ class LibvirtDriver(driver.ComputeDriver):
bdmi.type = disk_info['type']
bdmi.format = disk_info.get('format')
bdmi.boot_index = disk_info.get('boot_index')
volume_id = connection_info.get('volume_id')
volume_secret = None
if volume_id:
volume_secret = self._host.find_secret('volume', volume_id)
volume_secret = self._host.find_secret('volume', vol.volume_id)
if volume_secret:
bdmi.encryption_secret_uuid = volume_secret.UUIDString()

3
nova/virt/libvirt/volume/volume.py
View File

@ -22,6 +22,7 @@ from oslo_log import log as logging
import nova.conf
from nova import exception
from nova import profiler
from nova.virt import block_device as driver_block_device
from nova.virt.libvirt import config as vconfig
import nova.virt.libvirt.driver
from nova.virt.libvirt import utils as libvirt_utils
@ -109,7 +110,7 @@ class LibvirtBaseVolumeDriver(object):
# a shareable disk.
conf.shareable = True
volume_id = connection_info.get('data', {}).get('volume_id')
volume_id = driver_block_device.get_volume_id(connection_info)
volume_secret = None
if volume_id:
volume_secret = self.host.find_secret('volume', volume_id)