Add new style volume attachment support to block_device.py
This plumbs in the new-style volume attachment support to the virt driver block device code which does the actual work of attaching the volume via the driver and completing the attachment with cinder (the thing that makes the volume status "in-use"). At this point, none of the new flow is exercised outside of tests because we are not yet setting the attachment_id when attaching volumes, that comes in a later change to the API. It's worth noting that when nova creates a volume during boot from volume for a source_type=blank/image/snapshot BDM, we won't have an attachment_id for that volume so we'll attach using the old flow. For now this is OK, but might be something we need to revisit in the future when we eventually want to remove the old flow. The structure here is to separate the old/new attach flows into separate methods, despite quite a bit of copy/paste code in both places. A single method using conditionals in different places is arguably uglier (as seen in the test code) and we could likely consolidate some of the common code into helper methods later. Note that test_refresh_connection_info_with_attachment_id is removed since it's now redundant with test_refresh_connection. Co-Authored-By: Matt Riedemann <mriedem.os@gmail.com> Part of blueprint cinder-new-attach-apis Change-Id: Ib1b6b223c9d04579828d47607006ecd98b472e5a
This commit is contained in:
John Griffith
Matt Riedemann
parent
82de8bc5ac
commit
2d68fbe565
@ -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
|
||||
|
||||
@ -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)
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user