diff options
author | Brian Rosmaita <rosmaita.fossdev@gmail.com> | 2020-08-26 22:00:12 -0400 |
---|---|---|
committer | Brian Rosmaita <rosmaita.fossdev@gmail.com> | 2020-09-01 17:22:11 -0400 |
commit | 00ac80bbadeaba9c42d2453d49410c48269a2e15 (patch) | |
tree | c08286d6a349502f838a6614e9d30aefe12196ce | |
parent | 70fa875cd7f719067015b0441e02cc4137e40940 (diff) | |
download | cinder-00ac80bbadeaba9c42d2453d49410c48269a2e15.tar.gz |
Include deleted volumes for online data migrations
The two online data migrations which moved volumes to the __DEFAULT__
volume type did not include deleted volumes, which meant that the follow
up migration which makes it a nonnullable value does not work because
the deleted volumes will continue to have `null` as their
volume_type_id.
This patch fixes that by including deleted volumes when running the
online database migration. Also adds tests.
(Patch is proposed directly to stable/ussuri because the online migration
code is removed from master by change
I78681ea89762790f544178725999903a41d75ad1)
Co-authored-by: Mohammed Naser <mnaser@vexxhost.com>
Change-Id: I3dc5ab78266dd895829e827066f78c6bebf67d0d
Closes-Bug: #1893107
(cherry picked from commit 2440cb66e31fe54b2624550e2ac636a4b9a674fe)
conflicts:
- replaced the Ussuri release note with one specific to Train
-rw-r--r-- | cinder/db/sqlalchemy/api.py | 8 | ||||
-rw-r--r-- | cinder/tests/unit/test_db_api.py | 109 | ||||
-rw-r--r-- | releasenotes/notes/bug-1893107-train-45bb91952c3170e1.yaml | 78 |
3 files changed, 191 insertions, 4 deletions
diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index 9e251a13d..4f238127d 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -580,11 +580,11 @@ def untyped_volumes_online_data_migration(context, max_count): session = get_session() with session.begin(): total = model_query(context, - models.Volume, + models.Volume, read_deleted='yes', session=session).filter_by( volume_type_id=None).limit(max_count).count() volumes = model_query(context, - models.Volume, + models.Volume, read_deleted='yes', session=session).filter_by( volume_type_id=None).limit(max_count).all() for volume in volumes: @@ -605,11 +605,11 @@ def untyped_snapshots_online_data_migration(context, max_count): session = get_session() with session.begin(): total = model_query(context, - models.Snapshot, + models.Snapshot, read_deleted='yes', session=session).filter_by( volume_type_id=None).limit(max_count).count() snapshots = model_query(context, - models.Snapshot, + models.Snapshot, read_deleted='yes', session=session).filter_by( volume_type_id=None).limit(max_count).all() for snapshot in snapshots: diff --git a/cinder/tests/unit/test_db_api.py b/cinder/tests/unit/test_db_api.py index f20231b01..45a8a3502 100644 --- a/cinder/tests/unit/test_db_api.py +++ b/cinder/tests/unit/test_db_api.py @@ -21,6 +21,7 @@ import enum import mock from mock import call from oslo_config import cfg +from oslo_db import exception as oslo_exc from oslo_utils import timeutils from oslo_utils import uuidutils import six @@ -1689,6 +1690,57 @@ class DBAPIVolumeTestCase(BaseTest): new_cluster_name + vols[i].cluster_name[len(cluster_name):], db_vols[i].cluster_name) + def test_untyped_volumes_online_data_migration(self): + # Bug #1893107: need to make sure we migrate even deleted + # volumes so that the DB doesn't need to be purged before + # making the volume_type_id column nullable in a subsequent + # release. + + # need this or volume_create will fail when creating a deleted volume + self.ctxt.read_deleted = 'yes' + + db_volumes = [ + # non-deleted with volume_type + db.volume_create(self.ctxt, + {'id': fake.VOLUME_ID, + 'volume_type_id': fake.VOLUME_TYPE_ID, + 'deleted': False}), + # deleted with volume_type + db.volume_create(self.ctxt, + {'id': fake.VOLUME2_ID, + 'volume_type_id': fake.VOLUME_TYPE_ID, + 'deleted': True})] + expected_total = 0 + expected_updated = 0 + + # if the volume_type_id column is nullable, add some that will + # have to be migrated + try: + # non-deleted with NO volume_type + v3 = db.volume_create(self.ctxt, + {'id': fake.VOLUME3_ID, + 'volume_type_id': None, + 'deleted': False}) + # deleted with NO volume_type + v4 = db.volume_create(self.ctxt, + {'id': fake.VOLUME4_ID, + 'volume_type_id': None, + 'deleted': True}) + db_volumes.append([v3, v4]) + expected_total = 2 + expected_updated = 2 + + except oslo_exc.DBError: + pass + + # restore context to normal + self.ctxt.read_deleted = 'no' + + total, updated = db.untyped_volumes_online_data_migration( + self.ctxt, max_count=10) + self.assertEqual(expected_total, total) + self.assertEqual(expected_updated, updated) + @ddt.ddt class DBAPISnapshotTestCase(BaseTest): @@ -2065,6 +2117,63 @@ class DBAPISnapshotTestCase(BaseTest): self.assertEqual(should_be, db.snapshot_metadata_get(self.ctxt, 1)) + def test_untyped_snapshots_online_data_migration(self): + # Bug #1893107: need to make sure we migrate even deleted + # snapshots so that the DB doesn't need to be purged before + # making the volume_type_id column nullable in a subsequent + # release. + + # need this or snapshot_create will fail when creating a deleted + # snapshot + self.ctxt.read_deleted = 'yes' + + db_snapshots = [ + # non-deleted with volume_type + db.snapshot_create(self.ctxt, + {'id': fake.SNAPSHOT_ID, + 'volume_id': fake.VOLUME_ID, + 'volume_type_id': fake.VOLUME_TYPE_ID, + 'deleted': False}), + # deleted with volume_type + db.snapshot_create(self.ctxt, + {'id': fake.SNAPSHOT2_ID, + 'volume_id': fake.VOLUME2_ID, + 'volume_type_id': fake.VOLUME_TYPE_ID, + 'deleted': True})] + expected_total = 0 + expected_updated = 0 + + # if the volume_type_id column is nullable, add some that will + # have to be migrated + try: + # non-deleted with NO volume_type + s3 = db.snapshot_create(self.ctxt, + {'id': fake.SNAPSHOT3_ID, + 'volume_id': fake.VOLUME3_ID, + 'volume_type_id': None, + 'deleted': False}) + # deleted with NO volume_type + s4 = db.snapshot_create(self.ctxt, + {'id': fake.UUID4, + 'volume_id': fake.VOLUME4_ID, + 'volume_type_id': None, + 'deleted': True}) + + db_snapshots.append([s3, s4]) + expected_total = 2 + expected_updated = 2 + + except oslo_exc.DBError: + pass + + # restore context to normal + self.ctxt.read_deleted = 'no' + + total, updated = db.untyped_snapshots_online_data_migration( + self.ctxt, max_count=10) + self.assertEqual(expected_total, total) + self.assertEqual(expected_updated, updated) + @ddt.ddt class DBAPIConsistencygroupTestCase(BaseTest): diff --git a/releasenotes/notes/bug-1893107-train-45bb91952c3170e1.yaml b/releasenotes/notes/bug-1893107-train-45bb91952c3170e1.yaml new file mode 100644 index 000000000..536ff6515 --- /dev/null +++ b/releasenotes/notes/bug-1893107-train-45bb91952c3170e1.yaml @@ -0,0 +1,78 @@ +--- +upgrade: + - | + This release modifies the online database migrations to address an + an upgrade issue (`Bug #1893107 + <https://bugs.launchpad.net/cinder/+bug/1893107>`_). The issue does + not manifest itself in the Train release of cinder, but under specific + circumstances it can prevent a cinder database upgrade from Train to + Ussuri. + + This upgrade notice applies to you only if **all** of the following + conditions are met: + + #. You upgraded to Train from Stein + #. Before upgrading from Stein, you did **not** purge the cinder database + #. Your original upgrade from Stein was to cinder version 15.3.0 or + earlier. + + .. note:: + If you are upgrading a Stein installation directly to this release + (cinder 15.3.1) or later, this notice does *not* apply to you. + + If all the above three items apply to you, as part of your upgrade + to cinder 15.3.1 you should re-run the online database migrations + contained in this release. This will prepare your cinder database + for an eventual upgrade to the Ussuri release. + + .. note:: + The online database migrations in this release require the existence + of a volume type named ``__DEFAULT__``. A ``__DEFAULT__`` volume + type was created as part of your original installation of/upgrade to + a Train release of cinder. If you have renamed (or renamed and deleted) + the ``__DEFAULT__`` volume type, you must re-create it before running + the online migrations. (If you renamed it, you don't have to un-rename + it; you can create a new one just for the purposes of the online + database migration.) + + If necessary, you can create a new ``__DEFAULT__`` volume type as + follows using the Block Storage API, or by using the + python-cinderclient or python-openstackclient to do the equivalent: + + API request: ``POST /v3/{project_id}/types`` + + Request body:: + + { + "volume_type": { + "name": "__DEFAULT__", + "description": "Default Volume Type", + "os-volume-type-access:is_public": true + } + } + + The ``__DEFAULT__`` volume type may safely be renamed (or renamed + and deleted) after you have run the online migrations as long as + the ``default_volume_type`` configuration option is set to a valid + existing volume type. + +fixes: + - | + `Bug #1893107 <https://bugs.launchpad.net/cinder/+bug/1893107>`_: + The Ussuri release changes the cinder database schema to make the + ``volume_type_id`` column in the ``volumes`` and ``snapshots`` tables + non-nullable because all volumes have been required to have a volume type + since the Train release. The online database migration in the cinder + Train series (release 15.3.0 or earlier), however, did not process + soft-deleted rows, leaving the possibility that there could be a + deleted volume or snapshot with a null ``volume_type_id``, which in + turn will make the database upgrade fail when the non-nullability + constraint cannot be applied when a Train installation is upgraded + to Ussuri. + + If you are upgrading to this release from an earlier release in the + Train series (that is, you are upgrading from cinder>=15.0.0,<=15.3.0), + under specific circumstances you should re-run the online database + migrations so that your database will be in the correct state when + you eventually upgrade to a Ussuri release. See the "Upgrade Notes" + for more information. |