From 4e2e6a597dd187175e99f2d45042db78f6ee7f9f Mon Sep 17 00:00:00 2001 From: huangtianhua Date: Tue, 12 May 2015 12:09:53 +0800 Subject: [PATCH] Correct volume restore behavior Now cinder volume restore will enter UpdateReplacement flow, the volume will be deleted and a new one be created. This patch will correct the restore behavior to call backup-restore API to just restore the volume. Change-Id: I2d2c330572279113c403e8fda41999521bfd4966 Closes-Bug: #1451359 --- heat/engine/clients/os/cinder.py | 8 +++ .../resources/openstack/cinder/volume.py | 58 +++++++++++++++++-- heat/tests/cinder/test_volume.py | 27 +++++---- 3 files changed, 79 insertions(+), 14 deletions(-) diff --git a/heat/engine/clients/os/cinder.py b/heat/engine/clients/os/cinder.py index e93bbeafff..41b0f5f579 100644 --- a/heat/engine/clients/os/cinder.py +++ b/heat/engine/clients/os/cinder.py @@ -215,6 +215,14 @@ class VolumeResizeProgress(object): self.size = size +class VolumeBackupRestoreProgress(object): + def __init__(self, vol_id, backup_id): + self.called = False + self.complete = False + self.vol_id = vol_id + self.backup_id = backup_id + + class VolumeConstraint(constraints.BaseCustomConstraint): expected_exceptions = (exception.EntityNotFound,) diff --git a/heat/engine/resources/openstack/cinder/volume.py b/heat/engine/resources/openstack/cinder/volume.py index 7bcd344926..73f4f43940 100644 --- a/heat/engine/resources/openstack/cinder/volume.py +++ b/heat/engine/resources/openstack/cinder/volume.py @@ -76,7 +76,8 @@ class CinderVolume(vb.BaseVolume): ), BACKUP_ID: properties.Schema( properties.Schema.STRING, - _('If specified, the backup to create the volume from.') + _('If specified, the backup to create the volume from.'), + update_allowed=True, ), NAME: properties.Schema( properties.Schema.STRING, @@ -301,12 +302,43 @@ class CinderVolume(vb.BaseVolume): LOG.info(_LI('Volume %(id)s resize complete'), {'id': vol.id}) return True + def _backup_restore(self, vol_id, backup_id): + try: + self.client().restores.restore(backup_id, vol_id) + except Exception as ex: + if self.client_plugin().is_client_exception(ex): + raise exception.Error(_( + "Failed to restore volume %(vol)s from backup %(backup)s " + "- %(err)s") % {'vol': vol_id, + 'backup': backup_id, + 'err': ex}) + else: + raise + return True + + def _check_backup_restore_complete(self): + vol = self.client().volumes.get(self.resource_id) + if vol.status == 'restoring-backup': + LOG.debug("Volume %s is being restoring from backup" % vol.id) + return False + + if vol.status != 'available': + LOG.info(_LI("Restore failed: Volume %(vol)s is in %(status)s " + "state."), {'vol': vol.id, 'status': vol.status}) + raise resource.ResourceUnknownStatus( + resource_status=vol.status, + result=_('Volume backup restore failed')) + + LOG.info(_LI('Volume %(id)s backup restore complete'), {'id': vol.id}) + return True + def handle_update(self, json_snippet, tmpl_diff, prop_diff): vol = None cinder = self.client() prg_resize = None prg_attach = None prg_detach = None + prg_backup_restore = None # update the name and description for cinder volume if self.NAME in prop_diff or self.DESCRIPTION in prop_diff: vol = cinder.volumes.get(self.resource_id) @@ -339,6 +371,11 @@ class CinderVolume(vb.BaseVolume): if self.READ_ONLY in prop_diff: flag = prop_diff.get(self.READ_ONLY) cinder.volumes.update_readonly_flag(self.resource_id, flag) + # restore the volume from backup + if self.BACKUP_ID in prop_diff: + prg_backup_restore = heat_cinder.VolumeBackupRestoreProgress( + vol_id=self.resource_id, + backup_id=prop_diff.get(self.BACKUP_ID)) # extend volume size if self.SIZE in prop_diff: if not vol: @@ -366,7 +403,7 @@ class CinderVolume(vb.BaseVolume): prg_attach = heat_cinder.VolumeAttachProgress( server_id, vol.id, device) - return prg_detach, prg_resize, prg_attach + return prg_backup_restore, prg_detach, prg_resize, prg_attach def _detach_volume_to_complete(self, prg_detach): if not prg_detach.called: @@ -396,7 +433,17 @@ class CinderVolume(vb.BaseVolume): return prg_attach.complete def check_update_complete(self, checkers): - prg_detach, prg_resize, prg_attach = checkers + prg_backup_restore, prg_detach, prg_resize, prg_attach = checkers + if prg_backup_restore: + if not prg_backup_restore.called: + prg_backup_restore.called = self._backup_restore( + prg_backup_restore.vol_id, + prg_backup_restore.backup_id) + return False + if not prg_backup_restore.complete: + prg_backup_restore.complete = \ + self._check_backup_restore_complete() + return prg_backup_restore.complete and not prg_resize if not prg_resize: return True # detach volume @@ -518,8 +565,11 @@ class CinderVolume(vb.BaseVolume): def handle_restore(self, defn, restore_data): backup_id = restore_data['resource_data']['backup_id'] + # we can't ignore 'size' property: if user update the size + # of volume after snapshot, we need to change to old size + # when restore the volume. ignore_props = ( - self.IMAGE_REF, self.IMAGE, self.SOURCE_VOLID, self.SIZE) + self.IMAGE_REF, self.IMAGE, self.SOURCE_VOLID) props = dict( (key, value) for (key, value) in six.iteritems(defn.properties(self.properties_schema)) diff --git a/heat/tests/cinder/test_volume.py b/heat/tests/cinder/test_volume.py index b09b4653e4..e168e7aded 100644 --- a/heat/tests/cinder/test_volume.py +++ b/heat/tests/cinder/test_volume.py @@ -998,9 +998,8 @@ class CinderVolumeTest(vt_base.BaseVolumeTest): stack_name, combinations, err_msg, exception.StackValidationFailed) - def test_volume_restore(self): - stack_name = 'test_cvolume_restore_stack' - + def _test_volume_restore(self, stack_name, final_status='available', + stack_final_status=('RESTORE', 'COMPLETE')): # create script cinder.CinderClientPlugin._create().MultipleTimes().AndReturn( self.cinder_fc) @@ -1023,12 +1022,12 @@ class CinderVolumeTest(vt_base.BaseVolumeTest): # restore script fvbr = vt_base.FakeBackupRestore('vol-123') self.m.StubOutWithMock(self.cinder_fc.restores, 'restore') - self.cinder_fc.restores.restore('backup-123').AndReturn(fvbr) - self.cinder_fc.volumes.get('vol-123').AndReturn(fv) - self.cinder_fc.volumes.update('vol-123', - description='test_description', - name='test_name') - self.cinder_fc.volumes.get('vol-123').AndReturn(fv) + self.cinder_fc.restores.restore('backup-123', + 'vol-123').AndReturn(fvbr) + fv_restoring = vt_base.FakeVolume('restoring-backup', id=fv.id) + self.cinder_fc.volumes.get('vol-123').AndReturn(fv_restoring) + fv_final = vt_base.FakeVolume(final_status, id=fv.id) + self.cinder_fc.volumes.get('vol-123').AndReturn(fv_final) self.m.ReplayAll() @@ -1048,10 +1047,18 @@ class CinderVolumeTest(vt_base.BaseVolumeTest): stack.restore(fake_snapshot) - self.assertEqual((stack.RESTORE, stack.COMPLETE), stack.state) + self.assertEqual(stack_final_status, stack.state) self.m.VerifyAll() + def test_volume_restore_success(self): + self._test_volume_restore(stack_name='test_volume_restore_success') + + def test_volume_restore_failed(self): + self._test_volume_restore(stack_name='test_volume_restore_failed', + final_status='error', + stack_final_status=('RESTORE', 'FAILED')) + def test_handle_delete_snapshot_no_backup(self): stack_name = 'test_handle_delete_snapshot_no_backup' mock_vs = {