From de2ffaff36e3713e3862b15816f59c4d3dd8abca Mon Sep 17 00:00:00 2001 From: Ivan Kolodyazhny Date: Fri, 1 Sep 2017 18:08:31 +0300 Subject: [PATCH] Add ability to specify backup driver via class name This patch also deprecates backup driver initialization using module name. Change-Id: Id6bee9e7d0da8ead224a04f86fe79ddfb5b286cf Related-Blueprint: generic-backup-implementation --- cinder/backup/manager.py | 36 +++++++++++++------ cinder/db/base.py | 8 ++++- cinder/tests/unit/backup/test_backup.py | 25 +++++++------ ...driver-configuration-36357733962dab03.yaml | 11 ++++++ 4 files changed, 56 insertions(+), 24 deletions(-) create mode 100644 releasenotes/notes/backup-driver-configuration-36357733962dab03.yaml diff --git a/cinder/backup/manager.py b/cinder/backup/manager.py index a71e309e3de..1512eebbd17 100644 --- a/cinder/backup/manager.py +++ b/cinder/backup/manager.py @@ -60,7 +60,7 @@ LOG = logging.getLogger(__name__) backup_manager_opts = [ cfg.StrOpt('backup_driver', - default='cinder.backup.drivers.swift', + default='cinder.backup.drivers.swift.SwiftBackupDriver', help='Driver to use for backups.',), cfg.BoolOpt('backup_service_inithost_offload', default=True, @@ -90,7 +90,6 @@ class BackupManager(manager.ThreadPoolManager): target = messaging.Target(version=RPC_API_VERSION) def __init__(self, *args, **kwargs): - self.service = importutils.import_module(self.driver_name) self.az = CONF.storage_availability_zone self.backup_rpcapi = backup_rpcapi.BackupAPI() self.volume_rpcapi = volume_rpcapi.VolumeAPI() @@ -115,6 +114,23 @@ class BackupManager(manager.ThreadPoolManager): return mapper[service] return service + 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 + def _update_backup_error(self, backup, err, status=fields.BackupStatus.ERROR): backup.status = status @@ -133,7 +149,7 @@ class BackupManager(manager.ThreadPoolManager): LOG.exception("Problem cleaning incomplete backup operations.") def _setup_backup_driver(self, ctxt): - backup_service = self.service.get_backup_driver(ctxt) + backup_service = self.get_backup_driver(ctxt) backup_service.check_for_setup_error() self.is_initialized = True raise loopingcall.LoopingCallDone() @@ -389,7 +405,7 @@ class BackupManager(manager.ThreadPoolManager): self._notify_about_backup_usage(context, backup, "create.end") def _run_backup(self, context, backup, volume): - backup_service = self.service.get_backup_driver(context) + backup_service = self.get_backup_driver(context) properties = utils.brick_get_connector_properties() try: @@ -505,7 +521,7 @@ class BackupManager(manager.ThreadPoolManager): self._notify_about_backup_usage(context, backup, "restore.end") def _run_restore(self, context, backup, volume): - backup_service = self.service.get_backup_driver(context) + backup_service = self.get_backup_driver(context) properties = utils.brick_get_connector_properties() secure_enabled = ( @@ -569,7 +585,7 @@ class BackupManager(manager.ThreadPoolManager): raise exception.InvalidBackup(reason=err) try: - backup_service = self.service.get_backup_driver(context) + backup_service = self.get_backup_driver(context) backup_service.delete_backup(backup) except Exception as err: with excutils.save_and_reraise_exception(): @@ -653,7 +669,7 @@ class BackupManager(manager.ThreadPoolManager): # Call driver to create backup description string try: - backup_service = self.service.get_backup_driver(context) + backup_service = self.get_backup_driver(context) driver_info = backup_service.export_record(backup) backup_url = backup.encode_record(driver_info=driver_info) backup_record['backup_url'] = backup_url @@ -709,7 +725,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.service.get_backup_driver(context) + backup_service = self.get_backup_driver(context) backup_service.import_record(backup, driver_options) except Exception as err: msg = six.text_type(err) @@ -813,7 +829,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.service.get_backup_driver(context) + backup_service = self.get_backup_driver(context) if isinstance(backup_service, driver.BackupDriverWithVerify): backup_service.verify(backup.id) @@ -878,7 +894,7 @@ class BackupManager(manager.ThreadPoolManager): :param context: running context """ - backup_service = self.service.get_backup_driver(context) + backup_service = self.get_backup_driver(context) return backup_service.support_force_delete def _attach_device(self, ctxt, backup_device, diff --git a/cinder/db/base.py b/cinder/db/base.py index facb71f1c6d..fd0043f3c0c 100644 --- a/cinder/db/base.py +++ b/cinder/db/base.py @@ -19,6 +19,7 @@ from oslo_config import cfg from oslo_utils import importutils +import six db_driver_opt = cfg.StrOpt('db_driver', @@ -38,5 +39,10 @@ class Base(object): super(Base, self).__init__() if not db_driver: db_driver = CONF.db_driver - self.db = importutils.import_module(db_driver) # pylint: disable=C0103 + + # pylint: disable=C0103 + if isinstance(db_driver, six.string_types): + self.db = importutils.import_module(db_driver) + else: + self.db = db_driver self.db.dispose_engine() diff --git a/cinder/tests/unit/backup/test_backup.py b/cinder/tests/unit/backup/test_backup.py index 3442903601b..332d63c0410 100644 --- a/cinder/tests/unit/backup/test_backup.py +++ b/cinder/tests/unit/backup/test_backup.py @@ -36,7 +36,6 @@ from cinder import objects from cinder.objects import fields from cinder import test from cinder.tests import fake_driver -from cinder.tests.unit.backup import fake_service_with_verify as fake_service from cinder.tests.unit import utils from cinder.volume import rpcapi as volume_rpcapi @@ -650,9 +649,9 @@ class BackupTestCase(BaseBackupTest): vol_size = 1 vol_id = self._create_volume_db_entry(size=vol_size) backup = self._create_backup_db_entry(volume_id=vol_id, - parent_id = 'mock') + parent_id='mock') - with mock.patch.object(self.backup_mgr.service, 'get_backup_driver') as \ + with mock.patch.object(self.backup_mgr, 'get_backup_driver') as \ mock_get_backup_driver: mock_get_backup_driver.return_value.backup.return_value = ( {'parent_id': None}) @@ -687,7 +686,7 @@ 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.service, 'get_backup_driver') as \ + with mock.patch.object(self.backup_mgr, 'get_backup_driver') as \ mock_get_backup_driver: mock_get_backup_driver.return_value.backup.return_value = ( {'parent_id': parent_backup.id}) @@ -720,7 +719,7 @@ 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.service, 'get_backup_driver') as \ + with mock.patch.object(self.backup_mgr, 'get_backup_driver') as \ mock_get_backup_driver: mock_get_backup_driver.return_value.backup.side_effect = ( FakeBackupException('fake')) @@ -759,7 +758,7 @@ class BackupTestCase(BaseBackupTest): backup_service = lambda: None backup_service.backup = mock.Mock( return_value=mock.sentinel.backup_update) - self.backup_mgr.service.get_backup_driver = lambda x: backup_service + self.backup_mgr.get_backup_driver = lambda x: backup_service vol_id = self._create_volume_db_entry() backup = self._create_backup_db_entry(volume_id=vol_id) @@ -1343,7 +1342,7 @@ class BackupTestCase(BaseBackupTest): record where the backup driver returns an exception. """ export = self._create_exported_record_entry() - backup_driver = self.backup_mgr.service.get_backup_driver(self.ctxt) + backup_driver = self.backup_mgr.get_backup_driver(self.ctxt) _mock_record_import_class = ('%s.%s.%s' % (backup_driver.__module__, backup_driver.__class__.__name__, @@ -1415,7 +1414,7 @@ class BackupTestCaseWithVerify(BaseBackupTest): imported_record = self._create_export_record_db_entry( backup_id=backup_id) backup_hosts = [] - backup_driver = self.backup_mgr.service.get_backup_driver(self.ctxt) + backup_driver = self.backup_mgr.get_backup_driver(self.ctxt) _mock_backup_verify_class = ('%s.%s.%s' % (backup_driver.__module__, backup_driver.__class__.__name__, @@ -1449,7 +1448,7 @@ class BackupTestCaseWithVerify(BaseBackupTest): imported_record = self._create_export_record_db_entry( backup_id=backup_id) backup_hosts = [] - backup_driver = self.backup_mgr.service.get_backup_driver(self.ctxt) + backup_driver = self.backup_mgr.get_backup_driver(self.ctxt) _mock_backup_verify_class = ('%s.%s.%s' % (backup_driver.__module__, backup_driver.__class__.__name__, @@ -1481,7 +1480,8 @@ class BackupTestCaseWithVerify(BaseBackupTest): '_map_service_to_driver') as \ mock_map_service_to_driver: # It should works when the service name is a string - mock_map_service_to_driver.return_value = 'swift' + backup_driver = 'cinder.tests.unit.backup.fake_service_with_verify' + mock_map_service_to_driver.return_value = backup_driver self.backup_mgr.reset_status(self.ctxt, backup, fields.BackupStatus.AVAILABLE) @@ -1490,8 +1490,7 @@ class BackupTestCaseWithVerify(BaseBackupTest): self.assertEqual(fields.BackupStatus.AVAILABLE, new_backup['status']) - mock_map_service_to_driver.return_value = \ - fake_service.get_backup_driver(self.ctxt) + mock_map_service_to_driver.return_value = backup_driver self.backup_mgr.reset_status(self.ctxt, backup, fields.BackupStatus.ERROR) @@ -1512,7 +1511,7 @@ class BackupTestCaseWithVerify(BaseBackupTest): backup = self._create_backup_db_entry(status=fields.BackupStatus.ERROR, volume_id=volume['id']) - backup_driver = self.backup_mgr.service.get_backup_driver(self.ctxt) + backup_driver = self.backup_mgr.get_backup_driver(self.ctxt) _mock_backup_verify_class = ('%s.%s.%s' % (backup_driver.__module__, backup_driver.__class__.__name__, diff --git a/releasenotes/notes/backup-driver-configuration-36357733962dab03.yaml b/releasenotes/notes/backup-driver-configuration-36357733962dab03.yaml new file mode 100644 index 00000000000..f63089c22fa --- /dev/null +++ b/releasenotes/notes/backup-driver-configuration-36357733962dab03.yaml @@ -0,0 +1,11 @@ +--- +features: + - | + Add ability to specify backup driver via class name. +upgrade: + - | + Operators should change backup driver configuration value to use class + name to get backup service working in a 'S' release. +deprecations: + - | + Backup driver initialization using module name is deprecated.