diff --git a/cinder/tests/unit/volume/drivers/solidfire/test_solidfire.py b/cinder/tests/unit/volume/drivers/solidfire/test_solidfire.py index beb65cbf72a..f534da5f10f 100644 --- a/cinder/tests/unit/volume/drivers/solidfire/test_solidfire.py +++ b/cinder/tests/unit/volume/drivers/solidfire/test_solidfire.py @@ -118,9 +118,8 @@ class SolidFireVolumeTestCase(test.TestCase): self.ctxt, volume_id=self.vol.id) self.fake_sfaccount = {'accountID': 25, - 'name': 'testprjid', 'targetSecret': 'shhhh', - 'username': 'john-wayne', + 'username': 'prefix-testprjid', 'volumes': [6, 7, 20]} self.fake_sfvol = {'volumeID': 6, @@ -304,6 +303,16 @@ class SolidFireVolumeTestCase(test.TestCase): "snapshotID": 1 } } + elif method is 'ListAccounts': + return { + 'result': { + 'accounts': [{ + 'accountID': 5, + 'targetSecret': 'shhhh', + 'username': 'prefix-testprjid' + }] + } + } else: # Crap, unimplemented API call in Fake return None @@ -351,9 +360,8 @@ class SolidFireVolumeTestCase(test.TestCase): 'created_at': timeutils.utcnow()} fake_sfaccounts = [{'accountID': 5, - 'name': 'testprjid', 'targetSecret': 'shhhh', - 'username': 'john-wayne'}] + 'username': 'prefix-testprjid'}] test_type = {'name': 'sf-1', 'qos_specs_id': 'fb0576d7-b4b5-4cad-85dc-ca92e6a497d1', @@ -415,9 +423,8 @@ class SolidFireVolumeTestCase(test.TestCase): 'volume_type_id': None, 'created_at': timeutils.utcnow()} fake_sfaccounts = [{'accountID': 5, - 'name': 'testprjid', 'targetSecret': 'shhhh', - 'username': 'john-wayne'}] + 'username': 'prefix-testprjid'}] sfv = solidfire.SolidFireDriver(configuration=self.configuration) with mock.patch.object(sfv, @@ -445,9 +452,8 @@ class SolidFireVolumeTestCase(test.TestCase): 'volume_type_id': None, 'created_at': timeutils.utcnow()} fake_sfaccounts = [{'accountID': 5, - 'name': 'testprjid', 'targetSecret': 'shhhh', - 'username': 'john-wayne'}] + 'username': 'prefix-testprjid'}] sfv = solidfire.SolidFireDriver(configuration=self.configuration) with mock.patch.object(sfv, @@ -487,7 +493,8 @@ class SolidFireVolumeTestCase(test.TestCase): mock.patch.object(sfv, '_get_sfaccounts_for_tenant', return_value=[{'accountID': 5, - 'name': 'testprjid'}]): + 'username': + 'prefix-testprjid'}]): sfv.create_snapshot(testsnap) sfv.delete_snapshot(testsnap) @@ -587,7 +594,7 @@ class SolidFireVolumeTestCase(test.TestCase): self.mock_object(solidfire.SolidFireDriver, '_issue_api_request', self.fake_issue_api_request) - account = sfv._create_sfaccount('project-id') + account = sfv._create_sfaccount('some-name') self.assertIsNotNone(account) def test_create_sfaccount_fails(self): @@ -598,6 +605,22 @@ class SolidFireVolumeTestCase(test.TestCase): self.assertRaises(exception.SolidFireAPIException, sfv._create_sfaccount, 'project-id') + def test_get_sfaccounts_for_tenant(self): + sfv = solidfire.SolidFireDriver(configuration=self.configuration) + self.mock_object(solidfire.SolidFireDriver, + '_issue_api_request', + self.fake_issue_api_request) + accounts = sfv._get_sfaccounts_for_tenant('some-name') + self.assertIsNotNone(accounts) + + def test_get_sfaccounts_for_tenant_fails(self): + sfv = solidfire.SolidFireDriver(configuration=self.configuration) + self.mock_object(solidfire.SolidFireDriver, + '_issue_api_request', + self.fake_issue_api_request_fails) + self.assertRaises(exception.SolidFireAPIException, + sfv._get_sfaccounts_for_tenant, 'some-name') + def test_get_sfaccount_by_name(self): sfv = solidfire.SolidFireDriver(configuration=self.configuration) self.mock_object(solidfire.SolidFireDriver, @@ -606,6 +629,80 @@ class SolidFireVolumeTestCase(test.TestCase): account = sfv._get_sfaccount_by_name('some-name') self.assertIsNotNone(account) + def test_get_account_create_availability_no_account(self): + fake_sfaccounts = [] + sfv = solidfire.SolidFireDriver(configuration=self.configuration) + sfaccount = sfv._get_account_create_availability(fake_sfaccounts) + self.assertIsNone(sfaccount) + + def test_get_account_create_availability(self): + fake_sfaccounts = [{'accountID': 29, + 'targetSecret': 'shhhh', + 'username': 'prefix-testprjid', + 'volumes': [6, 7, 20]}] + sfv = solidfire.SolidFireDriver(configuration=self.configuration) + sfaccount = sfv._get_account_create_availability(fake_sfaccounts) + self.assertIsNotNone(sfaccount) + self.assertEqual(sfaccount['accountID'], + fake_sfaccounts[0]['accountID']) + + def test_get_account_create_availability_primary_full(self): + fake_sfaccounts = [{'accountID': 30, + 'targetSecret': 'shhhh', + 'username': 'prefix-testprjid'}] + get_sfaccount_result = {'accountID': 31, + 'targetSecret': 'shhhh', + 'username': 'prefix-testprjid_'} + get_vol_result = list(range(1, 2001)) + sfv = solidfire.SolidFireDriver(configuration=self.configuration) + with mock.patch.object(sfv, + '_get_sfaccounts_for_tenant', + return_value=fake_sfaccounts), \ + mock.patch.object(sfv, + '_get_volumes_for_account', + return_value=get_vol_result): + sfaccount = sfv._get_account_create_availability(fake_sfaccounts) + self.assertIsNotNone(sfaccount) + self.assertEqual(sfaccount['username'], + get_sfaccount_result['username']) + + def test_get_account_create_availability_both_full(self): + fake_sfaccounts = [{'accountID': 32, + 'targetSecret': 'shhhh', + 'username': 'prefix-testprjid'}, + {'accountID': 33, + 'targetSecret': 'shhhh', + 'username': 'prefix-testprjid_'}] + get_vol_result = list(range(1, 2001)) + sfv = solidfire.SolidFireDriver(configuration=self.configuration) + with mock.patch.object(sfv, + '_get_sfaccounts_for_tenant', + return_value=fake_sfaccounts), \ + mock.patch.object(sfv, + '_get_volumes_for_account', + return_value=get_vol_result): + sfaccount = sfv._get_account_create_availability(fake_sfaccounts) + self.assertIsNone(sfaccount) + + def test_get_create_account(self): + fake_sfaccounts = [{'accountID': 34, + 'targetSecret': 'shhhh', + 'username': 'prefix-testprjid'}, + {'accountID': 35, + 'targetSecret': 'shhhh', + 'username': 'prefix-testprjid_'}] + get_vol_result = list(range(1, 2001)) + sfv = solidfire.SolidFireDriver(configuration=self.configuration) + with mock.patch.object(sfv, + '_get_sfaccounts_for_tenant', + return_value=fake_sfaccounts), \ + mock.patch.object(sfv, + '_get_volumes_for_account', + return_value=get_vol_result): + sfaccount = sfv._get_account_create_availability(fake_sfaccounts) + self.assertRaises(exception.SolidFireDriverException, + sfv._get_create_account, sfaccount) + def test_get_sfaccount_by_name_fails(self): sfv = solidfire.SolidFireDriver(configuration=self.configuration) self.mock_object(solidfire.SolidFireDriver, @@ -616,9 +713,8 @@ class SolidFireVolumeTestCase(test.TestCase): def test_get_sfvol_by_cinder_vref_no_provider_id(self): fake_sfaccounts = [{'accountID': 25, - 'name': 'testprjid', 'targetSecret': 'shhhh', - 'username': 'john-wayne', + 'username': 'prefix-testprjid', 'volumes': [6, 7, 20]}] self.mock_vref = mock_vref() @@ -651,9 +747,8 @@ class SolidFireVolumeTestCase(test.TestCase): def test_get_sfvol_by_cinder_vref_no_provider_id_nomatch(self): fake_sfaccounts = [{'accountID': 5, - 'name': 'testprjid', 'targetSecret': 'shhhh', - 'username': 'john-wayne', + 'username': 'prefix-testprjid', 'volumes': [5, 6, 7, 8]}] self.mock_vref = mock_vref() @@ -673,9 +768,8 @@ class SolidFireVolumeTestCase(test.TestCase): def test_get_sfvol_by_cinder_vref_nomatch(self): fake_sfaccounts = [{'accountID': 5, - 'name': 'testprjid', 'targetSecret': 'shhhh', - 'username': 'john-wayne', + 'username': 'prefix-testprjid', 'volumes': [5, 6, 7, 8]}] self.mock_vref = mock_vref() @@ -695,9 +789,8 @@ class SolidFireVolumeTestCase(test.TestCase): def test_get_sfvol_by_cinder_vref(self): fake_sfaccounts = [{'accountID': 5, - 'name': 'testprjid', 'targetSecret': 'shhhh', - 'username': 'john-wayne', + 'username': 'prefix-testprjid', 'volumes': [5, 6, 7, 8]}] self.mock_vref = mock_vref() @@ -777,9 +870,8 @@ class SolidFireVolumeTestCase(test.TestCase): multiattach=True) fake_sfaccounts = [{'accountID': 5, - 'name': 'testprjid', 'targetSecret': 'shhhh', - 'username': 'john-wayne'}] + 'username': 'prefix-testprjid'}] get_vol_result = {'volumeID': 5, 'name': 'test_volume', @@ -812,9 +904,8 @@ class SolidFireVolumeTestCase(test.TestCase): def test_delete_volume_no_volume_on_backend(self): fake_sfaccounts = [{'accountID': 5, - 'name': 'testprjid', 'targetSecret': 'shhhh', - 'username': 'john-wayne'}] + 'username': 'prefix-testprjid'}] fake_no_volumes = [] testvol = test_utils.create_volume(self.ctxt) @@ -829,9 +920,8 @@ class SolidFireVolumeTestCase(test.TestCase): def test_delete_snapshot_no_snapshot_on_backend(self): fake_sfaccounts = [{'accountID': 5, - 'name': 'testprjid', 'targetSecret': 'shhhh', - 'username': 'john-wayne'}] + 'username': 'prefix-testprjid'}] fake_no_volumes = [] testvol = test_utils.create_volume( self.ctxt, @@ -1142,9 +1232,8 @@ class SolidFireVolumeTestCase(test.TestCase): 'migration_status': 'target:' 'a720b3c0-d1f0-11e1-9b23-0800200c9a66'} fake_sfaccounts = [{'accountID': 5, - 'name': 'testprjid', 'targetSecret': 'shhhh', - 'username': 'john-wayne'}] + 'username': 'prefix-testprjid'}] def _fake_do_v_create(project_id, params): return project_id, params diff --git a/cinder/volume/drivers/solidfire.py b/cinder/volume/drivers/solidfire.py index 3029bd65678..fe2aba42dd8 100644 --- a/cinder/volume/drivers/solidfire.py +++ b/cinder/volume/drivers/solidfire.py @@ -647,7 +647,7 @@ class SolidFireDriver(san.SanISCSIDriver): return sfaccount - def _create_sfaccount(self, project_id): + def _create_sfaccount(self, sf_account_name): """Create account on SolidFire device if it doesn't already exist. We're first going to check if the account already exists, if it does @@ -655,7 +655,6 @@ class SolidFireDriver(san.SanISCSIDriver): """ - sf_account_name = self._get_sf_account_name(project_id) sfaccount = self._get_sfaccount_by_name(sf_account_name) if sfaccount is None: LOG.debug('solidfire account: %s does not exist, create it...', @@ -1102,7 +1101,8 @@ class SolidFireDriver(san.SanISCSIDriver): # Also: we expect this to be sorted, so we get the primary first # in the list return sorted([acc for acc in accounts if - cinder_project_id in acc['username']]) + cinder_project_id in acc['username']], + key=lambda k: k['accountID']) def _get_all_active_volumes(self, cinder_uuid=None): params = {} @@ -1132,19 +1132,21 @@ class SolidFireDriver(san.SanISCSIDriver): # if it exists and return whichever one has count # available. for acc in accounts: - if self._get_volumes_for_account( - acc['accountID']) > self.max_volumes_per_account: + if len(self._get_volumes_for_account( + acc['accountID'])) < self.max_volumes_per_account: return acc if len(accounts) == 1: - sfaccount = self._create_sfaccount(accounts[0]['name'] + '_') + sfaccount = self._create_sfaccount(accounts[0]['username'] + '_') return sfaccount return None def _get_create_account(self, proj_id): # Retrieve SolidFire accountID to be used for creating volumes. sf_accounts = self._get_sfaccounts_for_tenant(proj_id) + if not sf_accounts: - sf_account = self._create_sfaccount(proj_id) + sf_account_name = self._get_sf_account_name(proj_id) + sf_account = self._create_sfaccount(sf_account_name) else: # Check availability for creates sf_account = self._get_account_create_availability(sf_accounts) @@ -2050,7 +2052,7 @@ class SolidFireDriver(san.SanISCSIDriver): if new_project != volume['project_id']: # do a create_sfaccount here as this tenant # may not exist on the cluster yet - sfaccount = self._create_sfaccount(new_project) + sfaccount = self._get_create_account(new_project) params = { 'volumeID': sf_vol['volumeID'], @@ -2118,7 +2120,7 @@ class SolidFireDriver(san.SanISCSIDriver): 'ListActiveVolumes', params)['result']['volumes'] sf_ref = vols[0] - sfaccount = self._create_sfaccount(volume['project_id']) + sfaccount = self._get_create_account(volume['project_id']) attributes = {} qos = self._retrieve_qos_setting(volume) diff --git a/releasenotes/notes/fix-solidfire-python3-support-ee02ff2c1ec920f2.yaml b/releasenotes/notes/fix-solidfire-python3-support-ee02ff2c1ec920f2.yaml new file mode 100644 index 00000000000..b16928222fb --- /dev/null +++ b/releasenotes/notes/fix-solidfire-python3-support-ee02ff2c1ec920f2.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fix python3 imcompability issues and make SolidFire driver fully compatible + with python3.