From 54f97d28b04b996ed1c8e7fa15f1f4d8f3d58c50 Mon Sep 17 00:00:00 2001 From: Tom Barron Date: Sat, 25 Jul 2015 17:11:29 -0400 Subject: [PATCH] Fix cleanup_temp_volume_snapshots for missing vol The cleanup_temp_volume_snapshots method raises an exception that causes the backup service to exit if there is a backup without a corresponding volume. This commit catches these exceptions so that the backup service is not prevented from starting in this circumstance. Change-Id: Ia1aac2fe78048df5fce595d5de181cb4930ea78d Closes-bug: 1478599 --- cinder/backup/manager.py | 14 ++++++++++---- cinder/tests/unit/test_backup.py | 16 ++++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/cinder/backup/manager.py b/cinder/backup/manager.py index f0a90f7b2..8b6514da4 100644 --- a/cinder/backup/manager.py +++ b/cinder/backup/manager.py @@ -265,10 +265,16 @@ class BackupManager(manager.SchedulerDependentManager): # create by the backup job are deleted when service is started. ctxt = context.get_admin_context() for backup in backups: - volume = self.db.volume_get(ctxt, backup.volume_id) - volume_host = volume_utils.extract_host(volume['host'], 'backend') - backend = self._get_volume_backend(host=volume_host) - mgr = self._get_manager(backend) + try: + volume = self.db.volume_get(ctxt, backup.volume_id) + volume_host = volume_utils.extract_host(volume['host'], + 'backend') + backend = self._get_volume_backend(host=volume_host) + mgr = self._get_manager(backend) + except (KeyError, exception.VolumeNotFound): + LOG.debug("Could not find a volume to clean up for " + "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) diff --git a/cinder/tests/unit/test_backup.py b/cinder/tests/unit/test_backup.py index 836d5dd56..1c48e7bc8 100644 --- a/cinder/tests/unit/test_backup.py +++ b/cinder/tests/unit/test_backup.py @@ -17,6 +17,7 @@ Tests for Backup code. """ +import ddt import tempfile import mock @@ -170,6 +171,7 @@ class BaseBackupTest(test.TestCase): return backup +@ddt.ddt class BackupTestCase(BaseBackupTest): """Test Case for backups.""" @@ -227,6 +229,20 @@ class BackupTestCase(BaseBackupTest): self.assertTrue(mock_delete_volume.called) self.assertTrue(mock_delete_snapshot.called) + @mock.patch.object(db, 'volume_get') + @ddt.data(KeyError, exception.VolumeNotFound) + def test_cleanup_temp_volumes_snapshots(self, + err, + mock_volume_get): + """Ensure we handle missing volume for a backup.""" + mock_volume_get.side_effect = [err] + + backup1 = self._create_backup_db_entry(status='creating') + backups = [backup1] + + self.assertIsNone(self.backup_mgr._cleanup_temp_volumes_snapshots( + backups)) + 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) -- 2.45.2