Move snapshot image property inheritance

The xenapi snapshotting code had support for inheriting properties
from the base image of the snapshot (by way of the system metadata
set on the snapshotted instance).  The problem, however, came in the
fact that some image properties were set in nova.compute.api, but
those properties were not excluded from this inheritance logic
except through a configuration option called
"non_inheritable_image_properties".  I had previously updated the
default setting for this option to work around the bugs introduced
by setting image properties in two different locations, but now it
is time for the real fix.

This change moves the inheritance logic into
nova.compute.api:API._create_image.  Note that two properties are
still set through the xenapi snapshotting logic: the "os_type" and
the "auto_disk_config" properties, which are presumed to be xenapi
specific.  The change also alters the inheritance logic to ensure
that the work-around listing of image properties in
non_inheritable_image_properties is no longer necessary; the
default for this configuration option is updated accordingly.

(Note: It will not harm anything to have these image properties
still listed in non_inheritable_image_properties, so configurations
that override this option do not need to be altered.)

Change-Id: I3514da432cc10c75418e1de9752f60640d579136

2 files changed, 23 insertions, 5 deletions

diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py
index 78968afab..cad27b544 100644
--- a/nova/tests/compute/test_compute.py
+++ b/nova/tests/compute/test_compute.py
@@ -3532,6 +3532,27 @@ class ComputeAPITestCase(BaseTestCase):
 
         db.instance_destroy(self.context, instance['uuid'])
 
+    def test_snapshot_image_metadata_inheritance(self):
+        # Ensure image snapshots inherit metadata from the base image
+        self.flags(non_inheritable_image_properties=['spam'])
+
+        def fake_instance_system_metadata_get(context, uuid):
+            return dict(image_a=1, image_b=2, image_c='c', d='d', spam='spam')
+
+        self.stubs.Set(db, 'instance_system_metadata_get',
+                       fake_instance_system_metadata_get)
+
+        instance = self._create_fake_instance()
+        image = self.compute_api.snapshot(self.context, instance, 'snap1',
+                                          {'extra_param': 'value1'})
+
+        properties = image['properties']
+        self.assertEqual(properties['a'], 1)
+        self.assertEqual(properties['b'], 2)
+        self.assertEqual(properties['c'], 'c')
+        self.assertEqual(properties['d'], 'd')
+        self.assertFalse('spam' in properties)
+
     def test_backup(self):
         """Can't backup an instance which is already being backed up."""
         instance = self._create_fake_instance()
diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py
index 5f967862b..adacffbb5 100644
--- a/nova/tests/test_xenapi.py
+++ b/nova/tests/test_xenapi.py
@@ -2296,9 +2296,6 @@ class VmUtilsTestCase(test.TestCase):
     """Unit tests for xenapi utils."""
 
     def test_upload_image(self):
-        """Ensure image properties include instance system metadata
-           as well as few local settings."""
-
         def fake_instance_system_metadata_get(context, uuid):
             return dict(image_a=1, image_b=2, image_c='c', d='d')
 
@@ -2337,8 +2334,8 @@ class VmUtilsTestCase(test.TestCase):
         vm_utils.upload_image(ctx, session, instance, "vmi uuids", "image id")
 
         actual = self.kwargs['properties']
-        expected = dict(a=1, b=2, c='c', d='d',
-                        auto_disk_config='auto disk config',
+        # Inheritance happens in another place, now
+        expected = dict(auto_disk_config='auto disk config',
                         os_type='os type')
         self.assertEquals(expected, actual)