Merge "Handle missing volumes during cleanup of incomplete backups"

cinder/backup/manager.py

@@ -211,7 +211,13 @@ class BackupManager(manager.SchedulerDependentManager):
                               "snapshots for backup %(bkup)s.",
                               {'bkup': backup['id']})
 
-    def _cleanup_one_volume(self, ctxt, volume):
+    def _cleanup_one_volume(self, ctxt, volume_id):
+        try:
+            volume = objects.Volume.get_by_id(ctxt, volume_id)
+        except exception.VolumeNotFound:
+            LOG.info('Volume %s does not exist anymore. Ignoring.', volume_id)
+            return
+
         if volume['status'] == 'backing-up':
             self._detach_all_attachments(ctxt, volume)
             LOG.info('Resetting volume %(vol_id)s to previous '
@@ -231,19 +237,14 @@ class BackupManager(manager.SchedulerDependentManager):
         if backup['status'] == fields.BackupStatus.CREATING:
             LOG.info('Resetting backup %s to error (was creating).',
                      backup['id'])
-
-            volume = objects.Volume.get_by_id(ctxt, backup.volume_id)
-            self._cleanup_one_volume(ctxt, volume)
-
+            self._cleanup_one_volume(ctxt, backup.volume_id)
             err = 'incomplete backup reset on manager restart'
             volume_utils.update_backup_error(backup, err)
         elif backup['status'] == fields.BackupStatus.RESTORING:
             LOG.info('Resetting backup %s to '
                      'available (was restoring).',
                      backup['id'])
-            volume = objects.Volume.get_by_id(ctxt, backup.restore_volume_id)
-            self._cleanup_one_volume(ctxt, volume)
-
+            self._cleanup_one_volume(ctxt, backup.restore_volume_id)
             backup.status = fields.BackupStatus.AVAILABLE
             backup.save()
         elif backup['status'] == fields.BackupStatus.DELETING:

cinder/tests/unit/backup/test_backup.py

@@ -443,7 +443,7 @@ class BackupTestCase(BaseBackupTest):
                                                  previous_status='available')
         volume = db.volume_get(self.ctxt, volume_id)
 
-        self.backup_mgr._cleanup_one_volume(self.ctxt, volume)
+        self.backup_mgr._cleanup_one_volume(self.ctxt, volume_id)
 
         volume = db.volume_get(self.ctxt, volume_id)
         self.assertEqual('available', volume['status'])
@@ -454,11 +454,30 @@ class BackupTestCase(BaseBackupTest):
         volume_id = self._create_volume_db_entry(status='restoring-backup')
         volume = db.volume_get(self.ctxt, volume_id)
 
-        self.backup_mgr._cleanup_one_volume(self.ctxt, volume)
+        self.backup_mgr._cleanup_one_volume(self.ctxt, volume_id)
 
         volume = db.volume_get(self.ctxt, volume_id)
         self.assertEqual('error_restoring', volume['status'])
 
+    @ddt.data(fields.BackupStatus.CREATING,
+              fields.BackupStatus.RESTORING)
+    def test_cleanup_one_backup_with_deleted_volume(self, backup_status):
+        """Test cleanup_one_backup for non-existing volume."""
+
+        volume_id = str(uuid.uuid4())
+        backup = self._create_backup_db_entry(
+            status=backup_status,
+            volume_id=volume_id,
+            restore_volume_id=volume_id
+        )
+
+        mock_log = self.mock_object(manager, 'LOG')
+        self.backup_mgr._cleanup_one_backup(self.ctxt, backup)
+
+        mock_log.info.assert_called_with(
+            'Volume %s does not exist anymore. Ignoring.', volume_id
+        )
+
     def test_cleanup_one_creating_backup(self):
         """Test cleanup_one_backup for volume status 'creating'."""
 

releasenotes/notes/bug-2016138-56f07bc9376f55f7.yaml

@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    `Bug #2016138 <https://bugs.launchpad.net/cinder/+bug/2016138>`_:
+    Handle missing volumes during cleanup of incomplete backups.