From 01dfcb172e20ff18ab26629758d344df8bec74a8 Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Thu, 19 Mar 2020 14:54:31 +0100 Subject: [PATCH] DBMS: Fix db_sync between N and N+1 releases If we start cinderlib from the N release with the DBMS persistence plugin on a DB where we have already run it with release N+1 then we'll get an `oslo_db.exception.DBMigrationError` exception. This is because cinderlib automatically does a `db_sync` on each initialization, but on N this will fail because there will be unknown (as in newer) migrations applied to the DB. This patch takes this possibility and checks the exception raised by `db_sync` and ignores it if the `migrate` package complains with a VersionNotFoundError. Closes-Bug: #1868145 Change-Id: I539919e01f603d19cde8750ad92e3d2b4ac2fbc7 --- cinderlib/persistence/dbms.py | 11 +++++++- cinderlib/tests/unit/persistence/test_dbms.py | 27 +++++++++++++++++++ .../notes/mysql-db-sync-8a9e50a12bbe724d.yaml | 6 +++++ 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/mysql-db-sync-8a9e50a12bbe724d.yaml diff --git a/cinderlib/persistence/dbms.py b/cinderlib/persistence/dbms.py index cc6a9b5..983f5d6 100644 --- a/cinderlib/persistence/dbms.py +++ b/cinderlib/persistence/dbms.py @@ -23,6 +23,7 @@ from cinder.db.sqlalchemy import api as sqla_api from cinder.db.sqlalchemy import models from cinder import exception as cinder_exception from cinder import objects as cinder_objs +import migrate from oslo_config import cfg from oslo_db import exception from oslo_log import log @@ -72,7 +73,15 @@ class DBPersistence(persistence_base.PersistenceDriverBase): self.original_get_by_id = self.db_instance.get_by_id self.db_instance.get_by_id = self.get_by_id - migration.db_sync() + try: + migration.db_sync() + except exception.DBMigrationError as exc: + # We can be running 2 Cinder versions at the same time on the same + # DB while we upgrade, so we must ignore the fact that the DB is + # now on a newer version. + if not isinstance(getattr(exc, 'inner_exception', None), + migrate.exceptions.VersionNotFoundError): + raise self._create_key_value_table() # NOTE : At this point, the persistence isn't ready so we need to use diff --git a/cinderlib/tests/unit/persistence/test_dbms.py b/cinderlib/tests/unit/persistence/test_dbms.py index 1ddfb47..e0b0cce 100644 --- a/cinderlib/tests/unit/persistence/test_dbms.py +++ b/cinderlib/tests/unit/persistence/test_dbms.py @@ -14,6 +14,7 @@ # under the License. import tempfile +from unittest import mock from cinder.db.sqlalchemy import api as sqla_api from cinder import objects as cinder_ovos @@ -121,5 +122,31 @@ class TestDBPersistence(base.BasePersistenceTest): self.assertEqual('__DEFAULT__', self.persistence.DEFAULT_TYPE.name) +class TestDBPersistenceNewerSchema(base.helper.TestHelper): + """Test DBMS plugin can start when the DB has a newer schema.""" + CONNECTION = 'sqlite:///' + tempfile.NamedTemporaryFile().name + PERSISTENCE_CFG = {'storage': 'db', + 'connection': CONNECTION} + + @classmethod + def setUpClass(cls): + pass + + def _raise_exc(self): + inner_exc = dbms.migrate.exceptions.VersionNotFoundError() + exc = dbms.exception.DBMigrationError(inner_exc) + self.original_db_sync() + raise(exc) + + def test_newer_db_schema(self): + self.original_db_sync = dbms.migration.db_sync + with mock.patch.object(dbms.migration, 'db_sync', + side_effect=self._raise_exc) as db_sync_mock: + super(TestDBPersistenceNewerSchema, self).setUpClass() + db_sync_mock.assert_called_once() + self.assertIsInstance(cinderlib.Backend.persistence, + dbms.DBPersistence) + + class TestMemoryDBPersistence(TestDBPersistence): PERSISTENCE_CFG = {'storage': 'memory_db'} diff --git a/releasenotes/notes/mysql-db-sync-8a9e50a12bbe724d.yaml b/releasenotes/notes/mysql-db-sync-8a9e50a12bbe724d.yaml new file mode 100644 index 0000000..b75e8d7 --- /dev/null +++ b/releasenotes/notes/mysql-db-sync-8a9e50a12bbe724d.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Bug #1868145: Support rolling upgrades with the DBMS persistence plugin. + Can run an N release version even if we have already run once an N+1 + release.