openstack/cinder
Code Issues Proposed changes

Merge "Make volume soft delete more thorough"

Browse Source
This commit is contained in:
Zuul 2020-01-14 11:32:29 +00:00 committed by Gerrit Code Review
commit cd36b0ce8b
6 changed files with 73 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
cinder/db/sqlalchemy/api.py
View File

@ -1652,35 +1652,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
cinder/tests/unit/db/test_orm_relationships.py Normal file
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
cinder/tests/unit/test_db_api.py
View File

@ -657,6 +657,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,

6
cinder/volume/manager.py
View File

@ -936,9 +936,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
@ -3563,9 +3560,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
lower-constraints.txt
View File

@ -138,6 +138,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
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