openstack/trove
Code Issues Proposed changes

Added checks for deleting datastore version

Browse Source 
* Hard delete the datastore_configuration_parameters table record.
* Make 'datastore_version_id' nullable for 'instances' table.
* Check if the datastore version is still being used before removal.

Story: 2007563
Task: 39451
Change-Id: I84e4a31f14f9327cc01ff2d699167d91112e1565
This commit is contained in:
Lingxian Kong 2020-04-17 15:14:54 +12:00
parent 8c3df10aa5
commit a4057b10af
17 changed files with 269 additions and 92 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
releasenotes/notes/ussuri-fix-delete-datastore-version.yaml Normal file
View File

@ -0,0 +1,7 @@
---
fixes:
- Fixed the issue that datastore version cannot be deleted because of
dependency of deleted instances. Now, when instance or backup is
deleted, the datastore version attribute is set to NULL in database.
When datastore configuration parameter is deleted, the record is
deleted from database rather than only set 'deleted' field to 1.

24
tools/trove-pylint.config
View File

@ -771,6 +771,30 @@
"Instance of 'Table' has no 'create_column' member",
"upgrade"
],
[
"trove/db/sqlalchemy/migrate_repo/versions/044_remove_datastore_configuration_parameters_deleted.py",
"E1101",
"Instance of 'Table' has no 'drop_column' member",
"upgrade"
],
[
"trove/db/sqlalchemy/migrate_repo/versions/044_remove_datastore_configuration_parameters_deleted.py",
"no-member",
"Instance of 'Table' has no 'drop_column' member",
"upgrade"
],
[
"trove/db/sqlalchemy/migrate_repo/versions/044_remove_datastore_configuration_parameters_deleted.py",
"E1120",
"No value for argument 'dml' in method call",
"upgrade"
],
[
"trove/db/sqlalchemy/migrate_repo/versions/044_remove_datastore_configuration_parameters_deleted.py",
"no-value-for-parameter",
"No value for argument 'dml' in method call",
"upgrade"
],
[
"trove/db/sqlalchemy/migration.py",
"E0611",

5
trove/common/exception.py
View File

@ -188,6 +188,11 @@ class DatastoreVersionsExist(BadRequest):
message = _("Datastore versions exist for datastore %(datastore)s.")
class DatastoreVersionsInUse(BadRequest):
message = _("Datastore version is in use by %(resource)s.")
class DatastoreDefaultDatastoreNotFound(TroveError):
message = _("Please specify datastore. Default datastore "

77
trove/configuration/models.py
View File

@ -189,7 +189,7 @@ class Configuration(object):
id=self.configuration_id)
LOG.debug("config_items: %s", config_items)
detail_list = DatastoreConfigurationParameters.load_parameters(
datastore_v.id, show_deleted=True)
datastore_v.id)
for i in config_items:
LOG.debug("config item: %s", i)
@ -265,8 +265,6 @@ class DBDatastoreConfigurationParameters(dbmodels.DatabaseModelBase):
'max_size',
'min_size',
'data_type',
'deleted',
'deleted_at',
]
_table_name = "datastore_configuration_parameters"
@ -287,22 +285,12 @@ class DatastoreConfigurationParameters(object):
ds_v_id = kwargs.get('datastore_version_id')
config_param_name = kwargs.get('name')
try:
param = DatastoreConfigurationParameters.load_parameter_by_name(
DatastoreConfigurationParameters.load_parameter_by_name(
ds_v_id,
config_param_name,
show_deleted=True)
if param.deleted == 1:
param.restart_required = kwargs.get('restart_required')
param.data_type = kwargs.get('data_type')
param.max_size = kwargs.get('max_size')
param.min_size = kwargs.get('min_size')
param.deleted = 0
param.save()
return param
else:
raise exception.ConfigurationParameterAlreadyExists(
parameter_name=config_param_name,
datastore_version=ds_v_id)
config_param_name)
raise exception.ConfigurationParameterAlreadyExists(
parameter_name=config_param_name,
datastore_version=ds_v_id)
except exception.NotFound:
pass
config_param = DBDatastoreConfigurationParameters.create(
@ -313,54 +301,29 @@ class DatastoreConfigurationParameters(object):
def delete(version_id, config_param_name):
config_param = DatastoreConfigurationParameters.load_parameter_by_name(
version_id, config_param_name)
config_param.deleted = True
config_param.deleted_at = timeutils.utcnow()
config_param.save()
config_param.delete()
@classmethod
def load_parameters(cls, datastore_version_id, show_deleted=False):
try:
if show_deleted:
return DBDatastoreConfigurationParameters.find_all(
datastore_version_id=datastore_version_id
)
else:
return DBDatastoreConfigurationParameters.find_all(
datastore_version_id=datastore_version_id,
deleted=False
)
except exception.NotFound:
raise exception.NotFound(uuid=datastore_version_id)
def load_parameters(cls, datastore_version_id):
return DBDatastoreConfigurationParameters.find_all(
datastore_version_id=datastore_version_id)
@classmethod
def load_parameter(cls, config_id, show_deleted=False):
def load_parameter(cls, config_id):
try:
if show_deleted:
return DBDatastoreConfigurationParameters.find_by(
id=config_id
)
else:
return DBDatastoreConfigurationParameters.find_by(
id=config_id, deleted=False
)
return DBDatastoreConfigurationParameters.find_by(
id=config_id, deleted=False
)
except exception.NotFound:
raise exception.NotFound(uuid=config_id)
@classmethod
def load_parameter_by_name(cls, datastore_version_id, config_param_name,
show_deleted=False):
def load_parameter_by_name(cls, datastore_version_id, config_param_name):
try:
if show_deleted:
return DBDatastoreConfigurationParameters.find_by(
datastore_version_id=datastore_version_id,
name=config_param_name
)
else:
return DBDatastoreConfigurationParameters.find_by(
datastore_version_id=datastore_version_id,
name=config_param_name,
deleted=False
)
return DBDatastoreConfigurationParameters.find_by(
datastore_version_id=datastore_version_id,
name=config_param_name
)
except exception.NotFound:
raise exception.NotFound(uuid=config_param_name)
@ -376,7 +339,7 @@ def create_or_update_datastore_configuration_parameter(name,
datastore_version_id)
try:
config = DatastoreConfigurationParameters.load_parameter_by_name(
datastore_version_id, name, show_deleted=True)
datastore_version_id, name)
config.restart_required = restart_required
config.max_size = max_size
config.min_size = min_size

