diff --git a/cinder/backup/manager.py b/cinder/backup/manager.py index 231f460924f..363a5784bb8 100644 --- a/cinder/backup/manager.py +++ b/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: diff --git a/cinder/tests/unit/backup/test_backup.py b/cinder/tests/unit/backup/test_backup.py index 356bbd166dc..88635b25112 100644 --- a/cinder/tests/unit/backup/test_backup.py +++ b/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'.""" diff --git a/releasenotes/notes/bug1938488-a528893c103c03af.yaml b/releasenotes/notes/bug1938488-a528893c103c03af.yaml new file mode 100644 index 00000000000..ff3415c4cb8 --- /dev/null +++ b/releasenotes/notes/bug1938488-a528893c103c03af.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + `Bug #1938488 `_: + When cleaning up a failed backup, clean up the snapshot status when the + backup source is a snapshot