Merge "Cells: Reduce the create_image call depth for cells"

4 files changed, 152 insertions, 20 deletions

diff --git a/nova/compute/api.py b/nova/compute/api.py
index 293ab2d74..bddb83449 100644
--- a/nova/compute/api.py
+++ b/nova/compute/api.py
@@ -1288,7 +1288,7 @@ class API(base.Base):
     @wrap_check_policy
     @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED])
     def backup(self, context, instance, name, backup_type, rotation,
-               extra_properties=None):
+               extra_properties=None, image_id=None):
         """Backup the given instance
 
         :param instance: nova.db.sqlalchemy.models.Instance
@@ -1301,9 +1301,14 @@ class API(base.Base):
         instance = self.update(context, instance,
                                task_state=task_states.IMAGE_BACKUP,
                                expected_task_state=None)
-        image_meta = self._create_image(context, instance, name, 'backup',
-                            backup_type=backup_type, rotation=rotation,
-                            extra_properties=extra_properties)
+        if image_id:
+            # The image entry has already been created, so just pull the
+            # metadata.
+            image_meta = self.image_service.show(context, image_id)
+        else:
+            image_meta = self._create_image(context, instance, name,
+                    'backup', backup_type=backup_type,
+                    rotation=rotation, extra_properties=extra_properties)
         self.compute_rpcapi.snapshot_instance(context, instance=instance,
                 image_id=image_meta['id'], image_type='backup',
                 backup_type=backup_type, rotation=rotation)
@@ -1311,7 +1316,8 @@ class API(base.Base):
 
     @wrap_check_policy
     @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.STOPPED])
-    def snapshot(self, context, instance, name, extra_properties=None):
+    def snapshot(self, context, instance, name, extra_properties=None,
+                 image_id=None):
         """Snapshot the given instance.
 
         :param instance: nova.db.sqlalchemy.models.Instance
@@ -1323,8 +1329,13 @@ class API(base.Base):
         instance = self.update(context, instance,
                                task_state=task_states.IMAGE_SNAPSHOT,
                                expected_task_state=None)
-        image_meta = self._create_image(context, instance, name,
-                'snapshot', extra_properties=extra_properties)
+        if image_id:
+            # The image entry has already been created, so just pull the
+            # metadata.
+            image_meta = self.image_service.show(context, image_id)
+        else:
+            image_meta = self._create_image(context, instance, name,
+                    'snapshot', extra_properties=extra_properties)
         self.compute_rpcapi.snapshot_instance(context, instance=instance,
                 image_id=image_meta['id'], image_type='snapshot')
         return image_meta
diff --git a/nova/compute/cells_api.py b/nova/compute/cells_api.py
index 698c6eed0..47d60aec4 100644
--- a/nova/compute/cells_api.py
+++ b/nova/compute/cells_api.py
@@ -115,15 +115,28 @@ class ComputeCellsAPI(compute_api.API):
         """
         return
 
-    def _create_image(self, context, instance, name, image_type,
-            backup_type=None, rotation=None, extra_properties=None):
-        if backup_type:
-            return self._call_to_cells(context, instance, 'backup',
-                    name, backup_type, rotation,
-                    extra_properties=extra_properties)
-        else:
-            return self._call_to_cells(context, instance, 'snapshot',
-                    name, extra_properties=extra_properties)
+    def backup(self, context, instance, name, backup_type, rotation,
+               extra_properties=None, image_id=None):
+        """Backup the given instance."""
+        image_meta = super(ComputeCellsAPI, self).backup(context,
+                instance, name, backup_type, rotation,
+                extra_properties=extra_properties, image_id=image_id)
+        image_id = image_meta['id']
+        self._cast_to_cells(context, instance, 'backup', name,
+                backup_type=backup_type, rotation=rotation,
+                extra_properties=extra_properties, image_id=image_id)
+        return image_meta
+
+    def snapshot(self, context, instance, name, extra_properties=None,
+                 image_id=None):
+        """Snapshot the given instance."""
+        image_meta = super(ComputeCellsAPI, self).snapshot(context,
+                instance, name, extra_properties=extra_properties,
+                image_id=image_id)
+        image_id = image_meta['id']
+        self._cast_to_cells(context, instance, 'snapshot',
+                name, extra_properties=extra_properties, image_id=image_id)
+        return image_meta
 
     def create(self, *args, **kwargs):
         """We can use the base functionality, but I left this here just
diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py
index b79fbd042..7fac63e09 100644
--- a/nova/tests/compute/test_compute.py
+++ b/nova/tests/compute/test_compute.py
@@ -4376,6 +4376,31 @@ class ComputeAPITestCase(BaseTestCase):
 
         db.instance_destroy(self.context, instance['uuid'])
 
