From df7442ee5aeb519d579208e71040af02fad6c428 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9la=20Vancsics?= Date: Mon, 24 Jul 2017 11:59:59 +0200 Subject: [PATCH] Transform rescue/unrescue instance notifications The rescue (instance.rescue.start and instance.rescue.end) and unrescue (instance.unrescue.start and instance.unrescue.end) notifications are transformed to the versioned framework. This patch also fixes the power state of the server when unrescuing it with the fake compute driver. Co-Authored-By: Takashi Natsume Change-Id: Ib1d03c6d693e3b04886c638c956e35809fed8fc2 Implements: bp versioned-notification-transformation-queens Closes-Bug: #1742133 --- .../os-rescue/server-get-resp-unrescue.json | 2 +- .../InstanceActionRescuePayload.json | 8 +++ .../instance-rescue-end.json | 12 +++++ .../instance-rescue-start.json | 11 ++++ .../instance-unrescue-end.json | 6 +++ .../instance-unrescue-start.json | 13 +++++ nova/compute/manager.py | 15 ++++++ nova/compute/utils.py | 33 ++++++++++++ nova/notifications/objects/instance.py | 31 ++++++++++- nova/tests/fixtures.py | 4 ++ .../server-get-resp-unrescue.json.tpl | 2 +- .../test_instance.py | 52 ++++++++++++++++--- nova/tests/unit/compute/test_compute.py | 26 +++++++++- nova/tests/unit/compute/test_compute_mgr.py | 16 +++++- nova/tests/unit/compute/test_compute_utils.py | 36 +++++++++++++ .../objects/test_notification.py | 2 + nova/virt/fake.py | 2 +- 17 files changed, 256 insertions(+), 15 deletions(-) create mode 100644 doc/notification_samples/common_payloads/InstanceActionRescuePayload.json create mode 100644 doc/notification_samples/instance-rescue-end.json create mode 100644 doc/notification_samples/instance-rescue-start.json create mode 100644 doc/notification_samples/instance-unrescue-end.json create mode 100644 doc/notification_samples/instance-unrescue-start.json diff --git a/doc/api_samples/os-rescue/server-get-resp-unrescue.json b/doc/api_samples/os-rescue/server-get-resp-unrescue.json index 0dd02e07fa..a8c9f271aa 100644 --- a/doc/api_samples/os-rescue/server-get-resp-unrescue.json +++ b/doc/api_samples/os-rescue/server-get-resp-unrescue.json @@ -59,7 +59,7 @@ "OS-EXT-SRV-ATTR:host": "b8b357f7100d4391828f2177c922ef93", "OS-EXT-SRV-ATTR:hypervisor_hostname": "fake-mini", "OS-EXT-SRV-ATTR:instance_name": "instance-00000001", - "OS-EXT-STS:power_state": 4, + "OS-EXT-STS:power_state": 1, "OS-EXT-STS:task_state": null, "OS-EXT-STS:vm_state": "active", "os-extended-volumes:volumes_attached": [], diff --git a/doc/notification_samples/common_payloads/InstanceActionRescuePayload.json b/doc/notification_samples/common_payloads/InstanceActionRescuePayload.json new file mode 100644 index 0000000000..8938ec1858 --- /dev/null +++ b/doc/notification_samples/common_payloads/InstanceActionRescuePayload.json @@ -0,0 +1,8 @@ +{ + "$ref": "InstanceActionPayload.json", + "nova_object.data": { + "rescue_image_ref": "a2459075-d96c-40d5-893e-577ff92e721c" + }, + "nova_object.name": "InstanceActionRescuePayload", + "nova_object.version": "1.0" +} diff --git a/doc/notification_samples/instance-rescue-end.json b/doc/notification_samples/instance-rescue-end.json new file mode 100644 index 0000000000..0563c6b027 --- /dev/null +++ b/doc/notification_samples/instance-rescue-end.json @@ -0,0 +1,12 @@ +{ + "event_type": "instance.rescue.end", + "payload": { + "$ref": "common_payloads/InstanceActionRescuePayload.json#", + "nova_object.data": { + "state": "rescued", + "power_state": "shutdown" + } + }, + "priority":"INFO", + "publisher_id":"nova-compute:compute" +} diff --git a/doc/notification_samples/instance-rescue-start.json b/doc/notification_samples/instance-rescue-start.json new file mode 100644 index 0000000000..90719cde5d --- /dev/null +++ b/doc/notification_samples/instance-rescue-start.json @@ -0,0 +1,11 @@ +{ + "event_type": "instance.rescue.start", + "payload": { + "$ref": "common_payloads/InstanceActionRescuePayload.json#", + "nova_object.data": { + "task_state": "rescuing" + } + }, + "priority": "INFO", + "publisher_id": "nova-compute:compute" +} diff --git a/doc/notification_samples/instance-unrescue-end.json b/doc/notification_samples/instance-unrescue-end.json new file mode 100644 index 0000000000..ce4c025e22 --- /dev/null +++ b/doc/notification_samples/instance-unrescue-end.json @@ -0,0 +1,6 @@ +{ + "event_type": "instance.unrescue.end", + "payload":{"$ref": "common_payloads/InstanceActionPayload.json#"}, + "priority": "INFO", + "publisher_id": "nova-compute:compute" +} diff --git a/doc/notification_samples/instance-unrescue-start.json b/doc/notification_samples/instance-unrescue-start.json new file mode 100644 index 0000000000..f91fe80020 --- /dev/null +++ b/doc/notification_samples/instance-unrescue-start.json @@ -0,0 +1,13 @@ +{ + "event_type": "instance.unrescue.start", + "payload":{ + "$ref": "common_payloads/InstanceActionPayload.json#", + "nova_object.data": { + "power_state": "shutdown", + "task_state": "unrescuing", + "state": "rescued" + } + }, + "priority": "INFO", + "publisher_id": "nova-compute:compute" +} diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 9ab7f57f3b..d559cc8b46 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -3549,6 +3549,10 @@ class ComputeManager(manager.Manager): self._notify_about_instance_usage(context, instance, "rescue.start", extra_usage_info=extra_usage_info, network_info=network_info) + compute_utils.notify_about_instance_rescue_action( + context, instance, self.host, rescue_image_ref, + action=fields.NotificationAction.RESCUE, + phase=fields.NotificationPhase.START) try: self._power_off_instance(context, instance, clean_shutdown) @@ -3576,6 +3580,10 @@ class ComputeManager(manager.Manager): self._notify_about_instance_usage(context, instance, "rescue.end", extra_usage_info=extra_usage_info, network_info=network_info) + compute_utils.notify_about_instance_rescue_action( + context, instance, self.host, rescue_image_ref, + action=fields.NotificationAction.RESCUE, + phase=fields.NotificationPhase.END) @wrap_exception() @reverts_task_state @@ -3588,6 +3596,10 @@ class ComputeManager(manager.Manager): network_info = self.network_api.get_instance_nw_info(context, instance) self._notify_about_instance_usage(context, instance, "unrescue.start", network_info=network_info) + compute_utils.notify_about_instance_action(context, instance, + self.host, action=fields.NotificationAction.UNRESCUE, + phase=fields.NotificationPhase.START) + with self._error_out_instance_on_exception(context, instance): self.driver.unrescue(instance, network_info) @@ -3601,6 +3613,9 @@ class ComputeManager(manager.Manager): instance, "unrescue.end", network_info=network_info) + compute_utils.notify_about_instance_action(context, instance, + self.host, action=fields.NotificationAction.UNRESCUE, + phase=fields.NotificationPhase.END) @wrap_exception() @wrap_instance_fault diff --git a/nova/compute/utils.py b/nova/compute/utils.py index 272e88233c..e0354f5fc0 100644 --- a/nova/compute/utils.py +++ b/nova/compute/utils.py @@ -445,6 +445,39 @@ def notify_about_volume_attach_detach(context, instance, host, action, phase, notification.emit(context) +@rpc.if_notifications_enabled +def notify_about_instance_rescue_action( + context, instance, host, rescue_image_ref, action, phase=None, + source=fields.NotificationSource.COMPUTE, exception=None): + """Send versioned notification about the action made on the instance + + :param instance: the instance which the action performed on + :param host: the host emitting the notification + :param rescue_image_ref: the rescue image ref + :param action: the name of the action + :param phase: the phase of the action + :param source: the source of the notification + :param exception: the thrown exception (used in error notifications) + """ + fault, priority = _get_fault_and_priority_from_exc(exception) + payload = instance_notification.InstanceActionRescuePayload( + instance=instance, + fault=fault, + rescue_image_ref=rescue_image_ref) + + notification = instance_notification.InstanceActionRescueNotification( + context=context, + priority=priority, + publisher=notification_base.NotificationPublisher( + host=host, source=source), + event_type=notification_base.EventType( + object='instance', + action=action, + phase=phase), + payload=payload) + notification.emit(context) + + @rpc.if_notifications_enabled def notify_about_keypair_action(context, keypair, action, phase): """Send versioned notification about the keypair action on the instance diff --git a/nova/notifications/objects/instance.py b/nova/notifications/objects/instance.py index aa84ca0048..7e05f24880 100644 --- a/nova/notifications/objects/instance.py +++ b/nova/notifications/objects/instance.py @@ -244,6 +244,21 @@ class InstanceUpdatePayload(InstancePayload): for instance_tag in instance.tags.objects] +@nova_base.NovaObjectRegistry.register_notification +class InstanceActionRescuePayload(InstanceActionPayload): + # Version 1.0: Initial version + VERSION = '1.0' + fields = { + 'rescue_image_ref': fields.UUIDField(nullable=True) + } + + def __init__(self, instance, fault, rescue_image_ref): + super(InstanceActionRescuePayload, self).__init__( + instance=instance, + fault=fault) + self.rescue_image_ref = rescue_image_ref + + @nova_base.NovaObjectRegistry.register_notification class IpPayload(base.NotificationPayloadBase): # Version 1.0: Initial version @@ -451,8 +466,8 @@ class InstanceStateUpdatePayload(base.NotificationPayloadBase): @base.notification_sample('instance-soft_delete-end.json') @base.notification_sample('instance-trigger_crash_dump-start.json') @base.notification_sample('instance-trigger_crash_dump-end.json') -# @base.notification_sample('instance-unrescue-start.json') -# @base.notification_sample('instance-unrescue-end.json') +@base.notification_sample('instance-unrescue-start.json') +@base.notification_sample('instance-unrescue-end.json') @base.notification_sample('instance-unshelve-start.json') @base.notification_sample('instance-unshelve-end.json') @nova_base.NovaObjectRegistry.register_notification @@ -529,6 +544,18 @@ class InstanceActionSnapshotNotification(base.NotificationBase): } +@base.notification_sample('instance-rescue-start.json') +@base.notification_sample('instance-rescue-end.json') +@nova_base.NovaObjectRegistry.register_notification +class InstanceActionRescueNotification(base.NotificationBase): + # Version 1.0: Initial version + VERSION = '1.0' + + fields = { + 'payload': fields.ObjectField('InstanceActionRescuePayload') + } + + @nova_base.NovaObjectRegistry.register_notification class InstanceActionSnapshotPayload(InstanceActionPayload): # Version 1.6: Initial version. It starts at version 1.6 as diff --git a/nova/tests/fixtures.py b/nova/tests/fixtures.py index 380704a6cd..f7134f1e98 100644 --- a/nova/tests/fixtures.py +++ b/nova/tests/fixtures.py @@ -1464,6 +1464,8 @@ class CinderFixture(fixtures.Fixture): lambda *args, **kwargs: None) self.test.stub_out('nova.volume.cinder.API.unreserve_volume', fake_unreserve_volume) + self.test.stub_out('nova.volume.cinder.API.check_attached', + lambda *args, **kwargs: None) # TODO(mriedem): We can probably pull some of the common parts from the @@ -1638,6 +1640,8 @@ class CinderFixtureNewAttachFlow(fixtures.Fixture): lambda *args, **kwargs: None) self.test.stub_out('nova.volume.cinder.is_microversion_supported', lambda *args, **kwargs: None) + self.test.stub_out('nova.volume.cinder.API.check_attached', + lambda *args, **kwargs: None) class PlacementApiClient(object): diff --git a/nova/tests/functional/api_sample_tests/api_samples/os-rescue/server-get-resp-unrescue.json.tpl b/nova/tests/functional/api_sample_tests/api_samples/os-rescue/server-get-resp-unrescue.json.tpl index dcf8c453b8..3240c49372 100644 --- a/nova/tests/functional/api_sample_tests/api_samples/os-rescue/server-get-resp-unrescue.json.tpl +++ b/nova/tests/functional/api_sample_tests/api_samples/os-rescue/server-get-resp-unrescue.json.tpl @@ -59,7 +59,7 @@ "OS-EXT-SRV-ATTR:host": "%(compute_host)s", "OS-EXT-SRV-ATTR:hypervisor_hostname": "%(hypervisor_hostname)s", "OS-EXT-SRV-ATTR:instance_name": "%(instance_name)s", - "OS-EXT-STS:power_state": 4, + "OS-EXT-STS:power_state": 1, "OS-EXT-STS:task_state": null, "OS-EXT-STS:vm_state": "active", "os-extended-volumes:volumes_attached": [], diff --git a/nova/tests/functional/notification_sample_tests/test_instance.py b/nova/tests/functional/notification_sample_tests/test_instance.py index 020e9bedb1..524672177b 100644 --- a/nova/tests/functional/notification_sample_tests/test_instance.py +++ b/nova/tests/functional/notification_sample_tests/test_instance.py @@ -263,8 +263,7 @@ class TestInstanceNotificationSample( self._test_reboot_server_error, self._test_trigger_crash_dump, self._test_volume_detach_attach_server, - self._test_rescue_server, - self._test_unrescue_server, + self._test_rescue_unrescue_server, self._test_soft_delete_server, self._test_attach_volume_error, self._test_interface_attach_and_detach, @@ -1217,11 +1216,52 @@ class TestInstanceNotificationSample( 'uuid': server['id']}, actual=fake_notifier.VERSIONED_NOTIFICATIONS[1]) - def _test_rescue_server(self, server): - pass + def _test_rescue_unrescue_server(self, server): + # Both "rescue" and "unrescue" notification asserts are made here + # rescue notification asserts + post = { + "rescue": { + "rescue_image_ref": 'a2459075-d96c-40d5-893e-577ff92e721c' + } + } + self.api.post_server_action(server['id'], post) + self._wait_for_state_change(self.admin_api, server, 'RESCUE') - def _test_unrescue_server(self, server): - pass + self.assertEqual(2, len(fake_notifier.VERSIONED_NOTIFICATIONS)) + self._verify_notification( + 'instance-rescue-start', + replacements={ + 'reservation_id': server['reservation_id'], + 'uuid': server['id']}, + actual=fake_notifier.VERSIONED_NOTIFICATIONS[0]) + self._verify_notification( + 'instance-rescue-end', + replacements={ + 'reservation_id': server['reservation_id'], + 'uuid': server['id']}, + actual=fake_notifier.VERSIONED_NOTIFICATIONS[1]) + fake_notifier.reset() + + # unrescue notification asserts + post = { + 'unrescue': None + } + self.api.post_server_action(server['id'], post) + self._wait_for_state_change(self.admin_api, server, 'ACTIVE') + + self.assertEqual(2, len(fake_notifier.VERSIONED_NOTIFICATIONS)) + self._verify_notification( + 'instance-unrescue-start', + replacements={ + 'reservation_id': server['reservation_id'], + 'uuid': server['id']}, + actual=fake_notifier.VERSIONED_NOTIFICATIONS[0]) + self._verify_notification( + 'instance-unrescue-end', + replacements={ + 'reservation_id': server['reservation_id'], + 'uuid': server['id']}, + actual=fake_notifier.VERSIONED_NOTIFICATIONS[1]) def _test_soft_delete_server(self, server): self.flags(reclaim_instance_interval=30) diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 1abf135d4c..71e02ba7f3 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -2389,13 +2389,17 @@ class ComputeTestCase(BaseTestCase, self.compute.terminate_instance(self.context, instance, [], []) - def test_rescue_notifications(self): + @mock.patch.object(nova.compute.utils, + 'notify_about_instance_rescue_action') + @mock.patch('nova.context.RequestContext.elevated') + def test_rescue_notifications(self, mock_context, mock_notify): # Ensure notifications on instance rescue. def fake_rescue(self, context, instance_ref, network_info, image_meta, rescue_password): pass self.stub_out('nova.virt.fake.FakeDriver.rescue', fake_rescue) + mock_context.return_value = self.context instance = self._create_fake_instance_obj() self.compute.build_and_run_instance(self.context, instance, {}, {}, {}, block_device_mapping=[]) @@ -2412,6 +2416,14 @@ class ComputeTestCase(BaseTestCase, 'compute.instance.rescue.end'] self.assertEqual([m.event_type for m in fake_notifier.NOTIFICATIONS], expected_notifications) + mock_notify.assert_has_calls([ + mock.call(self.context, instance, 'fake-mini', + uuids.fake_image_ref_1, + action='rescue', phase='start'), + mock.call(self.context, instance, 'fake-mini', + uuids.fake_image_ref_1, + action='rescue', phase='end')]) + for n, msg in enumerate(fake_notifier.NOTIFICATIONS): self.assertEqual(msg.event_type, expected_notifications[n]) self.assertEqual(msg.priority, 'INFO') @@ -2433,12 +2445,16 @@ class ComputeTestCase(BaseTestCase, self.compute.terminate_instance(self.context, instance, [], []) - def test_unrescue_notifications(self): + @mock.patch.object(nova.compute.utils, 'notify_about_instance_action') + @mock.patch('nova.context.RequestContext.elevated') + def test_unrescue_notifications(self, mock_context, mock_notify): # Ensure notifications on instance rescue. def fake_unrescue(self, instance_ref, network_info): pass self.stub_out('nova.virt.fake.FakeDriver.unrescue', fake_unrescue) + context = self.context + mock_context.return_value = context instance = self._create_fake_instance_obj() self.compute.build_and_run_instance(self.context, instance, {}, {}, {}, @@ -2453,6 +2469,12 @@ class ComputeTestCase(BaseTestCase, 'compute.instance.unrescue.end'] self.assertEqual([m.event_type for m in fake_notifier.NOTIFICATIONS], expected_notifications) + mock_notify.assert_has_calls([ + mock.call(context, instance, 'fake-mini', + action='unrescue', phase='start'), + mock.call(context, instance, 'fake-mini', + action='unrescue', phase='end')]) + for n, msg in enumerate(fake_notifier.NOTIFICATIONS): self.assertEqual(msg.event_type, expected_notifications[n]) self.assertEqual(msg.priority, 'INFO') diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index ee17f54f59..a9e509e26a 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -3166,7 +3166,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): volume_id=volume_id), ]) - def _test_rescue(self, clean_shutdown=True): + @mock.patch('nova.compute.utils.notify_about_instance_rescue_action') + def _test_rescue(self, mock_notify, clean_shutdown=True): instance = fake_instance.fake_instance_obj( self.context, vm_state=vm_states.ACTIVE) fake_nw_info = network_model.NetworkInfo() @@ -3226,6 +3227,11 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): notify_usage_exists.assert_called_once_with(self.compute.notifier, self.context, instance, current_period=True) + mock_notify.assert_has_calls([ + mock.call(self.context, instance, 'fake-mini', None, + action='rescue', phase='start'), + mock.call(self.context, instance, 'fake-mini', None, + action='rescue', phase='end')]) instance_save.assert_called_once_with( expected_task_state=task_states.RESCUING) @@ -3236,7 +3242,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): def test_rescue_forced_shutdown(self): self._test_rescue(clean_shutdown=False) - def test_unrescue(self): + @mock.patch('nova.compute.utils.notify_about_instance_action') + def test_unrescue(self, mock_notify): instance = fake_instance.fake_instance_obj( self.context, vm_state=vm_states.RESCUED) fake_nw_info = network_model.NetworkInfo() @@ -3273,6 +3280,11 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase): notify_instance_usage.assert_has_calls(notify_calls) driver_unrescue.assert_called_once_with(instance, fake_nw_info) + mock_notify.assert_has_calls([ + mock.call(self.context, instance, 'fake-mini', + action='unrescue', phase='start'), + mock.call(self.context, instance, 'fake-mini', + action='unrescue', phase='end')]) instance_save.assert_called_once_with( expected_task_state=task_states.UNRESCUING) diff --git a/nova/tests/unit/compute/test_compute_utils.py b/nova/tests/unit/compute/test_compute_utils.py index 6d568410f6..ad301614ff 100644 --- a/nova/tests/unit/compute/test_compute_utils.py +++ b/nova/tests/unit/compute/test_compute_utils.py @@ -754,6 +754,42 @@ class UsageInfoTestCase(test.TestCase): self.assertEqual('nova.tests.unit.compute.test_compute_utils', exception_payload['module_name']) + def test_notify_about_instance_rescue_action(self): + instance = create_instance(self.context) + + compute_utils.notify_about_instance_rescue_action( + self.context, + instance, + 'fake-compute', + uuids.rescue_image_ref, + action='rescue', + phase='start') + + self.assertEqual(len(fake_notifier.VERSIONED_NOTIFICATIONS), 1) + notification = fake_notifier.VERSIONED_NOTIFICATIONS[0] + + self.assertEqual(notification['priority'], 'INFO') + self.assertEqual(notification['event_type'], 'instance.rescue.start') + self.assertEqual(notification['publisher_id'], + 'nova-compute:fake-compute') + + payload = notification['payload']['nova_object.data'] + self.assertEqual(payload['tenant_id'], self.project_id) + self.assertEqual(payload['user_id'], self.user_id) + self.assertEqual(payload['uuid'], instance['uuid']) + + flavorid = flavors.get_flavor_by_name('m1.tiny')['flavorid'] + flavor = payload['flavor']['nova_object.data'] + self.assertEqual(str(flavor['flavorid']), flavorid) + + for attr in ('display_name', 'created_at', 'launched_at', + 'state', 'task_state', 'display_description', 'locked', + 'auto_disk_config', 'key_name'): + self.assertIn(attr, payload, "Key %s not in payload" % attr) + + self.assertEqual(payload['image_uuid'], uuids.fake_image_ref) + self.assertEqual(payload['rescue_image_ref'], uuids.rescue_image_ref) + def test_notify_usage_exists_instance_not_found(self): # Ensure 'exists' notification generates appropriate usage data. instance = create_instance(self.context) diff --git a/nova/tests/unit/notifications/objects/test_notification.py b/nova/tests/unit/notifications/objects/test_notification.py index ddd9b779c4..9d0fe9c37e 100644 --- a/nova/tests/unit/notifications/objects/test_notification.py +++ b/nova/tests/unit/notifications/objects/test_notification.py @@ -380,6 +380,8 @@ notification_object_data = { 'FlavorPayload': '1.4-2e7011b8b4e59167fe8b7a0a81f0d452', 'InstanceActionNotification': '1.0-a73147b93b520ff0061865849d3dfa56', 'InstanceActionPayload': '1.5-fb2804ce9b681bfb217e729153c22611', + 'InstanceActionRescueNotification': '1.0-a73147b93b520ff0061865849d3dfa56', + 'InstanceActionRescuePayload': '1.0-a29f3339d0b8c3bcc997ab5d19d898d5', 'InstanceActionVolumeNotification': '1.0-a73147b93b520ff0061865849d3dfa56', 'InstanceActionVolumePayload': '1.3-f175b22ac6d6d0aea2bac21e12156e77', 'InstanceActionVolumeSwapNotification': diff --git a/nova/virt/fake.py b/nova/virt/fake.py index 601eec07aa..5628f0c800 100644 --- a/nova/virt/fake.py +++ b/nova/virt/fake.py @@ -225,7 +225,7 @@ class FakeDriver(driver.ComputeDriver): pass def unrescue(self, instance, network_info): - pass + self.instances[instance.uuid].state = power_state.RUNNING def poll_rebooting_instances(self, timeout, instances): pass