openstack
/
nova
Code Issues Proposed changes

Fix host mapping saving 

Changes return ability to use HostMapping.save method,
it was broken.

It fixes two issues:
- HostMapping._save_in_db got unexpected parameter
  (self.host instead of self object from the save call).
  It's clear.

- "sqlalchemy.orm.exc.DetachedInstanceError: Parent instance
  <HostMapping> is not bound to a Session;"
  while trying to get the cell_mapping attribute on saved HostMapping
  instance.

  As HostMapping cannot be without cell_mapping, solution is to load
  cell_mapping attribute just after getting it from DB.
  To be consistent I used the code from _create_in_db method.

Closes-bug: 1595587
Change-Id: Ia2e427f5bd4ab43d1c273de72ef7bb8c01d8d1af
This commit is contained in:
Andrey Volkov 2016-06-21 11:47:28 +03:00
parent 7a8f1369c5
commit daad6c26ec
3 changed files with 40 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
nova/objects/host_mapping.py
View File

@ -26,6 +26,17 @@ def _cell_id_in_updates(updates):
updates["cell_id"] = cell_mapping_obj.id
def _apply_updates(context, db_mapping, updates):
db_mapping.update(updates)
db_mapping.save(context.session)
# NOTE: This is done because a later access will trigger a lazy load
# outside of the db session so it will fail. We don't lazy load
# cell_mapping on the object later because we never need a HostMapping
# without the CellMapping.
db_mapping.cell_mapping
return db_mapping
@base.NovaObjectRegistry.register
class HostMapping(base.NovaTimestampObject, base.NovaObject):
# Version 1.0: Initial version
@ -61,7 +72,7 @@ class HostMapping(base.NovaTimestampObject, base.NovaObject):
if key == "cell_mapping":
# NOTE(dheeraj): If cell_mapping is stashed in db object
# we load it here. Otherwise, lazy loading will happen
# when .cell_mapping is accessd later
# when .cell_mapping is accessed later
if not db_value:
continue
db_value = cell_mapping.CellMapping._from_db_object(
@ -91,14 +102,7 @@ class HostMapping(base.NovaTimestampObject, base.NovaObject):
@db_api.api_context_manager.writer
def _create_in_db(context, updates):
db_mapping = api_models.HostMapping()
db_mapping.update(updates)
db_mapping.save(context.session)
# NOTE: This is done because a later access will trigger a lazy load
# outside of the db session so it will fail. We don't lazy load
# cell_mapping on the object later because we never need a HostMapping
# without the CellMapping.
db_mapping.cell_mapping
return db_mapping
return _apply_updates(context, db_mapping, updates)
@base.remotable
def create(self):
@ -115,16 +119,14 @@ class HostMapping(base.NovaTimestampObject, base.NovaObject):
id=obj.id).first()
if not db_mapping:
raise exception.HostMappingNotFound(name=obj.host)
db_mapping.update(updates)
return db_mapping
return _apply_updates(context, db_mapping, updates)
@base.remotable
def save(self):
changes = self.obj_get_changes()
# cell_mapping must be mapped to cell_id for updates
_cell_id_in_updates(changes)
db_mapping = self._save_in_db(self._context, self.host, changes)
db_mapping = self._save_in_db(self._context, self, changes)
self._from_db_object(self._context, self, db_mapping)
self.obj_reset_changes()

31
nova/tests/functional/db/test_host_mapping.py
View File

@ -85,19 +85,26 @@ class HostMappingTestCase(test.NoDBTestCase):
self.mapping_obj._get_by_host_from_db, self.context,
'fake-host2')
def test_save_in_db(self):
mapping = create_mapping()
new_cell = create_cell_mapping(id=42)
self.mapping_obj._save_in_db(self.context, mapping,
{'cell_id': new_cell["id"]})
db_mapping = self.mapping_obj._get_by_host_from_db(
self.context, mapping['host'])
self.assertNotEqual(db_mapping['cell_id'], mapping['cell_id'])
for key in [key for key in self.mapping_obj.fields.keys()
if key not in ('updated_at', 'cell_id')]:
if key == "cell_mapping":
def test_update_cell_mapping(self):
db_hm = create_mapping()
db_cell = create_cell_mapping(id=42)
cell = cell_mapping.CellMapping.get_by_uuid(
self.context, db_cell['uuid'])
hm = host_mapping.HostMapping(self.context)
hm.id = db_hm['id']
hm.cell_mapping = cell
hm.save()
self.assertNotEqual(db_hm['cell_id'], hm.cell_mapping.id)
for key in hm.fields.keys():
if key in ('updated_at', 'cell_mapping'):
continue
self.assertEqual(db_mapping[key], mapping[key])
model_field = getattr(hm, key)
if key == 'created_at':
model_field = model_field.replace(tzinfo=None)
self.assertEqual(db_hm[key], model_field, 'field %s' % key)
db_hm_new = host_mapping.HostMapping._get_by_host_from_db(
self.context, db_hm['host'])
self.assertNotEqual(db_hm['cell_id'], db_hm_new['cell_id'])
def test_destroy_in_db(self):
mapping = create_mapping()

8
nova/tests/unit/objects/test_host_mapping.py
View File

@ -13,6 +13,8 @@
import mock
from nova import objects
from nova import test
from nova.objects import host_mapping
from nova.tests.unit.objects import test_cell_mapping
from nova.tests.unit.objects import test_objects
@ -109,6 +111,7 @@ class _TestHostMappingObject(object):
host = db_mapping['host']
mapping_obj = objects.HostMapping(self.context)
mapping_obj.host = host
mapping_obj.id = db_mapping['id']
new_fake_cell = test_cell_mapping.get_db_mapping(id=10)
fake_cell_obj = objects.CellMapping(self.context, **new_fake_cell)
mapping_obj.cell_mapping = fake_cell_obj
@ -117,9 +120,10 @@ class _TestHostMappingObject(object):
mapping_obj.save()
save_in_db.assert_called_once_with(self.context,
db_mapping['host'],
test.MatchType(host_mapping.HostMapping),
{'cell_id': new_fake_cell["id"],
'host': host})
'host': host,
'id': db_mapping['id']})
self.compare_obj(mapping_obj, db_mapping,
subs={'cell_mapping': 'cell_id'},
comparators={