+    def test_snapshot_given_image_uuid(self):
+        """Ensure a snapshot of an instance can be created when image UUID
+        is already known.
+        """
+        instance = self._create_fake_instance()
+        name = 'snap1'
+        extra_properties = {'extra_param': 'value1'}
+        recv_meta = self.compute_api.snapshot(self.context, instance, name,
+                                              extra_properties)
+        image_id = recv_meta['id']
+
+        def fake_show(meh, context, id):
+            return recv_meta
+
+        instance = db.instance_update(self.context, instance['uuid'],
+                {'task_state': None})
+        fake_image.stub_out_image_service(self.stubs)
+        self.stubs.Set(fake_image._FakeImageService, 'show', fake_show)
+        image = self.compute_api.snapshot(self.context, instance, name,
+                                          extra_properties,
+                                          image_id=image_id)
+        self.assertEqual(image, recv_meta)
+
+        db.instance_destroy(self.context, instance['uuid'])
+
     def test_snapshot_minram_mindisk_VHD(self):
         """Ensure a snapshots min_ram and min_disk are correct.
 
@@ -4467,7 +4492,10 @@ class ComputeAPITestCase(BaseTestCase):
         def fake_show(*args):
             raise exception.ImageNotFound(image_id="fake")
 
-        self.stubs.Set(fake_image._FakeImageService, 'show', fake_show)
+        if not self.__class__.__name__ == "CellsComputeAPITestCase":
+            # Cells tests will call this a 2nd time in child cell with
+            # the newly created image_id, and we want that one to succeed.
+            self.stubs.Set(fake_image._FakeImageService, 'show', fake_show)
 
         instance = self._create_fake_instance()
 
diff --git a/nova/tests/compute/test_compute_cells.py b/nova/tests/compute/test_compute_cells.py
index aa4b448d4..3c25f9b43 100644
--- a/nova/tests/compute/test_compute_cells.py
+++ b/nova/tests/compute/test_compute_cells.py
@@ -16,7 +16,11 @@
 """
 Tests For Compute w/ Cells
 """
+import functools
+
 from nova.compute import cells_api as compute_cells_api
+from nova import db
+from nova.openstack.common import jsonutils
 from nova.openstack.common import log as logging
 from nova.tests.compute import test_compute
 
@@ -28,17 +32,57 @@ ORIG_COMPUTE_API = None
 
 def stub_call_to_cells(context, instance, method, *args, **kwargs):
     fn = getattr(ORIG_COMPUTE_API, method)
+    original_instance = kwargs.pop('original_instance', None)
+    if original_instance:
+        instance = original_instance
+        # Restore this in 'child cell DB'
+        db.instance_update(context, instance['uuid'],
+                dict(vm_state=instance['vm_state'],
+                     task_state=instance['task_state']))
+
     return fn(context, instance, *args, **kwargs)
 
 
 def stub_cast_to_cells(context, instance, method, *args, **kwargs):
     fn = getattr(ORIG_COMPUTE_API, method)
+    original_instance = kwargs.pop('original_instance', None)
+    if original_instance:
+        instance = original_instance
+        # Restore this in 'child cell DB'
+        db.instance_update(context, instance['uuid'],
+                dict(vm_state=instance['vm_state'],
+                     task_state=instance['task_state']))
     fn(context, instance, *args, **kwargs)
 
 
-def deploy_stubs(stubs, api):
-    stubs.Set(api, '_call_to_cells', stub_call_to_cells)
-    stubs.Set(api, '_cast_to_cells', stub_cast_to_cells)
+def deploy_stubs(stubs, api, original_instance=None):
+    call = stub_call_to_cells
+    cast = stub_cast_to_cells
+
+    if original_instance:
+        kwargs = dict(original_instance=original_instance)
+        call = functools.partial(stub_call_to_cells, **kwargs)
+        cast = functools.partial(stub_cast_to_cells, **kwargs)
+
+    stubs.Set(api, '_call_to_cells', call)
+    stubs.Set(api, '_cast_to_cells', cast)
+
+
+def wrap_create_instance(func):
+    @functools.wraps(func)
+    def wrapper(self, *args, **kwargs):
+        instance = self._create_fake_instance()
+
+        def fake(*args, **kwargs):
+            return instance
+
+        self.stubs.Set(self, '_create_fake_instance', fake)
+        original_instance = jsonutils.to_primitive(instance)
+        deploy_stubs(self.stubs, self.compute_api,
+                     original_instance=original_instance)
+        return func(self, *args, **kwargs)
+
+    return wrapper
 
 
 class CellsComputeAPITestCase(test_compute.ComputeAPITestCase):
@@ -84,6 +128,42 @@ class CellsComputeAPITestCase(test_compute.ComputeAPITestCase):
     def test_get_backdoor_port(self):
         self.skipTest("Test is incompatible with cells.")
 
+    def test_snapshot_given_image_uuid(self):
+        self.skipTest("Test doesn't apply to API cell.")
+
+    @wrap_create_instance
+    def test_snapshot(self):
+        return super(CellsComputeAPITestCase, self).test_snapshot()
+
+    @wrap_create_instance
+    def test_snapshot_image_metadata_inheritance(self):
+        return super(CellsComputeAPITestCase,
+                self).test_snapshot_image_metadata_inheritance()
+
+    @wrap_create_instance
+    def test_snapshot_minram_mindisk(self):
+        return super(CellsComputeAPITestCase,
+                self).test_snapshot_minram_mindisk()
+
+    @wrap_create_instance
+    def test_snapshot_minram_mindisk_VHD(self):
+        return super(CellsComputeAPITestCase,
+                self).test_snapshot_minram_mindisk_VHD()
+
+    @wrap_create_instance
+    def test_snapshot_minram_mindisk_img_missing_minram(self):
+        return super(CellsComputeAPITestCase,
+                self).test_snapshot_minram_mindisk_img_missing_minram()
+
+    @wrap_create_instance
+    def test_snapshot_minram_mindisk_no_image(self):
+        return super(CellsComputeAPITestCase,
+                self).test_snapshot_minram_mindisk_no_image()
+
+    @wrap_create_instance
+    def test_backup(self):
+        return super(CellsComputeAPITestCase, self).test_backup()
+
 
 class CellsComputePolicyTestCase(test_compute.ComputePolicyTestCase):
     def setUp(self):