Prevent redundant instance.update notifications

Use _sync_cells within Instance.save() to prevent redundant
notifications by nova-cells in api cells from updates made in compute
cells.

Commit eaaa6593 used the Instance.skip_cells_sync() contextmanager
with Instance.save() in instance_update_at_top() to prevent update
cycles between api and compute cells, but the notification in
Instance.save() is not guarded by _sync_cells, so those notifications
generated in compute cells are also generated in api cells.

Change-Id: I16dca3c3691833931dba77cd700da64ee23f583c
Co-Authored-By: Philip Schwartz <philip.schwartz@rackspace.com>
Closes-Bug: #1516174

nova/objects/instance.py

@@ -670,12 +670,22 @@ class Instance(base.NovaPersistentObject, base.NovaObject,
                 cells_api = cells_rpcapi.CellsAPI()
                 cells_api.instance_update_at_top(context, stale_instance)
 
-        # NOTE(danms): We have to be super careful here not to trigger
-        # any lazy-loads that will unmigrate or unbackport something. So,
-        # make a copy of the instance for notifications first.
-        new_ref = self.obj_clone()
+        def _notify():
+            # NOTE(danms): We have to be super careful here not to trigger
+            # any lazy-loads that will unmigrate or unbackport something. So,
+            # make a copy of the instance for notifications first.
+            new_ref = self.obj_clone()
+
+            notifications.send_update(context, old_ref, new_ref)
+
+        # NOTE(alaski): If cell synchronization is blocked it means we have
+        # already run this block of code in either the parent or child of this
+        # cell.  Therefore this notification has already been sent.
+        if not self._sync_cells:
+            _notify = lambda: None  # noqa: F811
+
+        _notify()
 
-        notifications.send_update(context, old_ref, new_ref)
         self.obj_reset_changes()
 
     @base.remotable

nova/tests/unit/objects/test_instance.py

@@ -522,11 +522,12 @@ class _TestInstanceObject(object):
             inst.save()
             self.assertFalse(mock_upd.called)
 
+    @mock.patch.object(notifications, 'send_update')
     @mock.patch.object(cells_rpcapi.CellsAPI, 'instance_update_from_api')
     @mock.patch.object(cells_rpcapi.CellsAPI, 'instance_update_at_top')
     @mock.patch.object(db, 'instance_update_and_get_original')
     def _test_skip_cells_sync_helper(self, mock_db_update, mock_update_at_top,
-            mock_update_from_api, cell_type):
+            mock_update_from_api, mock_notif_update, cell_type):
         self.flags(enable=True, cell_type=cell_type, group='cells')
         inst = fake_instance.fake_instance_obj(self.context, cell_name='fake')
         inst.vm_state = 'foo'
@@ -544,6 +545,7 @@ class _TestInstanceObject(object):
 
         mock_update_at_top.assert_has_calls([])
         mock_update_from_api.assert_has_calls([])
+        self.assertFalse(mock_notif_update.called)
 
         inst.vm_state = 'bar'
         inst.task_state = 'foo'
@@ -561,6 +563,7 @@ class _TestInstanceObject(object):
             inst.save()
 
         self.assertEqual('foo!bar@baz', inst.cell_name)
+        self.assertTrue(mock_notif_update.called)
         if cell_type == 'compute':
             mock_update_at_top.assert_called_once_with(self.context, mock.ANY)
             # Compare primitives since we can't check instance object equality