diff --git a/doc/notification_samples/instance-update-tags-action.json b/doc/notification_samples/instance-update-tags-action.json new file mode 100644 index 000000000000..4e704f86320b --- /dev/null +++ b/doc/notification_samples/instance-update-tags-action.json @@ -0,0 +1,90 @@ +{ + "event_type": "instance.update", + "payload": { + "nova_object.data": { + "architecture": "x86_64", + "audit_period": { + "nova_object.data": { + "audit_period_beginning": "2012-10-01T00:00:00Z", + "audit_period_ending": "2012-10-29T13:42:11Z"}, + "nova_object.name": "AuditPeriodPayload", + "nova_object.namespace": "nova", + "nova_object.version": "1.0"}, + "auto_disk_config": "MANUAL", + "availability_zone": "nova", + "bandwidth": [], + "created_at": "2012-10-29T13:42:11Z", + "deleted_at": null, + "display_description": "some-server", + "display_name": "some-server", + "flavor": { + "nova_object.data": { + "disabled": false, + "ephemeral_gb": 0, + "extra_specs": { + "hw:watchdog_action": "disabled" + }, + "flavorid": "a22d5517-147c-4147-a0d1-e698df5cd4e3", + "is_public": true, + "memory_mb": 512, + "name": "test_flavor", + "projects": null, + "root_gb": 1, + "rxtx_factor": 1.0, + "swap": 0, + "vcpu_weight": 0, + "vcpus": 1}, + "nova_object.name": "FlavorPayload", + "nova_object.namespace": "nova", + "nova_object.version": "1.3"}, + "host": "compute", + "host_name": "some-server", + "image_uuid": "155d900f-4e14-4e4c-a73d-069cbf4541e6", + "ip_addresses": [ + { + "nova_object.data": { + "address": "192.168.1.3", + "device_name": "tapce531f90-19", + "label": "private-network", + "mac": "fa:16:3e:4c:2c:30", + "meta": {}, + "port_uuid": "ce531f90-199f-48c0-816c-13e38010b442", + "version": 4}, + "nova_object.name": "IpPayload", + "nova_object.namespace": "nova", + "nova_object.version": "1.0" + } + ], + "kernel_id": "", + "launched_at": "2012-10-29T13:42:11Z", + "locked": false, + "metadata": {}, + "node": "fake-mini", + "old_display_name": null, + "os_type": null, + "power_state": "running", + "progress": 0, + "ramdisk_id": "", + "reservation_id": "r-53ad660s", + "state": "active", + "state_update": { + "nova_object.data": { + "new_task_state": null, + "old_state": "active", + "old_task_state": null, + "state": "active"}, + "nova_object.name": "InstanceStateUpdatePayload", + "nova_object.namespace": "nova", + "nova_object.version": "1.0"}, + "tags": ["tag1"], + "task_state": null, + "tenant_id": "6f70656e737461636b20342065766572", + "terminated_at": null, + "user_id": "fake", + "uuid": "d5ca7280-1f12-4238-a86f-378bb5d93c38"}, + "nova_object.name": "InstanceUpdatePayload", + "nova_object.namespace": "nova", + "nova_object.version": "1.3"}, + "priority": "INFO", + "publisher_id": "nova-api:fake-mini" +} diff --git a/nova/api/openstack/compute/server_tags.py b/nova/api/openstack/compute/server_tags.py index 5ee6a780fcd0..25413b6b3ebc 100644 --- a/nova/api/openstack/compute/server_tags.py +++ b/nova/api/openstack/compute/server_tags.py @@ -25,6 +25,7 @@ from nova.compute import vm_states from nova import context as nova_context from nova import exception from nova.i18n import _ +from nova.notifications import base as notifications_base from nova import objects from nova.policies import server_tags as st_policies @@ -58,6 +59,7 @@ class ServerTagsController(wsgi.Controller): method=action) common.raise_http_conflict_for_instance_invalid_state(exc, action, server_id) + return instance @wsgi.Controller.api_version("2.26") @wsgi.response(204) @@ -106,8 +108,8 @@ class ServerTagsController(wsgi.Controller): im = _get_instance_mapping(context, server_id) with nova_context.target_cell(context, im.cell_mapping) as cctxt: - self._check_instance_in_valid_state(cctxt, server_id, - 'update tag') + instance = self._check_instance_in_valid_state( + cctxt, server_id, 'update tag') try: jsonschema.validate(id, parameter_types.tag) @@ -136,9 +138,14 @@ class ServerTagsController(wsgi.Controller): with nova_context.target_cell(context, im.cell_mapping) as cctxt: tag = objects.Tag(context=cctxt, resource_id=server_id, tag=id) tag.create() + instance.tags = objects.TagList.get_by_resource_id(cctxt, + server_id) except exception.InstanceNotFound as e: raise webob.exc.HTTPNotFound(explanation=e.format_message()) + notifications_base.send_instance_update_notification( + context, instance, service="nova-api") + response = webob.Response(status_int=201) response.headers['Location'] = self._view_builder.get_location( req, server_id, id) @@ -153,15 +160,19 @@ class ServerTagsController(wsgi.Controller): im = _get_instance_mapping(context, server_id) with nova_context.target_cell(context, im.cell_mapping) as cctxt: - self._check_instance_in_valid_state(cctxt, server_id, - 'update tags') + instance = self._check_instance_in_valid_state( + cctxt, server_id, 'update tags') try: with nova_context.target_cell(context, im.cell_mapping) as cctxt: tags = objects.TagList.create(cctxt, server_id, body['tags']) + instance.tags = tags except exception.InstanceNotFound as e: raise webob.exc.HTTPNotFound(explanation=e.format_message()) + notifications_base.send_instance_update_notification( + context, instance, service="nova-api") + return {'tags': _get_tags_names(tags)} @wsgi.Controller.api_version("2.26") @@ -173,16 +184,21 @@ class ServerTagsController(wsgi.Controller): im = _get_instance_mapping(context, server_id) with nova_context.target_cell(context, im.cell_mapping) as cctxt: - self._check_instance_in_valid_state(cctxt, server_id, - 'delete tag') + instance = self._check_instance_in_valid_state( + cctxt, server_id, 'delete tag') try: with nova_context.target_cell(context, im.cell_mapping) as cctxt: objects.Tag.destroy(cctxt, server_id, id) + instance.tags = objects.TagList.get_by_resource_id(cctxt, + server_id) except (exception.InstanceTagNotFound, exception.InstanceNotFound) as e: raise webob.exc.HTTPNotFound(explanation=e.format_message()) + notifications_base.send_instance_update_notification( + context, instance, service="nova-api") + @wsgi.Controller.api_version("2.26") @wsgi.response(204) @extensions.expected_errors((404, 409)) @@ -192,11 +208,15 @@ class ServerTagsController(wsgi.Controller): im = _get_instance_mapping(context, server_id) with nova_context.target_cell(context, im.cell_mapping) as cctxt: - self._check_instance_in_valid_state(cctxt, server_id, - 'delete tags') + instance = self._check_instance_in_valid_state( + cctxt, server_id, 'delete tags') try: with nova_context.target_cell(context, im.cell_mapping) as cctxt: objects.TagList.destroy(cctxt, server_id) + instance.tags = objects.TagList() except exception.InstanceNotFound as e: raise webob.exc.HTTPNotFound(explanation=e.format_message()) + + notifications_base.send_instance_update_notification( + context, instance, service="nova-api") diff --git a/nova/notifications/base.py b/nova/notifications/base.py index 2478dc0c39c4..05808ceea286 100644 --- a/nova/notifications/base.py +++ b/nova/notifications/base.py @@ -134,7 +134,7 @@ def send_update(context, old_instance, new_instance, service="compute", old_display_name = None if new_instance["display_name"] != old_instance["display_name"]: old_display_name = old_instance["display_name"] - _send_instance_update_notification(context, new_instance, + send_instance_update_notification(context, new_instance, service=service, host=host, old_display_name=old_display_name) except exception.InstanceNotFound: @@ -176,7 +176,7 @@ def send_update_with_states(context, instance, old_vm_state, new_vm_state, if fire_update: # send either a state change or a regular notification try: - _send_instance_update_notification(context, instance, + send_instance_update_notification(context, instance, old_vm_state=old_vm_state, old_task_state=old_task_state, new_vm_state=new_vm_state, new_task_state=new_task_state, service=service, host=host) @@ -217,7 +217,8 @@ def _compute_states_payload(instance, old_vm_state=None, return states_payload -def _send_instance_update_notification(context, instance, old_vm_state=None, +@rpc.if_notifications_enabled +def send_instance_update_notification(context, instance, old_vm_state=None, old_task_state=None, new_vm_state=None, new_task_state=None, service="compute", host=None, old_display_name=None): """Send 'compute.instance.update' notification to inform observers diff --git a/nova/tests/functional/notification_sample_tests/test_instance.py b/nova/tests/functional/notification_sample_tests/test_instance.py index 38037d4f3ab5..f658e461db18 100644 --- a/nova/tests/functional/notification_sample_tests/test_instance.py +++ b/nova/tests/functional/notification_sample_tests/test_instance.py @@ -97,12 +97,13 @@ class TestInstanceNotificationSample( extra_params={'networks': [{'port': self.neutron.port_1['id']}]}) self.api.delete_server(server['id']) self._wait_until_deleted(server) - self.assertEqual(6, len(fake_notifier.VERSIONED_NOTIFICATIONS)) + self.assertEqual(7, len(fake_notifier.VERSIONED_NOTIFICATIONS)) # This list needs to be in order. expected_notifications = [ 'instance-create-start', 'instance-create-end', + 'instance-update-tags-action', 'instance-delete-start', 'instance-shutdown-start', 'instance-shutdown-end', @@ -166,15 +167,19 @@ class TestInstanceNotificationSample( server = self._boot_a_server( extra_params={'networks': [{'port': self.neutron.port_1['id']}]}) - instance_updates = self._wait_for_notifications('instance.update', 7) + instance_updates = self._wait_for_notifications('instance.update', 8) - # The first notification comes from the nova-conductor the + # The first notification comes from the nova-conductor, the + # eighth notification comes from nova-api the # rest is from the nova-compute. To keep the test simpler # assert this fact and then modify the publisher_id of the - # first notification to match the template + # first and eighth notification to match the template self.assertEqual('conductor:fake-mini', instance_updates[0]['publisher_id']) + self.assertEqual('nova-api:fake-mini', + instance_updates[7]['publisher_id']) instance_updates[0]['publisher_id'] = 'nova-compute:fake-mini' + instance_updates[7]['publisher_id'] = 'nova-compute:fake-mini' create_steps = [ # nothing -> scheduling @@ -237,6 +242,11 @@ class TestInstanceNotificationSample( 'state': 'active', 'task_state': None, 'power_state': 'running'}, + + # tag added + {'state_update.old_task_state': None, + 'state_update.old_state': 'active', + 'tags': ['tag1']}, ] replacements = self._verify_instance_update_steps( @@ -708,19 +718,19 @@ class TestInstanceNotificationSample( self.cinder.SWAP_NEW_VOL) self._wait_until_swap_volume(server, self.cinder.SWAP_NEW_VOL) - self.assertEqual(6, len(fake_notifier.VERSIONED_NOTIFICATIONS)) + self.assertEqual(7, len(fake_notifier.VERSIONED_NOTIFICATIONS)) self._verify_notification( 'instance-volume_swap-start', replacements={ 'reservation_id': server['reservation_id'], 'uuid': server['id']}, - actual=fake_notifier.VERSIONED_NOTIFICATIONS[4]) + actual=fake_notifier.VERSIONED_NOTIFICATIONS[5]) self._verify_notification( 'instance-volume_swap-end', replacements={ 'reservation_id': server['reservation_id'], 'uuid': server['id']}, - actual=fake_notifier.VERSIONED_NOTIFICATIONS[5]) + actual=fake_notifier.VERSIONED_NOTIFICATIONS[6]) def test_volume_swap_server_with_error(self): server = self._boot_a_server( @@ -740,12 +750,13 @@ class TestInstanceNotificationSample( # which generates the last notification (compute.exception). # 0. instance-create-start # 1. instance-create-end - # 2. instance-volume_attach-start - # 3. instance-volume_attach-end - # 4. instance-volume_swap-start - # 5. instance-volume_swap-error - # 6. compute.exception - self.assertTrue(len(fake_notifier.VERSIONED_NOTIFICATIONS) >= 6, + # 2. instance-update + # 3. instance-volume_attach-start + # 4. instance-volume_attach-end + # 5. instance-volume_swap-start + # 6. instance-volume_swap-error + # 7. compute.exception + self.assertTrue(len(fake_notifier.VERSIONED_NOTIFICATIONS) >= 7, 'Unexpected number of versioned notifications. ' 'Expected at least 6, got: %s' % len(fake_notifier.VERSIONED_NOTIFICATIONS)) @@ -756,13 +767,13 @@ class TestInstanceNotificationSample( 'old_volume_id': self.cinder.SWAP_ERR_OLD_VOL, 'reservation_id': server['reservation_id'], 'uuid': server['id']}, - actual=fake_notifier.VERSIONED_NOTIFICATIONS[4]) + actual=fake_notifier.VERSIONED_NOTIFICATIONS[5]) self._verify_notification( 'instance-volume_swap-error', replacements={ 'reservation_id': server['reservation_id'], 'uuid': server['id']}, - actual=fake_notifier.VERSIONED_NOTIFICATIONS[5]) + actual=fake_notifier.VERSIONED_NOTIFICATIONS[6]) def _test_revert_server(self, server): pass diff --git a/nova/tests/unit/api/openstack/compute/test_server_tags.py b/nova/tests/unit/api/openstack/compute/test_server_tags.py index 7545eb8e82e3..5c53a4f60ff7 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_tags.py +++ b/nova/tests/unit/api/openstack/compute/test_server_tags.py @@ -88,8 +88,9 @@ class ServerTagsTest(test.TestCase): self.assertEqual(TAGS, res.get('tags')) mock_db_get_inst_tags.assert_called_once_with(mock.ANY, UUID) + @mock.patch('nova.notifications.base.send_instance_update_notification') @mock.patch('nova.db.instance_tag_set') - def test_update_all(self, mock_db_set_inst_tags): + def test_update_all(self, mock_db_set_inst_tags, mock_notify): self.stub_out('nova.api.openstack.common.get_instance', return_server) fake_tags = [self._get_tag(tag) for tag in TAGS] mock_db_set_inst_tags.return_value = fake_tags @@ -99,6 +100,7 @@ class ServerTagsTest(test.TestCase): self.assertEqual(TAGS, res['tags']) mock_db_set_inst_tags.assert_called_once_with(mock.ANY, UUID, TAGS) + self.assertEqual(1, mock_notify.call_count) def test_update_all_too_many_tags(self): self.stub_out('nova.api.openstack.common.get_instance', return_server) @@ -160,9 +162,11 @@ class ServerTagsTest(test.TestCase): self.assertRaises(exc.HTTPNotFound, self.controller.show, req, UUID, TAG1) + @mock.patch('nova.notifications.base.send_instance_update_notification') @mock.patch('nova.db.instance_tag_add') @mock.patch('nova.db.instance_tag_get_by_instance_uuid') - def test_update(self, mock_db_get_inst_tags, mock_db_add_inst_tags): + def test_update(self, mock_db_get_inst_tags, mock_db_add_inst_tags, + mock_notify): self.stub_out('nova.api.openstack.common.get_instance', return_server) mock_db_get_inst_tags.return_value = [self._get_tag(TAG1)] mock_db_add_inst_tags.return_value = self._get_tag(TAG2) @@ -176,7 +180,8 @@ class ServerTagsTest(test.TestCase): self.assertEqual(0, len(res.body)) self.assertEqual(location, res.headers['Location']) mock_db_add_inst_tags.assert_called_once_with(mock.ANY, UUID, TAG2) - mock_db_get_inst_tags.assert_called_once_with(mock.ANY, UUID) + self.assertEqual(2, mock_db_get_inst_tags.call_count) + self.assertEqual(1, mock_notify.call_count) @mock.patch('nova.db.instance_tag_get_by_instance_uuid') def test_update_existing_tag(self, mock_db_get_inst_tags): @@ -232,13 +237,18 @@ class ServerTagsTest(test.TestCase): self.assertRaises(exc.HTTPConflict, self.controller.update, req, UUID, TAG1, body=None) + @mock.patch('nova.db.instance_tag_get_by_instance_uuid') + @mock.patch('nova.notifications.base.send_instance_update_notification') @mock.patch('nova.db.instance_tag_delete') - def test_delete(self, mock_db_delete_inst_tags): + def test_delete(self, mock_db_delete_inst_tags, mock_notify, + mock_db_get_inst_tags): self.stub_out('nova.api.openstack.common.get_instance', return_server) req = self._get_request( '/v2/fake/servers/%s/tags/%s' % (UUID, TAG2), 'DELETE') self.controller.delete(req, UUID, TAG2) mock_db_delete_inst_tags.assert_called_once_with(mock.ANY, UUID, TAG2) + mock_db_get_inst_tags.assert_called_once_with(mock.ANY, UUID) + self.assertEqual(1, mock_notify.call_count) @mock.patch('nova.db.instance_tag_delete') def test_delete_non_existing_tag(self, mock_db_delete_inst_tags): @@ -263,12 +273,14 @@ class ServerTagsTest(test.TestCase): self.assertRaises(exc.HTTPConflict, self.controller.delete, req, UUID, TAG1) + @mock.patch('nova.notifications.base.send_instance_update_notification') @mock.patch('nova.db.instance_tag_delete_all') - def test_delete_all(self, mock_db_delete_inst_tags): + def test_delete_all(self, mock_db_delete_inst_tags, mock_notify): self.stub_out('nova.api.openstack.common.get_instance', return_server) req = self._get_request('/v2/fake/servers/%s/tags' % UUID, 'DELETE') self.controller.delete_all(req, UUID) mock_db_delete_inst_tags.assert_called_once_with(mock.ANY, UUID) + self.assertEqual(1, mock_notify.call_count) def test_delete_all_invalid_instance_state(self): self.stub_out('nova.api.openstack.common.get_instance', diff --git a/nova/tests/unit/notifications/test_base.py b/nova/tests/unit/notifications/test_base.py index b5b622ad0c2e..2136c489261a 100644 --- a/nova/tests/unit/notifications/test_base.py +++ b/nova/tests/unit/notifications/test_base.py @@ -14,6 +14,8 @@ # under the License. import datetime +import mock + from nova.notifications import base from nova import test from nova import utils @@ -44,3 +46,19 @@ class TestNullSafeUtils(test.NoDBTestCase): self.assertEqual('', base.null_safe_int(number)) number = 10 self.assertEqual(number, base.null_safe_int(number)) + + +class TestSendInstanceUpdateNotification(test.NoDBTestCase): + + @mock.patch.object(base, 'info_from_instance', + new_callable=mock.NonCallableMock) # asserts not called + # TODO(mriedem): Rather than mock is_enabled, it would be better to + # configure oslo_messaging_notifications.driver=['noop'] + @mock.patch('nova.rpc.NOTIFIER.is_enabled', return_value=False) + def test_send_instance_update_notification_disabled(self, mock_enabled, + mock_info): + """Tests the case that notifications are disabled which makes + send_instance_update_notification a noop. + """ + base.send_instance_update_notification(mock.sentinel.ctxt, + mock.sentinel.instance) diff --git a/nova/tests/unit/test_notifications.py b/nova/tests/unit/test_notifications.py index 7b0b6d78c5a7..d94b54b36426 100644 --- a/nova/tests/unit/test_notifications.py +++ b/nova/tests/unit/test_notifications.py @@ -551,7 +551,7 @@ class NotificationsTestCase(test.TestCase): def sending_no_state_change(context, instance, **kwargs): called[0] = True self.stub_out('nova.notifications.base.' - '_send_instance_update_notification', + 'send_instance_update_notification', sending_no_state_change) notifications.send_update(self.context, self.instance, self.instance) self.assertTrue(called[0]) @@ -560,7 +560,7 @@ class NotificationsTestCase(test.TestCase): def fail_sending(context, instance, **kwargs): raise Exception('failed to notify') self.stub_out('nova.notifications.base.' - '_send_instance_update_notification', + 'send_instance_update_notification', fail_sending) notifications.send_update(self.context, self.instance, self.instance) @@ -572,7 +572,7 @@ class NotificationsTestCase(test.TestCase): # not logged as an error. notfound = exception.InstanceNotFound(instance_id=self.instance.uuid) with mock.patch.object(notifications, - '_send_instance_update_notification', + 'send_instance_update_notification', side_effect=notfound): notifications.send_update( self.context, self.instance, self.instance) @@ -586,7 +586,7 @@ class NotificationsTestCase(test.TestCase): # not logged as an error. notfound = exception.InstanceNotFound(instance_id=self.instance.uuid) with mock.patch.object(notifications, - '_send_instance_update_notification', + 'send_instance_update_notification', side_effect=notfound): notifications.send_update_with_states( self.context, self.instance, diff --git a/releasenotes/notes/send-notification-when-instance-tag-changes-67c08000b6e0cd2a.yaml b/releasenotes/notes/send-notification-when-instance-tag-changes-67c08000b6e0cd2a.yaml new file mode 100644 index 000000000000..2873a2db8067 --- /dev/null +++ b/releasenotes/notes/send-notification-when-instance-tag-changes-67c08000b6e0cd2a.yaml @@ -0,0 +1,4 @@ +--- +features: + - Versioned instance.update notification will be sent + when server's tags field is updated.