From f3575eab34a6cd532be7f8862838e9b85dc47fb8 Mon Sep 17 00:00:00 2001 From: Patrick East Date: Fri, 22 Aug 2014 16:43:20 -0700 Subject: [PATCH] Add DB table for driver specific data This will allow drivers to store key/value pairs in the database for any needs they have requiring persisted storage for initiator information across cinder service hosts. Implements: blueprint driver-private-data Change-Id: Ib5d80dcdda6df26f07c7a98c4dfd0135cb55d72c --- cinder/db/api.py | 14 +++++ cinder/db/sqlalchemy/api.py | 48 ++++++++++++++- .../038_add_driver_initiator_data_table.py | 58 ++++++++++++++++++ cinder/db/sqlalchemy/models.py | 14 +++++ cinder/tests/test_db_api.py | 61 +++++++++++++++++++ cinder/tests/test_migrations.py | 32 ++++++++++ cinder/tests/test_volume.py | 59 ++++++++++++++++++ cinder/volume/driver.py | 23 ++++++- cinder/volume/manager.py | 56 ++++++++++++++++- 9 files changed, 361 insertions(+), 4 deletions(-) create mode 100644 cinder/db/sqlalchemy/migrate_repo/versions/038_add_driver_initiator_data_table.py diff --git a/cinder/db/api.py b/cinder/db/api.py index 84d62571b..8061613ed 100644 --- a/cinder/db/api.py +++ b/cinder/db/api.py @@ -905,3 +905,17 @@ def purge_deleted_rows(context, age_in_days): :returns: number of deleted rows """ return IMPL.purge_deleted_rows(context, age_in_days=age_in_days) + + +################### + + +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_get(context, initiator, namespace): + """Query for an DriverPrivateData 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 6e3d85050..5b180d102 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -3386,7 +3386,8 @@ def purge_deleted_rows(context, age_in_days): tables = [] for model_class in models.__dict__.itervalues(): - if hasattr(model_class, "__tablename__"): + if hasattr(model_class, "__tablename__") \ + and hasattr(model_class, "deleted"): tables.append(model_class.__tablename__) # Reorder the list so the volumes table is last to avoid FK constraints @@ -3411,3 +3412,48 @@ def purge_deleted_rows(context, age_in_days): rows_purged = result.rowcount LOG.info(_LI("Deleted %(row)d rows from table=%(table)s"), {'row': rows_purged, 'table': table}) + + +############################### + + +@require_context +def driver_initiator_data_update(context, initiator, namespace, updates): + 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() + + +@require_context +def driver_initiator_data_get(context, initiator, namespace): + session = get_session() + with session.begin(): + return session.query(models.DriverInitiatorData).\ + filter_by(initiator=initiator).\ + filter_by(namespace=namespace).\ + all() diff --git a/cinder/db/sqlalchemy/migrate_repo/versions/038_add_driver_initiator_data_table.py b/cinder/db/sqlalchemy/migrate_repo/versions/038_add_driver_initiator_data_table.py new file mode 100644 index 000000000..63974340e --- /dev/null +++ b/cinder/db/sqlalchemy/migrate_repo/versions/038_add_driver_initiator_data_table.py @@ -0,0 +1,58 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from sqlalchemy import Column, DateTime, Integer +from sqlalchemy import MetaData, String, Table, UniqueConstraint + +from cinder.i18n import _LE +from cinder.openstack.common import log as logging + +LOG = logging.getLogger(__name__) + + +def upgrade(migrate_engine): + meta = MetaData() + meta.bind = migrate_engine + + # New table + initiator_data = Table( + 'driver_initiator_data', meta, + Column('created_at', DateTime(timezone=False)), + Column('updated_at', DateTime(timezone=False)), + Column('id', Integer, primary_key=True, nullable=False), + Column('initiator', String(length=255), index=True, nullable=False), + Column('namespace', String(length=255), nullable=False), + Column('key', String(length=255), nullable=False), + Column('value', String(length=255)), + UniqueConstraint('initiator', 'namespace', 'key'), + mysql_engine='InnoDB', + mysql_charset='utf8' + ) + + try: + initiator_data.create() + except Exception: + LOG.error(_LE("Table |%s| not created!"), repr(initiator_data)) + raise + + +def downgrade(migrate_engine): + meta = MetaData() + meta.bind = migrate_engine + table_name = 'driver_initiator_data' + initiator_data = Table(table_name, meta, autoload=True) + try: + initiator_data.drop() + except Exception: + LOG.error(_LE("%(table_name)s table not dropped"), + {'table_name': table_name}) + raise diff --git a/cinder/db/sqlalchemy/models.py b/cinder/db/sqlalchemy/models.py index b024f9b50..40f93d55e 100644 --- a/cinder/db/sqlalchemy/models.py +++ b/cinder/db/sqlalchemy/models.py @@ -548,6 +548,20 @@ class Transfer(BASE, CinderBase): 'Transfer.deleted == False)') +class DriverInitiatorData(BASE, models.TimestampMixin, models.ModelBase): + """Represents private key-value pair specific an initiator for drivers""" + __tablename__ = 'driver_initiator_data' + __table_args__ = ( + schema.UniqueConstraint("initiator", "namespace", "key"), + {'mysql_engine': 'InnoDB'} + ) + id = Column(Integer, primary_key=True, nullable=False) + initiator = Column(String(255), index=True, nullable=False) + namespace = Column(String(255), nullable=False) + key = Column(String(255), nullable=False) + value = Column(String(255)) + + def register_models(): """Register Models and create metadata. diff --git a/cinder/tests/test_db_api.py b/cinder/tests/test_db_api.py index d1120aed8..f4b84f34e 100644 --- a/cinder/tests/test_db_api.py +++ b/cinder/tests/test_db_api.py @@ -1540,3 +1540,64 @@ class DBAPIProcessSortParamTestCase(test.TestCase): sqlalchemy_api.process_sort_params, ['key'], dirs) + + +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 + } + } + + 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_key, data[0]['key']) + self.assertEqual(data_value, data[0]['value']) + + 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) diff --git a/cinder/tests/test_migrations.py b/cinder/tests/test_migrations.py index 0da942e2c..091198020 100644 --- a/cinder/tests/test_migrations.py +++ b/cinder/tests/test_migrations.py @@ -731,6 +731,38 @@ class MigrationsMixin(test_migrations.WalkVersionsMixin): consistencygroups = db_utils.get_table(engine, 'consistencygroups') self.assertNotIn('cgsnapshot_id', consistencygroups.c) + def _check_038(self, engine, data): + """Test adding and removing driver_initiator_data table.""" + + has_table = engine.dialect.has_table(engine.connect(), + "driver_initiator_data") + self.assertTrue(has_table) + + private_data = db_utils.get_table( + engine, + 'driver_initiator_data' + ) + + self.assertIsInstance(private_data.c.created_at.type, + self.TIME_TYPE) + self.assertIsInstance(private_data.c.updated_at.type, + self.TIME_TYPE) + self.assertIsInstance(private_data.c.id.type, + sqlalchemy.types.INTEGER) + self.assertIsInstance(private_data.c.initiator.type, + sqlalchemy.types.VARCHAR) + self.assertIsInstance(private_data.c.namespace.type, + sqlalchemy.types.VARCHAR) + self.assertIsInstance(private_data.c.key.type, + sqlalchemy.types.VARCHAR) + self.assertIsInstance(private_data.c.value.type, + sqlalchemy.types.VARCHAR) + + def _post_downgrade_038(self, engine): + has_table = engine.dialect.has_table(engine.connect(), + "driver_initiator_data") + self.assertFalse(has_table) + def test_walk_versions(self): self.walk_versions(True, False) diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py index 800d8fb88..fa21ee746 100644 --- a/cinder/tests/test_volume.py +++ b/cinder/tests/test_volume.py @@ -1675,6 +1675,65 @@ class VolumeTestCase(BaseVolumeTestCase): 'fake_volume_id', connector) + @mock.patch.object(db, 'volume_admin_metadata_get') + @mock.patch.object(db, 'volume_update') + @mock.patch.object(db, 'volume_get') + @mock.patch.object(fake_driver.FakeISCSIDriver, 'initialize_connection') + @mock.patch.object(db, 'driver_initiator_data_get') + @mock.patch.object(db, 'driver_initiator_data_update') + def test_initialize_connection_initiator_data(self, mock_data_update, + mock_data_get, + mock_driver_init, + mock_volume_get, + mock_volume_update, + mock_metadata_get): + + fake_admin_meta = {'fake-key': 'fake-value'} + fake_volume = {'volume_type_id': None, + 'name': 'fake_name', + 'host': 'fake_host', + 'id': 'fake_volume_id', + 'volume_admin_metadata': fake_admin_meta} + + mock_volume_get.return_value = fake_volume + mock_volume_update.return_value = fake_volume + connector = {'ip': 'IP', 'initiator': 'INITIATOR'} + mock_driver_init.return_value = { + 'driver_volume_type': 'iscsi', + 'data': {'access_mode': 'rw'} + } + mock_data_get.return_value = [] + self.volume.initialize_connection(self.context, 'id', connector) + mock_driver_init.assert_called_with(fake_volume, connector) + + data = [{'key': 'key1', 'value': 'value1'}] + mock_data_get.return_value = data + self.volume.initialize_connection(self.context, 'id', connector) + mock_driver_init.assert_called_with(fake_volume, connector, data) + + update = { + 'set_values': { + 'foo': 'bar' + }, + 'remove_values': [ + 'foo', + 'foo2' + ] + } + mock_driver_init.return_value['initiator_update'] = update + self.volume.initialize_connection(self.context, 'id', connector) + mock_driver_init.assert_called_with(fake_volume, connector, data) + mock_data_update.assert_called_with(self.context, 'INITIATOR', + 'FakeISCSIDriver', update) + + connector['initiator'] = None + mock_data_update.reset_mock() + mock_data_get.reset_mock() + self.volume.initialize_connection(self.context, 'id', connector) + mock_driver_init.assert_called_with(fake_volume, connector) + self.assertFalse(mock_data_get.called) + self.assertFalse(mock_data_update.called) + def test_run_attach_detach_volume_for_instance(self): """Make sure volume can be attached and detached from instance.""" mountpoint = "/dev/sdf" diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py index b697c087e..55ddc591f 100644 --- a/cinder/volume/driver.py +++ b/cinder/volume/driver.py @@ -193,6 +193,9 @@ volume_opts = [ help='Password for specified CHAP account name.', deprecated_opts=deprecated_chap_password_opts, secret=True), + cfg.StrOpt('driver_data_namespace', + default=None, + help='Namespace for driver private data values to be saved in.') ] # for backward compatibility @@ -701,8 +704,24 @@ class BaseVD(object): return @abc.abstractmethod - def initialize_connection(self, volume, connector): - """Allow connection to connector and return connection info.""" + def initialize_connection(self, volume, connector, initiator_data=None): + """Allow connection to connector and return connection info. + + :param volume: The volume to be attached + :param connector: Dictionary containing information about what is being + connected to. + :param initiator_data (optional): A dictionary of driver_initiator_data + objects with key-value pairs that have been saved for this initiator by + a driver in previous initialize_connection calls. + :returns conn_info: A dictionary of connection information. This can + optionally include a "initiator_updates" field. + + The "initiator_updates" field must be a dictionary containing a + "set_values" and/or "remove_values" field. The "set_values" field must + be a dictionary of key-value pairs to be set/updated in the db. The + "remove_values" field must be a list of keys, previously set with + "set_values", that will be deleted from the db. + """ return @abc.abstractmethod diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index 4ad9fdb58..e009f71f1 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -874,6 +874,47 @@ class VolumeManager(manager.SchedulerDependentManager): LOG.warn(_LW("Error occurred while deleting image %s."), image_id, exc_info=True) + def _driver_data_namespace(self): + return self.driver.configuration.safe_get('driver_data_namespace') \ + or self.driver.configuration.safe_get('volume_backend_name') \ + or self.driver.__class__.__name__ + + def _get_driver_initiator_data(self, context, connector): + data = None + initiator = connector.get('initiator', False) + if initiator: + namespace = self._driver_data_namespace() + try: + data = self.db.driver_initiator_data_get( + context, + initiator, + namespace + ) + except exception.CinderException: + LOG.exception(_LE("Failed to get driver initiator data for" + " initiator %(initiator)s and namespace" + " %(namespace)s"), + {'initiator': initiator, + 'namespace': namespace}) + raise + return data + + def _save_driver_initiator_data(self, context, connector, model_update): + if connector.get('initiator', False) and model_update: + namespace = self._driver_data_namespace() + try: + self.db.driver_initiator_data_update(context, + connector['initiator'], + namespace, + model_update) + except exception.CinderException: + LOG.exception(_LE("Failed to update initiator data for" + " initiator %(initiator)s and backend" + " %(backend)s"), + {'initiator': connector['initiator'], + 'backend': namespace}) + raise + def initialize_connection(self, context, volume_id, connector): """Prepare volume for connection from host represented by connector. @@ -948,8 +989,15 @@ class VolumeManager(manager.SchedulerDependentManager): {'volume_id': volume_id, 'model': model_update}) raise exception.ExportFailure(reason=ex) + initiator_data = self._get_driver_initiator_data(context, connector) try: - conn_info = self.driver.initialize_connection(volume, connector) + if initiator_data: + conn_info = self.driver.initialize_connection(volume, + connector, + initiator_data) + else: + conn_info = self.driver.initialize_connection(volume, + connector) except Exception as err: err_msg = (_('Unable to fetch connection information from ' 'backend: %(err)s') % {'err': err}) @@ -959,6 +1007,12 @@ class VolumeManager(manager.SchedulerDependentManager): raise exception.VolumeBackendAPIException(data=err_msg) + initiator_update = conn_info.get('initiator_update', None) + if initiator_update: + self._save_driver_initiator_data(context, connector, + initiator_update) + del conn_info['initiator_update'] + # Add qos_specs to connection info typeid = volume['volume_type_id'] specs = None -- 2.45.2