diff --git a/cinder/db/api.py b/cinder/db/api.py index d5842196297..5a28bb8f1ce 100644 --- a/cinder/db/api.py +++ b/cinder/db/api.py @@ -1100,14 +1100,24 @@ def get_booleans_for_table(table_name): ################### -def driver_initiator_data_update(context, initiator, namespace, updates): - """Create DriverPrivateData from the values dictionary.""" - return IMPL.driver_initiator_data_update(context, initiator, - namespace, updates) +def driver_initiator_data_insert_by_key(context, initiator, + namespace, key, value): + """Updates DriverInitiatorData entry. + + Sets the value for the specified key within the namespace. + + If the entry already exists return False, if it inserted successfully + return True. + """ + return IMPL.driver_initiator_data_insert_by_key(context, + initiator, + namespace, + key, + value) def driver_initiator_data_get(context, initiator, namespace): - """Query for an DriverPrivateData that has the specified key""" + """Query for an DriverInitiatorData that has the specified key""" return IMPL.driver_initiator_data_get(context, initiator, namespace) diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index aa925842098..c6ad4ecad2a 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -4563,35 +4563,20 @@ def message_destroy(context, message): @require_context -def driver_initiator_data_update(context, initiator, namespace, updates): +def driver_initiator_data_insert_by_key(context, initiator, namespace, + key, value): + data = models.DriverInitiatorData() + data.initiator = initiator + data.namespace = namespace + data.key = key + data.value = value session = get_session() - with session.begin(): - set_values = updates.get('set_values', {}) - for key, value in set_values.items(): - data = session.query(models.DriverInitiatorData).\ - filter_by(initiator=initiator).\ - filter_by(namespace=namespace).\ - filter_by(key=key).\ - first() - - if data: - data.update({'value': value}) - data.save(session=session) - else: - data = models.DriverInitiatorData() - data.initiator = initiator - data.namespace = namespace - data.key = key - data.value = value - session.add(data) - - remove_values = updates.get('remove_values', []) - for key in remove_values: - session.query(models.DriverInitiatorData).\ - filter_by(initiator=initiator).\ - filter_by(namespace=namespace).\ - filter_by(key=key).\ - delete() + try: + with session.begin(): + session.add(data) + return True + except db_exc.DBDuplicateEntry: + return False @require_context diff --git a/cinder/tests/unit/test_db_api.py b/cinder/tests/unit/test_db_api.py index 083d16d40e7..118ad1e478f 100644 --- a/cinder/tests/unit/test_db_api.py +++ b/cinder/tests/unit/test_db_api.py @@ -2274,61 +2274,22 @@ class DBAPIDriverInitiatorDataTestCase(BaseTest): initiator = 'iqn.1993-08.org.debian:01:222' namespace = 'test_ns' - def test_driver_initiator_data_set_and_remove(self): - data_key = 'key1' - data_value = 'value1' - update = { - 'set_values': { - data_key: data_value - } - } + def _test_insert(self, key, value, expected_result=True): + result = db.driver_initiator_data_insert_by_key( + self.ctxt, self.initiator, self.namespace, key, value) + self.assertEqual(expected_result, result) - db.driver_initiator_data_update(self.ctxt, self.initiator, - self.namespace, update) data = db.driver_initiator_data_get(self.ctxt, self.initiator, self.namespace) + self.assertEqual(data[0].key, key) + self.assertEqual(data[0].value, value) - self.assertIsNotNone(data) - self.assertEqual(data_key, data[0]['key']) - self.assertEqual(data_value, data[0]['value']) + def test_insert(self): + self._test_insert('key1', 'foo') - update = {'remove_values': [data_key]} - - db.driver_initiator_data_update(self.ctxt, self.initiator, - self.namespace, update) - data = db.driver_initiator_data_get(self.ctxt, self.initiator, - self.namespace) - - self.assertIsNotNone(data) - self.assertEqual([], data) - - def test_driver_initiator_data_no_changes(self): - db.driver_initiator_data_update(self.ctxt, self.initiator, - self.namespace, {}) - data = db.driver_initiator_data_get(self.ctxt, self.initiator, - self.namespace) - - self.assertIsNotNone(data) - self.assertEqual([], data) - - def test_driver_initiator_data_update_existing_values(self): - data_key = 'key1' - data_value = 'value1' - update = {'set_values': {data_key: data_value}} - db.driver_initiator_data_update(self.ctxt, self.initiator, - self.namespace, update) - data_value = 'value2' - update = {'set_values': {data_key: data_value}} - db.driver_initiator_data_update(self.ctxt, self.initiator, - self.namespace, update) - data = db.driver_initiator_data_get(self.ctxt, self.initiator, - self.namespace) - self.assertEqual(data_value, data[0]['value']) - - def test_driver_initiator_data_remove_not_existing(self): - update = {'remove_values': ['key_that_doesnt_exist']} - db.driver_initiator_data_update(self.ctxt, self.initiator, - self.namespace, update) + def test_insert_already_exists(self): + self._test_insert('key2', 'bar') + self._test_insert('key2', 'bar', expected_result=False) class DBAPIImageVolumeCacheEntryTestCase(BaseTest): diff --git a/cinder/tests/unit/test_pure.py b/cinder/tests/unit/test_pure.py index d122a6495b2..028bf28e91a 100644 --- a/cinder/tests/unit/test_pure.py +++ b/cinder/tests/unit/test_pure.py @@ -2098,18 +2098,15 @@ class PureISCSIDriverTestCase(PureDriverTestCase): self.driver._connect(VOLUME, ISCSI_CONNECTOR) result["auth_username"] = chap_user result["auth_password"] = chap_password - expected_update = { - "set_values": { - pure.CHAP_SECRET_KEY: chap_password - }, - } + self.assertDictMatch(result, real_result) self.array.set_host.assert_called_with(PURE_HOST_NAME, host_user=chap_user, host_password=chap_password) - self.mock_utils.save_driver_initiator_data.assert_called_with( + self.mock_utils.insert_driver_initiator_data.assert_called_with( ISCSI_CONNECTOR['initiator'], - expected_update + pure.CHAP_SECRET_KEY, + chap_password ) @mock.patch(ISCSI_DRIVER_OBJ + "._get_host", autospec=True) @@ -2158,6 +2155,39 @@ class PureISCSIDriverTestCase(PureDriverTestCase): self.assertTrue(self.array.connect_host.called) self.assertTrue(self.array.list_volume_private_connections) + @mock.patch(ISCSI_DRIVER_OBJ + "._generate_chap_secret") + def test_get_chap_credentials_create_new(self, mock_generate_secret): + self.mock_utils.get_driver_initiator_data.return_value = [] + host = 'host1' + expected_password = 'foo123' + mock_generate_secret.return_value = expected_password + self.mock_utils.insert_driver_initiator_data.return_value = True + username, password = self.driver._get_chap_credentials(host, + INITIATOR_IQN) + self.assertEqual(host, username) + self.assertEqual(expected_password, password) + self.mock_utils.insert_driver_initiator_data.assert_called_once_with( + INITIATOR_IQN, pure.CHAP_SECRET_KEY, expected_password + ) + + @mock.patch(ISCSI_DRIVER_OBJ + "._generate_chap_secret") + def test_get_chap_credentials_create_new_fail_to_set(self, + mock_generate_secret): + host = 'host1' + expected_password = 'foo123' + mock_generate_secret.return_value = 'badpassw0rd' + self.mock_utils.insert_driver_initiator_data.return_value = False + self.mock_utils.get_driver_initiator_data.side_effect = [ + [], + [{'key': pure.CHAP_SECRET_KEY, 'value': expected_password}], + exception.PureDriverException(reason='this should never be hit'), + ] + + username, password = self.driver._get_chap_credentials(host, + INITIATOR_IQN) + self.assertEqual(host, username) + self.assertEqual(expected_password, password) + class PureFCDriverTestCase(PureDriverTestCase): diff --git a/cinder/volume/driver_utils.py b/cinder/volume/driver_utils.py index cc408f44dd5..3a8a0bb8656 100644 --- a/cinder/volume/driver_utils.py +++ b/cinder/volume/driver_utils.py @@ -48,16 +48,25 @@ class VolumeDriverUtils(object): 'namespace': self._data_namespace}) raise - def save_driver_initiator_data(self, initiator, model_update, ctxt=None): + def insert_driver_initiator_data(self, initiator, key, value, ctxt=None): + """Update the initiator data at key with value. + + If the key has already been set to something return False, otherwise + if saved successfully return True. + """ try: - self._db.driver_initiator_data_update(self._get_context(ctxt), - initiator, - self._data_namespace, - model_update) + return self._db.driver_initiator_data_insert_by_key( + self._get_context(ctxt), + initiator, + self._data_namespace, + key, + value + ) except exception.CinderException: - LOG.exception(_LE("Failed to update initiator data for" + LOG.exception(_LE("Failed to insert initiator data for" " initiator %(initiator)s and backend" - " %(backend)s"), + " %(backend)s for key %(key)s."), {'initiator': initiator, - 'backend': self._data_namespace}) + 'backend': self._data_namespace, + 'key': key}) raise diff --git a/cinder/volume/drivers/pure.py b/cinder/volume/drivers/pure.py index 7ecccd8f895..0af5b382198 100644 --- a/cinder/volume/drivers/pure.py +++ b/cinder/volume/drivers/pure.py @@ -1552,26 +1552,27 @@ class PureISCSIDriver(PureBaseVolumeDriver, san.SanISCSIDriver): def _generate_chap_secret(): return volume_utils.generate_password() - def _get_chap_credentials(self, host, initiator): + def _get_chap_secret_from_init_data(self, initiator): data = self.driver_utils.get_driver_initiator_data(initiator) - initiator_updates = None - username = host - password = None if data: for d in data: if d["key"] == CHAP_SECRET_KEY: - password = d["value"] - break + return d["value"] + return None + + def _get_chap_credentials(self, host, initiator): + username = host + password = self._get_chap_secret_from_init_data(initiator) if not password: password = self._generate_chap_secret() - initiator_updates = { - "set_values": { - CHAP_SECRET_KEY: password - } - } - if initiator_updates: - self.driver_utils.save_driver_initiator_data(initiator, - initiator_updates) + success = self.driver_utils.insert_driver_initiator_data( + initiator, CHAP_SECRET_KEY, password) + if not success: + # The only reason the save would have failed is if someone + # else (read: another thread/instance of the driver) set + # one before we did. In that case just do another query. + password = self._get_chap_secret_from_init_data(initiator) + return username, password @utils.synchronized(CONNECT_LOCK_NAME, external=True)