openstack
/
nova
Code Issues Proposed changes

Merge "Add a new check to volume attach"

This commit is contained in:
Zuul 2017-12-06 15:59:11 +00:00 committed by Gerrit Code Review
parent 4fd6714594 4dd0656961
commit b09e5fe8ff
2 changed files with 77 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
nova/compute/api.py
View File

@ -3633,6 +3633,26 @@ class API(base.Base):
device_type=device_type, tag=tag)
return volume_bdm
def _check_volume_already_attached_to_instance(self, context, instance,
volume_id):
"""Avoid attaching the same volume to the same instance twice.
As the new Cinder flow (microversion 3.44) is handling the checks
differently and allows to attach the same volume to the same
instance twice to enable live_migrate we are checking whether the
BDM already exists for this combination for the new flow and fail
if it does.
"""
try:
objects.BlockDeviceMapping.get_by_volume_and_instance(
context, volume_id, instance.uuid)
msg = _("volume %s already attached") % volume_id
raise exception.InvalidVolume(reason=msg)
except exception.VolumeBDMNotFound:
pass
def _check_attach_and_reserve_volume(self, context, volume_id, instance):
volume = self.volume_api.get(context, volume_id)
self.volume_api.check_availability_zone(context, volume,
@ -3819,6 +3839,13 @@ class API(base.Base):
# we cast to the compute host.
self.volume_api.reserve_volume(context, new_volume['id'])
else:
try:
self._check_volume_already_attached_to_instance(
context, instance, new_volume['id'])
except exception.InvalidVolume:
with excutils.save_and_reraise_exception():
self.volume_api.roll_detaching(context, old_volume['id'])
# This is a new-style attachment so for the volume that we are
# going to swap to, create a new volume attachment.
new_attachment_id = self.volume_api.attachment_create(

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

@ -2368,7 +2368,12 @@ class _ComputeAPIUnitTestMixIn(object):
self._test_swap_volume(expected_exception=AttributeError,
attachment_id=uuids.attachment_id)
def _test_swap_volume(self, expected_exception=None, attachment_id=None):
def test_swap_volume_new_vol_already_attached_new_flow(self):
self._test_swap_volume(attachment_id=uuids.attachment_id,
volume_already_attached=True)
def _test_swap_volume(self, expected_exception=None, attachment_id=None,
volume_already_attached=False):
volumes = self._get_volumes_for_test_swap_volume()
instance = self._get_instance_for_test_swap_volume()
@ -2402,6 +2407,12 @@ class _ComputeAPIUnitTestMixIn(object):
if volumes[uuids.new_volume]['status'] == 'reserved':
volumes[uuids.new_volume]['status'] = 'available'
def fake_volume_is_attached(context, instance, volume_id):
if volume_already_attached:
raise exception.InvalidVolume(reason='Volume already attached')
else:
pass
@mock.patch.object(compute_api.API, '_record_action_start')
@mock.patch.object(self.compute_api.compute_rpcapi, 'swap_volume',
return_value=True)
@ -2411,6 +2422,9 @@ class _ComputeAPIUnitTestMixIn(object):
side_effect=fake_vol_api_attachment_delete)
@mock.patch.object(self.compute_api.volume_api, 'reserve_volume',
side_effect=fake_vol_api_reserve)
@mock.patch.object(self.compute_api,
'_check_volume_already_attached_to_instance',
side_effect=fake_volume_is_attached)
@mock.patch.object(self.compute_api.volume_api, 'attachment_create',
side_effect=fake_vol_api_attachment_create)
@mock.patch.object(self.compute_api.volume_api, 'roll_detaching',
@ -2421,8 +2435,9 @@ class _ComputeAPIUnitTestMixIn(object):
side_effect=fake_vol_api_begin_detaching)
def _do_test(mock_begin_detaching, mock_get_by_volume_and_instance,
mock_roll_detaching, mock_attachment_create,
mock_reserve_volume, mock_attachment_delete,
mock_unreserve_volume, mock_swap_volume, mock_record):
mock_check_volume_attached, mock_reserve_volume,
mock_attachment_delete, mock_unreserve_volume,
mock_swap_volume, mock_record):
bdm = objects.BlockDeviceMapping(
**fake_block_device.FakeDbBlockDeviceDict(
{'no_device': False, 'volume_id': '1', 'boot_index': 0,
@ -2454,6 +2469,27 @@ class _ComputeAPIUnitTestMixIn(object):
mock_unreserve_volume.assert_not_called()
mock_attachment_delete.assert_called_once_with(
self.context, attachment_id)
# Assert the call to the rpcapi.
mock_swap_volume.assert_called_once_with(
self.context, instance=instance,
old_volume_id=uuids.old_volume,
new_volume_id=uuids.new_volume,
new_attachment_id=attachment_id)
mock_record.assert_called_once_with(
self.context, instance, instance_actions.SWAP_VOLUME)
elif volume_already_attached:
self.assertRaises(exception.InvalidVolume,
self.compute_api.swap_volume, self.context,
instance, volumes[uuids.old_volume],
volumes[uuids.new_volume])
self.assertEqual('in-use', volumes[uuids.old_volume]['status'])
self.assertEqual('available',
volumes[uuids.new_volume]['status'])
mock_check_volume_attached.assert_called_once_with(
self.context, instance, uuids.new_volume)
mock_roll_detaching.assert_called_once_with(self.context,
uuids.old_volume)
else:
self.compute_api.swap_volume(self.context, instance,
volumes[uuids.old_volume],
@ -2466,22 +2502,24 @@ class _ComputeAPIUnitTestMixIn(object):
mock_reserve_volume.assert_called_once_with(
self.context, uuids.new_volume)
mock_attachment_create.assert_not_called()
mock_check_volume_attached.assert_not_called()
else:
# New style attachment, so reserve was not called and
# attachment_create was called.
mock_reserve_volume.assert_not_called()
mock_check_volume_attached.assert_called_once_with(
self.context, instance, uuids.new_volume)
mock_attachment_create.assert_called_once_with(
self.context, uuids.new_volume, instance.uuid)
# Assert the call to the rpcapi.
mock_swap_volume.assert_called_once_with(
self.context, instance=instance,
old_volume_id=uuids.old_volume,
new_volume_id=uuids.new_volume,
new_attachment_id=attachment_id)
mock_record.assert_called_once_with(self.context,
instance,
instance_actions.SWAP_VOLUME)
# Assert the call to the rpcapi.
mock_swap_volume.assert_called_once_with(
self.context, instance=instance,
old_volume_id=uuids.old_volume,
new_volume_id=uuids.new_volume,
new_attachment_id=attachment_id)
mock_record.assert_called_once_with(
self.context, instance, instance_actions.SWAP_VOLUME)
_do_test()