Address potential races in SolidFire VAG
While building the initial volume access group solution I realized there were race conditions present. This patch attempts to address those by being far more careful during VAG creation and removal. Tracking multiattach volume detaches around VAG volume removal introduced all sorts of future race conditions so instead multiattach volumes are treated differently and are only removed from VAGs on volume removal. Not ideal but it'll get the job done. Think I covered all the new branches with unit tests. Complexity was introduced but it should be worth it due to the safety that was also introduced. Change-Id: I80034cdf759295c8d379c02ed9e42659590b41a9
This commit is contained in:
parent
23bfbd0407
commit
250127530e
@ -499,43 +499,48 @@ class SolidFireVolumeTestCase(test.TestCase):
|
||||
self.assertRaises(exception.SolidFireAPIException,
|
||||
sfv._get_sfaccount_by_name, 'some-name')
|
||||
|
||||
@mock.patch.object(solidfire.SolidFireDriver, '_issue_api_request')
|
||||
@mock.patch.object(solidfire.SolidFireDriver, '_create_template_account')
|
||||
def test_delete_volume(self,
|
||||
_mock_create_template_account,
|
||||
_mock_issue_api_request):
|
||||
_mock_issue_api_request.return_value = self.mock_stats_data
|
||||
_mock_create_template_account.return_value = 1
|
||||
def test_delete_volume(self):
|
||||
testvol = {'project_id': 'testprjid',
|
||||
'name': 'test_volume',
|
||||
'size': 1,
|
||||
'id': 'a720b3c0-d1f0-11e1-9b23-0800200c9a66',
|
||||
'created_at': timeutils.utcnow(),
|
||||
'provider_id': '1 5 None',
|
||||
'multiattach': True
|
||||
}
|
||||
fake_sfaccounts = [{'accountID': 5,
|
||||
'name': 'testprjid',
|
||||
'targetSecret': 'shhhh',
|
||||
'username': 'john-wayne'}]
|
||||
|
||||
def _fake_do_v_create(project_id, params):
|
||||
return project_id, params
|
||||
get_vol_result = [{'volumeID': 5,
|
||||
'name': 'test_volume',
|
||||
'accountID': 25,
|
||||
'sliceCount': 1,
|
||||
'totalSize': 1 * units.Gi,
|
||||
'enable512e': True,
|
||||
'access': "readWrite",
|
||||
'status': "active",
|
||||
'attributes': {},
|
||||
'qos': None,
|
||||
'iqn': 'super_fake_iqn'}]
|
||||
|
||||
sfv = solidfire.SolidFireDriver(configuration=self.configuration)
|
||||
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), \
|
||||
mock.patch.object(sfv,
|
||||
'_get_account_create_availability',
|
||||
return_value=fake_sfaccounts[0]), \
|
||||
mock.patch.object(sfv,
|
||||
'_do_volume_create',
|
||||
side_effect=_fake_do_v_create):
|
||||
mock.patch.object(sfv,
|
||||
'_get_volumes_for_account',
|
||||
return_value=get_vol_result), \
|
||||
mock.patch.object(sfv,
|
||||
'_issue_api_request'), \
|
||||
mock.patch.object(sfv,
|
||||
'_remove_volume_from_vags') as rem_vol:
|
||||
|
||||
sfv.delete_volume(testvol)
|
||||
rem_vol.assert_called_with(get_vol_result[0]['volumeID'])
|
||||
|
||||
def test_delete_volume_no_volume_on_backend(self):
|
||||
fake_sfaccounts = [{'accountID': 5,
|
||||
@ -1112,73 +1117,11 @@ class SolidFireVolumeTestCase(test.TestCase):
|
||||
sfv, '_issue_api_request', side_effect=_fake_issue_api_req):
|
||||
self.assertEqual(5, sfv._get_sf_volume(test_name, 8)['volumeID'])
|
||||
|
||||
def test_create_vag(self):
|
||||
global counter
|
||||
counter = 0
|
||||
|
||||
def _trick_get_vag(vag_name):
|
||||
# On the second call to get_vag we want to return a fake VAG
|
||||
# result as required by logic of _sf_initialize_connection.
|
||||
global counter
|
||||
vag = {'attributes': {},
|
||||
'deletedVolumes': [],
|
||||
'initiators': [],
|
||||
'name': 'TESTIQN',
|
||||
'volumeAccessGroupID': 1,
|
||||
'volumes': [],
|
||||
'virtualNetworkIDs': []}
|
||||
|
||||
if counter == 1:
|
||||
return [vag]
|
||||
counter += 1
|
||||
|
||||
def test_sf_init_conn_with_vag(self):
|
||||
# Verify with the _enable_vag conf set that we correctly create a VAG.
|
||||
mod_conf = self.configuration
|
||||
mod_conf.sf_enable_vag = True
|
||||
sfv = solidfire.SolidFireDriver(configuration=mod_conf)
|
||||
|
||||
testvol = {'project_id': 'testprjid',
|
||||
'name': 'testvol',
|
||||
'size': 1,
|
||||
'id': 'a720b3c0-d1f0-11e1-9b23-0800200c9a66',
|
||||
'volume_type_id': None,
|
||||
'provider_location': '10.10.7.1:3260 iqn.2010-01.com.'
|
||||
'solidfire:87hg.uuid-2cc06226-cc'
|
||||
'74-4cb7-bd55-14aed659a0cc.4060 0',
|
||||
'provider_auth': 'CHAP stack-1-a60e2611875f40199931f2'
|
||||
'c76370d66b 2FE0CQ8J196R',
|
||||
'provider_geometry': '4096 4096',
|
||||
'created_at': timeutils.utcnow(),
|
||||
'provider_id': "1 1 1"
|
||||
}
|
||||
|
||||
connector = {'initiator': 'iqn.2012-07.org.fake:01'}
|
||||
|
||||
def add_volume_to_vag_check(vol_id, vag_id):
|
||||
self.assertEqual(1, vol_id)
|
||||
self.assertEqual(1, vag_id)
|
||||
|
||||
with mock.patch.object(sfv,
|
||||
'_create_vag',
|
||||
return_value=1), \
|
||||
mock.patch.object(sfv,
|
||||
'_get_vags',
|
||||
side_effect=_trick_get_vag), \
|
||||
mock.patch.object(sfv,
|
||||
'_add_initiator_to_vag'), \
|
||||
mock.patch.object(sfv,
|
||||
'_add_volume_to_vag',
|
||||
side_effect=add_volume_to_vag_check):
|
||||
|
||||
sfv.initialize_connection(testvol, connector)
|
||||
|
||||
def test_remove_vag(self):
|
||||
vag = {'attributes': {},
|
||||
'deletedVolumes': [],
|
||||
'initiators': [],
|
||||
'name': 'TESTIQN',
|
||||
'volumeAccessGroupID': 1,
|
||||
'volumes': [1],
|
||||
'virtualNetworkIDs': []}
|
||||
testvol = {'project_id': 'testprjid',
|
||||
'name': 'testvol',
|
||||
'size': 1,
|
||||
@ -1194,55 +1137,384 @@ class SolidFireVolumeTestCase(test.TestCase):
|
||||
'provider_id': "1 1 1"
|
||||
}
|
||||
connector = {'initiator': 'iqn.2012-07.org.fake:01'}
|
||||
mod_conf = self.configuration
|
||||
mod_conf.sf_enable_vag = True
|
||||
sfv = solidfire.SolidFireDriver(configuration=mod_conf)
|
||||
|
||||
with mock.patch.object(sfv,
|
||||
'_get_vags',
|
||||
return_value=[vag]), \
|
||||
mock.patch.object(sfv,
|
||||
'_remove_vag') as mock_rem_vag:
|
||||
sfv.terminate_connection(testvol, connector, force=False)
|
||||
mock_rem_vag.assert_called_with(vag['volumeAccessGroupID'])
|
||||
|
||||
def test_remove_volume_from_vag(self):
|
||||
vag = {'attributes': {},
|
||||
'deletedVolumes': [],
|
||||
'initiators': [],
|
||||
'name': 'TESTIQN',
|
||||
'volumeAccessGroupID': 1,
|
||||
'volumes': [1, 2],
|
||||
'virtualNetworkIDs': []}
|
||||
testvol = {'project_id': 'testprjid',
|
||||
'name': 'testvol',
|
||||
'size': 1,
|
||||
'id': 'a720b3c0-d1f0-11e1-9b23-0800200c9a66',
|
||||
'volume_type_id': None,
|
||||
'provider_location': '10.10.7.1:3260 iqn.2010-01.com.'
|
||||
'solidfire:87hg.uuid-2cc06226-cc'
|
||||
'74-4cb7-bd55-14aed659a0cc.4060 0',
|
||||
'provider_auth': 'CHAP stack-1-a60e2611875f40199931f2'
|
||||
'c76370d66b 2FE0CQ8J196R',
|
||||
'provider_geometry': '4096 4096',
|
||||
'created_at': timeutils.utcnow(),
|
||||
'provider_id': "1 1 1"
|
||||
}
|
||||
connector = {'initiator': 'iqn.2012-07.org.fake:01'}
|
||||
mod_conf = self.configuration
|
||||
mod_conf.sf_enable_vag = True
|
||||
sfv = solidfire.SolidFireDriver(configuration=mod_conf)
|
||||
provider_id = testvol['provider_id']
|
||||
vol_id = int(sfv._parse_provider_id_string(provider_id)[0])
|
||||
vag_id = 1
|
||||
|
||||
with mock.patch.object(sfv,
|
||||
'_get_vags',
|
||||
return_value=[vag]), \
|
||||
'_safe_create_vag',
|
||||
return_value=vag_id) as create_vag, \
|
||||
mock.patch.object(sfv,
|
||||
'_remove_vag') as mock_rem_vag, \
|
||||
'_add_volume_to_vag') as add_vol:
|
||||
sfv._sf_initialize_connection(testvol, connector)
|
||||
create_vag.assert_called_with(connector['initiator'],
|
||||
vol_id)
|
||||
add_vol.assert_called_with(vol_id,
|
||||
connector['initiator'],
|
||||
vag_id)
|
||||
|
||||
def test_sf_term_conn_with_vag_rem_vag(self):
|
||||
# Verify we correctly remove an empty VAG on detach.
|
||||
mod_conf = self.configuration
|
||||
mod_conf.sf_enable_vag = True
|
||||
sfv = solidfire.SolidFireDriver(configuration=mod_conf)
|
||||
testvol = {'project_id': 'testprjid',
|
||||
'name': 'testvol',
|
||||
'size': 1,
|
||||
'id': 'a720b3c0-d1f0-11e1-9b23-0800200c9a66',
|
||||
'volume_type_id': None,
|
||||
'provider_location': '10.10.7.1:3260 iqn.2010-01.com.'
|
||||
'solidfire:87hg.uuid-2cc06226-cc'
|
||||
'74-4cb7-bd55-14aed659a0cc.4060 0',
|
||||
'provider_auth': 'CHAP stack-1-a60e2611875f40199931f2'
|
||||
'c76370d66b 2FE0CQ8J196R',
|
||||
'provider_geometry': '4096 4096',
|
||||
'created_at': timeutils.utcnow(),
|
||||
'provider_id': "1 1 1",
|
||||
'multiattach': False
|
||||
}
|
||||
connector = {'initiator': 'iqn.2012-07.org.fake:01'}
|
||||
vag_id = 1
|
||||
vags = [{'attributes': {},
|
||||
'deletedVolumes': [],
|
||||
'initiators': [connector['initiator']],
|
||||
'name': 'fakeiqn',
|
||||
'volumeAccessGroupID': vag_id,
|
||||
'volumes': [1],
|
||||
'virtualNetworkIDs': []}]
|
||||
|
||||
with mock.patch.object(sfv,
|
||||
'_get_vags_by_name',
|
||||
return_value=vags), \
|
||||
mock.patch.object(sfv,
|
||||
'_remove_volume_from_vag') as mock_rem_vol_vag:
|
||||
sfv.terminate_connection(testvol, connector, force=False)
|
||||
mock_rem_vol_vag.assert_called_with(vol_id,
|
||||
vag['volumeAccessGroupID'])
|
||||
mock_rem_vag.assert_not_called()
|
||||
'_remove_vag') as rem_vag:
|
||||
sfv._sf_terminate_connection(testvol, connector, False)
|
||||
rem_vag.assert_called_with(vag_id)
|
||||
|
||||
def test_sf_term_conn_with_vag_rem_vol(self):
|
||||
# Verify we correctly remove a the volume from a non-empty VAG.
|
||||
mod_conf = self.configuration
|
||||
mod_conf.sf_enable_vag = True
|
||||
sfv = solidfire.SolidFireDriver(configuration=mod_conf)
|
||||
testvol = {'project_id': 'testprjid',
|
||||
'name': 'testvol',
|
||||
'size': 1,
|
||||
'id': 'a720b3c0-d1f0-11e1-9b23-0800200c9a66',
|
||||
'volume_type_id': None,
|
||||
'provider_location': '10.10.7.1:3260 iqn.2010-01.com.'
|
||||
'solidfire:87hg.uuid-2cc06226-cc'
|
||||
'74-4cb7-bd55-14aed659a0cc.4060 0',
|
||||
'provider_auth': 'CHAP stack-1-a60e2611875f40199931f2'
|
||||
'c76370d66b 2FE0CQ8J196R',
|
||||
'provider_geometry': '4096 4096',
|
||||
'created_at': timeutils.utcnow(),
|
||||
'provider_id': "1 1 1",
|
||||
'multiattach': False
|
||||
}
|
||||
provider_id = testvol['provider_id']
|
||||
vol_id = int(sfv._parse_provider_id_string(provider_id)[0])
|
||||
connector = {'initiator': 'iqn.2012-07.org.fake:01'}
|
||||
vag_id = 1
|
||||
vags = [{'attributes': {},
|
||||
'deletedVolumes': [],
|
||||
'initiators': [connector['initiator']],
|
||||
'name': 'fakeiqn',
|
||||
'volumeAccessGroupID': vag_id,
|
||||
'volumes': [1, 2],
|
||||
'virtualNetworkIDs': []}]
|
||||
|
||||
with mock.patch.object(sfv,
|
||||
'_get_vags_by_name',
|
||||
return_value=vags), \
|
||||
mock.patch.object(sfv,
|
||||
'_remove_volume_from_vag') as rem_vag:
|
||||
sfv._sf_terminate_connection(testvol, connector, False)
|
||||
rem_vag.assert_called_with(vol_id, vag_id)
|
||||
|
||||
def test_safe_create_vag_simple(self):
|
||||
# Test the sunny day call straight into _create_vag.
|
||||
sfv = solidfire.SolidFireDriver(configuration=self.configuration)
|
||||
iqn = 'fake_iqn'
|
||||
vol_id = 1
|
||||
|
||||
with mock.patch.object(sfv,
|
||||
'_get_vags_by_name',
|
||||
return_value=[]), \
|
||||
mock.patch.object(sfv,
|
||||
'_create_vag') as mock_create_vag:
|
||||
sfv._safe_create_vag(iqn, vol_id)
|
||||
mock_create_vag.assert_called_with(iqn, vol_id)
|
||||
|
||||
def test_safe_create_vag_matching_vag(self):
|
||||
# Vag exists, resuse.
|
||||
sfv = solidfire.SolidFireDriver(configuration=self.configuration)
|
||||
iqn = 'TESTIQN'
|
||||
vags = [{'attributes': {},
|
||||
'deletedVolumes': [],
|
||||
'initiators': [iqn],
|
||||
'name': iqn,
|
||||
'volumeAccessGroupID': 1,
|
||||
'volumes': [1, 2],
|
||||
'virtualNetworkIDs': []}]
|
||||
|
||||
with mock.patch.object(sfv,
|
||||
'_get_vags_by_name',
|
||||
return_value=vags), \
|
||||
mock.patch.object(sfv,
|
||||
'_create_vag') as create_vag, \
|
||||
mock.patch.object(sfv,
|
||||
'_add_initiator_to_vag') as add_iqn:
|
||||
vag_id = sfv._safe_create_vag(iqn, None)
|
||||
self.assertEqual(vag_id, vags[0]['volumeAccessGroupID'])
|
||||
create_vag.assert_not_called()
|
||||
add_iqn.assert_not_called()
|
||||
|
||||
def test_safe_create_vag_reuse_vag(self):
|
||||
# Reuse a matching vag.
|
||||
sfv = solidfire.SolidFireDriver(configuration=self.configuration)
|
||||
iqn = 'TESTIQN'
|
||||
vags = [{'attributes': {},
|
||||
'deletedVolumes': [],
|
||||
'initiators': [],
|
||||
'name': iqn,
|
||||
'volumeAccessGroupID': 1,
|
||||
'volumes': [1, 2],
|
||||
'virtualNetworkIDs': []}]
|
||||
vag_id = vags[0]['volumeAccessGroupID']
|
||||
|
||||
with mock.patch.object(sfv,
|
||||
'_get_vags_by_name',
|
||||
return_value=vags), \
|
||||
mock.patch.object(sfv,
|
||||
'_add_initiator_to_vag',
|
||||
return_value = vag_id) as add_init:
|
||||
res_vag_id = sfv._safe_create_vag(iqn, None)
|
||||
self.assertEqual(res_vag_id, vag_id)
|
||||
add_init.assert_called_with(iqn, vag_id)
|
||||
|
||||
def test_create_vag_iqn_fail(self):
|
||||
# Attempt to create a VAG with an already in-use initiator.
|
||||
sfv = solidfire.SolidFireDriver(configuration=self.configuration)
|
||||
iqn = 'TESTIQN'
|
||||
vag_id = 1
|
||||
vol_id = 42
|
||||
|
||||
def throw_request(method, params, version):
|
||||
msg = 'xExceededLimit: {}'.format(params['initiators'][0])
|
||||
raise exception.SolidFireAPIException(message=msg)
|
||||
|
||||
with mock.patch.object(sfv,
|
||||
'_issue_api_request',
|
||||
side_effect=throw_request), \
|
||||
mock.patch.object(sfv,
|
||||
'_safe_create_vag',
|
||||
return_value=vag_id) as create_vag, \
|
||||
mock.patch.object(sfv,
|
||||
'_purge_vags') as purge_vags:
|
||||
res_vag_id = sfv._create_vag(iqn, vol_id)
|
||||
self.assertEqual(res_vag_id, vag_id)
|
||||
create_vag.assert_called_with(iqn, vol_id)
|
||||
purge_vags.assert_not_called()
|
||||
|
||||
def test_create_vag_limit_fail(self):
|
||||
# Attempt to create a VAG with VAG limit reached.
|
||||
sfv = solidfire.SolidFireDriver(configuration=self.configuration)
|
||||
iqn = 'TESTIQN'
|
||||
vag_id = 1
|
||||
vol_id = 42
|
||||
|
||||
def throw_request(method, params, version):
|
||||
msg = 'xExceededLimit'
|
||||
raise exception.SolidFireAPIException(message=msg)
|
||||
|
||||
with mock.patch.object(sfv,
|
||||
'_issue_api_request',
|
||||
side_effect=throw_request), \
|
||||
mock.patch.object(sfv,
|
||||
'_safe_create_vag',
|
||||
return_value=vag_id) as create_vag, \
|
||||
mock.patch.object(sfv,
|
||||
'_purge_vags') as purge_vags:
|
||||
res_vag_id = sfv._create_vag(iqn, vol_id)
|
||||
self.assertEqual(res_vag_id, vag_id)
|
||||
create_vag.assert_called_with(iqn, vol_id)
|
||||
purge_vags.assert_called_with()
|
||||
|
||||
def test_add_initiator_duplicate(self):
|
||||
# Thrown exception should yield vag_id.
|
||||
sfv = solidfire.SolidFireDriver(configuration=self.configuration)
|
||||
iqn = 'TESTIQN'
|
||||
vag_id = 1
|
||||
|
||||
def throw_request(method, params, version):
|
||||
msg = 'xAlreadyInVolumeAccessGroup'
|
||||
raise exception.SolidFireAPIException(message=msg)
|
||||
|
||||
with mock.patch.object(sfv,
|
||||
'_issue_api_request',
|
||||
side_effect=throw_request):
|
||||
res_vag_id = sfv._add_initiator_to_vag(iqn, vag_id)
|
||||
self.assertEqual(vag_id, res_vag_id)
|
||||
|
||||
def test_add_initiator_missing_vag(self):
|
||||
# Thrown exception should result in create_vag call.
|
||||
sfv = solidfire.SolidFireDriver(configuration=self.configuration)
|
||||
iqn = 'TESTIQN'
|
||||
vag_id = 1
|
||||
|
||||
def throw_request(method, params, version):
|
||||
msg = 'xVolumeAccessGroupIDDoesNotExist'
|
||||
raise exception.SolidFireAPIException(message=msg)
|
||||
|
||||
with mock.patch.object(sfv,
|
||||
'_issue_api_request',
|
||||
side_effect=throw_request), \
|
||||
mock.patch.object(sfv,
|
||||
'_safe_create_vag',
|
||||
return_value=vag_id) as mock_create_vag:
|
||||
res_vag_id = sfv._add_initiator_to_vag(iqn, vag_id)
|
||||
self.assertEqual(vag_id, res_vag_id)
|
||||
mock_create_vag.assert_called_with(iqn)
|
||||
|
||||
def test_add_volume_to_vag_duplicate(self):
|
||||
# Thrown exception should yield vag_id
|
||||
sfv = solidfire.SolidFireDriver(configuration=self.configuration)
|
||||
iqn = 'TESTIQN'
|
||||
vag_id = 1
|
||||
vol_id = 42
|
||||
|
||||
def throw_request(method, params, version):
|
||||
msg = 'xAlreadyInVolumeAccessGroup'
|
||||
raise exception.SolidFireAPIException(message=msg)
|
||||
|
||||
with mock.patch.object(sfv,
|
||||
'_issue_api_request',
|
||||
side_effect=throw_request):
|
||||
res_vag_id = sfv._add_volume_to_vag(vol_id, iqn, vag_id)
|
||||
self.assertEqual(res_vag_id, vag_id)
|
||||
|
||||
def test_add_volume_to_vag_missing_vag(self):
|
||||
# Thrown exception should yield vag_id
|
||||
sfv = solidfire.SolidFireDriver(configuration=self.configuration)
|
||||
iqn = 'TESTIQN'
|
||||
vag_id = 1
|
||||
vol_id = 42
|
||||
|
||||
def throw_request(method, params, version):
|
||||
msg = 'xVolumeAccessGroupIDDoesNotExist'
|
||||
raise exception.SolidFireAPIException(message=msg)
|
||||
|
||||
with mock.patch.object(sfv,
|
||||
'_issue_api_request',
|
||||
side_effect=throw_request), \
|
||||
mock.patch.object(sfv,
|
||||
'_safe_create_vag',
|
||||
return_value=vag_id) as mock_create_vag:
|
||||
res_vag_id = sfv._add_volume_to_vag(vol_id, iqn, vag_id)
|
||||
self.assertEqual(res_vag_id, vag_id)
|
||||
mock_create_vag.assert_called_with(iqn, vol_id)
|
||||
|
||||
def test_remove_volume_from_vag_missing_volume(self):
|
||||
# Volume not in VAG, throws.
|
||||
sfv = solidfire.SolidFireDriver(configuration=self.configuration)
|
||||
vag_id = 1
|
||||
vol_id = 42
|
||||
|
||||
def throw_request(method, params, version):
|
||||
msg = 'xNotInVolumeAccessGroup'
|
||||
raise exception.SolidFireAPIException(message=msg)
|
||||
|
||||
with mock.patch.object(sfv,
|
||||
'_issue_api_request',
|
||||
side_effect=throw_request):
|
||||
sfv._remove_volume_from_vag(vol_id, vag_id)
|
||||
|
||||
def test_remove_volume_from_vag_missing_vag(self):
|
||||
# Volume not in VAG, throws.
|
||||
sfv = solidfire.SolidFireDriver(configuration=self.configuration)
|
||||
vag_id = 1
|
||||
vol_id = 42
|
||||
|
||||
def throw_request(method, params, version):
|
||||
msg = 'xVolumeAccessGroupIDDoesNotExist'
|
||||
raise exception.SolidFireAPIException(message=msg)
|
||||
|
||||
with mock.patch.object(sfv,
|
||||
'_issue_api_request',
|
||||
side_effect=throw_request):
|
||||
sfv._remove_volume_from_vag(vol_id, vag_id)
|
||||
|
||||
def test_remove_volume_from_vag_unknown_exception(self):
|
||||
# Volume not in VAG, throws.
|
||||
sfv = solidfire.SolidFireDriver(configuration=self.configuration)
|
||||
vag_id = 1
|
||||
vol_id = 42
|
||||
|
||||
def throw_request(method, params, version):
|
||||
msg = 'xUnknownException'
|
||||
raise exception.SolidFireAPIException(message=msg)
|
||||
|
||||
with mock.patch.object(sfv,
|
||||
'_issue_api_request',
|
||||
side_effect=throw_request):
|
||||
self.assertRaises(exception.SolidFireAPIException,
|
||||
sfv._remove_volume_from_vag,
|
||||
vol_id,
|
||||
vag_id)
|
||||
|
||||
def test_remove_volume_from_vags(self):
|
||||
# Remove volume from several VAGs.
|
||||
sfv = solidfire.SolidFireDriver(configuration=self.configuration)
|
||||
vol_id = 42
|
||||
vags = [{'volumeAccessGroupID': 1,
|
||||
'volumes': [vol_id]},
|
||||
{'volumeAccessGroupID': 2,
|
||||
'volumes': [vol_id, 43]}]
|
||||
|
||||
with mock.patch.object(sfv,
|
||||
'_base_get_vags',
|
||||
return_value=vags), \
|
||||
mock.patch.object(sfv,
|
||||
'_remove_volume_from_vag') as rem_vol:
|
||||
sfv._remove_volume_from_vags(vol_id)
|
||||
self.assertEqual(len(vags), rem_vol.call_count)
|
||||
|
||||
def test_purge_vags(self):
|
||||
# Remove subset of VAGs.
|
||||
sfv = solidfire.SolidFireDriver(configuration=self.configuration)
|
||||
vags = [{'initiators': [],
|
||||
'volumeAccessGroupID': 1,
|
||||
'deletedVolumes': [],
|
||||
'volumes': [],
|
||||
'attributes': {'openstack': True}},
|
||||
{'initiators': [],
|
||||
'volumeAccessGroupID': 2,
|
||||
'deletedVolumes': [],
|
||||
'volumes': [],
|
||||
'attributes': {'openstack': False}},
|
||||
{'initiators': [],
|
||||
'volumeAccessGroupID': 3,
|
||||
'deletedVolumes': [1],
|
||||
'volumes': [],
|
||||
'attributes': {'openstack': True}},
|
||||
{'initiators': [],
|
||||
'volumeAccessGroupID': 4,
|
||||
'deletedVolumes': [],
|
||||
'volumes': [1],
|
||||
'attributes': {'openstack': True}},
|
||||
{'initiators': ['fakeiqn'],
|
||||
'volumeAccessGroupID': 5,
|
||||
'deletedVolumes': [],
|
||||
'volumes': [],
|
||||
'attributes': {'openstack': True}}]
|
||||
with mock.patch.object(sfv,
|
||||
'_base_get_vags',
|
||||
return_value=vags), \
|
||||
mock.patch.object(sfv,
|
||||
'_remove_vag') as rem_vag:
|
||||
sfv._purge_vags()
|
||||
# Of the vags provided there is only one that is valid for purge
|
||||
# based on the limits of no initiators, volumes, deleted volumes,
|
||||
# and features the openstack attribute.
|
||||
self.assertEqual(1, rem_vag.call_count)
|
||||
rem_vag.assert_called_with(1)
|
||||
|
@ -97,6 +97,12 @@ sf_opts = [
|
||||
CONF = cfg.CONF
|
||||
CONF.register_opts(sf_opts)
|
||||
|
||||
# SolidFire API Error Constants
|
||||
xExceededLimit = 'xExceededLimit'
|
||||
xAlreadyInVolumeAccessGroup = 'xAlreadyInVolumeAccessGroup'
|
||||
xVolumeAccessGroupIDDoesNotExist = 'xVolumeAccessGroupIDDoesNotExist'
|
||||
xNotInVolumeAccessGroup = 'xNotInVolumeAccessGroup'
|
||||
|
||||
|
||||
def retry(exc_tuple, tries=5, delay=1, backoff=2):
|
||||
def retry_dec(f):
|
||||
@ -819,57 +825,153 @@ class SolidFireDriver(san.SanISCSIDriver):
|
||||
vlist = sorted(vlist, key=lambda k: k['volumeID'])
|
||||
return vlist
|
||||
|
||||
def _create_vag(self, vag_name):
|
||||
def _create_vag(self, iqn, vol_id=None):
|
||||
"""Create a volume access group(vag).
|
||||
|
||||
Returns the vag_id.
|
||||
"""
|
||||
params = {'name': vag_name}
|
||||
result = self._issue_api_request('CreateVolumeAccessGroup',
|
||||
params,
|
||||
version='7.0')
|
||||
return result['result']['volumeAccessGroupID']
|
||||
vag_name = re.sub('[^0-9a-zA-Z]+', '-', iqn)
|
||||
params = {'name': vag_name,
|
||||
'initiators': [iqn],
|
||||
'volumes': [vol_id],
|
||||
'attributes': {'openstack': True}}
|
||||
try:
|
||||
result = self._issue_api_request('CreateVolumeAccessGroup',
|
||||
params,
|
||||
version='7.0')
|
||||
return result['result']['volumeAccessGroupID']
|
||||
except exception.SolidFireAPIException as error:
|
||||
if xExceededLimit in error.msg:
|
||||
if iqn in error.msg:
|
||||
# Initiator double registered.
|
||||
return self._safe_create_vag(iqn, vol_id)
|
||||
else:
|
||||
# VAG limit reached. Purge and start over.
|
||||
self._purge_vags()
|
||||
return self._safe_create_vag(iqn, vol_id)
|
||||
else:
|
||||
raise
|
||||
|
||||
def _get_vags(self, vag_name):
|
||||
"""Retrieve SolidFire volume access group objects by name.
|
||||
def _safe_create_vag(self, iqn, vol_id=None):
|
||||
# Potential race condition with simultaneous volume attaches to the
|
||||
# same host. To help avoid this, VAG creation makes a best attempt at
|
||||
# finding and using an existing VAG.
|
||||
|
||||
Returns an array of vags with a matching name value.
|
||||
Returns an empty array if there are no matches.
|
||||
"""
|
||||
vags = self._get_vags_by_name(iqn)
|
||||
if vags:
|
||||
# Filter through the vags and find the one with matching initiator
|
||||
vag = next((v for v in vags if iqn in v['initiators']), None)
|
||||
if vag:
|
||||
return vag['volumeAccessGroupID']
|
||||
else:
|
||||
# No matches, use the first result, add initiator IQN.
|
||||
vag_id = vags[0]['volumeAccessGroupID']
|
||||
return self._add_initiator_to_vag(iqn, vag_id)
|
||||
return self._create_vag(iqn, vol_id)
|
||||
|
||||
def _base_get_vags(self):
|
||||
params = {}
|
||||
vags = self._issue_api_request(
|
||||
'ListVolumeAccessGroups',
|
||||
params,
|
||||
version='7.0')['result']['volumeAccessGroups']
|
||||
return vags
|
||||
|
||||
def _get_vags_by_name(self, iqn):
|
||||
"""Retrieve SolidFire volume access group objects by name.
|
||||
|
||||
Returns an array of vags with a matching name value.
|
||||
Returns an empty array if there are no matches.
|
||||
"""
|
||||
vags = self._base_get_vags()
|
||||
vag_name = re.sub('[^0-9a-zA-Z]+', '-', iqn)
|
||||
matching_vags = [vag for vag in vags if vag['name'] == vag_name]
|
||||
return matching_vags
|
||||
|
||||
def _add_initiator_to_vag(self, iqn, vag_id):
|
||||
# Added a vag_id return as there is a chance that we might have to
|
||||
# create a new VAG if our target VAG is deleted underneath us.
|
||||
params = {"initiators": [iqn],
|
||||
"volumeAccessGroupID": vag_id}
|
||||
self._issue_api_request('AddInitiatorsToVolumeAccessGroup',
|
||||
params,
|
||||
version='7.0')
|
||||
try:
|
||||
self._issue_api_request('AddInitiatorsToVolumeAccessGroup',
|
||||
params,
|
||||
version='7.0')
|
||||
return vag_id
|
||||
except exception.SolidFireAPIException as error:
|
||||
if xAlreadyInVolumeAccessGroup in error.msg:
|
||||
return vag_id
|
||||
elif xVolumeAccessGroupIDDoesNotExist in error.msg:
|
||||
# No locking means sometimes a VAG can be removed by a parallel
|
||||
# volume detach against the same host.
|
||||
return self._safe_create_vag(iqn)
|
||||
else:
|
||||
raise
|
||||
|
||||
def _add_volume_to_vag(self, vol_id, vag_id):
|
||||
def _add_volume_to_vag(self, vol_id, iqn, vag_id):
|
||||
# Added a vag_id return to be consistent with add_initiator_to_vag. It
|
||||
# isn't necessary but may be helpful in the future.
|
||||
params = {"volumeAccessGroupID": vag_id,
|
||||
"volumes": [vol_id]}
|
||||
self._issue_api_request('AddVolumesToVolumeAccessGroup',
|
||||
params,
|
||||
version='7.0')
|
||||
try:
|
||||
self._issue_api_request('AddVolumesToVolumeAccessGroup',
|
||||
params,
|
||||
version='7.0')
|
||||
return vag_id
|
||||
|
||||
except exception.SolidFireAPIException as error:
|
||||
if xAlreadyInVolumeAccessGroup in error.msg:
|
||||
return vag_id
|
||||
elif xVolumeAccessGroupIDDoesNotExist in error.msg:
|
||||
return self._safe_create_vag(iqn, vol_id)
|
||||
else:
|
||||
raise
|
||||
|
||||
def _remove_volume_from_vag(self, vol_id, vag_id):
|
||||
params = {"volumeAccessGroupID": vag_id,
|
||||
"volumes": [vol_id]}
|
||||
self._issue_api_request('RemoveVolumesFromVolumeAccessGroup',
|
||||
params,
|
||||
version='7.0')
|
||||
try:
|
||||
self._issue_api_request('RemoveVolumesFromVolumeAccessGroup',
|
||||
params,
|
||||
version='7.0')
|
||||
except exception.SolidFireAPIException as error:
|
||||
if xNotInVolumeAccessGroup in error.msg:
|
||||
pass
|
||||
elif xVolumeAccessGroupIDDoesNotExist in error.msg:
|
||||
pass
|
||||
else:
|
||||
raise
|
||||
|
||||
def _remove_volume_from_vags(self, vol_id):
|
||||
# Due to all sorts of uncertainty around multiattach, on volume
|
||||
# deletion we make a best attempt at removing the vol_id from VAGs.
|
||||
vags = self._base_get_vags()
|
||||
targets = [v for v in vags if vol_id in v['volumes']]
|
||||
for vag in targets:
|
||||
self._remove_volume_from_vag(vol_id, vag['volumeAccessGroupID'])
|
||||
|
||||
def _remove_vag(self, vag_id):
|
||||
params = {"volumeAccessGroupID": vag_id}
|
||||
self._issue_api_request('DeleteVolumeAccessGroup',
|
||||
params,
|
||||
version='7.0')
|
||||
try:
|
||||
self._issue_api_request('DeleteVolumeAccessGroup',
|
||||
params,
|
||||
version='7.0')
|
||||
except exception.SolidFireAPIException as error:
|
||||
if xVolumeAccessGroupIDDoesNotExist not in error.msg:
|
||||
raise
|
||||
|
||||
def _purge_vags(self, limit=10):
|
||||
# Purge up to limit number of VAGs that have no active volumes,
|
||||
# initiators, and an OpenStack attribute. Purge oldest VAGs first.
|
||||
vags = self._base_get_vags()
|
||||
targets = [v for v in vags if v['volumes'] == [] and
|
||||
v['initiators'] == [] and
|
||||
v['deletedVolumes'] == [] and
|
||||
v['attributes'].get('openstack')]
|
||||
sorted_targets = sorted(targets,
|
||||
key=lambda k: k['volumeAccessGroupID'])
|
||||
for vag in sorted_targets[:limit]:
|
||||
self._remove_vag(vag['volumeAccessGroupID'])
|
||||
|
||||
def clone_image(self, context,
|
||||
volume, image_location,
|
||||
@ -1022,6 +1124,8 @@ class SolidFireDriver(san.SanISCSIDriver):
|
||||
if sf_vol is not None:
|
||||
params = {'volumeID': sf_vol['volumeID']}
|
||||
self._issue_api_request('DeleteVolume', params)
|
||||
if volume.get('multiattach'):
|
||||
self._remove_volume_from_vags(sf_vol['volumeID'])
|
||||
else:
|
||||
LOG.error(_LE("Volume ID %s was not found on "
|
||||
"the SolidFire Cluster while attempting "
|
||||
@ -1409,33 +1513,14 @@ class SolidFireISCSI(iscsi_driver.SanISCSITarget):
|
||||
Optionally checks and utilizes volume access groups.
|
||||
"""
|
||||
if self.configuration.sf_enable_vag:
|
||||
raw_iqn = connector['initiator']
|
||||
vag_name = re.sub('[^0-9a-zA-Z]+', '-', raw_iqn)
|
||||
vag = self._get_vags(vag_name)
|
||||
iqn = connector['initiator']
|
||||
provider_id = volume['provider_id']
|
||||
vol_id = int(self._parse_provider_id_string(provider_id)[0])
|
||||
|
||||
if vag:
|
||||
vag_id = vag[0]['volumeAccessGroupID']
|
||||
vag = vag[0]
|
||||
else:
|
||||
vag_id = self._create_vag(vag_name)
|
||||
vag = self._get_vags(vag_name)[0]
|
||||
|
||||
# TODO(chrismorrell): There is a potential race condition if a
|
||||
# volume is attached and another is detached on the same
|
||||
# host/initiator. The detach may purge the VAG before the attach
|
||||
# has a chance to add the volume to the VAG. Propose combining
|
||||
# add_initiator_to_vag and add_volume_to_vag with a retry on
|
||||
# SFAPI exception regarding nonexistent VAG.
|
||||
|
||||
# Verify IQN matches.
|
||||
if raw_iqn not in vag['initiators']:
|
||||
self._add_initiator_to_vag(raw_iqn,
|
||||
vag_id)
|
||||
# Add volume to vag if not already.
|
||||
if vol_id not in vag['volumes']:
|
||||
self._add_volume_to_vag(vol_id, vag_id)
|
||||
# safe_create_vag may opt to reuse vs create a vag, so we need to
|
||||
# add our vol_id.
|
||||
vag_id = self._safe_create_vag(iqn, vol_id)
|
||||
self._add_volume_to_vag(vol_id, iqn, vag_id)
|
||||
|
||||
# Continue along with default behavior
|
||||
return super(SolidFireISCSI, self).initialize_connection(volume,
|
||||
@ -1448,13 +1533,15 @@ class SolidFireISCSI(iscsi_driver.SanISCSITarget):
|
||||
If the VAG is empty then the VAG is also removed.
|
||||
"""
|
||||
if self.configuration.sf_enable_vag:
|
||||
raw_iqn = properties['initiator']
|
||||
vag_name = re.sub('[^0-9a-zA-Z]+', '-', raw_iqn)
|
||||
vag = self._get_vags(vag_name)
|
||||
iqn = properties['initiator']
|
||||
vag = self._get_vags_by_name(iqn)
|
||||
provider_id = volume['provider_id']
|
||||
vol_id = int(self._parse_provider_id_string(provider_id)[0])
|
||||
|
||||
if vag:
|
||||
if vag and not volume['multiattach']:
|
||||
# Multiattach causes problems with removing volumes from VAGs.
|
||||
# Compromise solution for now is to remove multiattach volumes
|
||||
# from VAGs during volume deletion.
|
||||
vag = vag[0]
|
||||
vag_id = vag['volumeAccessGroupID']
|
||||
if [vol_id] == vag['volumes']:
|
||||
|
Loading…
Reference in New Issue
Block a user