From 738c45ee514a67a2fec37697272cc7a54d716c16 Mon Sep 17 00:00:00 2001
From: Balazs Gibizer <balazs.gibizer@ericsson.com>
Date: Thu, 2 Jul 2015 14:43:35 +0200
Subject: [PATCH] Make on_shared_storage optional in compute manager

To be able to later make the onSharedStorage parameter of the evacuate
API optional first we have to make the on_shared_storage parameter of
the rebuild_instance optional in the compute manager.

After this modification if the on_shared_storage is not provided then
the rebuild_instance call will use the instance_on_disk function of
the virt dirver to decide if the instance files need to be recreated or
they can be reused.

Implements: bp optional-on-shared-storage-flag-in-rebuild-instance
Change-Id: I4217bd00d8c253db522241885dba2847a26af6df
---
 nova/compute/manager.py                 | 20 ++++++++++++----
 nova/tests/unit/compute/test_compute.py | 31 +++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 8612e04e8176..51667e8ff959 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -2553,7 +2553,7 @@ class ComputeManager(manager.Manager):
     @wrap_instance_fault
     def rebuild_instance(self, context, instance, orig_image_ref, image_ref,
                          injected_files, new_pass, orig_sys_metadata,
-                         bdms, recreate, on_shared_storage,
+                         bdms, recreate, on_shared_storage=None,
                          preserve_ephemeral=False):
         """Destroy and re-make this instance.
 
@@ -2571,7 +2571,10 @@ class ComputeManager(manager.Manager):
         :param recreate: True if the instance is being recreated (e.g. the
             hypervisor it was on failed) - cleanup of old state will be
             skipped.
-        :param on_shared_storage: True if instance files on shared storage
+        :param on_shared_storage: True if instance files on shared storage.
+                                  If not provided then information from the
+                                  driver will be used to decide if the instance
+                                  files are available or not on the target host
         :param preserve_ephemeral: True if the default ephemeral storage
                                    partition must be preserved on rebuild
         """
@@ -2588,9 +2591,16 @@ class ComputeManager(manager.Manager):
 
                 self._check_instance_exists(context, instance)
 
-                # To cover case when admin expects that instance files are on
-                # shared storage, but not accessible and vice versa
-                if on_shared_storage != self.driver.instance_on_disk(instance):
+                if on_shared_storage is None:
+                    LOG.debug('on_shared_storage is not provided, using driver'
+                              'information to decide if the instance needs to'
+                              'be recreated')
+                    on_shared_storage = self.driver.instance_on_disk(instance)
+
+                elif (on_shared_storage !=
+                        self.driver.instance_on_disk(instance)):
+                    # To cover case when admin expects that instance files are
+                    # on shared storage, but not accessible and vice versa
                     raise exception.InvalidSharedStorage(
                             _("Invalid state of instance files on shared"
                               " storage"))
diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py
index f55c67469e57..8bc63ecec965 100644
--- a/nova/tests/unit/compute/test_compute.py
+++ b/nova/tests/unit/compute/test_compute.py
@@ -11212,6 +11212,37 @@ class EvacuateHostTestCase(BaseTestCase):
             self.assertRaises(exception.InstanceRecreateNotSupported,
                               lambda: self._rebuild(on_shared_storage=True))
 
+    def test_on_shared_storage_not_provided_host_without_shared_storage(self):
+        fake_image = {'id': 1,
+                      'name': 'fake_name',
+                      'properties': {'kernel_id': 'fake_kernel_id',
+                                     'ramdisk_id': 'fake_ramdisk_id'}}
+
+        self.mox.StubOutWithMock(self.compute.driver, 'spawn')
+        self.compute.driver.spawn(mox.IsA(self.context),
+                mox.IsA(objects.Instance), mox.IsA(fake_image),
+                mox.IgnoreArg(), mox.IsA('newpass'),
+                network_info=mox.IgnoreArg(),
+                block_device_info=mox.IgnoreArg())
+
+        self.stubs.Set(self.compute.driver, 'instance_on_disk',
+                       lambda x: False)
+        self.mox.ReplayAll()
+
+        self._rebuild(on_shared_storage=None)
+
+    def test_on_shared_storage_not_provided_host_with_shared_storage(self):
+        self.mox.StubOutWithMock(self.compute.driver, 'spawn')
+        self.compute.driver.spawn(mox.IsA(self.context),
+                mox.IsA(objects.Instance), {}, mox.IgnoreArg(), 'newpass',
+                network_info=mox.IgnoreArg(),
+                block_device_info=mox.IgnoreArg())
+
+        self.stubs.Set(self.compute.driver, 'instance_on_disk', lambda x: True)
+        self.mox.ReplayAll()
+
+        self._rebuild(on_shared_storage=None)
+
 
 class ComputeInjectedFilesTestCase(BaseTestCase):
     # Test that running instances with injected_files decodes files correctly