Fix ChunkedBackupDriver _create_container
Current _create_container method in ChunkedBackupDriver is not behaving as expected. According to update_container_name docstring the method should return None if the container name that comes in to the driver is to be used, but code was using backup_default_container instead. Also, if None is returned by update_container_name the logging will report that it will be using None instead of the name that it will be really using. If update_container_name returns the same value we already have in the DB, driver will unnecessarily update the DB. This patch fixes all this. Change-Id: I2aeac37f5533b6bf0c10dd6bfe45224a25f39ea6 Closes-Bug: #1534182
This commit is contained in:
parent
aa44c8b281
commit
788ac9d982
|
@ -159,12 +159,24 @@ class ChunkedBackupDriver(driver.BackupDriver):
|
|||
return
|
||||
|
||||
def _create_container(self, context, backup):
|
||||
backup.container = self.update_container_name(backup, backup.container)
|
||||
# Container's name will be decided by the driver (returned by method
|
||||
# update_container_name), if no change is required by the driver then
|
||||
# we'll use the one the backup object already has, but if it doesn't
|
||||
# have one backup_default_container will be used.
|
||||
new_container = self.update_container_name(backup, backup.container)
|
||||
if new_container:
|
||||
# If the driver is not really changing the name we don't want to
|
||||
# dirty the field in the object and save it to the DB with the same
|
||||
# value.
|
||||
if new_container != backup.container:
|
||||
backup.container = new_container
|
||||
elif backup.container is None:
|
||||
backup.container = self.backup_default_container
|
||||
|
||||
LOG.debug('_create_container started, container: %(container)s,'
|
||||
'backup: %(backup_id)s.',
|
||||
{'container': backup.container, 'backup_id': backup.id})
|
||||
if backup.container is None:
|
||||
backup.container = self.backup_default_container
|
||||
|
||||
backup.save()
|
||||
self.put_container(backup.container)
|
||||
return backup.container
|
||||
|
|
|
@ -253,7 +253,8 @@ class BackupSwiftTestCase(test.TestCase):
|
|||
backup = objects.Backup.get_by_id(self.ctxt, 123)
|
||||
service.backup(backup, self.volume_file)
|
||||
|
||||
def test_backup_default_container(self):
|
||||
@mock.patch.object(db, 'backup_update', wraps=db.backup_update)
|
||||
def test_backup_default_container(self, backup_update_mock):
|
||||
volume_id = '9552017f-c8b9-4e4e-a876-00000053349c'
|
||||
self._create_backup_db_entry(volume_id=volume_id,
|
||||
container=None)
|
||||
|
@ -263,6 +264,38 @@ class BackupSwiftTestCase(test.TestCase):
|
|||
service.backup(backup, self.volume_file)
|
||||
backup = objects.Backup.get_by_id(self.ctxt, 123)
|
||||
self.assertEqual('volumebackups', backup['container'])
|
||||
self.assertEqual(3, backup_update_mock.call_count)
|
||||
|
||||
@mock.patch.object(db, 'backup_update', wraps=db.backup_update)
|
||||
def test_backup_db_container(self, backup_update_mock):
|
||||
volume_id = '9552017f-c8b9-4e4e-a876-00000053349c'
|
||||
self._create_backup_db_entry(volume_id=volume_id,
|
||||
container='existing_name')
|
||||
service = swift_dr.SwiftBackupDriver(self.ctxt)
|
||||
self.volume_file.seek(0)
|
||||
backup = objects.Backup.get_by_id(self.ctxt, 123)
|
||||
|
||||
service.backup(backup, self.volume_file)
|
||||
backup = objects.Backup.get_by_id(self.ctxt, 123)
|
||||
self.assertEqual('existing_name', backup['container'])
|
||||
# Make sure we are not making a DB update when we are using the same
|
||||
# value that's already in the DB.
|
||||
self.assertEqual(2, backup_update_mock.call_count)
|
||||
|
||||
@mock.patch.object(db, 'backup_update', wraps=db.backup_update)
|
||||
def test_backup_driver_container(self, backup_update_mock):
|
||||
volume_id = '9552017f-c8b9-4e4e-a876-00000053349c'
|
||||
self._create_backup_db_entry(volume_id=volume_id,
|
||||
container=None)
|
||||
service = swift_dr.SwiftBackupDriver(self.ctxt)
|
||||
self.volume_file.seek(0)
|
||||
backup = objects.Backup.get_by_id(self.ctxt, 123)
|
||||
with mock.patch.object(service, 'update_container_name',
|
||||
return_value='driver_name'):
|
||||
service.backup(backup, self.volume_file)
|
||||
backup = objects.Backup.get_by_id(self.ctxt, 123)
|
||||
self.assertEqual('driver_name', backup['container'])
|
||||
self.assertEqual(3, backup_update_mock.call_count)
|
||||
|
||||
@mock.patch('cinder.backup.drivers.swift.SwiftBackupDriver.'
|
||||
'_send_progress_end')
|
||||
|
|
Loading…
Reference in New Issue