Merge "Add new style volume attachment support to block_device.py"

This commit is contained in:
Zuul 2017-12-09 02:58:58 +00:00 committed by Gerrit Code Review
commit 5d97937c3c
2 changed files with 182 additions and 59 deletions

View File

@ -32,6 +32,10 @@ from nova.volume import cinder
class TestDriverBlockDevice(test.NoDBTestCase):
# This is used to signal if we're dealing with a new style volume
# attachment (Cinder v3.44 flow).
attachment_id = None
driver_classes = {
'swap': driver_block_device.DriverSwapBlockDevice,
'ephemeral': driver_block_device.DriverEphemeralBlockDevice,
@ -101,7 +105,6 @@ class TestDriverBlockDevice(test.NoDBTestCase):
'boot_index': 0})
volume_driver_bdm = {
'attachment_id': None,
'mount_device': '/dev/sda1',
'connection_info': {"fake": "connection_info"},
'delete_on_termination': False,
@ -130,7 +133,6 @@ class TestDriverBlockDevice(test.NoDBTestCase):
'boot_index': -1})
snapshot_driver_bdm = {
'attachment_id': None,
'mount_device': '/dev/sda2',
'connection_info': {"fake": "connection_info"},
'delete_on_termination': True,
@ -159,7 +161,6 @@ class TestDriverBlockDevice(test.NoDBTestCase):
'boot_index': -1})
image_driver_bdm = {
'attachment_id': None,
'mount_device': '/dev/sda2',
'connection_info': {"fake": "connection_info"},
'delete_on_termination': True,
@ -188,7 +189,6 @@ class TestDriverBlockDevice(test.NoDBTestCase):
'boot_index': -1})
blank_driver_bdm = {
'attachment_id': None,
'mount_device': '/dev/sda2',
'connection_info': {"fake": "connection_info"},
'delete_on_termination': True,
@ -222,6 +222,14 @@ class TestDriverBlockDevice(test.NoDBTestCase):
self.blank_bdm = fake_block_device.fake_bdm_object(
self.context, self.blank_bdm_dict)
# Set the attachment_id on our fake class variables which we have
# to do in setUp so that any attachment_id set by a subclass will
# be used properly.
for name in ('volume', 'snapshot', 'image', 'blank'):
for attr in ('%s_bdm', '%s_driver_bdm'):
bdm = getattr(self, attr % name)
bdm['attachment_id'] = self.attachment_id
def test_no_device_raises(self):
for name, cls in self.driver_classes.items():
bdm = fake_block_device.fake_bdm_object(
@ -443,13 +451,20 @@ class TestDriverBlockDevice(test.NoDBTestCase):
fake_volume,
instance=instance).AndRaise(
test.TestingException)
# The @update_db decorator will save any changes.
driver_bdm._bdm_obj.save().AndReturn(None)
return instance, expected_conn_info
self.virt_driver.get_volume_connector(instance).AndReturn(connector)
self.volume_api.initialize_connection(
elevated_context, fake_volume['id'],
connector).AndReturn(connection_info)
if self.attachment_id is None:
self.volume_api.initialize_connection(
elevated_context, fake_volume['id'],
connector).AndReturn(connection_info)
else:
self.volume_api.attachment_update(
elevated_context, self.attachment_id, connector).AndReturn(
{'connection_info': connection_info})
if driver_attach:
encryptors.get_encryption_metadata(
elevated_context, self.volume_api, fake_volume['id'],
@ -468,34 +483,52 @@ class TestDriverBlockDevice(test.NoDBTestCase):
disk_bus=bdm_dict['disk_bus'],
device_type=bdm_dict['device_type'],
encryption=enc_data).AndRaise(test.TestingException)
self.volume_api.terminate_connection(
elevated_context, fake_volume['id'],
connector).AndReturn(None)
if self.attachment_id is None:
self.volume_api.terminate_connection(
elevated_context, fake_volume['id'],
connector).AndReturn(None)
else:
self.volume_api.attachment_delete(
elevated_context, self.attachment_id).AndReturn(None)
# The @update_db decorator will save any changes.
driver_bdm._bdm_obj.save().AndReturn(None)
return instance, expected_conn_info
if volume_attach:
# save updates before marking the volume as in-use
driver_bdm._bdm_obj.save().AndReturn(None)
if not fail_volume_attach:
self.volume_api.attach(elevated_context, fake_volume['id'],
uuids.uuid, bdm_dict['device_name'],
mode=access_mode).AndReturn(None)
if self.attachment_id is None:
self.volume_api.attach(elevated_context, fake_volume['id'],
uuids.uuid, bdm_dict['device_name'],
mode=access_mode).AndReturn(None)
else:
self.volume_api.attachment_complete(
elevated_context, self.attachment_id).AndReturn(None)
else:
self.volume_api.attach(elevated_context, fake_volume['id'],
uuids.uuid, bdm_dict['device_name'],
mode=access_mode).AndRaise(
test.TestingException)
if driver_attach:
self.virt_driver.detach_volume(
if self.attachment_id is None:
self.volume_api.attach(elevated_context, fake_volume['id'],
uuids.uuid, bdm_dict['device_name'],
mode=access_mode).AndRaise(
test.TestingException)
if driver_attach:
self.virt_driver.detach_volume(
expected_conn_info, instance,
bdm_dict['device_name'],
encryption=enc_data).AndReturn(None)
self.volume_api.terminate_connection(
self.volume_api.terminate_connection(
elevated_context, fake_volume['id'],
connector).AndReturn(None)
self.volume_api.detach(elevated_context,
fake_volume['id']).AndReturn(None)
self.volume_api.detach(elevated_context,
fake_volume['id']).AndReturn(None)
else:
self.volume_api.attachment_complete(
elevated_context, self.attachment_id).AndRaise(
test.TestingException)
self.volume_api.attachment_delete(
elevated_context, self.attachment_id).AndReturn(None)
# The @update_db decorator will save any changes.
driver_bdm._bdm_obj.save().AndReturn(None)
return instance, expected_conn_info
@ -648,10 +681,16 @@ class TestDriverBlockDevice(test.NoDBTestCase):
self.mox.StubOutWithMock(test_bdm._bdm_obj, 'save')
self.virt_driver.get_volume_connector(instance).AndReturn(connector)
self.volume_api.initialize_connection(
self.context, test_bdm.volume_id,
connector).AndReturn(connection_info)
if self.attachment_id is None:
self.virt_driver.get_volume_connector(instance).AndReturn(
connector)
self.volume_api.initialize_connection(
self.context, test_bdm.volume_id,
connector).AndReturn(connection_info)
else:
self.volume_api.attachment_get(
self.context, self.attachment_id).AndReturn(
{'connection_info': connection_info})
test_bdm._bdm_obj.save().AndReturn(None)
self.mox.ReplayAll()
@ -661,36 +700,14 @@ class TestDriverBlockDevice(test.NoDBTestCase):
self.assertThat(test_bdm['connection_info'],
matchers.DictMatches(expected_conn_info))
def test_refresh_connection_info_with_attachment_id(self):
"""Tests refreshing connection info when the DriverVolumeBlockDevice
has a new style attachment_id set, which is a call to attachment_get
rather than initialize_connection.
"""
test_bdm = self.driver_classes['volume'](self.volume_bdm)
test_bdm['attachment_id'] = uuids.attachment_id
connection_info = {'data': {'multipath_id': 'fake_multipath_id'}}
fake_attachment = dict(connection_info=connection_info)
expected_conn_info = {'data': {'multipath_id': 'fake_multipath_id'},
'serial': self.volume_bdm.volume_id}
with mock.patch('nova.volume.cinder.API') as volume_api:
with mock.patch.object(test_bdm, 'save') as bdm_save:
volume_api.attachment_get.return_value = fake_attachment
test_bdm.refresh_connection_info(
self.context, mock.sentinel.instance, volume_api,
mock.sentinel.virt_driver)
volume_api.attachment_get.assert_called_once_with(
self.context, uuids.attachment_id)
self.assertDictEqual(expected_conn_info, test_bdm['connection_info'])
bdm_save.assert_called_once_with()
def test_snapshot_attach_no_volume(self):
no_volume_snapshot = self.snapshot_bdm_dict.copy()
no_volume_snapshot['volume_id'] = None
test_bdm = self.driver_classes['snapshot'](
fake_block_device.fake_bdm_object(
self.context, no_volume_snapshot))
# When we create a volume, we attach it using the old flow.
self.attachment_id = None
snapshot = {'id': 'fake-volume-id-1',
'attach_status': 'detached'}
@ -721,6 +738,8 @@ class TestDriverBlockDevice(test.NoDBTestCase):
test_bdm = self.driver_classes['snapshot'](
fake_block_device.fake_bdm_object(
self.context, no_volume_snapshot))
# When we create a volume, we attach it using the old flow.
self.attachment_id = None
snapshot = {'id': 'fake-volume-id-1',
'attach_status': 'detached'}
@ -809,6 +828,8 @@ class TestDriverBlockDevice(test.NoDBTestCase):
test_bdm = self.driver_classes['image'](
fake_block_device.fake_bdm_object(
self.context, no_volume_image))
# When we create a volume, we attach it using the old flow.
self.attachment_id = None
image = {'id': 'fake-image-id-1'}
volume = {'id': 'fake-volume-id-2',
@ -836,6 +857,8 @@ class TestDriverBlockDevice(test.NoDBTestCase):
test_bdm = self.driver_classes['image'](
fake_block_device.fake_bdm_object(
self.context, no_volume_image))
# When we create a volume, we attach it using the old flow.
self.attachment_id = None
image = {'id': 'fake-image-id-1'}
volume = {'id': 'fake-volume-id-2',
@ -1131,3 +1154,10 @@ class TestDriverBlockDevice(test.NoDBTestCase):
# can't assert_not_called if the method isn't in the spec.
self.assertFalse(hasattr(test_eph, 'refresh_connection_info'))
self.assertFalse(hasattr(test_swap, 'refresh_connection_info'))
class TestDriverBlockDeviceNewFlow(TestDriverBlockDevice):
"""Virt block_device tests for the Cinder 3.44 volume attach flow
where a volume BDM has an attachment_id.
"""
attachment_id = uuids.attachment_id

View File

@ -358,17 +358,11 @@ class DriverVolumeBlockDevice(DriverBlockDevice):
else:
volume_api.attachment_delete(context, self['attachment_id'])
@update_db
def attach(self, context, instance, volume_api, virt_driver,
do_driver_attach=False, **kwargs):
volume = volume_api.get(context, self.volume_id)
volume_api.check_availability_zone(context, volume,
instance=instance)
def _legacy_volume_attach(self, context, volume, connector, instance,
volume_api, virt_driver,
do_driver_attach=False):
volume_id = volume['id']
context = context.elevated()
connector = virt_driver.get_volume_connector(instance)
connection_info = volume_api.initialize_connection(context,
volume_id,
connector)
@ -434,6 +428,96 @@ class DriverVolumeBlockDevice(DriverBlockDevice):
# happen, the detach request will be ignored.
volume_api.detach(context, volume_id)
def _volume_attach(self, context, volume, connector, instance,
volume_api, virt_driver, attachment_id,
do_driver_attach=False):
# This is where we actually (finally) make a call down to the device
# driver and actually create/establish the connection. We'll go from
# here to block driver-->os-brick and back up.
volume_id = volume['id']
if self.volume_size is None:
self.volume_size = volume.get('size')
LOG.debug("Updating existing volume attachment record: %s",
attachment_id, instance=instance)
connection_info = volume_api.attachment_update(
context, attachment_id, connector)['connection_info']
if 'serial' not in connection_info:
connection_info['serial'] = self.volume_id
self._preserve_multipath_id(connection_info)
if do_driver_attach:
encryption = encryptors.get_encryption_metadata(
context, volume_api, volume_id, connection_info)
try:
virt_driver.attach_volume(
context, connection_info, instance,
self['mount_device'], disk_bus=self['disk_bus'],
device_type=self['device_type'], encryption=encryption)
except Exception:
with excutils.save_and_reraise_exception():
LOG.exception("Driver failed to attach volume "
"%(volume_id)s at %(mountpoint)s",
{'volume_id': volume_id,
'mountpoint': self['mount_device']},
instance=instance)
volume_api.attachment_delete(context,
attachment_id)
# NOTE(mriedem): save our current state so connection_info is in
# the database before the volume status goes to 'in-use' because
# after that we can detach and connection_info is required for
# detach.
# TODO(mriedem): Technically for the new flow, we shouldn't have to
# rely on the BlockDeviceMapping.connection_info since it's stored
# with the attachment in Cinder (see refresh_connection_info).
# Therefore we should phase out code that relies on the
# BDM.connection_info and get it from Cinder if it's needed.
self['connection_info'] = connection_info
self.save()
try:
# This marks the volume as "in-use".
volume_api.attachment_complete(context, attachment_id)
except Exception:
with excutils.save_and_reraise_exception():
if do_driver_attach:
# Disconnect the volume from the host.
try:
virt_driver.detach_volume(connection_info,
instance,
self['mount_device'],
encryption=encryption)
except Exception:
LOG.warning("Driver failed to detach volume "
"%(volume_id)s at %(mount_point)s.",
{'volume_id': volume_id,
'mount_point': self['mount_device']},
exc_info=True, instance=instance)
# Delete the attachment to mark the volume as "available".
volume_api.attachment_delete(context, self['attachment_id'])
@update_db
def attach(self, context, instance, volume_api, virt_driver,
do_driver_attach=False, **kwargs):
volume = volume_api.get(context, self.volume_id)
volume_api.check_availability_zone(context, volume,
instance=instance)
context = context.elevated()
connector = virt_driver.get_volume_connector(instance)
if not self['attachment_id']:
self._legacy_volume_attach(context, volume, connector, instance,
volume_api, virt_driver,
do_driver_attach)
else:
self._volume_attach(context, volume, connector, instance,
volume_api, virt_driver,
self['attachment_id'],
do_driver_attach)
@update_db
def refresh_connection_info(self, context, instance,
volume_api, virt_driver):
@ -502,6 +586,9 @@ class DriverSnapshotBlockDevice(DriverVolumeBlockDevice):
self.volume_id = vol['id']
# TODO(mriedem): Create an attachment to reserve the volume and
# make us go down the new-style attach flow.
# Call the volume attach now
super(DriverSnapshotBlockDevice, self).attach(
context, instance, volume_api, virt_driver)
@ -524,6 +611,9 @@ class DriverImageBlockDevice(DriverVolumeBlockDevice):
self.volume_id = vol['id']
# TODO(mriedem): Create an attachment to reserve the volume and
# make us go down the new-style attach flow.
super(DriverImageBlockDevice, self).attach(
context, instance, volume_api, virt_driver)
@ -545,6 +635,9 @@ class DriverBlankBlockDevice(DriverVolumeBlockDevice):
self.volume_id = vol['id']
# TODO(mriedem): Create an attachment to reserve the volume and
# make us go down the new-style attach flow.
super(DriverBlankBlockDevice, self).attach(
context, instance, volume_api, virt_driver)