From d3097e52b3c5138a1521422dd2cea1c1f4f173da Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Tue, 13 Dec 2016 17:04:23 +0100 Subject: [PATCH] Transform missing delete notifications The Iddbe50ce0ad3c14562df800bbc09ec5a7e840485 patch only considered instance delete in the happy case when the instance is scheduled to a compute successfully and the compute is available when the delete action is executed. If the instance is never scheduled to a compute or the compute is not available when the instance is deleted legacy delete notifications are emitted from different places, compute.api instead of compute.manager. The original patch missed these places. There will be subsequent patch(es) handling the same edge cases for soft_delete and force_delete. Change-Id: If0693eab2ed31b5fbfe6cbafa5d67b69c2ed8442 Implements: bp versioned-notification-transformation-stein --- .../instance-delete-end_compute_down.json | 15 ++++++ .../instance-delete-end_not_scheduled.json | 19 +++++++ .../instance-delete-start_compute_down.json | 11 +++++ .../instance-delete-start_not_scheduled.json | 18 +++++++ nova/compute/manager.py | 3 +- nova/compute/utils.py | 22 ++++++++- nova/conductor/manager.py | 3 +- nova/tests/functional/api/client.py | 12 +++++ .../test_instance.py | 49 +++++++++++++++++++ nova/tests/unit/compute/test_compute.py | 34 ++++++++----- nova/tests/unit/compute/test_compute_api.py | 46 +++++++++++++---- nova/tests/unit/compute/test_compute_utils.py | 12 ++++- nova/tests/unit/conductor/test_conductor.py | 31 ++++++++---- nova/tests/unit/utils.py | 40 +++++++++++++-- 14 files changed, 276 insertions(+), 39 deletions(-) create mode 100644 doc/notification_samples/instance-delete-end_compute_down.json create mode 100644 doc/notification_samples/instance-delete-end_not_scheduled.json create mode 100644 doc/notification_samples/instance-delete-start_compute_down.json create mode 100644 doc/notification_samples/instance-delete-start_not_scheduled.json diff --git a/doc/notification_samples/instance-delete-end_compute_down.json b/doc/notification_samples/instance-delete-end_compute_down.json new file mode 100644 index 000000000000..d346095eb344 --- /dev/null +++ b/doc/notification_samples/instance-delete-end_compute_down.json @@ -0,0 +1,15 @@ +{ + "event_type":"instance.delete.end", + "payload":{ + "$ref":"common_payloads/InstanceActionPayload.json#", + "nova_object.data":{ + "block_devices":[], + "deleted_at":"2012-10-29T13:42:11Z", + "ip_addresses":[], + "state":"deleted", + "terminated_at":"2012-10-29T13:42:11Z" + } + }, + "priority":"INFO", + "publisher_id":"nova-api:fake-mini" +} diff --git a/doc/notification_samples/instance-delete-end_not_scheduled.json b/doc/notification_samples/instance-delete-end_not_scheduled.json new file mode 100644 index 000000000000..7cd0045e5427 --- /dev/null +++ b/doc/notification_samples/instance-delete-end_not_scheduled.json @@ -0,0 +1,19 @@ +{ + "event_type":"instance.delete.end", + "payload":{ + "$ref":"common_payloads/InstanceActionPayload.json#", + "nova_object.data":{ + "block_devices":[], + "deleted_at":"2012-10-29T13:42:11Z", + "host":null, + "ip_addresses":[], + "launched_at":null, + "node":null, + "power_state":"pending", + "state":"deleted", + "terminated_at":"2012-10-29T13:42:11Z" + } + }, + "priority":"INFO", + "publisher_id":"nova-api:fake-mini" +} diff --git a/doc/notification_samples/instance-delete-start_compute_down.json b/doc/notification_samples/instance-delete-start_compute_down.json new file mode 100644 index 000000000000..e3ceaf56691e --- /dev/null +++ b/doc/notification_samples/instance-delete-start_compute_down.json @@ -0,0 +1,11 @@ +{ + "event_type":"instance.delete.start", + "payload":{ + "$ref":"common_payloads/InstanceActionPayload.json#", + "nova_object.data":{ + "task_state":"deleting" + } + }, + "priority":"INFO", + "publisher_id":"nova-api:fake-mini" +} diff --git a/doc/notification_samples/instance-delete-start_not_scheduled.json b/doc/notification_samples/instance-delete-start_not_scheduled.json new file mode 100644 index 000000000000..32be5a3bb4d2 --- /dev/null +++ b/doc/notification_samples/instance-delete-start_not_scheduled.json @@ -0,0 +1,18 @@ +{ + "event_type":"instance.delete.start", + "payload":{ + "$ref":"common_payloads/InstanceActionPayload.json#", + "nova_object.data":{ + "block_devices":[], + "host":null, + "ip_addresses":[], + "launched_at":null, + "node":null, + "power_state":"pending", + "state":"error", + "task_state":"deleting" + } + }, + "priority":"INFO", + "publisher_id":"nova-api:fake-mini" +} diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 326b7f9dec1f..9c78593bb52d 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -2824,7 +2824,8 @@ class ComputeManager(manager.Manager): def soft_delete_instance(self, context, instance): """Soft delete an instance on this host.""" with compute_utils.notify_about_instance_delete( - self.notifier, context, instance, 'soft_delete'): + self.notifier, context, instance, 'soft_delete', + fields.NotificationSource.COMPUTE): compute_utils.notify_about_instance_action(context, instance, self.host, action=fields.NotificationAction.SOFT_DELETE, phase=fields.NotificationPhase.START) diff --git a/nova/compute/utils.py b/nova/compute/utils.py index f03dda8d9256..cf78c8aaaeb6 100644 --- a/nova/compute/utils.py +++ b/nova/compute/utils.py @@ -1157,11 +1157,31 @@ class UnlimitedSemaphore(object): @contextlib.contextmanager def notify_about_instance_delete(notifier, context, instance, - delete_type='delete'): + delete_type='delete', + source=fields.NotificationSource.API): try: notify_about_instance_usage(notifier, context, instance, "%s.start" % delete_type) + # Note(gibi): soft_delete and force_delete types will be handled in a + # subsequent patch + if delete_type == 'delete': + notify_about_instance_action( + context, + instance, + host=CONF.host, + source=source, + action=fields.NotificationAction.DELETE, + phase=fields.NotificationPhase.START) + yield finally: notify_about_instance_usage(notifier, context, instance, "%s.end" % delete_type) + if delete_type == 'delete': + notify_about_instance_action( + context, + instance, + host=CONF.host, + source=source, + action=fields.NotificationAction.DELETE, + phase=fields.NotificationPhase.END) diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index 1409720c914c..14df9c1e057e 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -1419,7 +1419,8 @@ class ComputeTaskManager(base.Base): # bdm, tags and instance record. with obj_target_cell(instance, cell) as cctxt: with compute_utils.notify_about_instance_delete( - self.notifier, cctxt, instance): + self.notifier, cctxt, instance, + source=fields.NotificationSource.CONDUCTOR): try: instance.destroy() except exception.InstanceNotFound: diff --git a/nova/tests/functional/api/client.py b/nova/tests/functional/api/client.py index 7b56e17b4812..718e8ae37ad9 100644 --- a/nova/tests/functional/api/client.py +++ b/nova/tests/functional/api/client.py @@ -506,3 +506,15 @@ class TestOpenStackClient(object): def get_hypervisor_stats(self): return self.api_get( '/os-hypervisors/statistics').body['hypervisor_statistics'] + + def get_service_id(self, binary_name): + for service in self.get_services(): + if service['binary'] == binary_name: + return service['id'] + raise OpenStackApiNotFoundException('Service cannot be found.') + + def put_service_force_down(self, service_id, forced_down): + req = { + 'forced_down': forced_down + } + return self.api_put('os-services/%s' % service_id, req).body['service'] diff --git a/nova/tests/functional/notification_sample_tests/test_instance.py b/nova/tests/functional/notification_sample_tests/test_instance.py index 127fc928a3b3..9011919cd4b9 100644 --- a/nova/tests/functional/notification_sample_tests/test_instance.py +++ b/nova/tests/functional/notification_sample_tests/test_instance.py @@ -455,6 +455,25 @@ class TestInstanceNotificationSample( 'fault.traceback': self.ANY}, actual=fake_notifier.VERSIONED_NOTIFICATIONS[1]) + fake_notifier.reset() + + self.api.delete_server(server['id']) + self._wait_until_deleted(server) + + self.assertEqual(2, len(fake_notifier.VERSIONED_NOTIFICATIONS)) + self._verify_notification( + 'instance-delete-start_not_scheduled', + replacements={ + 'reservation_id': server['reservation_id'], + 'uuid': server['id']}, + actual=fake_notifier.VERSIONED_NOTIFICATIONS[0]) + self._verify_notification( + 'instance-delete-end_not_scheduled', + replacements={ + 'reservation_id': server['reservation_id'], + 'uuid': server['id']}, + actual=fake_notifier.VERSIONED_NOTIFICATIONS[1]) + def test_instance_exists_usage_audit(self): # TODO(xavvior): Should create a functional test for the # "instance_usage_audit" periodic task. We didn't find usable @@ -495,6 +514,36 @@ class TestInstanceNotificationSample( }, actual=notifications[0]) + def test_delete_server_while_compute_is_down(self): + + server = self._boot_a_server( + expected_status='ACTIVE', + extra_params={'networks': [{'port': self.neutron.port_1['id']}]}) + self._attach_volume_to_server(server, self.cinder.SWAP_OLD_VOL) + + service_id = self.api.get_service_id('nova-compute') + self.admin_api.put_service_force_down(service_id, True) + fake_notifier.reset() + + self.api.delete_server(server['id']) + self._wait_until_deleted(server) + + self.assertEqual(2, len(fake_notifier.VERSIONED_NOTIFICATIONS)) + self._verify_notification( + 'instance-delete-start_compute_down', + replacements={ + 'reservation_id': server['reservation_id'], + 'uuid': server['id']}, + actual=fake_notifier.VERSIONED_NOTIFICATIONS[0]) + self._verify_notification( + 'instance-delete-end_compute_down', + replacements={ + 'reservation_id': server['reservation_id'], + 'uuid': server['id']}, + actual=fake_notifier.VERSIONED_NOTIFICATIONS[1]) + + self.admin_api.put_service_force_down(service_id, False) + def _verify_instance_update_steps(self, steps, notifications, initial=None): replacements = {} diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index f4d5c9b7ffa1..674de0ec6604 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -8273,6 +8273,7 @@ class ComputeTestCase(BaseTestCase, block_device_mapping=[]) self.assertEqual('Preserve this', instance.fault.message) + @mock.patch('nova.compute.utils.notify_about_instance_action') @mock.patch('nova.objects.Instance.destroy') @mock.patch('nova.context.target_cell') @mock.patch('nova.objects.InstanceMapping.get_by_instance_uuid') @@ -8280,8 +8281,8 @@ class ComputeTestCase(BaseTestCase, @mock.patch('nova.compute.utils.notify_about_instance_usage') @mock.patch('nova.objects.BuildRequest.get_by_instance_uuid') def test_delete_while_booting_instance_not_in_cell_db_cellsv2( - self, br_get_by_instance, notify, minimum_server_version, - im_get_by_instance, target_cell, instance_destroy): + self, br_get_by_instance, legacy_notify, minimum_server_version, + im_get_by_instance, target_cell, instance_destroy, notify): minimum_server_version.return_value = 15 im_get_by_instance.return_value = mock.Mock() @@ -8297,15 +8298,17 @@ class ComputeTestCase(BaseTestCase, # the instance is updated during the delete so we only match by uuid test_utils.assert_instance_delete_notification_by_uuid( - notify, instance.uuid, self.compute_api.notifier, self.context) + legacy_notify, notify, instance.uuid, self.compute_api.notifier, + self.context) + @mock.patch('nova.compute.utils.notify_about_instance_action') @mock.patch('nova.objects.Instance.destroy') @mock.patch('nova.objects.Service.get_minimum_version') @mock.patch('nova.compute.utils.notify_about_instance_usage') @mock.patch('nova.objects.BuildRequest.get_by_instance_uuid') def test_delete_while_booting_instance_not_in_cell_db_cellsv1( - self, br_get_by_instance, notify, minimum_server_version, - instance_destroy): + self, br_get_by_instance, legacy_notify, minimum_server_version, + instance_destroy, notify): minimum_server_version.return_value = 14 @@ -8316,15 +8319,17 @@ class ComputeTestCase(BaseTestCase, self.compute_api._delete_instance(self.context, instance) test_utils.assert_instance_delete_notification_by_uuid( - notify, instance.uuid, self.compute_api.notifier, self.context) + legacy_notify, notify, instance.uuid, self.compute_api.notifier, + self.context) + @mock.patch('nova.compute.utils.notify_about_instance_action') @mock.patch('nova.objects.Instance.destroy') @mock.patch('nova.objects.InstanceMapping.get_by_instance_uuid') @mock.patch('nova.compute.utils.notify_about_instance_usage') @mock.patch('nova.objects.BuildRequest.get_by_instance_uuid') def test_delete_while_booting_instance_not_scheduled_cellv1( - self, br_get_by_instance, notify, im_get_by_instance, - instance_destroy): + self, br_get_by_instance, legacy_notify, im_get_by_instance, + instance_destroy, notify): instance = self._create_fake_instance_obj() instance.host = None @@ -8340,16 +8345,20 @@ class ComputeTestCase(BaseTestCase, self.compute_api._delete_instance(self.context, instance) test_utils.assert_instance_delete_notification_by_uuid( - notify, instance.uuid, self.compute_api.notifier, self.context) + legacy_notify, notify, instance.uuid, self.compute_api.notifier, + self.context) + instance_destroy.assert_called_once_with() + + @mock.patch('nova.compute.utils.notify_about_instance_action') @mock.patch('nova.objects.Instance.destroy') @mock.patch('nova.context.target_cell') @mock.patch('nova.objects.InstanceMapping.get_by_instance_uuid') @mock.patch('nova.compute.utils.notify_about_instance_usage') @mock.patch('nova.objects.BuildRequest.get_by_instance_uuid') def test_delete_while_booting_instance_not_scheduled_cellv2( - self, br_get_by_instance, notify, im_get_by_instance, target_cell, - instance_destroy): + self, br_get_by_instance, legacy_notify, im_get_by_instance, + target_cell, instance_destroy, notify): target_cell.return_value.__enter__.return_value = self.context instance = self._create_fake_instance_obj() @@ -8367,7 +8376,8 @@ class ComputeTestCase(BaseTestCase, instance_destroy.assert_called_once_with() test_utils.assert_instance_delete_notification_by_uuid( - notify, instance.uuid, self.compute_api.notifier, self.context) + legacy_notify, notify, instance.uuid, self.compute_api.notifier, + self.context) @ddt.ddt diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index eeb448815c92..cd46be7c8112 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -1006,6 +1006,8 @@ class _ComputeAPIUnitTestMixIn(object): return snapshot_id + @mock.patch.object(compute_utils, + 'notify_about_instance_action') @mock.patch.object(objects.Migration, 'get_by_instance_and_status') @mock.patch.object(image_api.API, 'delete') @mock.patch.object(objects.InstanceMapping, 'save') @@ -1028,8 +1030,9 @@ class _ComputeAPIUnitTestMixIn(object): def _test_delete(self, delete_type, mock_save, mock_bdm_get, mock_elevated, mock_get_cn, mock_up, mock_record, mock_inst_update, mock_deallocate, mock_inst_meta, mock_inst_destroy, - mock_notify, mock_del_token, mock_get_inst, mock_save_im, - mock_image_delete, mock_mig_get, **attrs): + mock_notify_legacy, mock_del_token, mock_get_inst, + mock_save_im, mock_image_delete, mock_mig_get, + mock_notify, **attrs): expected_save_calls = [mock.call()] expected_record_calls = [] expected_elevated_calls = [] @@ -1154,13 +1157,13 @@ class _ComputeAPIUnitTestMixIn(object): test.MatchType(objects.Service)) if is_downed_host: if 'soft' in delete_type: - mock_notify.assert_has_calls([ + mock_notify_legacy.assert_has_calls([ mock.call(self.compute_api.notifier, self.context, inst, 'delete.start'), mock.call(self.compute_api.notifier, self.context, inst, 'delete.end')]) else: - mock_notify.assert_has_calls([ + mock_notify_legacy.assert_has_calls([ mock.call(self.compute_api.notifier, self.context, inst, '%s.start' % delete_type), mock.call(self.compute_api.notifier, self.context, @@ -1186,6 +1189,14 @@ class _ComputeAPIUnitTestMixIn(object): if is_shelved: mock_image_delete.assert_called_once_with(self.context, snapshot_id) + if not cast and delete_type == 'delete': + mock_notify.assert_has_calls([ + mock.call(self.context, inst, host='fake-mini', + source='nova-api', + action=delete_type, phase='start'), + mock.call(self.context, inst, host='fake-mini', + source='nova-api', + action=delete_type, phase='end')]) def test_delete(self): self._test_delete('delete') @@ -1325,6 +1336,7 @@ class _ComputeAPIUnitTestMixIn(object): self._test_delete('force_delete', vm_state=vm_state, task_state=task_states.RESIZE_MIGRATING) + @mock.patch.object(compute_utils, 'notify_about_instance_action') @mock.patch.object(compute_utils, 'notify_about_instance_usage') @mock.patch.object(db, 'instance_destroy') @mock.patch.object(db, 'constraint') @@ -1333,7 +1345,8 @@ class _ComputeAPIUnitTestMixIn(object): @mock.patch.object(objects.BuildRequest, 'get_by_instance_uuid') def test_delete_fast_if_host_not_set(self, mock_br_get, mock_save, mock_bdm_get, mock_cons, - mock_inst_destroy, mock_notify): + mock_inst_destroy, + mock_notify_legacy, mock_notify): self.useFixture(nova_fixtures.AllServicesCurrent()) inst = self._create_instance_obj() inst.host = '' @@ -1382,11 +1395,17 @@ class _ComputeAPIUnitTestMixIn(object): test.MatchType(objects.BlockDeviceMappingList), delete_type='delete') else: - mock_notify.assert_has_calls([ + mock_notify_legacy.assert_has_calls([ mock.call(self.compute_api.notifier, self.context, inst, 'delete.start'), mock.call(self.compute_api.notifier, self.context, inst, 'delete.end')]) + mock_notify.assert_has_calls([ + mock.call(self.context, inst, host='fake-mini', + source='nova-api', action='delete', phase='start'), + mock.call(self.context, inst, host='fake-mini', + source='nova-api', action='delete', phase='end')]) + mock_cons.assert_called_once_with(host=mock.ANY) mock_inst_destroy.assert_called_once_with( self.context, instance_uuid, constraint='constraint') @@ -1395,6 +1414,7 @@ class _ComputeAPIUnitTestMixIn(object): rservations=None, local=False): pass + @mock.patch.object(compute_utils, 'notify_about_instance_action') @mock.patch.object(objects.BlockDeviceMapping, 'destroy') @mock.patch.object(cinder.API, 'detach') @mock.patch.object(compute_utils, 'notify_about_instance_usage') @@ -1403,7 +1423,7 @@ class _ComputeAPIUnitTestMixIn(object): @mock.patch.object(objects.Instance, 'destroy') def test_local_delete_with_deleted_volume( self, mock_inst_destroy, mock_elevated, mock_dealloc, - mock_notify, mock_detach, mock_bdm_destroy): + mock_notify_legacy, mock_detach, mock_bdm_destroy, mock_notify): bdms = [objects.BlockDeviceMapping( **fake_block_device.FakeDbBlockDeviceDict( {'id': 42, 'volume_id': 'volume_id', @@ -1419,11 +1439,17 @@ class _ComputeAPIUnitTestMixIn(object): 'delete', self._fake_do_delete) - mock_notify.assert_has_calls([ + mock_notify_legacy.assert_has_calls([ mock.call(self.compute_api.notifier, self.context, inst, 'delete.start'), mock.call(self.compute_api.notifier, self.context, inst, 'delete.end')]) + mock_notify.assert_has_calls([ + mock.call(self.context, inst, host='fake-mini', source='nova-api', + action='delete', phase='start'), + mock.call(self.context, inst, host='fake-mini', source='nova-api', + action='delete', phase='end')]) + mock_elevated.assert_has_calls([mock.call(), mock.call()]) mock_detach.assert_called_once_with(mock.ANY, 'volume_id', inst.uuid) mock_bdm_destroy.assert_called_once_with() @@ -6308,6 +6334,7 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase): fields=['device_id']) self.assertEqual([], instances.objects) + @mock.patch.object(compute_utils, 'notify_about_instance_action') @mock.patch('nova.compute.api.API._delete_while_booting', return_value=False) @mock.patch('nova.compute.api.API._lookup_instance') @@ -6320,7 +6347,8 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase): def _test_delete_volume_backed_instance( self, vm_state, mock_instance_destroy, bdm_destroy, notify_about_instance_usage, mock_save, mock_elevated, - bdm_get_by_instance_uuid, mock_lookup, _mock_del_booting): + bdm_get_by_instance_uuid, mock_lookup, _mock_del_booting, + notify_about_instance_action): volume_id = uuidutils.generate_uuid() conn_info = {'connector': {'host': 'orig-host'}} bdms = [objects.BlockDeviceMapping( diff --git a/nova/tests/unit/compute/test_compute_utils.py b/nova/tests/unit/compute/test_compute_utils.py index bed6dd2fe5be..d0b149c9817d 100644 --- a/nova/tests/unit/compute/test_compute_utils.py +++ b/nova/tests/unit/compute/test_compute_utils.py @@ -1098,10 +1098,12 @@ class ComputeUtilsTestCase(test.NoDBTestCase): self.assertEqual([], addresses) mock_ifaddresses.assert_called_once_with(iface) + @mock.patch('nova.compute.utils.notify_about_instance_action') @mock.patch('nova.compute.utils.notify_about_instance_usage') @mock.patch('nova.objects.Instance.destroy') def test_notify_about_instance_delete(self, mock_instance_destroy, - mock_notify_usage): + mock_notify_usage, + mock_notify_action): instance = fake_instance.fake_instance_obj( self.context, expected_attrs=('system_metadata',)) with compute_utils.notify_about_instance_delete( @@ -1114,6 +1116,14 @@ class ComputeUtilsTestCase(test.NoDBTestCase): 'delete.end') ] mock_notify_usage.assert_has_calls(expected_notify_calls) + mock_notify_action.assert_has_calls([ + mock.call(self.context, instance, + host='fake-mini', source='nova-api', + action='delete', phase='start'), + mock.call(self.context, instance, + host='fake-mini', source='nova-api', + action='delete', phase='end'), + ]) def test_get_stashed_volume_connector_none(self): inst = fake_instance.fake_instance_obj(self.context) diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index 47f8b3a3d108..77a72b7cb239 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -1740,6 +1740,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): @mock.patch('nova.objects.TagList.destroy') @mock.patch('nova.objects.TagList.create') + @mock.patch('nova.compute.utils.notify_about_instance_action') @mock.patch('nova.compute.utils.notify_about_instance_usage') @mock.patch('nova.compute.rpcapi.ComputeAPI.build_and_run_instance') @mock.patch('nova.scheduler.rpcapi.SchedulerAPI.select_destinations') @@ -1750,6 +1751,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): br_destroy, select_destinations, build_and_run, + legacy_notify, notify, taglist_create, taglist_destroy): @@ -1769,10 +1771,13 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): self.assertIsNotNone(taglist_destroy.call_args[0][0].db_connection) test_utils.assert_instance_delete_notification_by_uuid( - notify, self.params['build_requests'][0].instance_uuid, + legacy_notify, notify, + self.params['build_requests'][0].instance_uuid, self.conductor.notifier, test.MatchType(context.RequestContext), - expect_targeted_context=True) + expect_targeted_context=True, expected_source='nova-conductor', + expected_host='host1') + @mock.patch('nova.compute.utils.notify_about_instance_action') @mock.patch('nova.objects.Instance.destroy') @mock.patch('nova.compute.utils.notify_about_instance_usage') @mock.patch('nova.compute.rpcapi.ComputeAPI.build_and_run_instance') @@ -1781,7 +1786,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): @mock.patch('nova.conductor.manager.ComputeTaskManager._bury_in_cell0') def test_schedule_and_build_delete_during_scheduling_host_changed( self, bury, br_destroy, select_destinations, - build_and_run, notify, instance_destroy): + build_and_run, legacy_notify, instance_destroy, notify): br_destroy.side_effect = exc.BuildRequestNotFound(uuid='foo') instance_destroy.side_effect = [ @@ -1797,11 +1802,15 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): self.assertFalse(bury.called) self.assertTrue(br_destroy.called) self.assertEqual(2, instance_destroy.call_count) - test_utils.assert_instance_delete_notification_by_uuid( - notify, self.params['build_requests'][0].instance_uuid, - self.conductor.notifier, test.MatchType(context.RequestContext), - expect_targeted_context=True) + test_utils.assert_instance_delete_notification_by_uuid( + legacy_notify, notify, + self.params['build_requests'][0].instance_uuid, + self.conductor.notifier, test.MatchType(context.RequestContext), + expect_targeted_context=True, expected_source='nova-conductor', + expected_host='host1') + + @mock.patch('nova.compute.utils.notify_about_instance_action') @mock.patch('nova.objects.Instance.destroy') @mock.patch('nova.compute.utils.notify_about_instance_usage') @mock.patch('nova.compute.rpcapi.ComputeAPI.build_and_run_instance') @@ -1810,7 +1819,7 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): @mock.patch('nova.conductor.manager.ComputeTaskManager._bury_in_cell0') def test_schedule_and_build_delete_during_scheduling_instance_not_found( self, bury, br_destroy, select_destinations, - build_and_run, notify, instance_destroy): + build_and_run, legacy_notify, instance_destroy, notify): br_destroy.side_effect = exc.BuildRequestNotFound(uuid='foo') instance_destroy.side_effect = [ @@ -1826,9 +1835,11 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): self.assertTrue(br_destroy.called) self.assertEqual(1, instance_destroy.call_count) test_utils.assert_instance_delete_notification_by_uuid( - notify, self.params['build_requests'][0].instance_uuid, + legacy_notify, notify, + self.params['build_requests'][0].instance_uuid, self.conductor.notifier, test.MatchType(context.RequestContext), - expect_targeted_context=True) + expect_targeted_context=True, expected_source='nova-conductor', + expected_host='host1') @mock.patch('nova.compute.rpcapi.ComputeAPI.build_and_run_instance') @mock.patch('nova.scheduler.rpcapi.SchedulerAPI.select_destinations') diff --git a/nova/tests/unit/utils.py b/nova/tests/unit/utils.py index ae64faf7c58b..c0eabbf162a4 100644 --- a/nova/tests/unit/utils.py +++ b/nova/tests/unit/utils.py @@ -299,21 +299,53 @@ class CustomMockCallMatcher(object): def assert_instance_delete_notification_by_uuid( - mock_notify, expected_instance_uuid, expected_notifier, - expected_context, expect_targeted_context=False): + mock_legacy_notify, mock_notify, expected_instance_uuid, + expected_notifier, expected_context, expect_targeted_context=False, + expected_source='nova-api', expected_host='fake-mini'): match_by_instance_uuid = CustomMockCallMatcher( lambda instance: instance.uuid == expected_instance_uuid) + assert_legacy_instance_delete_notification_by_uuid( + mock_legacy_notify, match_by_instance_uuid, expected_notifier, + expected_context, expect_targeted_context) + assert_versioned_instance_delete_notification_by_uuid( + mock_notify, match_by_instance_uuid, + expected_context, expected_source, expected_host=expected_host) + + +def assert_versioned_instance_delete_notification_by_uuid( + mock_notify, instance_matcher, + expected_context, expected_source, expected_host): + + mock_notify.assert_has_calls([ + mock.call(expected_context, + instance_matcher, + host=expected_host, + source=expected_source, + action='delete', + phase='start'), + mock.call(expected_context, + instance_matcher, + host=expected_host, + source=expected_source, + action='delete', + phase='end')]) + + +def assert_legacy_instance_delete_notification_by_uuid( + mock_notify, instance_matcher, expected_notifier, + expected_context, expect_targeted_context): + mock_notify.assert_has_calls([ mock.call(expected_notifier, expected_context, - match_by_instance_uuid, + instance_matcher, 'delete.start'), mock.call(expected_notifier, expected_context, - match_by_instance_uuid, + instance_matcher, 'delete.end')]) for call in mock_notify.call_args_list: