diff --git a/cinder/backup/manager.py b/cinder/backup/manager.py index 1110cebdb76..78c5af6777f 100644 --- a/cinder/backup/manager.py +++ b/cinder/backup/manager.py @@ -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, diff --git a/cinder/tests/unit/backup/test_backup.py b/cinder/tests/unit/backup/test_backup.py index b5df7fd908c..288db592b50 100644 --- a/cinder/tests/unit/backup/test_backup.py +++ b/cinder/tests/unit/backup/test_backup.py @@ -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__, diff --git a/releasenotes/notes/backup-path-removal-c411bb6c0d3887f1.yaml b/releasenotes/notes/backup-path-removal-c411bb6c0d3887f1.yaml new file mode 100644 index 00000000000..d4e404242c2 --- /dev/null +++ b/releasenotes/notes/backup-path-removal-c411bb6c0d3887f1.yaml @@ -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``.