openstack/nova
Code Issues Proposed changes

Add instance action record for attach/detach/swap volumes

Browse Source 
We currently don't record volume attach/detach/swap instance
actions. This is useful for auditing and debugging.

This patch adds volume attach/detach/swap actions.

Change-Id: I0a3d15f3e3d0d8d920a79b519e17e3228e99f293
partial-implements: blueprint fill-the-gap-for-instance-action-records
This commit is contained in:
Kevin_Zheng 2017-11-02 16:13:17 +08:00 committed by Matt Riedemann
parent 2c7302e33e
commit 1cea4f0135
7 changed files with 91 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
nova/compute/api.py
View File

@ -48,6 +48,7 @@ from nova.compute import power_state
from nova.compute import rpcapi as compute_rpcapi
from nova.compute import task_states
from nova.compute import utils as compute_utils
from nova.compute.utils import wrap_instance_event
from nova.compute import vm_states
from nova import conductor
import nova.conf
@ -3558,6 +3559,8 @@ class API(base.Base):
device_type=device_type, tag=tag)
try:
self._check_attach_and_reserve_volume(context, volume_id, instance)
self._record_action_start(
context, instance, instance_actions.ATTACH_VOLUME)
self.compute_rpcapi.attach_volume(context, instance, volume_bdm)
except Exception:
with excutils.save_and_reraise_exception():
@ -3577,16 +3580,22 @@ class API(base.Base):
therefore the actual attachment will be performed once the
instance will be unshelved.
"""
@wrap_instance_event(prefix='api')
def attach_volume(self, context, v_id, instance, dev):
self.volume_api.attach(context,
v_id,
instance.uuid,
dev)
volume_bdm = self._create_volume_bdm(
context, instance, device, volume_id, disk_bus=disk_bus,
device_type=device_type, is_local_creation=True)
try:
self._check_attach_and_reserve_volume(context, volume_id, instance)
self.volume_api.attach(context,
volume_id,
instance.uuid,
device)
self._record_action_start(
context, instance,
instance_actions.ATTACH_VOLUME)
attach_volume(self, context, volume_id, instance, device)
except Exception:
with excutils.save_and_reraise_exception():
volume_bdm.destroy()
@ -3641,6 +3650,8 @@ class API(base.Base):
attachment_id = None
if attachments and instance.uuid in attachments:
attachment_id = attachments[instance.uuid]['attachment_id']
self._record_action_start(
context, instance, instance_actions.DETACH_VOLUME)
self.compute_rpcapi.detach_volume(context, instance=instance,
volume_id=volume['id'], attachment_id=attachment_id)
@ -3653,13 +3664,20 @@ class API(base.Base):
If the volume has delete_on_termination option set then we call the
volume api delete as well.
"""
@wrap_instance_event(prefix='api')
def detach_volume(self, context, instance, bdms):
self._local_cleanup_bdm_volumes(bdms, instance, context)
try:
self.volume_api.begin_detaching(context, volume['id'])
except exception.InvalidInput as exc:
raise exception.InvalidVolume(reason=exc.format_message())
bdms = [objects.BlockDeviceMapping.get_by_volume_id(
context, volume['id'], instance.uuid)]
self._local_cleanup_bdm_volumes(bdms, instance, context)
self._record_action_start(
context, instance,
instance_actions.DETACH_VOLUME)
detach_volume(self, context, instance, bdms)
@check_instance_lock
@check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.PAUSED,
@ -3712,6 +3730,9 @@ class API(base.Base):
new_attachment_id = self.volume_api.attachment_create(
context, new_volume['id'], instance.uuid)['id']
self._record_action_start(
context, instance, instance_actions.SWAP_VOLUME)
try:
self.compute_rpcapi.swap_volume(
context, instance=instance,

3
nova/compute/instance_actions.py
View File

@ -62,3 +62,6 @@ TRIGGER_CRASH_DUMP = 'trigger_crash_dump'
EXTEND_VOLUME = 'extend_volume'
ATTACH_INTERFACE = 'attach_interface'
DETACH_INTERFACE = 'detach_interface'
ATTACH_VOLUME = 'attach_volume'
DETACH_VOLUME = 'detach_volume'
SWAP_VOLUME = 'swap_volume'

3
nova/compute/manager.py
View File

@ -5093,6 +5093,7 @@ class ComputeManager(manager.Manager):
return do_reserve()
@wrap_exception()
@wrap_instance_event(prefix='compute')
@wrap_instance_fault
def attach_volume(self, context, instance, bdm):
"""Attach a volume to an instance."""
@ -5235,6 +5236,7 @@ class ComputeManager(manager.Manager):
instance=instance)
@wrap_exception()
@wrap_instance_event(prefix='compute')
@wrap_instance_fault
def detach_volume(self, context, volume_id, instance, attachment_id=None):
"""Detach a volume from an instance.
@ -5399,6 +5401,7 @@ class ComputeManager(manager.Manager):
return (comp_ret, new_cinfo)
@wrap_exception()
@wrap_instance_event(prefix='compute')
@wrap_instance_fault
def swap_volume(self, context, old_volume_id, new_volume_id, instance,
new_attachment_id=None):

45
nova/tests/unit/compute/test_compute.py
View File

@ -46,6 +46,7 @@ from nova import block_device
from nova import compute
from nova.compute import api as compute_api
from nova.compute import flavors
from nova.compute import instance_actions
from nova.compute import manager as compute_manager
from nova.compute import power_state
from nova.compute import rpcapi as compute_rpcapi
@ -393,7 +394,8 @@ class ComputeVolumeTestCase(BaseTestCase):
self.stub_out('nova.db.block_device_mapping_create', store_cinfo)
self.stub_out('nova.db.block_device_mapping_update', store_cinfo)
def test_attach_volume_serial(self):
@mock.patch.object(compute_utils, 'EventReporter')
def test_attach_volume_serial(self, mock_event):
fake_bdm = objects.BlockDeviceMapping(context=self.context,
**self.fake_volume)
with (mock.patch.object(cinder.API, 'get_volume_encryption_metadata',
@ -401,10 +403,14 @@ class ComputeVolumeTestCase(BaseTestCase):
instance = self._create_fake_instance_obj()
self.compute.attach_volume(self.context, instance, bdm=fake_bdm)
self.assertEqual(self.cinfo.get('serial'), uuids.volume_id)
mock_event.assert_called_once_with(
self.context, 'compute_attach_volume', instance.uuid)
@mock.patch.object(compute_utils, 'EventReporter')
@mock.patch('nova.context.RequestContext.elevated')
@mock.patch('nova.compute.utils.notify_about_volume_attach_detach')
def test_attach_volume_raises(self, mock_notify, mock_elevate):
def test_attach_volume_raises(self, mock_notify, mock_elevate,
mock_event):
mock_elevate.return_value = self.context
fake_bdm = objects.BlockDeviceMapping(**self.fake_volume)
@ -436,8 +442,11 @@ class ComputeVolumeTestCase(BaseTestCase):
volume_id=uuids.volume_id,
exception=expected_exception),
])
mock_event.assert_called_once_with(
self.context, 'compute_attach_volume', instance.uuid)
def test_detach_volume_api_raises(self):
@mock.patch.object(compute_utils, 'EventReporter')
def test_detach_volume_api_raises(self, mock_event):
fake_bdm = objects.BlockDeviceMapping(**self.fake_volume)
instance = self._create_fake_instance_obj()
@ -454,8 +463,11 @@ class ComputeVolumeTestCase(BaseTestCase):
test.TestingException, self.compute.detach_volume,
self.context, 'fake', instance, 'fake_id')
self.assertFalse(mock_destroy.called)
mock_event.assert_called_once_with(
self.context, 'compute_detach_volume', instance.uuid)
def test_detach_volume_bdm_destroyed(self):
@mock.patch.object(compute_utils, 'EventReporter')
def test_detach_volume_bdm_destroyed(self, mock_event):
# Assert that the BDM is destroyed given a successful call to detach
# the volume from the instance in Cinder.
fake_bdm = objects.BlockDeviceMapping(**self.fake_volume)
@ -475,6 +487,8 @@ class ComputeVolumeTestCase(BaseTestCase):
instance.uuid,
uuids.attachment_id)
self.assertTrue(mock_destroy.called)
mock_event.assert_called_once_with(
self.context, 'compute_detach_volume', instance.uuid)
def test_await_block_device_created_too_slow(self):
self.flags(block_device_allocate_retries=2)
@ -10068,8 +10082,9 @@ class ComputeAPITestCase(BaseTestCase):
with test.nested(
mock.patch.object(compute_api.API,
'_check_attach_and_reserve_volume'),
mock.patch.object(cinder.API, 'attach')
) as (mock_attach_and_reserve, mock_attach):
mock.patch.object(cinder.API, 'attach'),
mock.patch.object(compute_utils, 'EventReporter')
) as (mock_attach_and_reserve, mock_attach, mock_event):
self.compute_api._attach_volume_shelved_offloaded(
self.context, instance, 'fake-volume-id',
'/dev/vdb', 'ide', 'cdrom')
@ -10080,6 +10095,9 @@ class ComputeAPITestCase(BaseTestCase):
'fake-volume-id',
instance.uuid,
'/dev/vdb')
mock_event.assert_called_once_with(
self.context, 'api_attach_volume', instance.uuid
)
self.assertTrue(mock_attach.called)
def test_attach_volume_no_device(self):
@ -10125,7 +10143,8 @@ class ComputeAPITestCase(BaseTestCase):
self.assertTrue(called.get('fake_rpc_reserve_block_device_name'))
self.assertTrue(called.get('fake_rpc_attach_volume'))
def test_detach_volume(self):
@mock.patch('nova.compute.api.API._record_action_start')
def test_detach_volume(self, mock_record):
# Ensure volume can be detached from instance
called = {}
instance = self._create_fake_instance_obj()
@ -10147,14 +10166,19 @@ class ComputeAPITestCase(BaseTestCase):
instance, volume)
self.assertTrue(called.get('fake_begin_detaching'))
self.assertTrue(called.get('fake_rpc_detach_volume'))
mock_record.assert_called_once_with(
self.context, instance, instance_actions.DETACH_VOLUME)
@mock.patch('nova.compute.api.API._record_action_start')
@mock.patch.object(compute_utils, 'EventReporter')
@mock.patch.object(nova.volume.cinder.API, 'begin_detaching')
@mock.patch.object(compute_api.API, '_local_cleanup_bdm_volumes')
@mock.patch.object(objects.BlockDeviceMapping, 'get_by_volume_id')
def test_detach_volume_shelved_offloaded(self,
mock_block_dev,
mock_local_cleanup,
mock_begin_detaching):
mock_begin_detaching,
mock_event, mock_record):
mock_block_dev.return_value = [block_device_obj.BlockDeviceMapping(
context=context)]
@ -10165,6 +10189,11 @@ class ComputeAPITestCase(BaseTestCase):
volume)
mock_begin_detaching.assert_called_once_with(self.context,
volume['id'])
mock_record.assert_called_once_with(
self.context, instance, instance_actions.DETACH_VOLUME)
mock_event.assert_called_once_with(self.context,
'api_detach_volume',
instance.uuid)
self.assertTrue(mock_local_cleanup.called)
@mock.patch.object(nova.volume.cinder.API, 'begin_detaching',

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

@ -381,9 +381,10 @@ class _ComputeAPIUnitTestMixIn(object):
self.assertEqual(result.volume_id, bdm.volume_id)
self.assertTrue(bdm_create.called)
@mock.patch.object(compute_api.API, '_record_action_start')
@mock.patch.object(compute_rpcapi.ComputeAPI, 'reserve_block_device_name')
@mock.patch.object(compute_rpcapi.ComputeAPI, 'attach_volume')
def test_attach_volume(self, mock_attach, mock_reserve):
def test_attach_volume(self, mock_attach, mock_reserve, mock_record):
instance = self._create_instance_obj()
volume = fake_volume.fake_volume(1, 'test-vol', 'test-vol',
None, None, None, None, None)
@ -404,10 +405,14 @@ class _ComputeAPIUnitTestMixIn(object):
volume['id'])
mock_attach.assert_called_once_with(self.context,
instance, fake_bdm)
mock_record.assert_called_once_with(
self.context, instance, instance_actions.ATTACH_VOLUME)
@mock.patch.object(compute_api.API, '_record_action_start')
@mock.patch.object(compute_rpcapi.ComputeAPI, 'reserve_block_device_name')
@mock.patch.object(compute_rpcapi.ComputeAPI, 'attach_volume')
def test_tagged_volume_attach(self, mock_attach, mock_reserve):
def test_tagged_volume_attach(self, mock_attach, mock_reserve,
mock_record):
instance = self._create_instance_obj()
volume = fake_volume.fake_volume(1, 'test-vol', 'test-vol',
None, None, None, None, None)
@ -432,6 +437,8 @@ class _ComputeAPIUnitTestMixIn(object):
volume['id'])
mock_attach.assert_called_once_with(self.context,
instance, fake_bdm)
mock_record.assert_called_once_with(
self.context, instance, instance_actions.ATTACH_VOLUME)
def test_attach_volume_shelved_instance(self):
instance = self._create_instance_obj()
@ -2307,6 +2314,7 @@ class _ComputeAPIUnitTestMixIn(object):
if volumes[uuids.new_volume]['status'] == 'reserved':
volumes[uuids.new_volume]['status'] = 'available'
@mock.patch.object(compute_api.API, '_record_action_start')
@mock.patch.object(self.compute_api.compute_rpcapi, 'swap_volume',
return_value=True)
@mock.patch.object(self.compute_api.volume_api, 'unreserve_volume',
@ -2326,7 +2334,7 @@ class _ComputeAPIUnitTestMixIn(object):
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_unreserve_volume, mock_swap_volume, mock_record):
bdm = objects.BlockDeviceMapping(
**fake_block_device.FakeDbBlockDeviceDict(
{'no_device': False, 'volume_id': '1', 'boot_index': 0,
@ -2383,6 +2391,9 @@ class _ComputeAPIUnitTestMixIn(object):
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()

5
nova/tests/unit/compute/test_compute_mgr.py
View File

@ -1832,6 +1832,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
do_test()
@mock.patch.object(compute_utils, 'EventReporter')
@mock.patch.object(virt_driver.ComputeDriver, 'get_volume_connector',
return_value={})
@mock.patch.object(manager.ComputeManager, '_instance_update',
@ -1854,6 +1855,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
mock_instance_fault_create,
mock_instance_update,
mock_get_volume_connector,
mock_event,
expected_exception=None):
# This test ensures that volume_id arguments are passed to volume_api
# and that volume states are OK
@ -1955,6 +1957,9 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
fields.NotificationAction.VOLUME_SWAP,
fields.NotificationPhase.END,
uuids.old_volume, uuids.new_volume)
mock_event.assert_called_once_with(self.context,
'compute_swap_volume',
instance1.uuid)
def _assert_volume_api(self, context, volume, *args):
self.assertTrue(uuidutils.is_uuid_like(volume))

3
releasenotes/notes/fill-instance-action-record-gaps-14b36eba313d6d87.yaml
View File

@ -5,3 +5,6 @@ features:
* attach_interface
* detach_interface
* attach_volume
* detach_volume
* swap_volume