Add instance action record for snapshot instances

We currently don't record snapshot instance
actions. This is useful for auditing and debugging.

This patch adds instance snapshot actions.

partial-implements: blueprint fill-the-gap-for-instance-action-records

Change-Id: I9ce48e768cc67543f27a6c87c57b47501fff38c2

nova/cells/messaging.py

@@ -909,6 +909,11 @@ class _TargetedMessageMethods(_BaseMessageMethods):
         instance.refresh()
         instance.task_state = task_states.IMAGE_SNAPSHOT_PENDING
         instance.save(expected_task_state=[None])
+
+        objects.InstanceAction.action_start(
+            message.ctxt, instance.uuid, instance_actions.CREATE_IMAGE,
+            want_result=False)
+
         self.compute_rpcapi.snapshot_instance(message.ctxt,
                                               instance,
                                               image_id)

nova/compute/api.py

@@ -2658,6 +2658,9 @@ class API(base.Base):
                                            state=state,
                                            method='snapshot')
 
+        self._record_action_start(context, instance,
+                                  instance_actions.CREATE_IMAGE)
+
         self.compute_rpcapi.snapshot_instance(context, instance,
                                               image_meta['id'])
 
@@ -2767,29 +2770,39 @@ class API(base.Base):
         bdms = objects.BlockDeviceMappingList.get_by_instance_uuid(
                 context, instance.uuid)
 
-        mapping = []
+        @wrap_instance_event(prefix='api')
-        for bdm in bdms:
+        def snapshot_instance(self, context, instance, bdms):
-            if bdm.no_device:
+            mapping = []
-                continue
+            for bdm in bdms:
+                if bdm.no_device:
+                    continue
 
-            if bdm.is_volume:
+                if bdm.is_volume:
-                # create snapshot based on volume_id
+                    # create snapshot based on volume_id
-                volume = self.volume_api.get(context, bdm.volume_id)
+                    volume = self.volume_api.get(context, bdm.volume_id)
-                # NOTE(yamahata): Should we wait for snapshot creation?
+                    # NOTE(yamahata): Should we wait for snapshot creation?
-                #                 Linux LVM snapshot creation completes in
+                    #                 Linux LVM snapshot creation completes in
-                #                 short time, it doesn't matter for now.
+                    #                 short time, it doesn't matter for now.
-                name = _('snapshot for %s') % image_meta['name']
+                    name = _('snapshot for %s') % image_meta['name']
-                LOG.debug('Creating snapshot from volume %s.', volume['id'],
+                    LOG.debug('Creating snapshot from volume %s.',
-                          instance=instance)
+                              volume['id'],
-                snapshot = self.volume_api.create_snapshot_force(
+                              instance=instance)
-                    context, volume['id'], name, volume['display_description'])
+                    snapshot = self.volume_api.create_snapshot_force(
-                mapping_dict = block_device.snapshot_from_bdm(snapshot['id'],
+                        context, volume['id'], name,
-                                                              bdm)
+                        volume['display_description'])
-                mapping_dict = mapping_dict.get_image_mapping()
+                    mapping_dict = block_device.snapshot_from_bdm(
-            else:
+                        snapshot['id'],
-                mapping_dict = bdm.get_image_mapping()
+                        bdm)
+                    mapping_dict = mapping_dict.get_image_mapping()
+                else:
+                    mapping_dict = bdm.get_image_mapping()
 
-            mapping.append(mapping_dict)
+                mapping.append(mapping_dict)
+            return mapping
+
+        self._record_action_start(context, instance,
+                                  instance_actions.CREATE_IMAGE)
+        mapping = snapshot_instance(self, context, instance, bdms)
 
         if quiesced:
             self.compute_rpcapi.unquiesce_instance(context, instance, mapping)

nova/compute/instance_actions.py

@@ -69,3 +69,4 @@ SWAP_VOLUME = 'swap_volume'
 LOCK = 'lock'
 UNLOCK = 'unlock'
 BACKUP = 'createBackup'
+CREATE_IMAGE = 'createImage'

nova/compute/manager.py

@@ -3229,6 +3229,7 @@ class ComputeManager(manager.Manager):
 
     @wrap_exception()
     @reverts_task_state
+    @wrap_instance_event(prefix='compute')
     @wrap_instance_fault
     @delete_image_on_error
     def snapshot_instance(self, context, image_id, instance):

nova/tests/unit/cells/test_cells_messaging.py

@@ -1343,8 +1343,9 @@ class CellsTargetedMethodsTestCase(test.NoDBTestCase):
         self._test_instance_action_method('inject_network_info',
                                           (), {}, (), {}, False)
 
-    def test_snapshot_instance(self):
+    @mock.patch.object(objects.InstanceAction, 'action_start')
-        inst = objects.Instance()
+    def test_snapshot_instance(self, action_start):
+        inst = objects.Instance(uuid=uuids.instance)
         meth_cls = self.tgt_methods_cls
 
         self.mox.StubOutWithMock(inst, 'refresh')
@@ -1371,6 +1372,9 @@ class CellsTargetedMethodsTestCase(test.NoDBTestCase):
         message.need_response = False
 
         meth_cls.snapshot_instance(message, inst, image_id='image-id')
