Browse Source

Merge "Make volume soft delete more thorough" into stable/train

changes/02/704102/1
Zuul Gerrit Code Review 1 month ago
parent
commit
900f769f59
6 changed files with 73 additions and 26 deletions
  1. +18
    -20
      cinder/db/sqlalchemy/api.py
  2. +46
    -0
      cinder/tests/unit/db/test_orm_relationships.py
  3. +7
    -0
      cinder/tests/unit/test_db_api.py
  4. +0
    -6
      cinder/volume/manager.py
  5. +1
    -0
      lower-constraints.txt
  6. +1
    -0
      test-requirements.txt

+ 18
- 20
cinder/db/sqlalchemy/api.py View File

@@ -1658,35 +1658,33 @@ def volume_data_get_for_project(context, project_id,
volume_type_id, host=host)


VOLUME_DEPENDENT_MODELS = frozenset([models.VolumeMetadata,
models.VolumeAdminMetadata,
models.Transfer,
models.VolumeGlanceMetadata,
models.VolumeAttachment])


@require_admin_context
@oslo_db_api.wrap_db_retry(max_retries=5, retry_on_deadlock=True)
def volume_destroy(context, volume_id):
session = get_session()
now = timeutils.utcnow()
updated_values = {'status': 'deleted',
'deleted': True,
'deleted_at': now,
'updated_at': literal_column('updated_at'),
'migration_status': None}
with session.begin():
updated_values = {'status': 'deleted',
'deleted': True,
'deleted_at': now,
'updated_at': literal_column('updated_at'),
'migration_status': None}
model_query(context, models.Volume, session=session).\
filter_by(id=volume_id).\
update(updated_values)
model_query(context, models.VolumeMetadata, session=session).\
filter_by(volume_id=volume_id).\
update({'deleted': True,
'deleted_at': now,
'updated_at': literal_column('updated_at')})
model_query(context, models.VolumeAdminMetadata, session=session).\
filter_by(volume_id=volume_id).\
update({'deleted': True,
'deleted_at': now,
'updated_at': literal_column('updated_at')})
model_query(context, models.Transfer, session=session).\
filter_by(volume_id=volume_id).\
update({'deleted': True,
'deleted_at': now,
'updated_at': literal_column('updated_at')})
for model in VOLUME_DEPENDENT_MODELS:
model_query(context, model, session=session).\
filter_by(volume_id=volume_id).\
update({'deleted': True,
'deleted_at': now,
'updated_at': literal_column('updated_at')})
del updated_values['updated_at']
return updated_values



+ 46
- 0
cinder/tests/unit/db/test_orm_relationships.py View File

@@ -0,0 +1,46 @@
# Copyright 2020 Red Hat, Inc.
#
# 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.

"""Tests for code that makes assumptions about ORM relationships."""

from sqlalchemy_utils import functions as saf

from cinder.db.sqlalchemy import api as db_api
from cinder.db.sqlalchemy import models
from cinder import test


class VolumeRelationshipsTestCase(test.TestCase):
"""Test cases for Volume ORM model relationshps."""

def test_volume_dependent_models_list(self):
"""Make sure the volume dependent tables list is accurate."""
# Addresses LP Bug #1542169

volume_declarative_base = saf.get_declarative_base(models.Volume)
volume_fks = saf.get_referencing_foreign_keys(models.Volume)

dependent_tables = []
for table, fks in saf.group_foreign_keys(volume_fks):
dependent_tables.append(table)

found_dependent_models = []
for table in dependent_tables:
found_dependent_models.append(saf.get_class_by_table(
volume_declarative_base, table))

self.assertEqual(len(found_dependent_models),
len(db_api.VOLUME_DEPENDENT_MODELS))
for model in found_dependent_models:
self.assertIn(model, db_api.VOLUME_DEPENDENT_MODELS)

+ 7
- 0
cinder/tests/unit/test_db_api.py View File

@@ -658,6 +658,13 @@ class DBAPIVolumeTestCase(BaseTest):
self.assertRaises(exception.VolumeNotFound, db.volume_get,
self.ctxt, volume['id'])

@mock.patch('cinder.db.sqlalchemy.api.model_query')
def test_volume_destroy_deletes_dependent_data(self, mock_model_query):
"""Addresses LP Bug #1542169."""
db.volume_destroy(self.ctxt, fake.VOLUME_ID)
expected_call_count = 1 + len(sqlalchemy_api.VOLUME_DEPENDENT_MODELS)
self.assertEqual(expected_call_count, mock_model_query.call_count)

def test_volume_get_all(self):
volumes = [db.volume_create(self.ctxt,
{'host': 'h%d' % i, 'size': i,


+ 0
- 6
cinder/volume/manager.py View File

@@ -937,9 +937,6 @@ class VolumeManager(manager.CleanableManager,
LOG.exception("Failed to update usages deleting volume.",
resource=volume)

# Delete glance metadata if it exists
self.db.volume_glance_metadata_delete_by_volume(context, volume.id)

volume.destroy()

# If deleting source/destination volume in a migration or a temp
@@ -3564,9 +3561,6 @@ class VolumeManager(manager.CleanableManager,
resource={'type': 'group',
'id': group.id})

# Delete glance metadata if it exists
self.db.volume_glance_metadata_delete_by_volume(context, vol.id)

vol.destroy()

# Commit the reservations


+ 1
- 0
lower-constraints.txt View File

@@ -140,6 +140,7 @@ sphinx-feature-classification==0.1.0
sphinxcontrib-websupport==1.0.1
sqlalchemy-migrate==0.11.0
SQLAlchemy==1.0.10
SQLAlchemy-Utils==0.36.1
sqlparse==0.2.4
statsd==3.2.2
stestr==2.2.0


+ 1
- 0
test-requirements.txt View File

@@ -14,6 +14,7 @@ oslotest>=3.2.0 # Apache-2.0
pycodestyle==2.5.0 # MIT License
PyMySQL>=0.7.6 # MIT License
psycopg2>=2.7 # LGPL/ZPL
SQLAlchemy-Utils>=0.36.1 # BSD License
testtools>=2.2.0 # MIT
testresources>=2.0.0 # Apache-2.0/BSD
testscenarios>=0.4 # Apache-2.0/BSD


Loading…
Cancel
Save