Merge "compute: Reject requests to commit intermediary snapshot of an inactive instance" into stable/wallaby
This commit is contained in:
commit
c12d07fe38
|
@ -5483,11 +5483,25 @@ class API(base.Base):
|
||||||
bdm = self._get_bdm_by_volume_id(
|
bdm = self._get_bdm_by_volume_id(
|
||||||
context, volume_id, expected_attrs=['instance'])
|
context, volume_id, expected_attrs=['instance'])
|
||||||
|
|
||||||
# We allow deleting the snapshot in any vm_state as long as there is
|
|
||||||
# no task being performed on the instance and it has a host.
|
|
||||||
@check_instance_host()
|
@check_instance_host()
|
||||||
@check_instance_state(vm_state=None)
|
@check_instance_state(vm_state=None)
|
||||||
def do_volume_snapshot_delete(self, context, instance):
|
def do_volume_snapshot_delete(self, context, instance):
|
||||||
|
# FIXME(lyarwood): Avoid bug #1919487 by rejecting the request
|
||||||
|
# to delete an intermediary volume snapshot offline as this isn't
|
||||||
|
# currently implemented within the libvirt driver and will fail.
|
||||||
|
# This should be fixed in a future release but as it is essentially
|
||||||
|
# a new feature wouldn't be something we could backport. As such
|
||||||
|
# reject the request here so n-api can respond correctly to c-vol.
|
||||||
|
if (delete_info.get('merge_target_file') is not None and
|
||||||
|
instance.vm_state != vm_states.ACTIVE
|
||||||
|
):
|
||||||
|
raise exception.InstanceInvalidState(
|
||||||
|
attr='vm_state',
|
||||||
|
instance_uuid=instance.uuid,
|
||||||
|
state=instance.vm_state,
|
||||||
|
method='volume_snapshot_delete'
|
||||||
|
)
|
||||||
|
|
||||||
self.compute_rpcapi.volume_snapshot_delete(context, instance,
|
self.compute_rpcapi.volume_snapshot_delete(context, instance,
|
||||||
volume_id, snapshot_id, delete_info)
|
volume_id, snapshot_id, delete_info)
|
||||||
|
|
||||||
|
|
|
@ -3601,23 +3601,29 @@ class _ComputeAPIUnitTestMixIn(object):
|
||||||
'boot_index': -1})
|
'boot_index': -1})
|
||||||
fake_bdm['instance'] = fake_instance.fake_db_instance(
|
fake_bdm['instance'] = fake_instance.fake_db_instance(
|
||||||
launched_at=timeutils.utcnow(),
|
launched_at=timeutils.utcnow(),
|
||||||
vm_state=vm_states.STOPPED)
|
vm_state=vm_states.ACTIVE)
|
||||||
fake_bdm['instance_uuid'] = fake_bdm['instance']['uuid']
|
fake_bdm['instance_uuid'] = fake_bdm['instance']['uuid']
|
||||||
fake_bdm = objects.BlockDeviceMapping._from_db_object(
|
fake_bdm = objects.BlockDeviceMapping._from_db_object(
|
||||||
self.context, objects.BlockDeviceMapping(),
|
self.context, objects.BlockDeviceMapping(),
|
||||||
fake_bdm, expected_attrs=['instance'])
|
fake_bdm, expected_attrs=['instance'])
|
||||||
|
|
||||||
mock_get_bdm.return_value = fake_bdm
|
mock_get_bdm.return_value = fake_bdm
|
||||||
|
delete_info = {
|
||||||
|
'merge_target_file': 'foo.qcow2',
|
||||||
|
}
|
||||||
|
|
||||||
with mock.patch.object(self.compute_api.compute_rpcapi,
|
with mock.patch.object(
|
||||||
'volume_snapshot_delete') as mock_snapshot:
|
self.compute_api.compute_rpcapi, 'volume_snapshot_delete'
|
||||||
self.compute_api.volume_snapshot_delete(self.context, volume_id,
|
) as mock_snapshot:
|
||||||
snapshot_id, {})
|
self.compute_api.volume_snapshot_delete(
|
||||||
|
self.context, volume_id, snapshot_id, delete_info
|
||||||
|
)
|
||||||
mock_get_bdm.assert_called_once_with(self.context, volume_id,
|
mock_get_bdm.assert_called_once_with(self.context, volume_id,
|
||||||
expected_attrs=['instance'])
|
expected_attrs=['instance'])
|
||||||
mock_snapshot.assert_called_once_with(
|
mock_snapshot.assert_called_once_with(
|
||||||
self.context, fake_bdm['instance'], volume_id, snapshot_id, {})
|
self.context, fake_bdm['instance'], volume_id, snapshot_id,
|
||||||
|
delete_info
|
||||||
|
)
|
||||||
|
|
||||||
@mock.patch.object(
|
@mock.patch.object(
|
||||||
objects.BlockDeviceMapping, 'get_by_volume',
|
objects.BlockDeviceMapping, 'get_by_volume',
|
||||||
|
@ -3651,6 +3657,43 @@ class _ComputeAPIUnitTestMixIn(object):
|
||||||
self.context, mock.sentinel.volume_id,
|
self.context, mock.sentinel.volume_id,
|
||||||
mock.sentinel.snapshot_id, mock.sentinel.delete_info)
|
mock.sentinel.snapshot_id, mock.sentinel.delete_info)
|
||||||
|
|
||||||
|
@mock.patch.object(compute_api.API, '_get_bdm_by_volume_id')
|
||||||
|
def test_volume_snapshot_delete_intermediary_commit(self, mock_get_bdm):
|
||||||
|
fake_bdm = fake_block_device.FakeDbBlockDeviceDict({
|
||||||
|
'id': 123,
|
||||||
|
'device_name': '/dev/sda2',
|
||||||
|
'source_type': 'volume',
|
||||||
|
'destination_type': 'volume',
|
||||||
|
'connection_info': "{'fake': 'connection_info'}",
|
||||||
|
'volume_id': uuids.volume_id,
|
||||||
|
'boot_index': -1
|
||||||
|
})
|
||||||
|
fake_bdm['instance'] = fake_instance.fake_db_instance(
|
||||||
|
launched_at=timeutils.utcnow(),
|
||||||
|
vm_state=vm_states.STOPPED)
|
||||||
|
fake_bdm['instance_uuid'] = fake_bdm['instance']['uuid']
|
||||||
|
fake_bdm = objects.BlockDeviceMapping._from_db_object(
|
||||||
|
self.context, objects.BlockDeviceMapping(),
|
||||||
|
fake_bdm, expected_attrs=['instance'])
|
||||||
|
mock_get_bdm.return_value = fake_bdm
|
||||||
|
|
||||||
|
# c-vol can provide delete_info with merge_target_file pointing to
|
||||||
|
# an intermediary snapshot to commit into it's base. This is only
|
||||||
|
# supported while the instance is running at present.
|
||||||
|
delete_info = {
|
||||||
|
'merge_target_file': 'snap.img'
|
||||||
|
}
|
||||||
|
|
||||||
|
# Assert that the request is rejected as offline commit isn't supported
|
||||||
|
self.assertRaises(
|
||||||
|
exception.InstanceInvalidState,
|
||||||
|
self.compute_api.volume_snapshot_delete,
|
||||||
|
self.context,
|
||||||
|
uuids.volume_id,
|
||||||
|
uuids.snapshot_id,
|
||||||
|
delete_info
|
||||||
|
)
|
||||||
|
|
||||||
def _create_instance_with_disabled_disk_config(self, object=False):
|
def _create_instance_with_disabled_disk_config(self, object=False):
|
||||||
sys_meta = {"image_auto_disk_config": "Disabled"}
|
sys_meta = {"image_auto_disk_config": "Disabled"}
|
||||||
params = {"system_metadata": sys_meta}
|
params = {"system_metadata": sys_meta}
|
||||||
|
|
Loading…
Reference in New Issue