From 762380e0ee1fb09462658a75216f35f7759ec122 Mon Sep 17 00:00:00 2001 From: Kevin_Zheng Date: Mon, 16 Oct 2017 14:57:17 +0800 Subject: [PATCH] Add instance action record for attach/detach interface We currently don't record attach/detach interface instance actions. This is useful for auditing and debuging. This patch adds attach/detach interface actions. Change-Id: I0874f52ca7c5e29d9cd619679e1e121b25b6b41e partial-implements: blueprint fill-the-gap-for-instance-action-records --- nova/compute/api.py | 4 ++++ nova/compute/instance_actions.py | 2 ++ nova/compute/manager.py | 2 ++ nova/tests/unit/compute/test_compute.py | 22 ++++++------------- nova/tests/unit/compute/test_compute_api.py | 15 ++++++++++++- nova/tests/unit/compute/test_compute_mgr.py | 20 ++++++++++++----- ...e-action-record-gaps-14b36eba313d6d87.yaml | 7 ++++++ 7 files changed, 51 insertions(+), 21 deletions(-) create mode 100644 releasenotes/notes/fill-instance-action-record-gaps-14b36eba313d6d87.yaml diff --git a/nova/compute/api.py b/nova/compute/api.py index 86a775f498db..32d700bd4d96 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -3742,6 +3742,8 @@ class API(base.Base): def attach_interface(self, context, instance, network_id, port_id, requested_ip, tag=None): """Use hotplug to add an network adapter to an instance.""" + self._record_action_start( + context, instance, instance_actions.ATTACH_INTERFACE) return self.compute_rpcapi.attach_interface(context, instance=instance, network_id=network_id, port_id=port_id, requested_ip=requested_ip, tag=tag) @@ -3752,6 +3754,8 @@ class API(base.Base): task_state=[None]) def detach_interface(self, context, instance, port_id): """Detach an network adapter from an instance.""" + self._record_action_start( + context, instance, instance_actions.DETACH_INTERFACE) self.compute_rpcapi.detach_interface(context, instance=instance, port_id=port_id) diff --git a/nova/compute/instance_actions.py b/nova/compute/instance_actions.py index 2faf1c4fa3ac..7b223e774fa3 100644 --- a/nova/compute/instance_actions.py +++ b/nova/compute/instance_actions.py @@ -60,3 +60,5 @@ TRIGGER_CRASH_DUMP = 'trigger_crash_dump' # is used for tracking this asynchronous operation so the user/admin can know # when it is done in case they need/want to reboot the guest operating system. EXTEND_VOLUME = 'extend_volume' +ATTACH_INTERFACE = 'attach_interface' +DETACH_INTERFACE = 'detach_interface' diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 3ea26bf9708a..b3840a0a1d1b 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -5469,6 +5469,7 @@ class ComputeManager(manager.Manager): pass @wrap_exception() + @wrap_instance_event(prefix='compute') @wrap_instance_fault def attach_interface(self, context, instance, network_id, port_id, requested_ip, tag=None): @@ -5532,6 +5533,7 @@ class ComputeManager(manager.Manager): return network_info[0] @wrap_exception() + @wrap_instance_event(prefix='compute') @wrap_instance_fault def detach_interface(self, context, instance, port_id): """Detach a network adapter from an instance.""" diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 1cfa02ecc04d..e273fef91af9 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -9823,11 +9823,7 @@ class ComputeAPITestCase(BaseTestCase): @mock.patch.object(compute_utils, 'notify_about_instance_action') def test_attach_interface(self, mock_notify): - new_type = flavors.get_flavor_by_flavor_id('4') - instance = objects.Instance(image_ref=uuids.image_instance, - system_metadata={}, - flavor=new_type, - host='fake-host') + instance = self._create_fake_instance_obj() nwinfo = [fake_network_cache_model.new_vif()] network_id = nwinfo[0]['network']['id'] port_id = nwinfo[0]['id'] @@ -9845,7 +9841,7 @@ class ComputeAPITestCase(BaseTestCase): self.assertEqual(vif['id'], network_id) mock_allocate.assert_called_once_with( self.context, instance, port_id, network_id, req_ip, - bind_host_id='fake-host', tag=None) + bind_host_id='fake-mini', tag=None) mock_notify.assert_has_calls([ mock.call(self.context, instance, self.compute.host, action='interface_attach', phase='start'), @@ -9855,11 +9851,7 @@ class ComputeAPITestCase(BaseTestCase): @mock.patch.object(compute_utils, 'notify_about_instance_action') def test_interface_tagged_attach(self, mock_notify): - new_type = flavors.get_flavor_by_flavor_id('4') - instance = objects.Instance(image_ref=uuids.image_instance, - system_metadata={}, - flavor=new_type, - host='fake-host') + instance = self._create_fake_instance_obj() nwinfo = [fake_network_cache_model.new_vif()] network_id = nwinfo[0]['network']['id'] port_id = nwinfo[0]['id'] @@ -9878,7 +9870,7 @@ class ComputeAPITestCase(BaseTestCase): self.assertEqual(vif['id'], network_id) mock_allocate.assert_called_once_with( self.context, instance, port_id, network_id, req_ip, - bind_host_id='fake-host', tag='foo') + bind_host_id='fake-mini', tag='foo') mock_notify.assert_has_calls([ mock.call(self.context, instance, self.compute.host, action='interface_attach', phase='start'), @@ -9947,7 +9939,7 @@ class ComputeAPITestCase(BaseTestCase): self.stub_out('nova.network.api.API.' 'deallocate_port_for_instance', lambda a, b, c, d: []) - instance = objects.Instance() + instance = self._create_fake_instance_obj() instance.info_cache = objects.InstanceInfoCache.new( self.context, uuids.info_cache_instance) instance.info_cache.network_info = network_model.NetworkInfo.hydrate( @@ -9962,7 +9954,7 @@ class ComputeAPITestCase(BaseTestCase): def test_detach_interface_failed(self): nwinfo, port_id = self.test_attach_interface() - instance = objects.Instance(id=42) + instance = self._create_fake_instance_obj() instance['uuid'] = uuids.info_cache_instance instance.info_cache = objects.InstanceInfoCache.new( self.context, uuids.info_cache_instance) @@ -9990,7 +9982,7 @@ class ComputeAPITestCase(BaseTestCase): # Tests that when deallocate_port_for_instance fails we log the failure # before exiting compute.detach_interface. nwinfo, port_id = self.test_attach_interface() - instance = objects.Instance(id=42, uuid=uuids.fake) + instance = self._create_fake_instance_obj() instance.info_cache = objects.InstanceInfoCache.new( self.context, uuids.info_cache_instance) instance.info_cache.network_info = network_model.NetworkInfo.hydrate( diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index 8a34b5259faa..2b0001ea1b21 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -5072,14 +5072,27 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase): self.assertItemsEqual(['default', uuids.secgroup_uuid], security_groups) + @mock.patch('nova.compute.api.API._record_action_start') @mock.patch.object(compute_rpcapi.ComputeAPI, 'attach_interface') - def test_tagged_interface_attach(self, mock_attach): + def test_tagged_interface_attach(self, mock_attach, mock_record): instance = self._create_instance_obj() self.compute_api.attach_interface(self.context, instance, None, None, None, tag='foo') mock_attach.assert_called_with(self.context, instance=instance, network_id=None, port_id=None, requested_ip=None, tag='foo') + mock_record.assert_called_once_with( + self.context, instance, instance_actions.ATTACH_INTERFACE) + + @mock.patch('nova.compute.api.API._record_action_start') + @mock.patch.object(compute_rpcapi.ComputeAPI, 'detach_interface') + def test_detach_interface(self, mock_detach, mock_record): + instance = self._create_instance_obj() + self.compute_api.detach_interface(self.context, instance, None) + mock_detach.assert_called_with(self.context, instance=instance, + port_id=None) + mock_record.assert_called_once_with( + self.context, instance, instance_actions.DETACH_INTERFACE) class Cellsv1DeprecatedTestMixIn(object): diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 5953015a43f0..6dc9e471ef8f 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -1783,6 +1783,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): db_instance) e = exception.InterfaceAttachFailed(instance_uuid=f_instance.uuid) + @mock.patch.object(compute_utils, 'EventReporter') @mock.patch.object(compute_utils, 'notify_about_instance_action') @mock.patch.object(compute_utils, 'add_instance_fault_from_exc') @mock.patch.object(self.compute.network_api, @@ -1790,7 +1791,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): side_effect=e) @mock.patch.object(self.compute, '_instance_update', side_effect=lambda *a, **k: {}) - def do_test(update, meth, add_fault, notify): + def do_test(update, meth, add_fault, notify, event): self.assertRaises(exception.InterfaceAttachFailed, self.compute.attach_interface, self.context, f_instance, 'net_id', 'port_id', @@ -1798,6 +1799,9 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): add_fault.assert_has_calls([ mock.call(self.context, f_instance, e, mock.ANY)]) + event.assert_called_once_with( + self.context, 'compute_attach_interface', + f_instance.uuid) with mock.patch.dict(self.compute.driver.capabilities, supports_attach_interface=True): @@ -1807,18 +1811,24 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): # Test that the fault methods are invoked when a detach fails # Build test data that will cause a PortNotFound exception - f_instance = mock.MagicMock() - f_instance.info_cache = mock.MagicMock() - f_instance.info_cache.network_info = [] + nw_info = network_model.NetworkInfo([]) + info_cache = objects.InstanceInfoCache(network_info=nw_info, + instance_uuid=uuids.instance) + f_instance = objects.Instance(id=3, uuid=uuids.instance, + info_cache=info_cache) + @mock.patch.object(compute_utils, 'EventReporter') @mock.patch.object(compute_utils, 'add_instance_fault_from_exc') @mock.patch.object(self.compute, '_set_instance_obj_error_state') - def do_test(meth, add_fault): + def do_test(meth, add_fault, event): self.assertRaises(exception.PortNotFound, self.compute.detach_interface, self.context, f_instance, 'port_id') add_fault.assert_has_calls( [mock.call(self.context, f_instance, mock.ANY, mock.ANY)]) + event.assert_called_once_with( + self.context, 'compute_detach_interface', + f_instance.uuid) do_test() diff --git a/releasenotes/notes/fill-instance-action-record-gaps-14b36eba313d6d87.yaml b/releasenotes/notes/fill-instance-action-record-gaps-14b36eba313d6d87.yaml new file mode 100644 index 000000000000..89cd5144d5be --- /dev/null +++ b/releasenotes/notes/fill-instance-action-record-gaps-14b36eba313d6d87.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + The following instance action records have been added: + + * attach_interface + * detach_interface