From cd078020f6140fb401ea59837728d58c5f9bcb34 Mon Sep 17 00:00:00 2001 From: Accela Zhao Date: Fri, 8 Jul 2016 21:55:28 +0800 Subject: [PATCH] Remove "host" from driver private data The "host" field in driver private data related db and api is not used and no longer needed. This patches removes it. Change-Id: Ifd0b290b2992f6b0792250b86466d9329dfd08a7 Partially-Implements: blueprint driver-private-storage --- manila/db/api.py | 18 +-- ...78_remove_host_from_driver_private_data.py | 108 ++++++++++++++++++ manila/db/sqlalchemy/api.py | 15 ++- manila/db/sqlalchemy/models.py | 1 - manila/share/drivers_private_data.py | 6 +- .../alembic/migrations_data_checks.py | 31 +++++ manila/tests/db/sqlalchemy/test_api.py | 64 +++++------ .../tests/share/test_drivers_private_data.py | 6 +- 8 files changed, 188 insertions(+), 61 deletions(-) create mode 100644 manila/db/migrations/alembic/versions/e8ea58723178_remove_host_from_driver_private_data.py diff --git a/manila/db/api.py b/manila/db/api.py index a05465c29e..7acd73d368 100644 --- a/manila/db/api.py +++ b/manila/db/api.py @@ -852,21 +852,21 @@ def share_type_extra_specs_update_or_create(context, share_type_id, extra_specs) -def driver_private_data_get(context, host, entity_id, key=None, default=None): - """Get one, list or all key-value pairs for given host and entity_id.""" - return IMPL.driver_private_data_get(context, host, entity_id, key, default) +def driver_private_data_get(context, entity_id, key=None, default=None): + """Get one, list or all key-value pairs for given entity_id.""" + return IMPL.driver_private_data_get(context, entity_id, key, default) -def driver_private_data_update(context, host, entity_id, details, +def driver_private_data_update(context, entity_id, details, delete_existing=False): - """Update key-value pairs for given host and entity_id.""" - return IMPL.driver_private_data_update(context, host, entity_id, details, + """Update key-value pairs for given entity_id.""" + return IMPL.driver_private_data_update(context, entity_id, details, delete_existing) -def driver_private_data_delete(context, host, entity_id, key=None): - """Remove one, list or all key-value pairs for given host and entity_id.""" - return IMPL.driver_private_data_delete(context, host, entity_id, key) +def driver_private_data_delete(context, entity_id, key=None): + """Remove one, list or all key-value pairs for given entity_id.""" + return IMPL.driver_private_data_delete(context, entity_id, key) #################### diff --git a/manila/db/migrations/alembic/versions/e8ea58723178_remove_host_from_driver_private_data.py b/manila/db/migrations/alembic/versions/e8ea58723178_remove_host_from_driver_private_data.py new file mode 100644 index 0000000000..7b09296a6a --- /dev/null +++ b/manila/db/migrations/alembic/versions/e8ea58723178_remove_host_from_driver_private_data.py @@ -0,0 +1,108 @@ +# Copyright (c) 2016 EMC Corporation. +# All Rights Reserved. +# +# 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. + +"""Remove host from driver private data + +Revision ID: e8ea58723178 +Revises: fdfb668d19e1 +Create Date: 2016-07-11 12:59:34.579291 + +""" + +# revision identifiers, used by Alembic. +revision = 'e8ea58723178' +down_revision = 'fdfb668d19e1' + +from alembic import op +from oslo_log import log +from oslo_utils import uuidutils +import sqlalchemy as sql + +from manila.db.migrations import utils +from manila.i18n import _LI, _LE + +LOG = log.getLogger(__name__) +TABLE_NAME = 'drivers_private_data' +COLUMN_HOST = 'host' +DEFAULT_HOST = 'unknown' +COLUMN_ENTITY = 'entity_uuid' +COLUMN_KEY = 'key' + + +def upgrade(): + try: + op.drop_column(TABLE_NAME, COLUMN_HOST) + except Exception: + LOG.error(_LE("Column '%s' could not be dropped"), COLUMN_HOST) + raise + + +def downgrade(): + connection = op.get_bind() + from_table = utils.load_table(TABLE_NAME, connection) + migration_table_name = "_migrating_%(table)s_%(session)s" % { + 'table': TABLE_NAME, + 'session': uuidutils.generate_uuid()[:8] + } + + LOG.info(_LI("Creating the migration table %(table)s"), { + 'table': migration_table_name + }) + migration_table = op.create_table( + migration_table_name, + sql.Column('created_at', sql.DateTime), + sql.Column('updated_at', sql.DateTime), + sql.Column('deleted_at', sql.DateTime), + sql.Column('deleted', sql.Integer, default=0), + sql.Column('host', sql.String(255), + nullable=False, primary_key=True), + sql.Column('entity_uuid', sql.String(36), + nullable=False, primary_key=True), + sql.Column('key', sql.String(255), + nullable=False, primary_key=True), + sql.Column('value', sql.String(1023), nullable=False), + mysql_engine='InnoDB', + ) + + LOG.info(_LI("Copying data from %(from_table)s to the migration " + "table %(migration_table)s") % { + 'from_table': TABLE_NAME, + 'migration_table': migration_table_name + }) + rows = [] + for row in op.get_bind().execute(from_table.select()): + rows.append({ + 'created_at': row.created_at, + 'updated_at': row.updated_at, + 'deleted_at': row.deleted_at, + 'deleted': row.deleted, + 'host': DEFAULT_HOST, + 'entity_uuid': row.entity_uuid, + 'key': row.key, + 'value': row.value + }) + op.bulk_insert(migration_table, rows) + + LOG.info(_LI("Dropping table %(from_table)s") % { + 'from_table': TABLE_NAME + }) + op.drop_table(TABLE_NAME) + + LOG.info(_LI("Rename the migration table %(migration_table)s to " + "the original table %(from_table)s") % { + 'migration_table': migration_table_name, + 'from_table': TABLE_NAME + }) + op.rename_table(migration_table_name, TABLE_NAME) diff --git a/manila/db/sqlalchemy/api.py b/manila/db/sqlalchemy/api.py index 78e659fb7e..a69901ffa2 100644 --- a/manila/db/sqlalchemy/api.py +++ b/manila/db/sqlalchemy/api.py @@ -2839,7 +2839,7 @@ def share_server_backend_details_delete(context, share_server_id, ################### -def _driver_private_data_query(session, context, host, entity_id, key=None, +def _driver_private_data_query(session, context, entity_id, key=None, read_deleted=False): query = model_query( context, models.DriverPrivateData, session=session, @@ -2857,12 +2857,12 @@ def _driver_private_data_query(session, context, host, entity_id, key=None, @require_context -def driver_private_data_get(context, host, entity_id, key=None, +def driver_private_data_get(context, entity_id, key=None, default=None, session=None): if not session: session = get_session() - query = _driver_private_data_query(session, context, host, entity_id, key) + query = _driver_private_data_query(session, context, entity_id, key) if key is None or isinstance(key, list): return {item.key: item.value for item in query.all()} @@ -2872,7 +2872,7 @@ def driver_private_data_get(context, host, entity_id, key=None, @require_context -def driver_private_data_update(context, host, entity_id, details, +def driver_private_data_update(context, entity_id, details, delete_existing=False, session=None): # NOTE(u_glide): following code modifies details dict, that's why we should # copy it @@ -2885,7 +2885,7 @@ def driver_private_data_update(context, host, entity_id, details, # Process existing data # NOTE(u_glide): read_deleted=None means here 'read all' original_data = _driver_private_data_query( - session, context, host, entity_id, read_deleted=None).all() + session, context, entity_id, read_deleted=None).all() for data_ref in original_data: in_new_details = data_ref['key'] in new_details @@ -2908,7 +2908,6 @@ def driver_private_data_update(context, host, entity_id, details, for key, value in new_details.items(): data_ref = models.DriverPrivateData() data_ref.update({ - "host": host, "entity_uuid": entity_id, "key": key, "value": six.text_type(value) @@ -2919,13 +2918,13 @@ def driver_private_data_update(context, host, entity_id, details, @require_context -def driver_private_data_delete(context, host, entity_id, key=None, +def driver_private_data_delete(context, entity_id, key=None, session=None): if not session: session = get_session() with session.begin(): - query = _driver_private_data_query(session, context, host, + query = _driver_private_data_query(session, context, entity_id, key) query.update({"deleted": 1, "deleted_at": timeutils.utcnow()}) diff --git a/manila/db/sqlalchemy/models.py b/manila/db/sqlalchemy/models.py index 39d8d0e840..49da5284c7 100644 --- a/manila/db/sqlalchemy/models.py +++ b/manila/db/sqlalchemy/models.py @@ -874,7 +874,6 @@ class NetworkAllocation(BASE, ManilaBase): class DriverPrivateData(BASE, ManilaBase): """Represents a private data as key-value pairs for a driver.""" __tablename__ = 'drivers_private_data' - host = Column(String(255), nullable=False, primary_key=True) entity_uuid = Column(String(36), nullable=False, primary_key=True) key = Column(String(255), nullable=False, primary_key=True) value = Column(String(1023), nullable=False) diff --git a/manila/share/drivers_private_data.py b/manila/share/drivers_private_data.py index 903a7be534..73ab50c9c5 100644 --- a/manila/share/drivers_private_data.py +++ b/manila/share/drivers_private_data.py @@ -83,18 +83,18 @@ class SqlStorageDriver(StorageDriver): def update(self, entity_id, details, delete_existing): return db_api.driver_private_data_update( - self.context, self.backend_host, entity_id, details, + self.context, entity_id, details, delete_existing ) def get(self, entity_id, key, default): return db_api.driver_private_data_get( - self.context, self.backend_host, entity_id, key, default + self.context, entity_id, key, default ) def delete(self, entity_id, key): return db_api.driver_private_data_delete( - self.context, self.backend_host, entity_id, key + self.context, entity_id, key ) diff --git a/manila/tests/db/migrations/alembic/migrations_data_checks.py b/manila/tests/db/migrations/alembic/migrations_data_checks.py index 46d2f992e9..e1898a2856 100644 --- a/manila/tests/db/migrations/alembic/migrations_data_checks.py +++ b/manila/tests/db/migrations/alembic/migrations_data_checks.py @@ -762,3 +762,34 @@ class NewGatewayColumnChecks(BaseMigrationChecks): self.test_case.assertTrue(db_result.rowcount >= len(ids)) for record in db_result: self.test_case.assertFalse(hasattr(record, 'gateway')) + + +@map_to_migration('e8ea58723178') +class RemoveHostFromDriverPrivateDataChecks(BaseMigrationChecks): + table_name = 'drivers_private_data' + host_column_name = 'host' + + def setup_upgrade_data(self, engine): + dpd_data = { + 'created_at': datetime.datetime(2016, 7, 14, 22, 31, 22), + 'deleted': 0, + 'host': 'host1', + 'entity_uuid': 'entity_uuid1', + 'key': 'key1', + 'value': 'value1' + } + dpd_table = utils.load_table(self.table_name, engine) + engine.execute(dpd_table.insert(dpd_data)) + + def check_upgrade(self, engine, data): + dpd_table = utils.load_table(self.table_name, engine) + rows = engine.execute(dpd_table.select()) + for row in rows: + self.test_case.assertFalse(hasattr(row, self.host_column_name)) + + def check_downgrade(self, engine): + dpd_table = utils.load_table(self.table_name, engine) + rows = engine.execute(dpd_table.select()) + for row in rows: + self.test_case.assertTrue(hasattr(row, self.host_column_name)) + self.test_case.assertEqual('unknown', row[self.host_column_name]) diff --git a/manila/tests/db/sqlalchemy/test_api.py b/manila/tests/db/sqlalchemy/test_api.py index 670e0dc9f3..2c8a7a9936 100644 --- a/manila/tests/db/sqlalchemy/test_api.py +++ b/manila/tests/db/sqlalchemy/test_api.py @@ -1195,7 +1195,7 @@ class DriverPrivateDataDatabaseAPITestCase(test.TestCase): self.ctxt = context.get_admin_context() def _get_driver_test_data(self): - return ("fake@host", uuidutils.generate_uuid()) + return uuidutils.generate_uuid() @ddt.data({"details": {"foo": "bar", "tee": "too"}, "valid": {"foo": "bar", "tee": "too"}}, @@ -1203,93 +1203,83 @@ class DriverPrivateDataDatabaseAPITestCase(test.TestCase): "valid": {"foo": "bar", "tee": six.text_type(["test"])}}) @ddt.unpack def test_update(self, details, valid): - test_host, test_id = self._get_driver_test_data() + test_id = self._get_driver_test_data() - initial_data = db_api.driver_private_data_get( - self.ctxt, test_host, test_id) - db_api.driver_private_data_update(self.ctxt, test_host, test_id, - details) - actual_data = db_api.driver_private_data_get( - self.ctxt, test_host, test_id) + initial_data = db_api.driver_private_data_get(self.ctxt, test_id) + db_api.driver_private_data_update(self.ctxt, test_id, details) + actual_data = db_api.driver_private_data_get(self.ctxt, test_id) self.assertEqual({}, initial_data) self.assertEqual(valid, actual_data) def test_update_with_duplicate(self): - test_host, test_id = self._get_driver_test_data() + test_id = self._get_driver_test_data() details = {"tee": "too"} - db_api.driver_private_data_update(self.ctxt, test_host, test_id, - details) - db_api.driver_private_data_update(self.ctxt, test_host, test_id, - details) + db_api.driver_private_data_update(self.ctxt, test_id, details) + db_api.driver_private_data_update(self.ctxt, test_id, details) - actual_result = db_api.driver_private_data_get( - self.ctxt, test_host, test_id) + actual_result = db_api.driver_private_data_get(self.ctxt, + test_id) self.assertEqual(details, actual_result) def test_update_with_delete_existing(self): - test_host, test_id = self._get_driver_test_data() + test_id = self._get_driver_test_data() details = {"key1": "val1", "key2": "val2", "key3": "val3"} details_update = {"key1": "val1_upd", "key4": "new_val"} # Create new details - db_api.driver_private_data_update(self.ctxt, test_host, test_id, - details) - db_api.driver_private_data_update(self.ctxt, test_host, test_id, + db_api.driver_private_data_update(self.ctxt, test_id, details) + db_api.driver_private_data_update(self.ctxt, test_id, details_update, delete_existing=True) actual_result = db_api.driver_private_data_get( - self.ctxt, test_host, test_id) + self.ctxt, test_id) self.assertEqual(details_update, actual_result) def test_get(self): - test_host, test_id = self._get_driver_test_data() + test_id = self._get_driver_test_data() test_key = "foo" test_keys = [test_key, "tee"] details = {test_keys[0]: "val", test_keys[1]: "val", "mee": "foo"} - db_api.driver_private_data_update(self.ctxt, test_host, test_id, - details) + db_api.driver_private_data_update(self.ctxt, test_id, details) actual_result_all = db_api.driver_private_data_get( - self.ctxt, test_host, test_id) + self.ctxt, test_id) actual_result_single_key = db_api.driver_private_data_get( - self.ctxt, test_host, test_id, test_key) + self.ctxt, test_id, test_key) actual_result_list = db_api.driver_private_data_get( - self.ctxt, test_host, test_id, test_keys) + self.ctxt, test_id, test_keys) self.assertEqual(details, actual_result_all) self.assertEqual(details[test_key], actual_result_single_key) self.assertEqual(dict.fromkeys(test_keys, "val"), actual_result_list) def test_delete_single(self): - test_host, test_id = self._get_driver_test_data() + test_id = self._get_driver_test_data() test_key = "foo" details = {test_key: "bar", "tee": "too"} valid_result = {"tee": "too"} - db_api.driver_private_data_update(self.ctxt, test_host, test_id, - details) + db_api.driver_private_data_update(self.ctxt, test_id, details) - db_api.driver_private_data_delete(self.ctxt, test_host, test_id, - test_key) + db_api.driver_private_data_delete(self.ctxt, test_id, test_key) actual_result = db_api.driver_private_data_get( - self.ctxt, test_host, test_id) + self.ctxt, test_id) self.assertEqual(valid_result, actual_result) def test_delete_all(self): - test_host, test_id = self._get_driver_test_data() + test_id = self._get_driver_test_data() details = {"foo": "bar", "tee": "too"} - db_api.driver_private_data_update(self.ctxt, test_host, test_id, - details) + db_api.driver_private_data_update(self.ctxt, test_id, details) - db_api.driver_private_data_delete(self.ctxt, test_host, test_id) + db_api.driver_private_data_delete(self.ctxt, test_id) actual_result = db_api.driver_private_data_get( - self.ctxt, test_host, test_id) + self.ctxt, test_id) self.assertEqual({}, actual_result) diff --git a/manila/tests/share/test_drivers_private_data.py b/manila/tests/share/test_drivers_private_data.py index 864c88c5e5..0c74bf23d0 100644 --- a/manila/tests/share/test_drivers_private_data.py +++ b/manila/tests/share/test_drivers_private_data.py @@ -147,7 +147,7 @@ class SqlStorageDriverTestCase(test.TestCase): "method_kwargs": create_arg_dict( ["entity_id", "details", "delete_existing"]), "valid_args": create_arg_list( - ["context", "backend_host", "entity_id", "details", + ["context", "entity_id", "details", "delete_existing"] ) }, @@ -155,13 +155,13 @@ class SqlStorageDriverTestCase(test.TestCase): "method_name": 'get', "method_kwargs": create_arg_dict(["entity_id", "key", "default"]), "valid_args": create_arg_list( - ["context", "backend_host", "entity_id", "key", "default"]), + ["context", "entity_id", "key", "default"]), }, { "method_name": 'delete', "method_kwargs": create_arg_dict(["entity_id", "key"]), "valid_args": create_arg_list( - ["context", "backend_host", "entity_id", "key"]), + ["context", "entity_id", "key"]), }) @ddt.unpack def test_methods(self, method_kwargs, method_name, valid_args):