Make instance.refresh() avoid recursion better

The instance.refresh() method is careful to prevent recursion, to avoid
lazy-loads on the object we just pulled from the database. However, it
was allowing those to raise OrphanedObjectError to the caller to make
them visible. The caller is not really at fault in that case, and can not
avoid it.

The only time this has ever come up is in the context of cellsv1 and the
keypairs field that it doesn't have set on instances in the child cells.
Thus, this changes refresh() to just skip fields that are set on the
current object and unset on the one we pull from the database, as we
would not be able to refresh those anyway. This logs the situation so that
it is visible (at DEBUG) if it becomes relevant.

Change-Id: Ibfca4ae922766b5b977e217594d12e1169ddeee8

nova/objects/instance.py

@@ -862,11 +862,20 @@ def refresh(self, use_slave=False):
         current._context = None
 
         for field in self.fields:
-            if self.obj_attr_is_set(field):
-                if field == 'info_cache':
-                    self.info_cache.refresh()
-                elif self[field] != current[field]:
-                    self[field] = current[field]
+            if field not in self:
+                continue
+            if field not in current:
+                # If the field isn't in current we should not
+                # touch it, triggering a likely-recursive lazy load.
+                # Log it so we can see it happening though, as it
+                # probably isn't expected in most cases.
+                LOG.debug('Field %s is set but not in refreshed '
+                          'instance, skipping', field)
+                continue
+            if field == 'info_cache':
+                self.info_cache.refresh()
+            elif self[field] != current[field]:
+                self[field] = current[field]
         self.obj_reset_changes()
 
     def _load_generic(self, attrname):

nova/tests/unit/objects/test_instance.py

@@ -386,7 +386,7 @@ def test_refresh_does_not_recurse(self, mock_get):
 
         mock_get.return_value = inst_copy
 
-        self.assertRaises(exception.OrphanedObjectError, inst.refresh)
+        inst.refresh()
         mock_get.assert_called_once_with(self.context, uuid=inst.uuid,
             expected_attrs=['metadata'], use_slave=False)