Send out notifications when instance tags changed

This patch adds notifications when instance
tags changed, as tag is commonly used for
searching and filtering, and projects like
searchlight depends on the notifications.

To avoid unnecessarily building a notification
payload when notifications are disabled, this
change also adds the if_notifications_enabled
decorator to send_instance_update_notification.

Change-Id: I03c8e8225e51fd80580772752c0b292987e34218
Implements: bp send-tag-notification

4 files changed, 65 insertions, 24 deletions

diff --git a/nova/tests/functional/notification_sample_tests/test_instance.py b/nova/tests/functional/notification_sample_tests/test_instance.py
index 38037d4f3a..f658e461db 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 7545eb8e82..5c53a4f60f 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 b5b622ad0c..2136c48926 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 7b0b6d78c5..d94b54b364 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,