50
trove/db/sqlalchemy/migrate_repo/versions/043_instance_ds_version_nullable.py Normal file
View File

@ -0,0 +1,50 @@
# Copyright 2012 OpenStack Foundation
#
# 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.
from sqlalchemy.schema import MetaData
from sqlalchemy import text
from trove.db.sqlalchemy.migrate_repo.schema import Table
from trove.db.sqlalchemy import utils as db_utils
meta = MetaData()
def upgrade(migrate_engine):
meta.bind = migrate_engine
instance_table = Table('instances', meta, autoload=True)
datastore_versions_table = Table('datastore_versions',
meta,
autoload=True)
constraint_names = db_utils.get_foreign_key_constraint_names(
engine=migrate_engine,
table='instances',
columns=[text('datastore_version_id')],
ref_table='datastore_versions',
ref_columns=[text('id')])
db_utils.drop_foreign_key_constraints(
constraint_names=constraint_names,
columns=[instance_table.c.datastore_version_id],
ref_columns=[datastore_versions_table.c.id])
# Make datastore_version_id nullable so that this field could be set to
# NULL when instance is deleted.
instance_table.c.datastore_version_id.alter(nullable=True)
db_utils.create_foreign_key_constraints(
constraint_names=constraint_names,
columns=[instance_table.c.datastore_version_id],
ref_columns=[datastore_versions_table.c.id])

37
trove/db/sqlalchemy/migrate_repo/versions/044_remove_datastore_configuration_parameters_deleted.py Normal file
View File

@ -0,0 +1,37 @@
# Copyright 2012 OpenStack Foundation
#
# 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.
from sqlalchemy.schema import MetaData
from trove.db.sqlalchemy.migrate_repo.schema import Table
meta = MetaData()
def upgrade(migrate_engine):
meta.bind = migrate_engine
ds_config_param = Table('datastore_configuration_parameters', meta,
autoload=True)
# Remove records with deleted=1
if 'deleted' in ds_config_param.c:
ds_config_param.delete(). \
where(ds_config_param.c.deleted == 1). \
execute()
# Delete columns deleted and deleted_at
if migrate_engine.name != "sqlite":
ds_config_param.drop_column('deleted')
ds_config_param.drop_column('deleted_at')

6
trove/extensions/mgmt/configuration/service.py
View File

@ -38,8 +38,7 @@ class ConfigurationsParameterController(wsgi.Controller):
"""List all configuration parameters."""
ds_version = ds_models.DatastoreVersion.load_by_uuid(version_id)
config_params = config_models.DatastoreConfigurationParameters
rules = config_params.load_parameters(
ds_version.id, show_deleted=True)
rules = config_params.load_parameters(ds_version.id)
return wsgi.Result(views.MgmtConfigurationParametersView(rules).data(),
200)
@ -48,8 +47,7 @@ class ConfigurationsParameterController(wsgi.Controller):
"""Show a configuration parameter."""
ds_models.DatastoreVersion.load_by_uuid(version_id)
config_params = config_models.DatastoreConfigurationParameters
rule = config_params.load_parameter_by_name(
version_id, id, show_deleted=True)
rule = config_params.load_parameter_by_name(version_id, id)
return wsgi.Result(views.MgmtConfigurationParameterView(rule).data(),
200)

2
trove/extensions/mgmt/configuration/views.py
View File

@ -31,8 +31,6 @@ class MgmtConfigurationParameterView(object):
"datastore_version_id": self.config.datastore_version_id,
"restart_required": restart_required,
"type": self.config.data_type,
"deleted": self.config.deleted,
"deleted_at": self.config.deleted_at,
}
if self.config.max_size:
ret["max_size"] = int(self.config.max_size)

18
trove/extensions/mgmt/datastores/service.py
View File

@ -16,14 +16,17 @@
from glanceclient import exc as glance_exceptions
from oslo_log import log as logging
from trove.backup import models as backup_model
from trove.common import apischema
from trove.common.auth import admin_context
from trove.common import clients
from trove.common import exception
from trove.common import utils
from trove.common import wsgi
from trove.configuration import models as config_model
from trove.datastore import models
from trove.extensions.mgmt.datastores import views
from trove.instance import models as instance_model
LOG = logging.getLogger(__name__)
@ -140,6 +143,21 @@ class DatastoreVersionController(wsgi.Controller):
@admin_context
def delete(self, req, tenant_id, id):
"""Remove an existing datastore version."""
instances = instance_model.DBInstance.find_all(
datastore_version_id=id, deleted=0).all()
if len(instances) > 0:
raise exception.DatastoreVersionsInUse(resource='instance')
backups = backup_model.DBBackup.find_all(
datastore_version_id=id, deleted=0).all()
if len(backups) > 0:
raise exception.DatastoreVersionsInUse(resource='backup')
configs = config_model.DBConfiguration.find_all(
datastore_version_id=id, deleted=0).all()
if len(configs) > 0:
raise exception.DatastoreVersionsInUse(resource='configuration')
datastore_version = models.DatastoreVersion.load_by_uuid(id)
datastore = models.Datastore.load(datastore_version.datastore_id)

1
trove/extensions/mgmt/instances/models.py
View File

