Merge "Move rebuild to conductor and add find host logic"

nova/compute/api.py

@@ -2204,11 +2204,12 @@ class API(base.Base):
 
         self._record_action_start(context, instance, instance_actions.REBUILD)
 
-        self.compute_rpcapi.rebuild_instance(context, instance=instance,
+        self.compute_task_api.rebuild_instance(context, instance=instance,
                 new_pass=admin_password, injected_files=files_to_inject,
                 image_ref=image_href, orig_image_ref=orig_image_ref,
                 orig_sys_metadata=orig_sys_metadata, bdms=bdms,
-                preserve_ephemeral=preserve_ephemeral, kwargs=kwargs)
+                preserve_ephemeral=preserve_ephemeral, host=instance.host,
+                kwargs=kwargs)
 
     @wrap_check_policy
     @check_instance_lock
@@ -3052,6 +3053,12 @@ class API(base.Base):
 
         Checking vm compute host state, if the host not in expected_state,
         raising an exception.
+
+        :param instance: The instance to evacuate
+        :param host: Target host. if not set, the scheduler will pick up one
+        :param on_shared_storage: True if instance files on shared storage
+        :param admin_password: password to set on rebuilt instance
+
         """
         LOG.debug('vm evacuation scheduled')
         inst_host = instance.host
@@ -3066,17 +3073,17 @@ class API(base.Base):
         instance.save(expected_task_state=[None])
         self._record_action_start(context, instance, instance_actions.EVACUATE)
 
-        return self.compute_rpcapi.rebuild_instance(context,
+        return self.compute_task_api.rebuild_instance(context,
-                                        instance=instance,
+                       instance=instance,
-                                        new_pass=admin_password,
+                       new_pass=admin_password,
-                                        injected_files=None,
+                       injected_files=None,
-                                        image_ref=None,
+                       image_ref=None,
-                                        orig_image_ref=None,
+                       orig_image_ref=None,
-                                        orig_sys_metadata=None,
+                       orig_sys_metadata=None,
-                                        bdms=None,
+                       bdms=None,
-                                        recreate=True,
+                       recreate=True,
-                                        on_shared_storage=on_shared_storage,
+                       on_shared_storage=on_shared_storage,
-                                        host=host)
+                       host=host)
 
     def get_migrations(self, context, filters):
         """Get all migrations for the given filters."""

nova/conductor/api.py

@@ -260,6 +260,24 @@ class LocalComputeTaskAPI(object):
         utils.spawn_n(self._manager.unshelve_instance, context,
                 instance=instance)
 
+    def rebuild_instance(self, context, instance, orig_image_ref, image_ref,
+                         injected_files, new_pass, orig_sys_metadata,
+                         bdms, recreate=False, on_shared_storage=False,
+                         preserve_ephemeral=False, host=None, kwargs=None):
+        # kwargs unused but required for cell compatibility.
+        utils.spawn_n(self._manager.rebuild_instance, context,
+                instance=instance,
+                new_pass=new_pass,
+                injected_files=injected_files,
+                image_ref=image_ref,
+                orig_image_ref=orig_image_ref,
+                orig_sys_metadata=orig_sys_metadata,
+                bdms=bdms,
+                recreate=recreate,
+                on_shared_storage=on_shared_storage,
+                host=host,
+                preserve_ephemeral=preserve_ephemeral)
+
 
 class API(LocalAPI):
     """Conductor API that does updates via RPC to the ConductorManager."""
@@ -351,3 +369,21 @@ class ComputeTaskAPI(object):
     def unshelve_instance(self, context, instance):
         self.conductor_compute_rpcapi.unshelve_instance(context,
                 instance=instance)
+
+    def rebuild_instance(self, context, instance, orig_image_ref, image_ref,
+                         injected_files, new_pass, orig_sys_metadata,
+                         bdms, recreate=False, on_shared_storage=False,
+                         preserve_ephemeral=False, host=None, kwargs=None):
+        # kwargs unused but required for cell compatibility
+        self.conductor_compute_rpcapi.rebuild_instance(context,
+                instance=instance,
+                new_pass=new_pass,
+                injected_files=injected_files,
+                image_ref=image_ref,
+                orig_image_ref=orig_image_ref,
+                orig_sys_metadata=orig_sys_metadata,
+                bdms=bdms,
+                recreate=recreate,
+                on_shared_storage=on_shared_storage,
+                preserve_ephemeral=preserve_ephemeral,
+                host=host)

nova/conductor/manager.py

@@ -452,7 +452,7 @@ class ComputeTaskManager(base.Base):
     may involve coordinating activities on multiple compute nodes.
     """
 
