openstack
/
cinder
Code Issues Proposed changes

Handle missing temp volume and snapshot during cleanup 

When backup service is started, we try to clean up temp volumes
and snapshots in previously failed backups. If the temp volume
or snapshot is already deleted, we will get VolumeNotFound or
SnapshotNotFound exceptions. These exceptions should be handled.
Also temp_volume_id and temp_snapshot_id should be set to None
after they are deleted.

Change-Id: Ia20834bcc89040364bce71fc66c32c1777a5ac11
Closes-Bug: #1484774
This commit is contained in:
Xing Yang 2015-08-09 22:41:21 -04:00
parent 1bbae62b3d
commit 454bdf0f0f
2 changed files with 71 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
cinder/backup/manager.py
View File

@ -273,22 +273,38 @@ class BackupManager(manager.SchedulerDependentManager):
"backup %s.", backup.id)
continue
if backup.temp_volume_id and backup.status == 'error':
temp_volume = self.db.volume_get(ctxt,
backup.temp_volume_id)
# The temp volume should be deleted directly thru the
# the volume driver, not thru the volume manager.
mgr.driver.delete_volume(temp_volume)
self.db.volume_destroy(ctxt, temp_volume['id'])
try:
temp_volume = self.db.volume_get(ctxt,
backup.temp_volume_id)
# The temp volume should be deleted directly thru the
# the volume driver, not thru the volume manager.
mgr.driver.delete_volume(temp_volume)
self.db.volume_destroy(ctxt, temp_volume['id'])
except exception.VolumeNotFound:
LOG.debug("Could not find temp volume %(vol)s to clean up "
"for backup %(backup)s.",
{'vol': backup.temp_volume_id,
'backup': backup.id})
backup.temp_volume_id = None
backup.save()
if backup.temp_snapshot_id and backup.status == 'error':
temp_snapshot = objects.Snapshot.get_by_id(
ctxt, backup.temp_snapshot_id)
# The temp snapshot should be deleted directly thru the
# volume driver, not thru the volume manager.
mgr.driver.delete_snapshot(temp_snapshot)
with temp_snapshot.obj_as_admin():
self.db.volume_glance_metadata_delete_by_snapshot(
ctxt, temp_snapshot.id)
temp_snapshot.destroy()
try:
temp_snapshot = objects.Snapshot.get_by_id(
ctxt, backup.temp_snapshot_id)
# The temp snapshot should be deleted directly thru the
# volume driver, not thru the volume manager.
mgr.driver.delete_snapshot(temp_snapshot)
with temp_snapshot.obj_as_admin():
self.db.volume_glance_metadata_delete_by_snapshot(
ctxt, temp_snapshot.id)
temp_snapshot.destroy()
except exception.SnapshotNotFound:
LOG.debug("Could not find temp snapshot %(snap)s to clean "
"up for backup %(backup)s.",
{'snap': backup.temp_snapshot_id,
'backup': backup.id})
backup.temp_snapshot_id = None
backup.save()
def create_backup(self, context, backup):
"""Create volume backups using configured backup service."""

43
cinder/tests/unit/test_backup.py
View File

@ -230,9 +230,8 @@ class BackupTestCase(BaseBackupTest):
@mock.patch.object(db, 'volume_get')
@ddt.data(KeyError, exception.VolumeNotFound)
def test_cleanup_temp_volumes_snapshots(self,
err,
mock_volume_get):
def test_cleanup_temp_volumes_snapshots_volume_not_found(
self, err, mock_volume_get):
"""Ensure we handle missing volume for a backup."""
mock_volume_get.side_effect = [err]
@ -242,6 +241,44 @@ class BackupTestCase(BaseBackupTest):
self.assertIsNone(self.backup_mgr._cleanup_temp_volumes_snapshots(
backups))
@mock.patch.object(lvm.LVMVolumeDriver, 'delete_snapshot')
def test_cleanup_temp_snapshot_not_found(self,
mock_delete_snapshot):
"""Ensure we handle missing temp snapshot for a backup."""
vol1_id = self._create_volume_db_entry()
self._create_volume_attach(vol1_id)
db.volume_update(self.ctxt, vol1_id, {'status': 'backing-up'})
backup1 = self._create_backup_db_entry(status='error',
volume_id=vol1_id,
temp_snapshot_id='fake')
backups = [backup1]
self.assertEqual('fake', backups[0].temp_snapshot_id)
self.assertIsNone(self.backup_mgr._cleanup_temp_volumes_snapshots(
backups))
self.assertFalse(mock_delete_snapshot.called)
self.assertIsNone(backups[0].temp_snapshot_id)
backup1.destroy()
db.volume_destroy(self.ctxt, vol1_id)
@mock.patch.object(lvm.LVMVolumeDriver, 'delete_volume')
def test_cleanup_temp_volume_not_found(self,
mock_delete_volume):
"""Ensure we handle missing temp volume for a backup."""
vol1_id = self._create_volume_db_entry()
self._create_volume_attach(vol1_id)
db.volume_update(self.ctxt, vol1_id, {'status': 'backing-up'})
backup1 = self._create_backup_db_entry(status='error',
volume_id=vol1_id,
temp_volume_id='fake')
backups = [backup1]
self.assertEqual('fake', backups[0].temp_volume_id)
self.assertIsNone(self.backup_mgr._cleanup_temp_volumes_snapshots(
backups))
self.assertFalse(mock_delete_volume.called)
self.assertIsNone(backups[0].temp_volume_id)
backup1.destroy()
db.volume_destroy(self.ctxt, vol1_id)
def test_create_backup_with_bad_volume_status(self):
"""Test creating a backup from a volume with a bad status."""
vol_id = self._create_volume_db_entry(status='restoring', size=1)