From e83dbe1205c9b40e5cde30d11c6cbf6f4875126f Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Thu, 22 Jun 2017 11:26:29 +0200 Subject: [PATCH] Send soft_delete from context manager This patch moves soft_delete notification sending to the notify_about_instance_delete context manager to unify the delete notification sending code practice. Implements: bp versioned-notification-transformation-stein Change-Id: Ib6f95c22ffd3ea235b60db4da32094d49c2efa2a --- nova/compute/api.py | 8 ++++---- nova/compute/manager.py | 11 ++--------- nova/compute/utils.py | 16 ++++++++-------- nova/conductor/manager.py | 2 +- nova/tests/unit/compute/test_compute_api.py | 2 +- nova/tests/unit/compute/test_compute_mgr.py | 15 +++++++++------ nova/tests/unit/compute/test_compute_utils.py | 2 +- 7 files changed, 26 insertions(+), 30 deletions(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 52d3d96574..9f8d279482 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -1833,7 +1833,7 @@ class API(base.Base): # FIXME: When the instance context is targeted, # we can remove this with compute_utils.notify_about_instance_delete( - self.notifier, cctxt, instance): + self.notifier, cctxt, instance, CONF.host): instance.destroy() else: instance.destroy() @@ -1891,7 +1891,7 @@ class API(base.Base): try: # Now destroy the instance from the cell it lives in. with compute_utils.notify_about_instance_delete( - self.notifier, context, instance): + self.notifier, context, instance, CONF.host): instance.destroy() except exception.InstanceNotFound: pass @@ -1962,7 +1962,7 @@ class API(base.Base): if not instance.host and not may_have_ports_or_volumes: try: with compute_utils.notify_about_instance_delete( - self.notifier, context, instance, + self.notifier, context, instance, CONF.host, delete_type if delete_type != 'soft_delete' else 'delete'): @@ -2111,7 +2111,7 @@ class API(base.Base): LOG.warning("instance's host %s is down, deleting from " "database", instance.host, instance=instance) with compute_utils.notify_about_instance_delete( - self.notifier, context, instance, + self.notifier, context, instance, CONF.host, delete_type if delete_type != 'soft_delete' else 'delete'): elevated = context.elevated() diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 9c78593bb5..1f6cf3f92d 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -2824,11 +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', - fields.NotificationSource.COMPUTE): - compute_utils.notify_about_instance_action(context, instance, - self.host, action=fields.NotificationAction.SOFT_DELETE, - phase=fields.NotificationPhase.START) + self.notifier, context, instance, self.host, 'soft_delete', + source=fields.NotificationSource.COMPUTE): try: self.driver.soft_delete(instance) except NotImplementedError: @@ -2839,10 +2836,6 @@ class ComputeManager(manager.Manager): instance.vm_state = vm_states.SOFT_DELETED instance.task_state = None instance.save(expected_task_state=[task_states.SOFT_DELETING]) - compute_utils.notify_about_instance_action( - context, instance, self.host, - action=fields.NotificationAction.SOFT_DELETE, - phase=fields.NotificationPhase.END) @wrap_exception() @reverts_task_state diff --git a/nova/compute/utils.py b/nova/compute/utils.py index cf78c8aaae..1317e9b484 100644 --- a/nova/compute/utils.py +++ b/nova/compute/utils.py @@ -1156,32 +1156,32 @@ class UnlimitedSemaphore(object): @contextlib.contextmanager -def notify_about_instance_delete(notifier, context, instance, +def notify_about_instance_delete(notifier, context, instance, host, 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 + # Note(gibi): force_delete types will be handled in a # subsequent patch - if delete_type == 'delete': + if delete_type in ['delete', 'soft_delete']: notify_about_instance_action( context, instance, - host=CONF.host, + host=host, source=source, - action=fields.NotificationAction.DELETE, + action=delete_type, phase=fields.NotificationPhase.START) yield finally: notify_about_instance_usage(notifier, context, instance, "%s.end" % delete_type) - if delete_type == 'delete': + if delete_type in ['delete', 'soft_delete']: notify_about_instance_action( context, instance, - host=CONF.host, + host=host, source=source, - action=fields.NotificationAction.DELETE, + action=delete_type, phase=fields.NotificationPhase.END) diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index 14df9c1e05..385b767a35 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -1419,7 +1419,7 @@ 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, CONF.host, source=fields.NotificationSource.CONDUCTOR): try: instance.destroy() diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index cd46be7c81..de352646ea 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -1712,7 +1712,7 @@ class _ComputeAPIUnitTestMixIn(object): _lookup_instance.assert_called_once_with( self.context, instance.uuid) notify_mock.assert_called_once_with( - self.compute_api.notifier, self.context, instance) + self.compute_api.notifier, self.context, instance, 'fake-mini') destroy_mock.assert_called_once_with() @mock.patch.object(context, 'target_cell') diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 72157a27dc..2a97a5ffc5 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -4818,20 +4818,23 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): def test_instance_soft_delete_notification(self): inst_obj = fake_instance.fake_instance_obj(self.context, vm_state=vm_states.ACTIVE) + inst_obj.system_metadata = {} with test.nested( mock.patch.object(nova.compute.utils, 'notify_about_instance_action'), mock.patch.object(nova.compute.utils, - 'notify_about_instance_delete'), + 'notify_about_instance_usage'), mock.patch.object(objects.Instance, 'save'), mock.patch.object(self.compute.driver, 'soft_delete') - ) as (fake_notify, fake_notify_usage, fake_save, fake_soft_delete): + ) as (fake_notify, fake_legacy_notify, fake_save, fake_soft_delete): self.compute.soft_delete_instance(self.context, inst_obj) fake_notify.assert_has_calls([ - mock.call(self.context, inst_obj, 'fake-mini', - action='soft_delete', phase='start'), - mock.call(self.context, inst_obj, 'fake-mini', - action='soft_delete', phase='end')]) + mock.call(self.context, inst_obj, action='soft_delete', + source='nova-compute', host='fake-mini', + phase='start'), + mock.call(self.context, inst_obj, action='soft_delete', + source='nova-compute', host='fake-mini', + phase='end')]) def test_get_scheduler_hints(self): # 1. No hints and no request_spec. diff --git a/nova/tests/unit/compute/test_compute_utils.py b/nova/tests/unit/compute/test_compute_utils.py index d0b149c981..9901e99795 100644 --- a/nova/tests/unit/compute/test_compute_utils.py +++ b/nova/tests/unit/compute/test_compute_utils.py @@ -1107,7 +1107,7 @@ class ComputeUtilsTestCase(test.NoDBTestCase): instance = fake_instance.fake_instance_obj( self.context, expected_attrs=('system_metadata',)) with compute_utils.notify_about_instance_delete( - mock.sentinel.notifier, self.context, instance): + mock.sentinel.notifier, self.context, instance, "fake-mini"): instance.destroy() expected_notify_calls = [ mock.call(mock.sentinel.notifier, self.context, instance,