@ -42,6 +42,7 @@ def load_mgmt_instances(context, deleted=None, client=None,
args['deleted'] = deleted
if not include_clustered:
args['cluster_id'] = None
db_infos = instance_models.DBInstance.find_all(**args)
instances = MgmtInstances.load_status_from_existing(context, db_infos,

27
trove/instance/models.py
View File

@ -179,15 +179,17 @@ class SimpleInstance(object):
self.root_pass = root_password
self._fault = None
self._fault_loaded = False
if ds_version is None:
self.ds_version = None
self.ds = None
self.locality = locality
self.slave_list = None
if ds_version is None and self.db_info.datastore_version_id:
self.ds_version = (datastore_models.DatastoreVersion.
load_by_uuid(self.db_info.datastore_version_id))
if ds is None:
if ds is None and self.ds_version:
self.ds = (datastore_models.Datastore.
load(self.ds_version.datastore_id))
self.locality = locality
self.slave_list = None
def __repr__(self, *args, **kwargs):
return "%s(%s)" % (self.name, self.id)
@ -410,11 +412,15 @@ class SimpleInstance(object):
@property
def volume_support(self):
return CONF.get(self.datastore_version.manager).volume_support
if self.datastore_version:
return CONF.get(self.datastore_version.manager).volume_support
return None
@property
def device_path(self):
return CONF.get(self.datastore_version.manager).device_path
if self.datastore_version:
return CONF.get(self.datastore_version.manager).device_path
return None
@property
def root_password(self):
@ -809,8 +815,13 @@ class BaseInstance(SimpleInstance):
deleted_at = timeutils.utcnow()
self._delete_resources(deleted_at)
LOG.debug("Setting instance %s to be deleted.", self.id)
# Also set FOREIGN KEY fields to NULL
self.update_db(deleted=True, deleted_at=deleted_at,
task_status=InstanceTasks.NONE)
task_status=InstanceTasks.NONE,
datastore_version_id=None,
configuration_id=None,
slave_of_id=None,
cluster_id=None)
self.set_servicestatus_deleted()
self.set_instance_fault_deleted()

14
trove/instance/views.py
View File

@ -37,10 +37,14 @@ class InstanceView(object):
"status": self.instance.status,
"links": self._build_links(),
"flavor": self._build_flavor_info(),
"datastore": {"type": self.instance.datastore.name,
"version": self.instance.datastore_version.name},
"datastore": {"type": None, "version": None},
"region": self.instance.region_name
}
if self.instance.datastore_version:
instance_dict['datastore'] = {
"type": self.instance.datastore.name,
"version": self.instance.datastore_version.name
}
if self.context.is_admin:
instance_dict['tenant_id'] = self.instance.tenant_id
if self.instance.volume_support:
@ -94,8 +98,10 @@ class InstanceDetailView(InstanceView):
result['instance']['service_status_updated'] = (self.instance.
service_status_updated)
result['instance']['datastore']['version'] = (self.instance.
datastore_version.name)
result['instance']['datastore']['version'] = None
if self.instance.datastore_version:
result['instance']['datastore']['version'] = \
self.instance.datastore_version.name
if self.instance.fault:
result['instance']['fault'] = self._build_fault_info()

14
trove/taskmanager/models.py
View File

@ -1427,6 +1427,14 @@ class BackupTasks(object):
@classmethod
def delete_backup(cls, context, backup_id):
"""Delete backup from swift."""
def _delete(backup):
backup.deleted = True
backup.deleted_at = timeutils.utcnow()
# Set datastore_version_id to None so that datastore_version could
# be deleted.
backup.datastore_version_id = None
backup.save()
LOG.info("Deleting backup %s.", backup_id)
backup = bkup_models.Backup.get_by_id(context, backup_id)
try:
@ -1434,11 +1442,11 @@ class BackupTasks(object):
if filename:
BackupTasks.delete_files_from_swift(context, filename)
except ValueError:
backup.delete()
_delete(backup)
except ClientException as e:
if e.http_status == 404:
# Backup already deleted in swift
backup.delete()
_delete(backup)
else:
LOG.exception("Error occurred when deleting from swift. "
"Details: %s", e)
@ -1447,7 +1455,7 @@ class BackupTasks(object):
raise TroveError(_("Failed to delete swift object for backup "
"%s.") % backup_id)
else:
backup.delete()
_delete(backup)
LOG.info("Deleted backup %s successfully.", backup_id)

25
trove/tests/api/configurations.py
View File

@ -633,18 +633,19 @@ class DeleteConfigurations(ConfigurationsTestBase):
@after_class(always_run=True)
def tearDown(self):
# need to "undelete" the parameter that was deleted from the mgmt call
ds = instance_info.dbaas_datastore
ds_v = instance_info.dbaas_datastore_version
version = instance_info.dbaas.datastore_versions.get(
ds, ds_v)
client = instance_info.dbaas_admin.mgmt_configs
print(self.config_parameter_dict)
client.create(version.id,
self.config_parameter_dict['name'],
self.config_parameter_dict['restart_required'],
self.config_parameter_dict['type'],
self.config_parameter_dict['max'],
self.config_parameter_dict['min'])
if instance_info.dbaas:
ds = instance_info.dbaas_datastore
ds_v = instance_info.dbaas_datastore_version
version = instance_info.dbaas.datastore_versions.get(
ds, ds_v)
client = instance_info.dbaas_admin.mgmt_configs
print(self.config_parameter_dict)
client.create(version.id,
self.config_parameter_dict['name'],
self.config_parameter_dict['restart_required'],
self.config_parameter_dict['type'],
self.config_parameter_dict['max'],
self.config_parameter_dict['min'])
@test
def test_delete_invalid_configuration_not_found(self):

