diff --git a/cinder/backup/manager.py b/cinder/backup/manager.py index 0ac16102e5f..231f460924f 100644 --- a/cinder/backup/manager.py +++ b/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: diff --git a/cinder/tests/unit/backup/test_backup.py b/cinder/tests/unit/backup/test_backup.py index 6e80f6f5a97..356bbd166dc 100644 --- a/cinder/tests/unit/backup/test_backup.py +++ b/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'.""" diff --git a/releasenotes/notes/bug-2016138-56f07bc9376f55f7.yaml b/releasenotes/notes/bug-2016138-56f07bc9376f55f7.yaml new file mode 100644 index 00000000000..0d0fe2808ad --- /dev/null +++ b/releasenotes/notes/bug-2016138-56f07bc9376f55f7.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + `Bug #2016138 `_: + Handle missing volumes during cleanup of incomplete backups.