diff --git a/cinder/tests/unit/volume/drivers/solidfire/test_solidfire.py b/cinder/tests/unit/volume/drivers/solidfire/test_solidfire.py index c1665e0cf7f..59efd0f8ed3 100644 --- a/cinder/tests/unit/volume/drivers/solidfire/test_solidfire.py +++ b/cinder/tests/unit/volume/drivers/solidfire/test_solidfire.py @@ -1275,6 +1275,45 @@ class SolidFireVolumeTestCase(test.TestCase): sfv._sf_terminate_connection(testvol, connector, False) rem_vag.assert_called_with(vol_id, vag_id) + def test_sf_term_conn_without_connector(self): + # Verify we correctly force the deletion of a volume. + 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(provider_id.split()[0]) + vag_id = 1 + vags = [{'attributes': {}, + 'deletedVolumes': [], + 'initiators': ['iqn.2012-07.org.fake:01'], + 'name': 'fakeiqn', + 'volumeAccessGroupID': vag_id, + 'volumes': [1, 2], + 'virtualNetworkIDs': []}] + + with mock.patch.object(sfv, + '_get_vags_by_volume', + return_value=vags), \ + mock.patch.object(sfv, + '_remove_volume_from_vags') as rem_vags: + sfv._sf_terminate_connection(testvol, None, False) + rem_vags.assert_called_with(vol_id) + def test_safe_create_vag_simple(self): # Test the sunny day call straight into _create_vag. sfv = solidfire.SolidFireDriver(configuration=self.configuration) @@ -1516,7 +1555,7 @@ class SolidFireVolumeTestCase(test.TestCase): 'volumes': [vol_id, 43]}] with mock.patch.object(sfv, - '_base_get_vags', + '_get_vags_by_volume', return_value=vags), \ mock.patch.object(sfv, '_remove_volume_from_vag') as rem_vol: diff --git a/cinder/volume/drivers/solidfire.py b/cinder/volume/drivers/solidfire.py index 65f4eaaf200..b51c8413909 100644 --- a/cinder/volume/drivers/solidfire.py +++ b/cinder/volume/drivers/solidfire.py @@ -1201,6 +1201,13 @@ class SolidFireDriver(san.SanISCSIDriver): matching_vags = [vag for vag in vags if vag['name'] == vag_name] return matching_vags + def _get_vags_by_volume(self, vol_id): + params = {"volumeID": vol_id} + vags = self._issue_api_request( + 'GetVolumeStats', + params)['result']['volumeStats']['volumeAccessGroups'] + return 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. @@ -1258,9 +1265,8 @@ class SolidFireDriver(san.SanISCSIDriver): 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: + vags = self._get_vags_by_volume(vol_id) + for vag in vags: self._remove_volume_from_vag(vol_id, vag['volumeAccessGroupID']) def _remove_vag(self, vag_id): @@ -2337,21 +2343,26 @@ class SolidFireISCSI(iscsi_driver.SanISCSITarget): If the VAG is empty then the VAG is also removed. """ if self.configuration.sf_enable_vag: - iqn = properties['initiator'] - vag = self._get_vags_by_name(iqn) provider_id = volume['provider_id'] vol_id = int(provider_id.split()[0]) - 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']: - self._remove_vag(vag_id) - elif vol_id in vag['volumes']: - self._remove_volume_from_vag(vol_id, vag_id) + if properties: + iqn = properties['initiator'] + vag = self._get_vags_by_name(iqn) + + 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']: + self._remove_vag(vag_id) + elif vol_id in vag['volumes']: + self._remove_volume_from_vag(vol_id, vag_id) + else: + self._remove_volume_from_vags(vol_id) return super(SolidFireISCSI, self).terminate_connection(volume, properties, diff --git a/releasenotes/notes/fix-netapp-force_detach-36bdf75dd2c9a030.yaml b/releasenotes/notes/fix-netapp-force_detach-36bdf75dd2c9a030.yaml new file mode 100644 index 00000000000..4ea36bf12a1 --- /dev/null +++ b/releasenotes/notes/fix-netapp-force_detach-36bdf75dd2c9a030.yaml @@ -0,0 +1,3 @@ +--- +fixes: + - Fixes force_detach behavior for volumes in NetApp SolidFire driver.