Merge "Fix snapshot status is always backing-up"

cinder/backup/manager.py

@@ -233,11 +233,29 @@ class BackupManager(manager.SchedulerDependentManager):
             self.db.volume_update(ctxt, volume['id'],
                                   {'status': 'error_restoring'})
 
+    def _cleanup_one_snapshot(self, ctxt, snapshot_id):
+        try:
+            snapshot = objects.Snapshot.get_by_id(ctxt, snapshot_id)
+        except exception.SnapshotNotFound:
+            LOG.info('Snapshot %s does not exist anymore. Ignoring.',
+                     snapshot_id)
+            return
+        if snapshot['status'] == 'backing-up':
+            LOG.info('Resetting snapshot %(snap_id)s to previous '
+                     'status %(status)s (was backing-up).',
+                     {'snap_id': snapshot['id'],
+                      'status': fields.SnapshotStatus.AVAILABLE})
+
+            snapshot.status = fields.SnapshotStatus.AVAILABLE
+            snapshot.save()
+
     def _cleanup_one_backup(self, ctxt, backup):
         if backup['status'] == fields.BackupStatus.CREATING:
             LOG.info('Resetting backup %s to error (was creating).',
                      backup['id'])
             self._cleanup_one_volume(ctxt, backup.volume_id)
+            if backup.snapshot_id:
+                self._cleanup_one_snapshot(ctxt, backup.snapshot_id)
             err = 'incomplete backup reset on manager restart'
             volume_utils.update_backup_error(backup, err)
         elif backup['status'] == fields.BackupStatus.RESTORING:
@@ -245,6 +263,8 @@ class BackupManager(manager.SchedulerDependentManager):
                      'available (was restoring).',
                      backup['id'])
             self._cleanup_one_volume(ctxt, backup.restore_volume_id)
+            if backup.snapshot_id:
+                self._cleanup_one_snapshot(ctxt, backup.snapshot_id)
             backup.status = fields.BackupStatus.AVAILABLE
             backup.save()
         elif backup['status'] == fields.BackupStatus.DELETING:

cinder/tests/unit/backup/test_backup.py

@@ -448,6 +448,18 @@ class BackupTestCase(BaseBackupTest):
         volume = db.volume_get(self.ctxt, volume_id)
         self.assertEqual('available', volume['status'])
 
+    def test_cleanup_one_backing_up_snapshot(self):
+        """Test cleanup_one_snapshot for snapshot status 'backing-up'."""
+
+        volume_id = str(uuid.uuid4())
+        snapshot_entry = self._create_snapshot_db_entry(status='backing-up',
+                                                        volume_id=volume_id)
+
+        self.backup_mgr._cleanup_one_snapshot(self.ctxt, snapshot_entry.id)
+
+        snapshot = db.snapshot_get(self.ctxt, snapshot_entry.id)
+        self.assertEqual('available', snapshot['status'])
+
     def test_cleanup_one_restoring_backup_volume(self):
         """Test cleanup_one_volume for volume status 'restoring-backup'."""
 
@@ -478,6 +490,27 @@ class BackupTestCase(BaseBackupTest):
             'Volume %s does not exist anymore. Ignoring.', volume_id
         )
 
+    @ddt.data(fields.BackupStatus.CREATING,
+              fields.BackupStatus.RESTORING)
+    def test_cleanup_one_backup_with_deleted_snapshot(self, backup_status):
+        """Test cleanup_one_backup for non-existing volume."""
+
+        volume_id = str(uuid.uuid4())
+        snapshot_id = str(uuid.uuid4())
+        backup = self._create_backup_db_entry(
+            status=backup_status,
+            volume_id=volume_id,
+            restore_volume_id=volume_id,
+            snapshot_id=snapshot_id
+        )
+
+        mock_log = self.mock_object(manager, 'LOG')
+        self.backup_mgr._cleanup_one_backup(self.ctxt, backup)
+
+        mock_log.info.assert_called_with(
+            'Snapshot %s does not exist anymore. Ignoring.', snapshot_id
+        )
+
     def test_cleanup_one_creating_backup(self):
         """Test cleanup_one_backup for volume status 'creating'."""
 

releasenotes/notes/bug1938488-a528893c103c03af.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    `Bug #1938488 <https://bugs.launchpad.net/cinder/+bug/1938488>`_:
+    When cleaning up a failed backup, clean up the snapshot status when the
+    backup source is a snapshot