From 86f62022ee92d9c57b0bfe9baa8993331f416472 Mon Sep 17 00:00:00 2001 From: Sean McGinnis Date: Wed, 22 Aug 2018 17:02:52 -0500 Subject: [PATCH] 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 --- cinder/backup/manager.py | 43 +++++------------ cinder/tests/unit/backup/test_backup.py | 48 +++++++++---------- .../backup-path-removal-c411bb6c0d3887f1.yaml | 8 ++++ 3 files changed, 43 insertions(+), 56 deletions(-) create mode 100644 releasenotes/notes/backup-path-removal-c411bb6c0d3887f1.yaml 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``.