Merge "Make system_metadata update in place"

3 files changed, 43 insertions, 9 deletions

diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index 393e1a03c..e7a871345 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -1811,9 +1811,10 @@ def instance_update_and_get_original(context, instance_uuid, values):
 # NOTE(danms): This updates the instance's metadata list in-place and in
 # the database to avoid stale data and refresh issues. It assumes the
 # delete=True behavior of instance_metadata_update(...)
-def _instance_metadata_update_in_place(context, instance, metadata, session):
+def _instance_metadata_update_in_place(context, instance, metadata_type, model,
+                                       metadata, session):
     to_delete = []
-    for keyvalue in instance['metadata']:
+    for keyvalue in instance[metadata_type]:
         key = keyvalue['key']
         if key in metadata:
             keyvalue['value'] = metadata.pop(key)
@@ -1821,15 +1822,14 @@ def _instance_metadata_update_in_place(context, instance, metadata, session):
             to_delete.append(keyvalue)
 
     for condemned in to_delete:
-        instance['metadata'].remove(condemned)
         condemned.soft_delete(session=session)
 
     for key, value in metadata.iteritems():
-        newitem = models.InstanceMetadata()
+        newitem = model()
         newitem.update({'key': key, 'value': value,
                         'instance_uuid': instance['uuid']})
         session.add(newitem)
-        instance['metadata'].append(newitem)
+        instance[metadata_type].append(newitem)
 
 
 def _instance_update(context, instance_uuid, values, copy_old_instance=False):
@@ -1877,14 +1877,18 @@ def _instance_update(context, instance_uuid, values, copy_old_instance=False):
         metadata = values.get('metadata')
         if metadata is not None:
             _instance_metadata_update_in_place(context, instance_ref,
+                                               'metadata',
+                                               models.InstanceMetadata,
                                                values.pop('metadata'),
                                                session)
 
         system_metadata = values.get('system_metadata')
         if system_metadata is not None:
-            instance_system_metadata_update(
-                 context, instance_ref['uuid'], values.pop('system_metadata'),
-                 delete=True, session=session)
+            _instance_metadata_update_in_place(context, instance_ref,
+                                               'system_metadata',
+                                               models.InstanceSystemMetadata,
+                                               values.pop('system_metadata'),
+                                               session)
 
         instance_ref.update(values)
         instance_ref.save(session=session)
diff --git a/nova/tests/test_db_api.py b/nova/tests/test_db_api.py
index 684f9fded..db1f16440 100644
--- a/nova/tests/test_db_api.py
+++ b/nova/tests/test_db_api.py
@@ -29,6 +29,7 @@ from nova.openstack.common import cfg
 from nova.openstack.common import timeutils
 from nova import test
 from nova.tests import matchers
+from nova import utils
 
 
 CONF = cfg.CONF
@@ -467,6 +468,34 @@ class DbApiTestCase(test.TestCase):
         self.assertEquals(spec, old_ref['extra_specs'])
         self.assertEquals(spec, new_ref['extra_specs'])
 
+    def _test_instance_update_updates_metadata(self, metadata_type):
+        ctxt = context.get_admin_context()
+
+        instance = db.instance_create(ctxt, {})
+
+        def set_and_check(meta):
+            inst = db.instance_update(ctxt, instance['uuid'],
+                               {metadata_type: dict(meta)})
+            _meta = utils.metadata_to_dict(inst[metadata_type])
+            self.assertEqual(meta, _meta)
+
+        meta = {'speed': '88', 'units': 'MPH'}
+        set_and_check(meta)
+
+        meta['gigawatts'] = '1.21'
+        set_and_check(meta)
+
+        del meta['gigawatts']
+        set_and_check(meta)
+
+    def test_instance_update_updates_system_metadata(self):
+        # Ensure that system_metadata is updated during instance_update
+        self._test_instance_update_updates_metadata('system_metadata')
+
+    def test_instance_update_updates_metadata(self):
+        # Ensure that metadata is updated during instance_update
+        self._test_instance_update_updates_metadata('metadata')
+
     def test_instance_fault_create(self):
         # Ensure we can create an instance fault.
         ctxt = context.get_admin_context()
diff --git a/nova/utils.py b/nova/utils.py
index 83bf55583..cb75709ad 100644
--- a/nova/utils.py
+++ b/nova/utils.py
@@ -1291,5 +1291,6 @@ def last_bytes(file_like_object, num):
 def metadata_to_dict(metadata):
     result = {}
     for item in metadata:
-        result[item['key']] = item['value']
+        if not item.get('deleted'):
+            result[item['key']] = item['value']
     return result