From 399f3abbf9da6ba9f3ae83650513616b151420cb Mon Sep 17 00:00:00 2001 From: Alexis Lee Date: Tue, 1 Sep 2015 17:56:54 +0100 Subject: [PATCH] Unify on _schedule_instances There's already a helper method for unshelve_instance. It turns out that the other two places in this file which call select_destinations can use the same helper. Doing this removes some code redundancy. I haven't pushed build_request_spec in as well because for bp check-destination-on-migrations we won't always be constructing a brand new request spec. Change-Id: I19bad96d997f61958e32f6254bc4a428395bc7ec --- nova/conductor/manager.py | 38 +++++++----------- nova/tests/unit/conductor/test_conductor.py | 44 ++++++++++----------- 2 files changed, 35 insertions(+), 47 deletions(-) diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index 1a226db251a9..726c76f763bc 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -330,8 +330,6 @@ class ComputeTaskManager(base.Base): security_groups, block_device_mapping=None, legacy_bdm=True): # TODO(ndipanov): Remove block_device_mapping and legacy_bdm in version # 2.0 of the RPC API. - request_spec = scheduler_utils.build_request_spec(context, image, - instances) # TODO(danms): Remove this in version 2.0 of the RPC API if (requested_networks and not isinstance(requested_networks, @@ -348,15 +346,15 @@ class ComputeTaskManager(base.Base): filter_properties = dict(filter_properties, instance_type=flavor) try: - scheduler_utils.setup_instance_group(context, request_spec, - filter_properties) # check retry policy. Rather ugly use of instances[0]... # but if we've exceeded max retries... then we really only # have a single instance. - scheduler_utils.populate_retry(filter_properties, - instances[0].uuid) - hosts = self.scheduler_client.select_destinations(context, - request_spec, filter_properties) + scheduler_utils.populate_retry( + filter_properties, instances[0].uuid) + request_spec = scheduler_utils.build_request_spec( + context, image, instances) + hosts = self._schedule_instances( + context, request_spec, filter_properties) except Exception as exc: updates = {'vm_state': vm_states.ERROR, 'task_state': None} for instance in instances: @@ -391,10 +389,7 @@ class ComputeTaskManager(base.Base): block_device_mapping=bdms, node=host['nodename'], limits=host['limits']) - def _schedule_instances(self, context, image, filter_properties, - *instances): - request_spec = scheduler_utils.build_request_spec(context, image, - instances) + def _schedule_instances(self, context, request_spec, filter_properties): scheduler_utils.setup_instance_group(context, request_spec, filter_properties) hosts = self.scheduler_client.select_destinations(context, @@ -442,9 +437,10 @@ class ComputeTaskManager(base.Base): filter_properties = {} scheduler_utils.populate_retry(filter_properties, instance.uuid) - hosts = self._schedule_instances(context, image, - filter_properties, - instance) + request_spec = scheduler_utils.build_request_spec( + context, image, [instance]) + hosts = self._schedule_instances( + context, request_spec, filter_properties) host_state = hosts[0] scheduler_utils.populate_filter_properties( filter_properties, host_state) @@ -484,15 +480,11 @@ class ComputeTaskManager(base.Base): # 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: - scheduler_utils.setup_instance_group(context, request_spec, - filter_properties) - hosts = self.scheduler_client.select_destinations(context, - request_spec, - filter_properties) + request_spec = scheduler_utils.build_request_spec( + context, image_ref, [instance]) + hosts = self._schedule_instances( + context, request_spec, filter_properties) host_dict = hosts.pop(0) host, node, limits = (host_dict['host'], host_dict['nodename'], diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index d42181f6ba9a..65663053f919 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -408,9 +408,7 @@ class _BaseTaskTestCase(object): instance_properties['system_metadata'] = flavors.save_flavor_info( {}, instance_type) - self.mox.StubOutWithMock(scheduler_utils, 'setup_instance_group') - self.mox.StubOutWithMock(self.conductor_manager.scheduler_client, - 'select_destinations') + self.mox.StubOutWithMock(self.conductor_manager, '_schedule_instances') self.mox.StubOutWithMock(db, 'block_device_mapping_get_all_by_instance') self.mox.StubOutWithMock(self.conductor_manager.compute_rpcapi, @@ -420,10 +418,9 @@ class _BaseTaskTestCase(object): 'instance_properties': instance_properties, 'instance_type': instance_type_p, 'num_instances': 2} - scheduler_utils.setup_instance_group(self.context, spec, {}) - self.conductor_manager.scheduler_client.select_destinations( - self.context, spec, - {'retry': {'num_attempts': 1, 'hosts': []}}).AndReturn( + filter_properties = {'retry': {'num_attempts': 1, 'hosts': []}} + self.conductor_manager._schedule_instances(self.context, + spec, filter_properties).AndReturn( [{'host': 'host1', 'nodename': 'node1', 'limits': []}, {'host': 'host2', 'nodename': 'node2', 'limits': []}]) db.block_device_mapping_get_all_by_instance(self.context, @@ -492,18 +489,14 @@ class _BaseTaskTestCase(object): 'instance_properties': instances[0]} exception = exc.NoValidHost(reason='fake-reason') self.mox.StubOutWithMock(scheduler_utils, 'build_request_spec') - self.mox.StubOutWithMock(scheduler_utils, 'setup_instance_group') + self.mox.StubOutWithMock(self.conductor_manager, '_schedule_instances') self.mox.StubOutWithMock(scheduler_utils, 'set_vm_state_and_notify') - self.mox.StubOutWithMock(self.conductor_manager.scheduler_client, - 'select_destinations') scheduler_utils.build_request_spec(self.context, image, mox.IgnoreArg()).AndReturn(spec) - scheduler_utils.setup_instance_group(self.context, spec, {}) - self.conductor_manager.scheduler_client.select_destinations( - self.context, spec, - {'retry': {'num_attempts': 1, - 'hosts': []}}).AndRaise(exception) + filter_properties = {'retry': {'num_attempts': 1, 'hosts': []}} + self.conductor_manager._schedule_instances(self.context, + spec, filter_properties).AndRaise(exception) updates = {'vm_state': vm_states.ERROR, 'task_state': None} for instance in instances: scheduler_utils.set_vm_state_and_notify( @@ -644,14 +637,17 @@ class _BaseTaskTestCase(object): system_metadata = instance.system_metadata self.mox.StubOutWithMock(self.conductor_manager.image_api, 'get') + self.mox.StubOutWithMock(scheduler_utils, 'build_request_spec') self.mox.StubOutWithMock(self.conductor_manager, '_schedule_instances') self.mox.StubOutWithMock(self.conductor_manager.compute_rpcapi, 'unshelve_instance') self.conductor_manager.image_api.get(self.context, 'fake_image_id', show_deleted=False).AndReturn('fake_image') + scheduler_utils.build_request_spec(self.context, 'fake_image', + mox.IgnoreArg()).AndReturn('req_spec') self.conductor_manager._schedule_instances(self.context, - 'fake_image', filter_properties, instance).AndReturn( + 'req_spec', filter_properties).AndReturn( [{'host': 'fake_host', 'nodename': 'fake_node', 'limits': {}}]) @@ -726,12 +722,15 @@ class _BaseTaskTestCase(object): 'hosts': []}} system_metadata = instance.system_metadata + self.mox.StubOutWithMock(scheduler_utils, 'build_request_spec') self.mox.StubOutWithMock(self.conductor_manager, '_schedule_instances') self.mox.StubOutWithMock(self.conductor_manager.compute_rpcapi, 'unshelve_instance') + scheduler_utils.build_request_spec(self.context, None, + mox.IgnoreArg()).AndReturn('req_spec') self.conductor_manager._schedule_instances(self.context, - None, filter_properties, instance).AndReturn( + 'req_spec', filter_properties).AndReturn( [{'host': 'fake_host', 'nodename': 'fake_node', 'limits': {}}]) @@ -1315,18 +1314,15 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): spec = {'fake': 'specs', 'instance_properties': instances[0]} self.mox.StubOutWithMock(scheduler_utils, 'build_request_spec') - self.mox.StubOutWithMock(scheduler_utils, 'setup_instance_group') - self.mox.StubOutWithMock(self.conductor_manager.scheduler_client, - 'select_destinations') + self.mox.StubOutWithMock(self.conductor_manager, '_schedule_instances') self.mox.StubOutWithMock(self.conductor_manager.compute_rpcapi, 'build_and_run_instance') scheduler_utils.build_request_spec(self.context, image, mox.IgnoreArg()).AndReturn(spec) - scheduler_utils.setup_instance_group(self.context, spec, {}) - self.conductor_manager.scheduler_client.select_destinations( - self.context, spec, - {'retry': {'num_attempts': 1, 'hosts': []}}).AndReturn( + filter_properties = {'retry': {'num_attempts': 1, 'hosts': []}} + self.conductor_manager._schedule_instances(self.context, + spec, filter_properties).AndReturn( [{'host': 'host1', 'nodename': 'node1', 'limits': []}, {'host': 'host2', 'nodename': 'node2', 'limits': []}]) instances[0].refresh().AndRaise(