SF ensure the correct volume is deleted
When two management systems are available openstack/cinder and SolidFire-API/GUI a volume can be deleted though either interface. Thus if a volume is deleted through the SolidFire-interface and then again through the openstack/cinder interface it was possible to delete another volume. The original code made an incorrect assumption on all the possible values returned from a query to find the target volume. This could lead to deleting a volume that was not the target of the original delete command from openstack/cinder. The correction in the module _get_sfvol_by_cinder_vref validates that the returned object/volumes-id matchs the requested volume-id with a comparison of svol['volumeID'] to int(sf_vid) Closes-Bug: #1782373 Change-Id: I92a8e76c64c4e23dd185875e009132938ace5091
This commit is contained in:
Jay Rubenstein
jarbassaidai
parent
5246d383e7
commit
d1b3adfbe4
|
|
@ -35,6 +35,29 @@ from cinder.volume import qos_specs
|
|||
from cinder.volume import volume_types
|
||||
|
||||
|
||||
class mock_vref(object):
|
||||
def __init__(self):
|
||||
self._name_id = None
|
||||
self.admin_metadata = {}
|
||||
self.attach_status = 'detached'
|
||||
self.id = '262b9ce2-a71a-4fbe-830c-c20c5596caea'
|
||||
self.project_id = '52423d9394ad4c67b3b9034da58cedbc'
|
||||
self.provider_id = '5 4 6ecebf5d-5521-4ce1-80f3-358ebc1b9cdc'
|
||||
self.size = 20
|
||||
|
||||
def __setitem__(self, item, value):
|
||||
self.__dict__[item] = value
|
||||
|
||||
def __getitem__(self, item):
|
||||
return self.__dict__[item]
|
||||
|
||||
def get(self, item, arg2 = None):
|
||||
return self.__dict__[item]
|
||||
|
||||
f_uuid = ['262b9ce2-a71a-4fbe-830c-c20c5596caea',
|
||||
'362b9ce2-a71a-4fbe-830c-c20c5596caea']
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class SolidFireVolumeTestCase(test.TestCase):
|
||||
|
||||
|
|
@ -74,7 +97,9 @@ class SolidFireVolumeTestCase(test.TestCase):
|
|||
'size': 1,
|
||||
'id': 'a720b3c0-d1f0-11e1-9b23-0800200c9a66',
|
||||
'volume_type_id': 'fast',
|
||||
'created_at': timeutils.utcnow()}
|
||||
'created_at': timeutils.utcnow(),
|
||||
'attributes':
|
||||
{'uuid': '262b9ce2-a71a-4fbe-830c-c20c5596caea'}}
|
||||
self.fake_image_meta = {'id': '17c550bb-a411-44c0-9aaf-0d96dd47f501',
|
||||
'updated_at': datetime.datetime(2013, 9,
|
||||
28, 15,
|
||||
|
|
@ -164,7 +189,7 @@ class SolidFireVolumeTestCase(test.TestCase):
|
|||
'enable512e': True,
|
||||
'access': "readWrite",
|
||||
'status': "active",
|
||||
'attributes': {},
|
||||
'attributes': {'uuid': f_uuid[0]},
|
||||
'qos': None,
|
||||
'iqn': test_name}]}}
|
||||
return result
|
||||
|
|
@ -184,6 +209,35 @@ class SolidFireVolumeTestCase(test.TestCase):
|
|||
'qos': None,
|
||||
'iqn': test_name}]}}
|
||||
return result
|
||||
elif method is 'ListVolumes':
|
||||
test_name = "get_sfvol_by_cinder"
|
||||
result = {'result': {
|
||||
'volumes': [{'volumeID': 5,
|
||||
'name': test_name,
|
||||
'accountID': 8,
|
||||
'sliceCount': 1,
|
||||
'totalSize': int(1.75 * units.Gi),
|
||||
'enable512e': True,
|
||||
'access': "readWrite",
|
||||
'status': "active",
|
||||
'attributes': {'uuid': f_uuid[0]},
|
||||
'qos': None,
|
||||
'iqn': test_name},
|
||||
{'volumeID': 15,
|
||||
'name': test_name,
|
||||
'accountID': 8,
|
||||
'sliceCount': 1,
|
||||
'totalSize': int(1.75 * units.Gi),
|
||||
'enable512e': True,
|
||||
'access': "readWrite",
|
||||
'status': "active",
|
||||
'attributes': {'uuid': f_uuid[1]},
|
||||
'qos': None,
|
||||
'iqn': test_name}]}}
|
||||
for v in result['result']['volumes']:
|
||||
if int(v['volumeID']) == int(params['startVolumeID']):
|
||||
break
|
||||
return v
|
||||
elif method is 'DeleteSnapshot':
|
||||
return {'result': {}}
|
||||
elif method is 'GetClusterVersionInfo':
|
||||
|
|
@ -500,6 +554,116 @@ class SolidFireVolumeTestCase(test.TestCase):
|
|||
self.assertRaises(exception.SolidFireAPIException,
|
||||
sfv._get_sfaccount_by_name, 'some-name')
|
||||
|
||||
def test_get_sfvol_by_cinder_vref_no_provider_id(self):
|
||||
fake_sfaccounts = [{'accountID': 25,
|
||||
'name': 'testprjid',
|
||||
'targetSecret': 'shhhh',
|
||||
'username': 'john-wayne',
|
||||
'volumes': [6, 7, 20]}]
|
||||
self.mock_vref = mock_vref()
|
||||
|
||||
vol_result = {'volumeID': 5,
|
||||
'name': 'test_volume',
|
||||
'accountID': 25,
|
||||
'sliceCount': 1,
|
||||
'totalSize': 1 * units.Gi,
|
||||
'enable512e': True,
|
||||
'access': "readWrite",
|
||||
'status': "active",
|
||||
'attributes': {'uuid': f_uuid[0]},
|
||||
'qos': None,
|
||||
'iqn': 'super_fake_iqn'}
|
||||
|
||||
mod_conf = self.configuration
|
||||
mod_conf.sf_enable_vag = True
|
||||
sfv = solidfire.SolidFireDriver(configuration=mod_conf)
|
||||
with mock.patch.object(sfv,
|
||||
'_get_sfaccounts_for_tenant',
|
||||
return_value = fake_sfaccounts), \
|
||||
mock.patch.object(sfv, '_issue_api_request',
|
||||
side_effect = self.fake_issue_api_request):
|
||||
self.mock_vref['provider_id'] = None
|
||||
sfvol = sfv._get_sfvol_by_cinder_vref(self.mock_vref)
|
||||
self.assertIsNotNone(sfvol)
|
||||
self.assertEqual(sfvol['attributes']['uuid'],
|
||||
vol_result['attributes']['uuid'])
|
||||
self.assertEqual(sfvol['volumeID'], vol_result['volumeID'])
|
||||
|
||||
def test_get_sfvol_by_cinder_vref_no_provider_id_nomatch(self):
|
||||
fake_sfaccounts = [{'accountID': 5,
|
||||
'name': 'testprjid',
|
||||
'targetSecret': 'shhhh',
|
||||
'username': 'john-wayne',
|
||||
'volumes': [5, 6, 7, 8]}]
|
||||
|
||||
self.mock_vref = mock_vref()
|
||||
mod_conf = self.configuration
|
||||
mod_conf.sf_enable_vag = True
|
||||
|
||||
sfv = solidfire.SolidFireDriver(configuration=mod_conf)
|
||||
with mock.patch.object(sfv,
|
||||
'_get_sfaccounts_for_tenant',
|
||||
return_value = fake_sfaccounts), \
|
||||
mock.patch.object(sfv, '_issue_api_request',
|
||||
side_effect = self.fake_issue_api_request):
|
||||
self.mock_vref['provider_id'] = None
|
||||
self.mock_vref['id'] = '142b9c32-a71A-4fbe-830c-c20c5596caea'
|
||||
sfvol = sfv._get_sfvol_by_cinder_vref(self.mock_vref)
|
||||
self.assertIsNone(sfvol)
|
||||
|
||||
def test_get_sfvol_by_cinder_vref_nomatch(self):
|
||||
fake_sfaccounts = [{'accountID': 5,
|
||||
'name': 'testprjid',
|
||||
'targetSecret': 'shhhh',
|
||||
'username': 'john-wayne',
|
||||
'volumes': [5, 6, 7, 8]}]
|
||||
|
||||
self.mock_vref = mock_vref()
|
||||
mod_conf = self.configuration
|
||||
mod_conf.sf_enable_vag = True
|
||||
sfv = solidfire.SolidFireDriver(configuration=mod_conf)
|
||||
with mock.patch.object(sfv,
|
||||
'_get_sfaccounts_for_tenant',
|
||||
return_value = fake_sfaccounts), \
|
||||
mock.patch.object(sfv, '_issue_api_request',
|
||||
side_effect = self.fake_issue_api_request):
|
||||
p_i = '324 8 6ecebf5d-5521-4ce1-80f3-358ebc1b9cdc'
|
||||
self.mock_vref['provider_id'] = p_i
|
||||
self.mock_vref['id'] = '142b9c32-a71A-4fbe-830c-c20c5596caea'
|
||||
sfvol = sfv._get_sfvol_by_cinder_vref(self.mock_vref)
|
||||
self.assertIsNone(sfvol)
|
||||
|
||||
def test_get_sfvol_by_cinder_vref(self):
|
||||
fake_sfaccounts = [{'accountID': 5,
|
||||
'name': 'testprjid',
|
||||
'targetSecret': 'shhhh',
|
||||
'username': 'john-wayne',
|
||||
'volumes': [5, 6, 7, 8]}]
|
||||
|
||||
self.mock_vref = mock_vref()
|
||||
|
||||
get_vol_result = {'volumeID': 5,
|
||||
'name': 'test_volume',
|
||||
'accountID': 25,
|
||||
'sliceCount': 1,
|
||||
'totalSize': 1 * units.Gi,
|
||||
'enable512e': True,
|
||||
'access': "readWrite",
|
||||
'status': "active",
|
||||
'attributes': {'uuid': f_uuid[0]},
|
||||
'qos': None,
|
||||
'iqn': 'super_fake_iqn'}
|
||||
|
||||
sfv = solidfire.SolidFireDriver(configuration=self.configuration)
|
||||
with mock.patch.object(sfv, '_get_sfaccounts_for_tenant',
|
||||
return_value = fake_sfaccounts), \
|
||||
mock.patch.object(sfv, '_issue_api_request',
|
||||
side_effect = self.fake_issue_api_request):
|
||||
|
||||
sfvol = sfv._get_sfvol_by_cinder_vref(self.mock_vref)
|
||||
self.assertIsNotNone(sfvol)
|
||||
self.assertEqual(get_vol_result['volumeID'], sfvol['volumeID'])
|
||||
|
||||
def test_delete_volume(self):
|
||||
vol_id = 'a720b3c0-d1f0-11e1-9b23-0800200c9a66'
|
||||
testvol = test_utils.create_volume(
|
||||
|
|
|
|||
|
|
@ -555,7 +555,12 @@ class SolidFireDriver(san.SanISCSIDriver):
|
|||
return vlist
|
||||
|
||||
def _get_sfvol_by_cinder_vref(self, vref):
|
||||
# sfvols is one or more element objects returned from a list call
|
||||
# sfvol is the single volume object that will be returned or it will
|
||||
# be None
|
||||
sfvols = None
|
||||
sfvol = None
|
||||
|
||||
provider_id = vref.get('provider_id', None)
|
||||
if provider_id:
|
||||
try:
|
||||
|
|
@ -573,6 +578,10 @@ class SolidFireDriver(san.SanISCSIDriver):
|
|||
{'startVolumeID': sf_vid,
|
||||
'limit': 1},
|
||||
version='8.0')['result']['volumes'][0]
|
||||
# Bug 1782373 validate the list returned has what we asked
|
||||
# for, check if there was no match
|
||||
if sfvol['volumeID'] != int(sf_vid):
|
||||
sfvol = None
|
||||
except Exception:
|
||||
pass
|
||||
if not sfvol:
|
||||
|
|
@ -582,18 +591,13 @@ class SolidFireDriver(san.SanISCSIDriver):
|
|||
sfvols = self._issue_api_request(
|
||||
'ListVolumesForAccount',
|
||||
{'accountID': account['accountID']})['result']['volumes']
|
||||
if len(sfvols) >= 1:
|
||||
sfvol = sfvols[0]
|
||||
break
|
||||
if not sfvol:
|
||||
# Hmmm, frankly if we get here there's a problem,
|
||||
# but try one last trick
|
||||
LOG.info("Failed to find volume by provider_id or account, "
|
||||
"attempting find by attributes.")
|
||||
for v in sfvols:
|
||||
if v['Attributes'].get('uuid', None):
|
||||
sfvol = v
|
||||
break
|
||||
# Bug 1782373 match single vref.id encase no provider as the
|
||||
# above call will return a list for the account
|
||||
for sfv in sfvols:
|
||||
if sfv['attributes'].get('uuid', None) == vref.id:
|
||||
sfvol = sfv
|
||||
break
|
||||
|
||||
return sfvol
|
||||
|
||||
def _get_sfaccount_by_name(self, sf_account_name, endpoint=None):
|
||||
|
|
@ -1008,6 +1012,8 @@ class SolidFireDriver(san.SanISCSIDriver):
|
|||
tvol['provider_location'] = template_vol['provider_location']
|
||||
tvol['provider_auth'] = template_vol['provider_auth']
|
||||
|
||||
attach_info = None
|
||||
|
||||
try:
|
||||
connector = {'multipath': False}
|
||||
conn = self.initialize_connection(tvol, connector)
|
||||
|
|
@ -1034,7 +1040,8 @@ class SolidFireDriver(san.SanISCSIDriver):
|
|||
exc)
|
||||
LOG.debug('Removing SolidFire Cache Volume (SF ID): %s',
|
||||
vol['volumeID'])
|
||||
self._detach_volume(context, attach_info, tvol, properties)
|
||||
if attach_info is not None:
|
||||
self._detach_volume(context, attach_info, tvol, properties)
|
||||
self._issue_api_request('DeleteVolume', params)
|
||||
self._issue_api_request('PurgeDeletedVolume', params)
|
||||
return
|
||||
|
|
|
|||
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
fixes:
|
||||
- |
|
||||
The Solidfire cinder driver has been fixed to ensure delete happens
|
||||
on the correct volume.
|
||||
Loading…
Reference in New Issue