-    target = messaging.Target(namespace='compute_task', version='1.7')
+    target = messaging.Target(namespace='compute_task', version='1.8')
 
     def __init__(self):
         super(ComputeTaskManager, self).__init__()
@@ -708,3 +708,44 @@ class ComputeTaskManager(base.Base):
                 del(sys_meta[key])
         instance.system_metadata = sys_meta
         instance.save()
+
+    def rebuild_instance(self, context, instance, orig_image_ref, image_ref,
+                         injected_files, new_pass, orig_sys_metadata,
+                         bdms, recreate, on_shared_storage,
+                         preserve_ephemeral=False, host=None):
+
+        with compute_utils.EventReporter(context, 'rebuild_server',
+                                          instance.uuid):
+            if not host:
+                # NOTE(lcostantino): Retrieve scheduler filters for the
+                # instance when the feature is available
+                filter_properties = {'ignore_hosts': [instance.host]}
+                request_spec = scheduler_utils.build_request_spec(context,
+                                                                  image_ref,
+                                                                  [instance])
+                try:
+                    hosts = self.scheduler_rpcapi.select_destinations(context,
+                                                            request_spec,
+                                                            filter_properties)
+                    host = hosts.pop(0)['host']
+                except exception.NoValidHost as ex:
+                    with excutils.save_and_reraise_exception():
+                        self._set_vm_state_and_notify(context,
+                                'rebuild_server',
+                                {'vm_state': instance.vm_state,
+                                 'task_state': None}, ex, request_spec)
+                        LOG.warning(_("No valid host found for rebuild"),
+                                      instance=instance)
+
+            self.compute_rpcapi.rebuild_instance(context,
+                    instance=instance,
+                    new_pass=new_pass,
+                    injected_files=injected_files,
+                    image_ref=image_ref,
+                    orig_image_ref=orig_image_ref,
+                    orig_sys_metadata=orig_sys_metadata,
+                    bdms=bdms,
+                    recreate=recreate,
+                    on_shared_storage=on_shared_storage,
+                    preserve_ephemeral=preserve_ephemeral,
+                    host=host)

nova/conductor/rpcapi.py

@@ -367,6 +367,7 @@ class ComputeTaskAPI(object):
     1.5 - Added the leagacy_bdm parameter to build_instances
     1.6 - Made migrate_server use instance objects
     1.7 - Do not send block_device_mapping and legacy_bdm to build_instances
