openstack/nova
Code Issues Proposed changes

Fix nova assisted volume snapshots

Browse Source 
When performing nova assisted volume snapshots, the nova api does
not receive an instance id, so there is not the usual instance
lookup from the cell which automatically targets the context for the
cell that the instance was pulled from, which is also where we'd get
the BDM.

Since we do not know which cell to target when fetching the BDM, we
have to iterate through *all* of them.

Closes-Bug: #1713735

Change-Id: Id2e3d3f177739a31d63790e4a1ae6ac41f438ddd
This commit is contained in:
Lucian Petrut 2017-08-29 17:30:18 +03:00 committed by Matt Riedemann
parent bd4a364e68
commit 074614f077
2 changed files with 72 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
nova/compute/api.py
View File

@ -4073,9 +4073,31 @@ class API(base.Base):
return objects.Migration.get_by_id_and_instance(
context, migration_id, instance_uuid)
def _get_bdm_by_volume_id(self, context, volume_id, expected_attrs=None):
"""Retrieve a BDM without knowing its cell.
.. note:: The context will be targeted to the cell in which the
BDM is found, if any.
:param context: The API request context.
:param volume_id: The ID of the volume.
:param expected_attrs: list of any additional attributes that should
be joined when the BDM is loaded from the database.
:raises: nova.exception.VolumeBDMNotFound if not found in any cell
"""
load_cells()
for cell in CELLS:
nova_context.set_target_cell(context, cell)
try:
return objects.BlockDeviceMapping.get_by_volume(
context, volume_id, expected_attrs=expected_attrs)
except exception.NotFound:
continue
raise exception.VolumeBDMNotFound(volume_id=volume_id)
def volume_snapshot_create(self, context, volume_id, create_info):
bdm = objects.BlockDeviceMapping.get_by_volume(
context, volume_id, expected_attrs=['instance'])
bdm = self._get_bdm_by_volume_id(
context, volume_id, expected_attrs=['instance'])
# We allow creating the snapshot in any vm_state as long as there is
# no task being performed on the instance and it has a host.
@ -4096,8 +4118,8 @@ class API(base.Base):
def volume_snapshot_delete(self, context, volume_id, snapshot_id,
delete_info):
bdm = objects.BlockDeviceMapping.get_by_volume(
context, volume_id, expected_attrs=['instance'])
bdm = self._get_bdm_by_volume_id(
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.

52
nova/tests/unit/compute/test_compute_api.py
View File

@ -2789,6 +2789,46 @@ class _ComputeAPIUnitTestMixIn(object):
self._test_snapshot_volume_backed(False, True,
vm_state=vm_states.SUSPENDED)
@mock.patch.object(context, 'set_target_cell')
@mock.patch.object(objects.BlockDeviceMapping, 'get_by_volume')
def test_get_bdm_by_volume_id(self, mock_get_by_volume,
mock_target_cell):
fake_cells = [mock.sentinel.cell0, mock.sentinel.cell1]
mock_get_by_volume.side_effect = [
exception.VolumeBDMNotFound(volume_id=mock.sentinel.volume_id),
mock.sentinel.bdm]
with mock.patch.object(compute_api, 'CELLS', fake_cells):
bdm = self.compute_api._get_bdm_by_volume_id(
self.context, mock.sentinel.volume_id,
mock.sentinel.expected_attrs)
self.assertEqual(mock.sentinel.bdm, bdm)
mock_target_cell.assert_has_calls([
mock.call(self.context, cell) for cell in fake_cells])
mock_get_by_volume.assert_has_calls(
[mock.call(self.context,
mock.sentinel.volume_id,
expected_attrs=mock.sentinel.expected_attrs)] * 2)
@mock.patch.object(context, 'set_target_cell')
@mock.patch.object(objects.BlockDeviceMapping, 'get_by_volume')
def test_get_missing_bdm_by_volume_id(self, mock_get_by_volume,
mock_target_cell):
fake_cells = [mock.sentinel.cell0, mock.sentinel.cell1]
mock_get_by_volume.side_effect = exception.VolumeBDMNotFound(
volume_id=mock.sentinel.volume_id)
with mock.patch.object(compute_api, 'CELLS', fake_cells):
self.assertRaises(
exception.VolumeBDMNotFound,
self.compute_api._get_bdm_by_volume_id,
self.context, mock.sentinel.volume_id,
mock.sentinel.expected_attrs)
def test_volume_snapshot_create(self):
volume_id = '1'
create_info = {'id': 'eyedee'}
@ -2808,12 +2848,12 @@ class _ComputeAPIUnitTestMixIn(object):
self.context, objects.BlockDeviceMapping(),
fake_bdm, expected_attrs=['instance'])
self.mox.StubOutWithMock(objects.BlockDeviceMapping,
'get_by_volume')
self.mox.StubOutWithMock(self.compute_api,
'_get_bdm_by_volume_id')
self.mox.StubOutWithMock(self.compute_api.compute_rpcapi,
'volume_snapshot_create')
objects.BlockDeviceMapping.get_by_volume(
self.compute_api._get_bdm_by_volume_id(
self.context, volume_id,
expected_attrs=['instance']).AndReturn(fake_bdm)
self.compute_api.compute_rpcapi.volume_snapshot_create(self.context,
@ -2883,12 +2923,12 @@ class _ComputeAPIUnitTestMixIn(object):
self.context, objects.BlockDeviceMapping(),
fake_bdm, expected_attrs=['instance'])
self.mox.StubOutWithMock(objects.BlockDeviceMapping,
'get_by_volume')
self.mox.StubOutWithMock(self.compute_api,
'_get_bdm_by_volume_id')
self.mox.StubOutWithMock(self.compute_api.compute_rpcapi,
'volume_snapshot_delete')
objects.BlockDeviceMapping.get_by_volume(
self.compute_api._get_bdm_by_volume_id(
self.context, volume_id,
expected_attrs=['instance']).AndReturn(fake_bdm)
self.compute_api.compute_rpcapi.volume_snapshot_delete(self.context,