NetApp SolidFire: Fix misbehavior on account creation

This change fixes a misbehavior when creating a new sfaccount and
verifying the availability for new sfaccounts. It also fixes
the driver compability with python3.

Change-Id: I35cd8d0725aad36e97becb5599b7eaf6c413f2e3
Closes-bug: #1813189

cinder/tests/unit/volume/drivers/solidfire/test_solidfire.py

@@ -117,9 +117,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,
@@ -300,6 +299,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
@@ -347,9 +356,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',
@@ -411,9 +419,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,
@@ -441,9 +448,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,
@@ -483,7 +489,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)
 
@@ -583,7 +590,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):
@@ -594,6 +601,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,
@@ -602,6 +625,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,
@@ -612,9 +709,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()
 
@@ -647,9 +743,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()
@@ -669,9 +764,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()
@@ -691,9 +785,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()
@@ -730,9 +823,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',
@@ -765,9 +857,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)
 
@@ -782,9 +873,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,
@@ -1076,9 +1166,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

cinder/volume/drivers/solidfire.py

@@ -635,7 +635,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
@@ -643,7 +643,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...',
@@ -1090,7 +1089,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 = {}
@@ -1120,19 +1120,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(
+            if len(self._get_volumes_for_account(
-                    acc['accountID']) > self.max_volumes_per_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)
@@ -2012,7 +2014,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'],
@@ -2080,7 +2082,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)

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.