diff --git a/cinder/backup/manager.py b/cinder/backup/manager.py index 7f83cc77376..4f1f94ea266 100644 --- a/cinder/backup/manager.py +++ b/cinder/backup/manager.py @@ -43,6 +43,7 @@ from oslo_service import loopingcall from oslo_service import periodic_task from oslo_utils import excutils from oslo_utils import importutils +from oslo_utils import timeutils import six from cinder.backup import driver @@ -544,17 +545,22 @@ class BackupManager(manager.ThreadPoolManager): backup.host = self.host backup.save() - expected_status = 'restoring-backup' - actual_status = volume['status'] - if actual_status != expected_status: + expected_status = [fields.VolumeStatus.RESTORING_BACKUP, + fields.VolumeStatus.CREATING] + volume_previous_status = volume['status'] + if volume_previous_status not in expected_status: err = (_('Restore backup aborted, expected volume status ' '%(expected_status)s but got %(actual_status)s.') % - {'expected_status': expected_status, - 'actual_status': actual_status}) + {'expected_status': ','.join(expected_status), + 'actual_status': volume_previous_status}) backup.status = fields.BackupStatus.AVAILABLE backup.save() - self.db.volume_update(context, volume_id, - {'status': 'error_restoring'}) + self.db.volume_update( + context, volume_id, + {'status': + (fields.VolumeStatus.ERROR if + volume_previous_status == fields.VolumeStatus.CREATING else + fields.VolumeStatus.ERROR_RESTORING)}) raise exception.InvalidVolume(reason=err) expected_status = fields.BackupStatus.RESTORING @@ -565,7 +571,8 @@ class BackupManager(manager.ThreadPoolManager): {'expected_status': expected_status, 'actual_status': actual_status}) self._update_backup_error(backup, err) - self.db.volume_update(context, volume_id, {'status': 'error'}) + self.db.volume_update(context, volume_id, + {'status': fields.VolumeStatus.ERROR}) raise exception.InvalidBackup(reason=err) if volume['size'] > backup['size']: @@ -587,22 +594,34 @@ class BackupManager(manager.ThreadPoolManager): } backup.status = fields.BackupStatus.AVAILABLE backup.save() - self.db.volume_update(context, volume_id, {'status': 'error'}) + self.db.volume_update(context, volume_id, + {'status': fields.VolumeStatus.ERROR}) raise exception.InvalidBackup(reason=err) + canceled = False try: - canceled = False self._run_restore(context, backup, volume) except exception.BackupRestoreCancel: canceled = True except Exception: with excutils.save_and_reraise_exception(): - self.db.volume_update(context, volume_id, - {'status': 'error_restoring'}) + self.db.volume_update( + context, volume_id, + {'status': (fields.VolumeStatus.ERROR if + actual_status == fields.VolumeStatus.CREATING + else fields.VolumeStatus.ERROR_RESTORING)}) backup.status = fields.BackupStatus.AVAILABLE backup.save() - volume.status = 'error' if canceled else 'available' + if canceled: + volume.status = fields.VolumeStatus.ERROR + else: + volume.status = fields.VolumeStatus.AVAILABLE + # NOTE(tommylikehu): If previous status is 'creating', this is + # just a new created volume and we need update the 'launched_at' + # attribute as well. + if volume_previous_status == fields.VolumeStatus.CREATING: + volume['launched_at'] = timeutils.utcnow() volume.save() backup.status = fields.BackupStatus.AVAILABLE backup.save() diff --git a/cinder/objects/fields.py b/cinder/objects/fields.py index bd58a122136..062fbbfee60 100644 --- a/cinder/objects/fields.py +++ b/cinder/objects/fields.py @@ -177,10 +177,12 @@ class VolumeStatus(BaseCinderEnum): IN_USE = 'in-use' DETACHING = 'detaching' MAINTENANCE = 'maintenance' + RESTORING_BACKUP = 'restoring-backup' + ERROR_RESTORING = 'error_restoring' ALL = (CREATING, AVAILABLE, DELETING, ERROR, ERROR_DELETING, ERROR_MANAGING, MANAGING, ATTACHING, IN_USE, DETACHING, - MAINTENANCE) + MAINTENANCE, RESTORING_BACKUP, ERROR_RESTORING) class VolumeStatusField(BaseEnumField): diff --git a/cinder/tests/unit/backup/test_backup.py b/cinder/tests/unit/backup/test_backup.py index f290b670f98..efe4b78b3e7 100644 --- a/cinder/tests/unit/backup/test_backup.py +++ b/cinder/tests/unit/backup/test_backup.py @@ -1134,6 +1134,44 @@ class BackupTestCase(BaseBackupTest): self.assertEqual(fields.BackupStatus.AVAILABLE, backup.status) self.assertTrue(mock_run_restore.called) + def test_restore_backup_with_creating_volume(self): + """Test restore backup with a creating volume.""" + vol_id = self._create_volume_db_entry( + status=fields.VolumeStatus.CREATING, + size=1) + backup = self._create_backup_db_entry( + status=fields.BackupStatus.RESTORING, volume_id=vol_id) + mock_run_restore = self.mock_object( + self.backup_mgr, + '_run_restore') + self.backup_mgr.restore_backup(self.ctxt, backup, vol_id) + vol = objects.Volume.get_by_id(self.ctxt, vol_id) + self.assertEqual(fields.VolumeStatus.AVAILABLE, vol.status) + self.assertIsNotNone(vol.launched_at) + backup.refresh() + self.assertEqual(fields.BackupStatus.AVAILABLE, backup.status) + self.assertTrue(mock_run_restore.called) + + def test_restore_backup_canceled_with_creating_volume(self): + """Test restore backup with a creating volume.""" + vol_id = self._create_volume_db_entry( + status=fields.VolumeStatus.CREATING, + size=1) + backup = self._create_backup_db_entry( + status=fields.BackupStatus.RESTORING, volume_id=vol_id) + mock_run_restore = self.mock_object( + self.backup_mgr, + '_run_restore') + mock_run_restore.side_effect = exception.BackupRestoreCancel( + vol_id=vol_id, back_id=backup.id) + # We shouldn't raise an exception on the call, it's OK to cancel + self.backup_mgr.restore_backup(self.ctxt, backup, vol_id) + vol = objects.Volume.get_by_id(self.ctxt, vol_id) + self.assertEqual(fields.VolumeStatus.ERROR, vol.status) + backup.refresh() + self.assertEqual(fields.BackupStatus.AVAILABLE, backup.status) + self.assertTrue(mock_run_restore.called) + def test_restore_backup_with_bad_service(self): """Test error handling. diff --git a/cinder/volume/flows/manager/create_volume.py b/cinder/volume/flows/manager/create_volume.py index 0be266e52aa..e9e6bd1f095 100644 --- a/cinder/volume/flows/manager/create_volume.py +++ b/cinder/volume/flows/manager/create_volume.py @@ -973,7 +973,6 @@ class CreateVolumeFromSpecTask(flow_utils.CinderTask): "backup service to restore the volume with backup.", {'id': backup_id}) model_update = self._create_raw_volume(volume, **kwargs) or {} - model_update.update({'status': 'restoring-backup'}) volume.update(model_update) volume.save()