+        action_start.assert_called_once_with(
+            message.ctxt, inst.uuid, instance_actions.CREATE_IMAGE,
+            want_result=False)
 
     @mock.patch.object(objects.InstanceAction, 'action_start')
     def test_backup_instance(self, action_start):

nova/tests/unit/compute/test_compute.py

@@ -3392,14 +3392,17 @@ class ComputeTestCase(BaseTestCase,
                       fake_delete)
 
         inst_obj = self._get_snapshotting_instance()
-        if method == 'snapshot':
+        with mock.patch.object(compute_utils,
-            self.assertRaises(test.TestingException,
+                               'EventReporter') as mock_event:
-                              self.compute.snapshot_instance,
+            if method == 'snapshot':
-                              self.context, image_id='fakesnap',
+                self.assertRaises(test.TestingException,
-                              instance=inst_obj)
+                                  self.compute.snapshot_instance,
-        else:
+                                  self.context, image_id='fakesnap',
-            with mock.patch.object(compute_utils,
+                                  instance=inst_obj)
-                                   'EventReporter') as mock_event:
+                mock_event.assert_called_once_with(self.context,
+                                                   'compute_snapshot_instance',
+                                                   inst_obj.uuid)
+            else:
                 self.assertRaises(test.TestingException,
                                   self.compute.backup_instance,
                                   self.context, image_id='fakesnap',

nova/tests/unit/compute/test_compute_api.py

@@ -2594,6 +2594,8 @@ class _ComputeAPIUnitTestMixIn(object):
                 res = self.compute_api.snapshot(self.context, instance,
                                           'fake-name',
                                           extra_properties=extra_props)
+                mock_record.assert_called_once_with(
+                    self.context, instance, instance_actions.CREATE_IMAGE)
             else:
                 res = self.compute_api.backup(self.context, instance,
                                         'fake-name',
@@ -2821,9 +2823,20 @@ class _ComputeAPIUnitTestMixIn(object):
                        fake_unquiesce_instance)
         fake_image.stub_out_image_service(self)
 
-        # No block devices defined
+        with test.nested(
-        self.compute_api.snapshot_volume_backed(
+                mock.patch.object(compute_api.API, '_record_action_start'),
-            self.context, instance, 'test-snapshot')
+                mock.patch.object(compute_utils, 'EventReporter')) as (
+            mock_record, mock_event):
+            # No block devices defined
+            self.compute_api.snapshot_volume_backed(
+                self.context, instance, 'test-snapshot')
+
+        mock_record.assert_called_once_with(self.context,
+                                            instance,
+                                            instance_actions.CREATE_IMAGE)
+        mock_event.assert_called_once_with(self.context,
+                                           'api_snapshot_instance',
+                                           instance.uuid)
 
         bdm = fake_block_device.FakeDbBlockDeviceDict(
                 {'no_device': False, 'volume_id': '1', 'boot_index': 0,
@@ -2843,13 +2856,24 @@ class _ComputeAPIUnitTestMixIn(object):
              'destination_type': 'volume', 'delete_on_termination': False,
              'tag': None})
 
-        # All the db_only fields and the volume ones are removed
+        with test.nested(
-        self.compute_api.snapshot_volume_backed(
+                mock.patch.object(compute_api.API, '_record_action_start'),
-            self.context, instance, 'test-snapshot')
+                mock.patch.object(compute_utils, 'EventReporter')) as (
+                mock_record, mock_event):
+            # All the db_only fields and the volume ones are removed
+            self.compute_api.snapshot_volume_backed(
+                self.context, instance, 'test-snapshot')
 
         self.assertEqual(quiesce_expected, quiesced[0])
         self.assertEqual(quiesce_expected, quiesced[1])
 
+        mock_record.assert_called_once_with(self.context,
+                                            instance,
+                                            instance_actions.CREATE_IMAGE)
+        mock_event.assert_called_once_with(self.context,
+                                           'api_snapshot_instance',
+                                           instance.uuid)
+
         instance.system_metadata['image_mappings'] = jsonutils.dumps(
             [{'virtual': 'ami', 'device': 'vda'},
              {'device': 'vda', 'virtual': 'ephemeral0'},
@@ -2882,13 +2906,25 @@ class _ComputeAPIUnitTestMixIn(object):
 
         quiesced = [False, False]
 
-        # Check that the mappings from the image properties are not included
+        with test.nested(
-        self.compute_api.snapshot_volume_backed(
+                mock.patch.object(compute_api.API, '_record_action_start'),
-            self.context, instance, 'test-snapshot')
+                mock.patch.object(compute_utils, 'EventReporter')) as (
+                mock_record, mock_event):
+            # Check that the mappings from the image properties are not
+            # included
+            self.compute_api.snapshot_volume_backed(
+                self.context, instance, 'test-snapshot')
 
         self.assertEqual(quiesce_expected, quiesced[0])
         self.assertEqual(quiesce_expected, quiesced[1])
 
+        mock_record.assert_called_once_with(self.context,
+                                            instance,
+                                            instance_actions.CREATE_IMAGE)
+        mock_event.assert_called_once_with(self.context,
+                                           'api_snapshot_instance',
+                                           instance.uuid)
+
     def test_snapshot_volume_backed(self):
         self._test_snapshot_volume_backed(False, False)
 

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

@@ -12,3 +12,4 @@ features:
     * unlock
     * shelveOffload
     * createBackup
+    * createImage