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.

Closes-Bug: #1772088

Change-Id: Ibfca4ae922766b5b977e217594d12e1169ddeee8
(cherry picked from commit 549e5a2226)
This commit is contained in:
Dan Smith 2018-05-18 09:28:18 -07:00 committed by Matt Riedemann
parent d8b336f473
commit 419e6198fa
2 changed files with 15 additions and 6 deletions

View File

@ -847,11 +847,20 @@ class Instance(base.NovaPersistentObject, base.NovaObject,
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):

View File

@ -380,7 +380,7 @@ class _TestInstanceObject(object):
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)