Merge "Add instance action record for attach/detach/swap volumes"

nova/compute/api.py

@@ -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
@@ -3569,6 +3570,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():
@@ -3588,16 +3591,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,
+            self._record_action_start(
-                                   volume_id,
+                context, instance,
-                                   instance.uuid,
+                instance_actions.ATTACH_VOLUME)
-                                   device)
+            attach_volume(self, context, volume_id, instance, device)
         except Exception:
             with excutils.save_and_reraise_exception():
                 volume_bdm.destroy()
@@ -3652,6 +3661,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)
 
@@ -3664,13 +3675,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,
@@ -3723,6 +3741,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,

nova/compute/instance_actions.py

@@ -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'

nova/compute/manager.py

@@ -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):

nova/tests/unit/compute/test_compute.py

@@ -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')
+             mock.patch.object(cinder.API, 'attach'),
-        ) as (mock_attach_and_reserve, mock_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',

nova/tests/unit/compute/test_compute_api.py

@@ -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()
 

nova/tests/unit/compute/test_compute_mgr.py

@@ -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))

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

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