openstack
/
cinder
Code Issues Proposed changes

Drop legacy backup service support 

Backup drivers were migrated from backup services around Havana with
Ic3fca567111f4bd1b221689c73cd5c3bab4a777b. This removes support for
still loading the old path names and cleans up a mapping dict that
wasn't even being used anymore.

Support for loading by class name and not module name was deprecated in
queen with Id6bee9e7d0da8ead224a04f86fe79ddfb5b286cf. This removes that
capability and requires that the backup driver class name is provided.

Change-Id: I3ada2dee1857074746b1893b82dd5f6641c6e579
Signed-off-by: Sean McGinnis <sean.mcginnis@gmail.com>
This commit is contained in:
Sean McGinnis 2018-08-22 17:02:52 -05:00
parent f68a0e80ed
commit 86f62022ee
No known key found for this signature in database
GPG Key ID: CE7EE4BFAF8D70C8
3 changed files with 43 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

43
cinder/backup/manager.py
View File

@ -37,7 +37,6 @@ from castellan import key_manager
from eventlet import tpool
from oslo_config import cfg
from oslo_log import log as logging
from oslo_log import versionutils
import oslo_messaging as messaging
from oslo_service import loopingcall
from oslo_service import periodic_task
@ -81,11 +80,6 @@ backup_manager_opts = [
'decreased for specific drivers that don\'t.'),
]
# This map doesn't need to be extended in the future since it's only
# for old backup services
mapper = {'cinder.backup.services.swift': 'cinder.backup.drivers.swift',
'cinder.backup.services.ceph': 'cinder.backup.drivers.ceph'}
CONF = cfg.CONF
CONF.register_opts(backup_manager_opts)
CONF.import_opt('use_multipath_for_image_xfer', 'cinder.volume.driver')
@ -94,7 +88,8 @@ QUOTAS = quota.QUOTAS
MAPPING = {
# Module name "google" conflicts with google library namespace inside the
# driver when it imports google.auth
'cinder.backup.drivers.google': 'cinder.backup.drivers.gcs',
'cinder.backup.drivers.google.GoogleBackupDriver':
'cinder.backup.drivers.gcs.GoogleBackupDriver',
}
SERVICE_PGRP = '' if os.name == 'nt' else os.getpgrp()
@ -127,23 +122,7 @@ class BackupManager(manager.ThreadPoolManager):
'configuration to the new path %s',
self.driver_name, new_name)
self.driver_name = new_name
def get_backup_driver(self, context):
driver = None
try:
# TODO(e0ne): remove backward compatibility in S release
service = importutils.import_module(self.driver_name)
msg = ("Backup driver initialization using module name "
"is deprecated and will be removed in a 'S' "
"release. Please, use classname for backup driver "
"reference in the config.")
versionutils.report_deprecated_feature(LOG, msg)
driver = service.get_backup_driver(context)
except ImportError:
driver_class = importutils.import_class(self.driver_name)
driver = driver_class(context=context, db=self.db)
return driver
self.service = importutils.import_class(self.driver_name)
def _update_backup_error(self, backup, err,
status=fields.BackupStatus.ERROR):
@ -169,7 +148,7 @@ class BackupManager(manager.ThreadPoolManager):
backups=backups)
def _setup_backup_driver(self, ctxt):
backup_service = self.get_backup_driver(ctxt)
backup_service = self.service(context=ctxt, db=self.db)
backup_service.check_for_setup_error()
self.is_initialized = True
raise loopingcall.LoopingCallDone()
@ -468,7 +447,7 @@ class BackupManager(manager.ThreadPoolManager):
volume.encryption_key_id)
backup.save()
backup_service = self.get_backup_driver(context)
backup_service = self.service(context)
properties = utils.brick_get_connector_properties()
@ -636,7 +615,7 @@ class BackupManager(manager.ThreadPoolManager):
def _run_restore(self, context, backup, volume):
orig_key_id = volume.encryption_key_id
backup_service = self.get_backup_driver(context)
backup_service = self.service(context)
properties = utils.brick_get_connector_properties()
secure_enabled = (
@ -754,7 +733,7 @@ class BackupManager(manager.ThreadPoolManager):
if backup.service:
try:
backup_service = self.get_backup_driver(context)
backup_service = self.service(context)
backup_service.delete_backup(backup)
except Exception as err:
with excutils.save_and_reraise_exception():
@ -843,7 +822,7 @@ class BackupManager(manager.ThreadPoolManager):
# Call driver to create backup description string
try:
backup_service = self.get_backup_driver(context)
backup_service = self.service(context)
driver_info = backup_service.export_record(backup)
backup_url = backup.encode_record(driver_info=driver_info)
backup_record['backup_url'] = backup_url
@ -899,7 +878,7 @@ class BackupManager(manager.ThreadPoolManager):
# Extract driver specific info and pass it to the driver
driver_options = backup_options.pop('driver_info', {})
backup_service = self.get_backup_driver(context)
backup_service = self.service(context)
backup_service.import_record(backup, driver_options)
except Exception as err:
msg = six.text_type(err)
@ -1002,7 +981,7 @@ class BackupManager(manager.ThreadPoolManager):
if (status == fields.BackupStatus.AVAILABLE and
backup['status'] != fields.BackupStatus.RESTORING):
# check whether we could verify the backup is ok or not
backup_service = self.get_backup_driver(context)
backup_service = self.service(context)
if isinstance(backup_service,
driver.BackupDriverWithVerify):
backup_service.verify(backup.id)
@ -1067,7 +1046,7 @@ class BackupManager(manager.ThreadPoolManager):
:param context: running context
"""
backup_service = self.get_backup_driver(context)
backup_service = self.service(context)
return backup_service.support_force_delete
def _attach_device(self, ctxt, backup_device,

48
cinder/tests/unit/backup/test_backup.py
View File

@ -728,9 +728,9 @@ class BackupTestCase(BaseBackupTest):
backup = self._create_backup_db_entry(volume_id=vol_id,
parent_id='mock')
with mock.patch.object(self.backup_mgr, 'get_backup_driver') as \
mock_get_backup_driver:
mock_get_backup_driver.return_value.backup.return_value = (
with mock.patch.object(self.backup_mgr, 'service') as \
mock_service:
mock_service.return_value.backup.return_value = (
{'parent_id': None})
with mock.patch.object(self.backup_mgr, '_detach_device'):
device_path = '/fake/disk/path/'
@ -763,9 +763,9 @@ class BackupTestCase(BaseBackupTest):
backup = self._create_backup_db_entry(volume_id=vol_id)
parent_backup = self._create_backup_db_entry(size=vol_size)
with mock.patch.object(self.backup_mgr, 'get_backup_driver') as \
mock_get_backup_driver:
mock_get_backup_driver.return_value.backup.return_value = (
with mock.patch.object(self.backup_mgr, 'service') as \
mock_service:
mock_service.return_value.backup.return_value = (
{'parent_id': parent_backup.id})
with mock.patch.object(self.backup_mgr, '_detach_device'):
device_path = '/fake/disk/path/'
@ -796,9 +796,9 @@ class BackupTestCase(BaseBackupTest):
vol_id = self._create_volume_db_entry()
backup = self._create_backup_db_entry(volume_id=vol_id)
with mock.patch.object(self.backup_mgr, 'get_backup_driver') as \
mock_get_backup_driver:
mock_get_backup_driver.return_value.backup.side_effect = (
with mock.patch.object(self.backup_mgr, 'service') as \
mock_service:
mock_service.return_value.backup.side_effect = (
FakeBackupException('fake'))
with mock.patch.object(self.backup_mgr, '_detach_device'):
device_path = '/fake/disk/path/'
@ -835,7 +835,7 @@ class BackupTestCase(BaseBackupTest):
backup_service = lambda: None
backup_service.backup = mock.Mock(
return_value=mock.sentinel.backup_update)
self.backup_mgr.get_backup_driver = lambda x: backup_service
self.backup_mgr.service = lambda x: backup_service
vol_id = self._create_volume_db_entry()
backup = self._create_backup_db_entry(volume_id=vol_id)
@ -1471,14 +1471,12 @@ class BackupTestCase(BaseBackupTest):
backup.save()
self.backup_mgr.delete_backup(self.ctxt, backup)
@ddt.data('cinder.tests.unit.backup.fake_service.FakeBackupService',
'cinder.tests.unit.backup.fake_service')
def test_delete_backup(self, service):
def test_delete_backup(self):
"""Test normal backup deletion."""
vol_id = self._create_volume_db_entry(size=1)
backup = self._create_backup_db_entry(
status=fields.BackupStatus.DELETING, volume_id=vol_id,
service=service)
service='cinder.tests.unit.backup.fake_service.FakeBackupService')
self.backup_mgr.delete_backup(self.ctxt, backup)
self.assertRaises(exception.BackupNotFound,
db.backup_get,
@ -1605,10 +1603,9 @@ class BackupTestCase(BaseBackupTest):
self.ctxt,
backup)
@ddt.data('cinder.tests.unit.backup.fake_service.FakeBackupService',
'cinder.tests.unit.backup.fake_service')
def test_export_record(self, service):
def test_export_record(self):
"""Test normal backup record export."""
service = 'cinder.tests.unit.backup.fake_service.FakeBackupService'
vol_size = 1
vol_id = self._create_volume_db_entry(status='available',
size=vol_size)
@ -1708,7 +1705,7 @@ class BackupTestCase(BaseBackupTest):
record where the backup driver returns an exception.
"""
export = self._create_exported_record_entry()
backup_driver = self.backup_mgr.get_backup_driver(self.ctxt)
backup_driver = self.backup_mgr.service(self.ctxt)
_mock_record_import_class = ('%s.%s.%s' %
(backup_driver.__module__,
backup_driver.__class__.__name__,
@ -1730,7 +1727,8 @@ class BackupTestCase(BaseBackupTest):
def test_not_supported_driver_to_force_delete(self):
"""Test force delete check method for not supported drivers."""
self.override_config('backup_driver', 'cinder.backup.drivers.ceph')
self.override_config('backup_driver',
'cinder.backup.drivers.ceph.CephBackupDriver')
self.backup_mgr = importutils.import_object(CONF.backup_manager)
result = self.backup_mgr.check_support_to_force_delete(self.ctxt)
self.assertFalse(result)
@ -1742,7 +1740,8 @@ class BackupTestCase(BaseBackupTest):
def test_check_support_to_force_delete(self, mock_check_configuration,
mock_init_backup_repo_path):
"""Test force delete check method for supported drivers."""
self.override_config('backup_driver', 'cinder.backup.drivers.nfs')
self.override_config('backup_driver',
'cinder.backup.drivers.nfs.NFSBackupDriver')
self.backup_mgr = importutils.import_object(CONF.backup_manager)
result = self.backup_mgr.check_support_to_force_delete(self.ctxt)
self.assertTrue(result)
@ -1785,7 +1784,8 @@ class BackupTestCaseWithVerify(BaseBackupTest):
def setUp(self):
self.override_config(
"backup_driver",
"cinder.tests.unit.backup.fake_service_with_verify")
"cinder.tests.unit.backup.fake_service_with_verify."
"FakeBackupServiceWithVerify")
super(BackupTestCaseWithVerify, self).setUp()
def test_import_record_with_verify(self):
@ -1801,7 +1801,7 @@ class BackupTestCaseWithVerify(BaseBackupTest):
imported_record = self._create_export_record_db_entry(
backup_id=backup_id)
backup_hosts = []
backup_driver = self.backup_mgr.get_backup_driver(self.ctxt)
backup_driver = self.backup_mgr.service(self.ctxt)
_mock_backup_verify_class = ('%s.%s.%s' %
(backup_driver.__module__,
backup_driver.__class__.__name__,
@ -1835,7 +1835,7 @@ class BackupTestCaseWithVerify(BaseBackupTest):
imported_record = self._create_export_record_db_entry(
backup_id=backup_id)
backup_hosts = []
backup_driver = self.backup_mgr.get_backup_driver(self.ctxt)
backup_driver = self.backup_mgr.service(self.ctxt)
_mock_backup_verify_class = ('%s.%s.%s' %
(backup_driver.__module__,
backup_driver.__class__.__name__,
@ -1896,7 +1896,7 @@ class BackupTestCaseWithVerify(BaseBackupTest):
backup = self._create_backup_db_entry(status=fields.BackupStatus.ERROR,
volume_id=volume['id'])
backup_driver = self.backup_mgr.get_backup_driver(self.ctxt)
backup_driver = self.backup_mgr.service(self.ctxt)
_mock_backup_verify_class = ('%s.%s.%s' %
(backup_driver.__module__,
backup_driver.__class__.__name__,

8
releasenotes/notes/backup-path-removal-c411bb6c0d3887f1.yaml Normal file
View File

@ -0,0 +1,8 @@
---
upgrade:
- |
The ability to specify a backup driver by module name was deprecated in
the Queens release and the ability has now been removed. Any configuration
in cinder.conf still using the module path should be updated to include the
full class name. For example, ``cinder.backup.drivers.swift`` should be
updated to ``cinder.backup.drivers.swift.SwiftBackupDriver``.