Detach volume on device validation failure
If device validation fails when attaching a volume for some driver
operation (copy_volume_data, copy_image_to_volume, copy_volume_to_image)
we may end up with an attached volume that we don't cleanup.
This patch tries to detach the volume if we fail when validating the
device after we have attached the volume.
This happens for example on multipath when we have properly detected the
paths but they are all in a failed state when we try to read from the
device on validation.
Closes-Bug: #1502138
Change-Id: I73be4206930eba7da064e22d86ff2136191acb0b
(cherry picked from commit c5323c6d6c)
This commit is contained in:
Gorka Eguileor
parent
9c6d20e8a3
commit
98c62d2b9d
|
|
@ -3497,7 +3497,7 @@ class VolumeTestCase(BaseVolumeTestCase):
|
|||
"""Test function of create_volume_from_image.
|
||||
|
||||
Test cases call this function to create a volume from image, caller
|
||||
can choose whether to fake out copy_image_to_volume and conle_image,
|
||||
can choose whether to fake out copy_image_to_volume and clone_image,
|
||||
after calling this, test cases should check status of the volume.
|
||||
"""
|
||||
def fake_local_path(volume):
|
||||
|
|
@ -3523,7 +3523,7 @@ class VolumeTestCase(BaseVolumeTestCase):
|
|||
self.stubs.Set(self.volume.driver, 'clone_image', fake_clone_image)
|
||||
self.stubs.Set(image_utils, 'fetch_to_raw', fake_fetch_to_raw)
|
||||
if fakeout_copy_image_to_volume:
|
||||
self.stubs.Set(self.volume, '_copy_image_to_volume',
|
||||
self.stubs.Set(self.volume.driver, 'copy_image_to_volume',
|
||||
fake_copy_image_to_volume)
|
||||
mock_clone_image_volume.return_value = ({}, clone_image_volume)
|
||||
mock_fetch_img.return_value = mock.MagicMock(
|
||||
|
|
@ -3622,6 +3622,32 @@ class VolumeTestCase(BaseVolumeTestCase):
|
|||
self.assertDictEqual(self.volume.stats['pools'],
|
||||
{'_pool0': {'allocated_capacity_gb': 1}})
|
||||
|
||||
@mock.patch('cinder.utils.brick_get_connector_properties')
|
||||
@mock.patch('cinder.utils.brick_get_connector')
|
||||
@mock.patch('cinder.volume.driver.BaseVD.secure_file_operations_enabled')
|
||||
@mock.patch('cinder.volume.driver.BaseVD._detach_volume')
|
||||
def test_create_volume_from_image_unavailable(self, mock_detach,
|
||||
mock_secure, *args):
|
||||
"""Test create volume with ImageCopyFailure
|
||||
|
||||
We'll raise an exception inside _connect_device after volume has
|
||||
already been attached to confirm that it detaches the volume.
|
||||
"""
|
||||
mock_secure.side_effect = NameError
|
||||
|
||||
# We want to test BaseVD copy_image_to_volume and since FakeISCSIDriver
|
||||
# inherits from LVM it overwrites it, so we'll mock it to use the
|
||||
# BaseVD implementation.
|
||||
unbound_copy_method = cinder.volume.driver.BaseVD.copy_image_to_volume
|
||||
bound_copy_method = unbound_copy_method.__get__(self.volume.driver)
|
||||
with mock.patch.object(self.volume.driver, 'copy_image_to_volume',
|
||||
side_effect=bound_copy_method):
|
||||
self.assertRaises(exception.ImageCopyFailure,
|
||||
self._create_volume_from_image,
|
||||
fakeout_copy_image_to_volume=False)
|
||||
# We must have called detach method.
|
||||
self.assertEqual(1, mock_detach.call_count)
|
||||
|
||||
def test_create_volume_from_image_clone_image_volume(self):
|
||||
"""Test create volume from image via image volume.
|
||||
|
||||
|
|
|
|||
|
|
@ -963,7 +963,26 @@ class BaseVD(object):
|
|||
LOG.error(err_msg)
|
||||
raise exception.VolumeBackendAPIException(data=ex_msg)
|
||||
raise exception.VolumeBackendAPIException(data=err_msg)
|
||||
return (self._connect_device(conn), volume)
|
||||
|
||||
try:
|
||||
attach_info = self._connect_device(conn)
|
||||
except exception.DeviceUnavailable as exc:
|
||||
# We may have reached a point where we have attached the volume,
|
||||
# so we have to detach it (do the cleanup).
|
||||
attach_info = exc.kwargs.get('attach_info', None)
|
||||
if attach_info:
|
||||
try:
|
||||
LOG.debug('Device for volume %s is unavailable but did '
|
||||
'attach, detaching it.', volume['id'])
|
||||
self._detach_volume(context, attach_info, volume,
|
||||
properties, force=True,
|
||||
remote=remote)
|
||||
except Exception:
|
||||
LOG.exception(_LE('Error detaching volume %s'),
|
||||
volume['id'])
|
||||
raise
|
||||
|
||||
return (attach_info, volume)
|
||||
|
||||
def _attach_snapshot(self, context, snapshot, properties, remote=False):
|
||||
"""Attach the snapshot."""
|
||||
|
|
@ -1036,17 +1055,26 @@ class BaseVD(object):
|
|||
device = connector.connect_volume(conn['data'])
|
||||
host_device = device['path']
|
||||
|
||||
# Secure network file systems will NOT run as root.
|
||||
root_access = not self.secure_file_operations_enabled()
|
||||
attach_info = {'conn': conn, 'device': device, 'connector': connector}
|
||||
|
||||
if not connector.check_valid_device(host_device, root_access):
|
||||
unavailable = True
|
||||
try:
|
||||
# Secure network file systems will NOT run as root.
|
||||
root_access = not self.secure_file_operations_enabled()
|
||||
unavailable = not connector.check_valid_device(host_device,
|
||||
root_access)
|
||||
except Exception:
|
||||
LOG.exception(_LE('Could not validate device %s'), host_device)
|
||||
|
||||
if unavailable:
|
||||
raise exception.DeviceUnavailable(path=host_device,
|
||||
attach_info=attach_info,
|
||||
reason=(_("Unable to access "
|
||||
"the backend storage "
|
||||
"via the path "
|
||||
"%(path)s.") %
|
||||
{'path': host_device}))
|
||||
return {'conn': conn, 'device': device, 'connector': connector}
|
||||
return attach_info
|
||||
|
||||
def clone_image(self, context, volume,
|
||||
image_location, image_meta,
|
||||
|
|
|
|||
Loading…
Reference in New Issue