Merge "Send out notifications when instance tags changed"

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,