diff --git a/cinder/backup/manager.py b/cinder/backup/manager.py index 1512eebbd17..aab2c49c2ae 100644 --- a/cinder/backup/manager.py +++ b/cinder/backup/manager.py @@ -490,7 +490,11 @@ class BackupManager(manager.ThreadPoolManager): backup_service = self._map_service_to_driver(backup['service']) configured_service = self.driver_name - if backup_service != configured_service: + # TODO(tommylikehu): We upgraded the 'driver_name' from module + # to class name, so we use 'in' here to match two namings, + # this can be replaced with equal sign during next + # release (Rocky). + if backup_service not in configured_service: err = _('Restore backup aborted, the backup service currently' ' configured [%(configured_service)s] is not the' ' backup service that was used to create this' @@ -574,7 +578,11 @@ class BackupManager(manager.ThreadPoolManager): backup_service = self._map_service_to_driver(backup['service']) if backup_service is not None: configured_service = self.driver_name - if backup_service != configured_service: + # TODO(tommylikehu): We upgraded the 'driver_name' from module + # to class name, so we use 'in' here to match two namings, + # this can be replaced with equal sign during next + # release (Rocky). + if backup_service not in configured_service: err = _('Delete backup aborted, the backup service currently' ' configured [%(configured_service)s] is not the' ' backup service that was used to create this' @@ -658,7 +666,11 @@ class BackupManager(manager.ThreadPoolManager): backup_record = {'backup_service': backup.service} backup_service = self._map_service_to_driver(backup.service) configured_service = self.driver_name - if backup_service != configured_service: + # TODO(tommylikehu): We upgraded the 'driver_name' from module + # to class name, so we use 'in' here to match two namings, + # this can be replaced with equal sign during next + # release (Rocky). + if backup_service not in configured_service: err = (_('Export record aborted, the backup service currently ' 'configured [%(configured_service)s] is not the ' 'backup service that was used to create this ' @@ -815,7 +827,11 @@ class BackupManager(manager.ThreadPoolManager): LOG.info('Backup service: %s.', backup_service_name) if backup_service_name is not None: configured_service = self.driver_name - if backup_service_name != configured_service: + # TODO(tommylikehu): We upgraded the 'driver_name' from module + # to class name, so we use 'in' here to match two namings, + # this can be replaced with equal sign during next + # release (Rocky). + if backup_service_name not in configured_service: err = _('Reset backup status aborted, the backup service' ' currently configured [%(configured_service)s] ' 'is not the backup service that was used to create' diff --git a/cinder/cmd/manage.py b/cinder/cmd/manage.py index c6880ab8909..a32d3fbd215 100644 --- a/cinder/cmd/manage.py +++ b/cinder/cmd/manage.py @@ -207,6 +207,7 @@ class DbCommands(object): online_migrations = ( # Added in Queens db.service_uuids_online_data_migration, + db.backup_service_online_migration, ) def __init__(self): diff --git a/cinder/db/api.py b/cinder/db/api.py index b71832029ab..d07f10a9b14 100644 --- a/cinder/db/api.py +++ b/cinder/db/api.py @@ -97,6 +97,10 @@ def service_uuids_online_data_migration(context, max_count): return IMPL.service_uuids_online_data_migration(context, max_count) +def backup_service_online_migration(context, max_count): + return IMPL.backup_service_online_migration(context, max_count) + + def service_destroy(context, service_id): """Destroy the service or raise if it does not exist.""" return IMPL.service_destroy(context, service_id) diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index 06b67c2bb2b..f3fe6f5dd4c 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -598,6 +598,40 @@ def service_uuids_online_data_migration(context, max_count): return total, updated +@require_admin_context +def backup_service_online_migration(context, max_count): + name_rules = {'cinder.backup.drivers.swift': + 'cinder.backup.drivers.swift.SwiftBackupDriver', + 'cinder.backup.drivers.ceph': + 'cinder.backup.drivers.ceph.CephBackupDriver', + 'cinder.backup.drivers.glusterfs': + 'cinder.backup.drivers.glusterfs.GlusterfsBackupDriver', + 'cinder.backup.drivers.google': + 'cinder.backup.drivers.google.GoogleBackupDriver', + 'cinder.backup.drivers.nfs': + 'cinder.backup.drivers.nfs.NFSBackupDriver', + 'cinder.backup.drivers.tsm': + 'cinder.backup.drivers.tsm.TSMBackupDriver', + 'cinder.backup.drivers.posix': + 'cinder.backup.drivers.posix.PosixBackupDriver'} + total = 0 + updated = 0 + session = get_session() + with session.begin(): + total = model_query( + context, models.Backup, session=session).filter( + models.Backup.service.in_(name_rules.keys())).count() + backups = (model_query( + context, models.Backup, session=session).filter( + models.Backup.service.in_( + name_rules.keys())).limit(max_count)).all() + if len(backups): + for backup in backups: + updated += 1 + backup.service = name_rules[backup.service] + + return total, updated + ################### diff --git a/cinder/tests/unit/backup/fake_service.py b/cinder/tests/unit/backup/fake_service.py index 14e48dac590..7a1f6287fe2 100644 --- a/cinder/tests/unit/backup/fake_service.py +++ b/cinder/tests/unit/backup/fake_service.py @@ -17,8 +17,8 @@ from cinder.backup import driver class FakeBackupService(driver.BackupDriver): - def __init__(self, context, db_driver=None): - super(FakeBackupService, self).__init__(context, db_driver) + def __init__(self, context, db=None): + super(FakeBackupService, self).__init__(context, db) def backup(self, backup, volume_file): pass diff --git a/cinder/tests/unit/backup/test_backup.py b/cinder/tests/unit/backup/test_backup.py index 332d63c0410..d37597891b9 100644 --- a/cinder/tests/unit/backup/test_backup.py +++ b/cinder/tests/unit/backup/test_backup.py @@ -1122,11 +1122,14 @@ class BackupTestCase(BaseBackupTest): backup.save() self.backup_mgr.delete_backup(self.ctxt, backup) - def test_delete_backup(self): + @ddt.data('cinder.tests.unit.backup.fake_service.FakeBackupService', + 'cinder.tests.unit.backup.fake_service') + def test_delete_backup(self, service): """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) + status=fields.BackupStatus.DELETING, volume_id=vol_id, + service=service) self.backup_mgr.delete_backup(self.ctxt, backup) self.assertRaises(exception.BackupNotFound, db.backup_get, @@ -1242,16 +1245,19 @@ class BackupTestCase(BaseBackupTest): self.ctxt, backup) - def test_export_record(self): + @ddt.data('cinder.tests.unit.backup.fake_service.FakeBackupService', + 'cinder.tests.unit.backup.fake_service') + def test_export_record(self, service): """Test normal backup record export.""" vol_size = 1 vol_id = self._create_volume_db_entry(status='available', size=vol_size) backup = self._create_backup_db_entry( - status=fields.BackupStatus.AVAILABLE, volume_id=vol_id) + status=fields.BackupStatus.AVAILABLE, volume_id=vol_id, + service=service) export = self.backup_mgr.export_record(self.ctxt, backup) - self.assertEqual(CONF.backup_driver, export['backup_service']) + self.assertEqual(service, export['backup_service']) self.assertIn('backup_url', export) def test_import_record_with_verify_not_implemented(self): diff --git a/cinder/tests/unit/conf_fixture.py b/cinder/tests/unit/conf_fixture.py index a52832eaa4b..9201af72d4f 100644 --- a/cinder/tests/unit/conf_fixture.py +++ b/cinder/tests/unit/conf_fixture.py @@ -43,7 +43,8 @@ def set_defaults(conf): conf.set_default('sqlite_synchronous', False, group='database') conf.set_default('policy_file', 'cinder.tests.unit/policy.json', group='oslo_policy') - conf.set_default('backup_driver', 'cinder.tests.unit.backup.fake_service') + conf.set_default('backup_driver', + 'cinder.tests.unit.backup.fake_service.FakeBackupService') conf.set_default('backend', 'castellan.tests.unit.key_manager.mock_key_manager.' 'MockKeyManager', diff --git a/cinder/tests/unit/test_db_api.py b/cinder/tests/unit/test_db_api.py index 35fe374ff68..891f0d8695a 100644 --- a/cinder/tests/unit/test_db_api.py +++ b/cinder/tests/unit/test_db_api.py @@ -210,6 +210,31 @@ class DBAPIServiceTestCase(BaseTest): self.assertEqual(1, total) self.assertEqual(1, updated) + @ddt.data({'count': 5, 'total': 3, 'updated': 3}, + {'count': 2, 'total': 3, 'updated': 2}) + @ddt.unpack + def test_backup_service_online_migration(self, count, total, updated): + volume = utils.create_volume(self.ctxt) + sqlalchemy_api.backup_create(self.ctxt, { + 'service': 'cinder.backup.drivers.swift', + 'volume_id': volume.id + }) + sqlalchemy_api.backup_create(self.ctxt, { + 'service': 'cinder.backup.drivers.ceph', + 'volume_id': volume.id + }) + sqlalchemy_api.backup_create(self.ctxt, { + 'service': 'cinder.backup.drivers.glusterfs', + 'volume_id': volume.id + }) + sqlalchemy_api.backup_create(self.ctxt, { + 'service': 'cinder.backup.drivers.fake_backup_service', + 'volume_id': volume.id + }) + t, u = db.backup_service_online_migration(self.ctxt, count) + self.assertEqual(total, t) + self.assertEqual(updated, u) + def test_service_create(self): # Add a cluster value to the service values = {'cluster_name': 'cluster'}