8
trove/tests/api/datastores.py
View File

@ -85,6 +85,14 @@ class Datastores(object):
@test
def test_create_inactive_datastore_by_admin(self):
datastores = self.rd_client.datastores.list()
for ds in datastores:
if ds.name == test_config.dbaas_datastore_name_no_versions:
for version in ds.versions:
if version['name'] == 'inactive_version':
return
# Get a valid image ID from a datastore version
datastore = self.rd_client.datastores.get(test_config.dbaas_datastore)
ds_version = self.rd_client.datastore_versions.list(datastore.id)[0]
ds_version_info = self.rd_admin.datastore_versions.get_by_uuid(

43
trove/tests/unittests/mgmt/test_datastore_controller.py
View File

@ -105,15 +105,56 @@ class TestDatastoreVersionController(trove_testtools.TestCase):
self.controller.show(self.req, self.tenant_id, id)
mock_ds_version_load.assert_called_with(id)
@patch('trove.configuration.models.DBConfiguration.find_all')
@patch('trove.backup.models.DBBackup.find_all')
@patch('trove.instance.models.DBInstance.find_all')
@patch.object(datastore_models.Datastore, 'load')
@patch.object(datastore_models.DatastoreVersion, 'load_by_uuid')
def test_delete_ds_version(self, mock_ds_version_load, mock_ds_load):
def test_delete_ds_version(self, mock_ds_version_load, mock_ds_load,
mock_instance_find, mock_backup_find,
mock_config_find):
ds_version_id = Mock()
ds_version = Mock()
mock_ds_version_load.return_value = ds_version
self.controller.delete(self.req, self.tenant_id, ds_version_id)
ds_version.delete.assert_called_with()
@patch('trove.instance.models.DBInstance.find_all')
def test_delete_ds_version_instance_in_use(self, mock_instance_find):
mock_instance_find.return_value.all.return_value = [Mock()]
self.assertRaises(
exception.DatastoreVersionsInUse,
self.controller.delete,
self.req, self.tenant_id, 'fake_version_id'
)
@patch('trove.backup.models.DBBackup.find_all')
@patch('trove.instance.models.DBInstance.find_all')
def test_delete_ds_version_backup_in_use(self, mock_instance_find,
mock_backup_find):
mock_backup_find.return_value.all.return_value = [Mock()]
self.assertRaises(
exception.DatastoreVersionsInUse,
self.controller.delete,
self.req, self.tenant_id, 'fake_version_id'
)
@patch('trove.configuration.models.DBConfiguration.find_all')
@patch('trove.backup.models.DBBackup.find_all')
@patch('trove.instance.models.DBInstance.find_all')
def test_delete_ds_version_config_in_use(self, mock_instance_find,
mock_backup_find,
mock_config_find):
mock_config_find.return_value.all.return_value = [Mock()]
self.assertRaises(
exception.DatastoreVersionsInUse,
self.controller.delete,
self.req, self.tenant_id, 'fake_version_id'
)
@patch.object(datastore_models.DatastoreVersion, 'load_by_uuid')
@patch.object(datastore_models.DatastoreVersions, 'load_all')
def test_index_ds_version(self, mock_ds_version_load_all,

3
trove/tests/unittests/taskmanager/test_models.py
View File

@ -1056,7 +1056,8 @@ class BackupTasksTest(trove_testtools.TestCase):
self.backup.location = ''
taskmanager_models.BackupTasks.delete_backup('dummy context',
self.backup.id)
self.backup.delete.assert_any_call()
self.assertTrue(self.backup.deleted)
@patch('trove.taskmanager.models.LOG')
@patch('trove.common.clients.create_swift_client')