+    1.8 - Add rebuild_instance
     """
 
     def __init__(self):
@@ -418,3 +419,17 @@ class ComputeTaskAPI(object):
     def unshelve_instance(self, context, instance):
         cctxt = self.client.prepare(version='1.3')
         cctxt.cast(context, 'unshelve_instance', instance=instance)
+
+    def rebuild_instance(self, ctxt, instance, new_pass, injected_files,
+            image_ref, orig_image_ref, orig_sys_metadata, bdms,
+            recreate=False, on_shared_storage=False, host=None,
+            preserve_ephemeral=False, kwargs=None):
+        cctxt = self.client.prepare(version='1.8')
+        cctxt.cast(ctxt, 'rebuild_instance',
+                   instance=instance, new_pass=new_pass,
+                   injected_files=injected_files, image_ref=image_ref,
+                   orig_image_ref=orig_image_ref,
+                   orig_sys_metadata=orig_sys_metadata, bdms=bdms,
+                   recreate=recreate, on_shared_storage=on_shared_storage,
+                   preserve_ephemeral=preserve_ephemeral,
+                   host=host)

nova/tests/compute/test_compute.py

@@ -7208,7 +7208,7 @@ class ComputeAPITestCase(BaseTestCase):
             info['image_ref'] = kwargs['instance'].image_ref
             info['clean'] = kwargs['instance'].obj_what_changed() == set()
 
-        self.stubs.Set(self.compute_api.compute_rpcapi, 'rebuild_instance',
+        self.stubs.Set(self.compute_api.compute_task_api, 'rebuild_instance',
                        fake_rpc_rebuild)
 
         image_ref = instance["image_ref"] + '-new_image_ref'
@@ -9195,7 +9195,7 @@ class ComputeAPITestCase(BaseTestCase):
 
         self.stubs.Set(self.compute_api.servicegroup_api, 'service_is_up',
                 fake_service_is_up)
-        self.stubs.Set(self.compute_api.compute_rpcapi, 'rebuild_instance',
+        self.stubs.Set(self.compute_api.compute_task_api, 'rebuild_instance',
                 fake_rebuild_instance)
         self.compute_api.evacuate(self.context.elevated(),
                                   instance,

nova/tests/compute/test_compute_api.py

@@ -1851,7 +1851,7 @@ class _ComputeAPIUnitTestMixIn(object):
         _get_image.return_value = (None, image)
         bdm_get_by_instance_uuid.return_value = bdms
 
-        with mock.patch.object(self.compute_api.compute_rpcapi,
+        with mock.patch.object(self.compute_api.compute_task_api,
                 'rebuild_instance') as rebuild_instance:
             self.compute_api.rebuild(self.context, instance, image_href,
                     admin_pass, files_to_inject)
@@ -1861,7 +1861,7 @@ class _ComputeAPIUnitTestMixIn(object):
                     injected_files=files_to_inject, image_ref=image_href,
                     orig_image_ref=image_href,
                     orig_sys_metadata=orig_system_metadata, bdms=bdms,
-                    preserve_ephemeral=False, kwargs={})
+                    preserve_ephemeral=False, host=instance.host, kwargs={})
 
         _check_auto_disk_config.assert_called_once_with(image=image)
         _checks_for_create_and_rebuild.assert_called_once_with(self.context,
@@ -1909,7 +1909,7 @@ class _ComputeAPIUnitTestMixIn(object):
         _get_image.side_effect = get_image
         bdm_get_by_instance_uuid.return_value = bdms
 
-        with mock.patch.object(self.compute_api.compute_rpcapi,
+        with mock.patch.object(self.compute_api.compute_task_api,
                 'rebuild_instance') as rebuild_instance:
             self.compute_api.rebuild(self.context, instance, new_image_href,
                     admin_pass, files_to_inject)
@@ -1919,7 +1919,7 @@ class _ComputeAPIUnitTestMixIn(object):
                     injected_files=files_to_inject, image_ref=new_image_href,
                     orig_image_ref=orig_image_href,
                     orig_sys_metadata=orig_system_metadata, bdms=bdms,
-                    preserve_ephemeral=False, kwargs={})
+                    preserve_ephemeral=False, host=instance.host, kwargs={})
 
         _check_auto_disk_config.assert_called_once_with(image=new_image)
         _checks_for_create_and_rebuild.assert_called_once_with(self.context,

nova/tests/conductor/test_conductor.py

@@ -54,6 +54,7 @@ from nova.tests import fake_block_device
 from nova.tests import fake_instance
 from nova.tests import fake_notifier
 from nova.tests import fake_server_actions
+from nova.tests import fake_utils
 from nova import utils
 
 
@@ -87,6 +88,8 @@ class _BaseTestCase(object):
         self.stubs.Set(rpc.RequestContextSerializer, 'deserialize_context',
                        fake_deserialize_context)
 
+        fake_utils.stub_out_utils_spawn_n(self.stubs)
+
     def _create_fake_instance(self, params=None, type_name='m1.tiny'):
         if not params:
             params = {}
@@ -1119,6 +1122,21 @@ class _BaseTaskTestCase(object):
         self.stubs.Set(rpc.RequestContextSerializer, 'deserialize_context',
                        fake_deserialize_context)
 
+    def _prepare_rebuild_args(self, update_args=None):
+        rebuild_args = {'new_pass': 'admin_password',
+                        'injected_files': 'files_to_inject',
+                        'image_ref': 'image_ref',
+                        'orig_image_ref': 'orig_image_ref',
+                        'orig_sys_metadata': 'orig_sys_meta',
+                        'bdms': {},
+                        'recreate': False,
+                        'on_shared_storage': False,
+                        'preserve_ephemeral': False,
+                        'host': 'compute-host'}
+        if update_args:
+            rebuild_args.update(update_args)
+        return rebuild_args
+
     def test_live_migrate(self):
         inst = fake_instance.fake_db_instance()
         inst_obj = objects.Instance._from_db_object(
@@ -1477,6 +1495,83 @@ class _BaseTaskTestCase(object):
         system_metadata['shelved_host'] = 'fake-mini'
         self.conductor_manager.unshelve_instance(self.context, instance)
 
+    def test_rebuild_instance(self):
+        db_instance = jsonutils.to_primitive(self._create_fake_instance())
+        inst_obj = objects.Instance.get_by_uuid(self.context,
+                                                db_instance['uuid'])
+        rebuild_args = self._prepare_rebuild_args({'host': inst_obj.host})
+
+        with contextlib.nested(
+            mock.patch.object(self.conductor_manager.compute_rpcapi,
+                              'rebuild_instance'),
+            mock.patch.object(self.conductor_manager.scheduler_rpcapi,
+                              'select_destinations')
+        ) as (rebuild_mock, select_dest_mock):
+            self.conductor_manager.rebuild_instance(context=self.context,
+                                            instance=inst_obj,
+                                            **rebuild_args)
+            self.assertFalse(select_dest_mock.called)
+            rebuild_mock.assert_called_once_with(self.context,
+                               instance=inst_obj,
+                               **rebuild_args)
+
+    def test_rebuild_instance_with_scheduler(self):
+        db_instance = jsonutils.to_primitive(self._create_fake_instance())
+        inst_obj = objects.Instance.get_by_uuid(self.context,
+                                                db_instance['uuid'])
+        inst_obj.host = 'noselect'
+        rebuild_args = self._prepare_rebuild_args({'host': None})
+        expected_host = 'thebesthost'
+        request_spec = {}
+        filter_properties = {'ignore_hosts': [(inst_obj.host)]}
+
+        with contextlib.nested(
+            mock.patch.object(self.conductor_manager.compute_rpcapi,
+                              'rebuild_instance'),
+            mock.patch.object(self.conductor_manager.scheduler_rpcapi,
+                              'select_destinations',
+                              return_value=[{'host': expected_host}]),
+            mock.patch('nova.scheduler.utils.build_request_spec',
+                       return_value=request_spec)
+        ) as (rebuild_mock, select_dest_mock, bs_mock):
+            self.conductor_manager.rebuild_instance(context=self.context,
+                                            instance=inst_obj,
+                                            **rebuild_args)
+            select_dest_mock.assert_called_once_with(self.context,
+                                                     request_spec,
+                                                     filter_properties)
+            rebuild_args['host'] = expected_host
+            rebuild_mock.assert_called_once_with(self.context,
+                                            instance=inst_obj,
+                                            **rebuild_args)
+
+    def test_rebuild_instance_with_scheduler_no_host(self):
+        db_instance = jsonutils.to_primitive(self._create_fake_instance())
+        inst_obj = objects.Instance.get_by_uuid(self.context,
+                                                db_instance['uuid'])
+        inst_obj.host = 'noselect'
+        rebuild_args = self._prepare_rebuild_args({'host': None})
+        request_spec = {}
+        filter_properties = {'ignore_hosts': [(inst_obj.host)]}
+
+        with contextlib.nested(
+            mock.patch.object(self.conductor_manager.compute_rpcapi,
+                              'rebuild_instance'),
+            mock.patch.object(self.conductor_manager.scheduler_rpcapi,
+                              'select_destinations',
+                              side_effect=exc.NoValidHost(reason='')),
+            mock.patch('nova.scheduler.utils.build_request_spec',
+                       return_value=request_spec)
+        ) as (rebuild_mock, select_dest_mock, bs_mock):
+            self.assertRaises(exc.NoValidHost,
+                              self.conductor_manager.rebuild_instance,
+                              context=self.context, instance=inst_obj,
+                              **rebuild_args)
+            select_dest_mock.assert_called_once_with(self.context,
+                                                     request_spec,
+                                                     filter_properties)
+            self.assertFalse(rebuild_mock.called)
+
 
 class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase):
     """ComputeTaskManager Tests."""