Merge "Fix host mapping saving"

nova/objects/host_mapping.py

@@ -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)
+        return _apply_updates(context, db_mapping, 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.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)
-
+        return _apply_updates(context, db_mapping, updates)
-        db_mapping.update(updates)
-        return db_mapping
 
     @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()
 

nova/tests/functional/db/test_host_mapping.py

@@ -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):
+    def test_update_cell_mapping(self):
-        mapping = create_mapping()
+        db_hm = create_mapping()
-        new_cell = create_cell_mapping(id=42)
+        db_cell = create_cell_mapping(id=42)
-        self.mapping_obj._save_in_db(self.context, mapping,
+        cell = cell_mapping.CellMapping.get_by_uuid(
-                {'cell_id': new_cell["id"]})
+            self.context, db_cell['uuid'])
-        db_mapping = self.mapping_obj._get_by_host_from_db(
+        hm = host_mapping.HostMapping(self.context)
-                self.context, mapping['host'])
+        hm.id = db_hm['id']
-        self.assertNotEqual(db_mapping['cell_id'], mapping['cell_id'])
+        hm.cell_mapping = cell
-        for key in [key for key in self.mapping_obj.fields.keys()
+        hm.save()
-                    if key not in ('updated_at', 'cell_id')]:
+        self.assertNotEqual(db_hm['cell_id'], hm.cell_mapping.id)
-            if key == "cell_mapping":
+        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()

nova/tests/unit/objects/test_host_mapping.py

@@ -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={