From 6b0ab40e4233a480c9bdcca657f594d7e90fadc8 Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Mon, 7 Aug 2017 15:12:25 +0200 Subject: [PATCH] Raise NoValidHost if no allocation candidates Placement took over the role of the CoreFilter, RamFilter and DiskFilter from the FilterScheduler. Therefore if placement returns no allocation candidates for a request then scheduling should be stopped as this means there is not enough VCPU, MEMORY_MB, or DISK_GB available in any compute node for the request. Change-Id: If20a20e5cce7ab490998643e32556a1016646b07 Closes-Bug: #1708637 --- nova/scheduler/manager.py | 8 +-- nova/tests/functional/test_servers.py | 23 +++----- nova/tests/unit/scheduler/test_scheduler.py | 65 +++++++-------------- 3 files changed, 30 insertions(+), 66 deletions(-) diff --git a/nova/scheduler/manager.py b/nova/scheduler/manager.py index 9ca44f867cdf..0db7ddbcfe72 100644 --- a/nova/scheduler/manager.py +++ b/nova/scheduler/manager.py @@ -134,13 +134,7 @@ class SchedulerManager(manager.Manager): "API. This may be a temporary occurrence as compute " "nodes start up and begin reporting inventory to " "the Placement service.") - # TODO(jaypipes): Setting provider_summaries to None triggers - # the scheduler to load all compute nodes to do scheduling "the - # old way". Really, we should raise NoValidHosts here, but all - # functional tests will fall over if we do that without - # changing the PlacementFixture to load compute node inventory - # into the placement database before starting functional tests. - provider_summaries = None + raise exception.NoValidHost(reason="") else: # Build a dict of lists of allocation requests, keyed by # provider UUID, so that when we attempt to claim resources for diff --git a/nova/tests/functional/test_servers.py b/nova/tests/functional/test_servers.py index 2e3e3acf2efe..34abfbf79646 100644 --- a/nova/tests/functional/test_servers.py +++ b/nova/tests/functional/test_servers.py @@ -1612,22 +1612,13 @@ class ServerMovingTests(test.TestCase, integrated_helpers.InstanceHelperMixin): } } - # NOTE(gibi): This is bug 1708637. Placement returns no allocation - # candidate and the scheduler falls back to the legacy filtering. As - # CoreFilter is not enabled the filtering result in hosts selected - # without enough VCPU resource - self.api.post_server_action(server['id'], resize_req) - server = self._wait_for_state_change(self.api, server, 'VERIFY_RESIZE') - self.assertEqual(dest_hostname, server['OS-EXT-SRV-ATTR:host']) - - # Note(gibi): After bug 1708637 is fixed the following is expected - # resp = self.api.post_server_action( - # server['id'], resize_req, check_response_status=[400]) - # self.assertEqual( - # resp['badRequest']['message'], - # "No valid host was found. No valid host found for resize") - # server = self.admin_api.get_server(server['id']) - # self.assertEqual(source_hostname, server['OS-EXT-SRV-ATTR:host']) + resp = self.api.post_server_action( + server['id'], resize_req, check_response_status=[400]) + self.assertEqual( + resp['badRequest']['message'], + "No valid host was found. No valid host found for resize") + server = self.admin_api.get_server(server['id']) + self.assertEqual(source_hostname, server['OS-EXT-SRV-ATTR:host']) # only the source host shall have usages after the failed resize source_usages = self._get_provider_usages(source_rp_uuid) diff --git a/nova/tests/unit/scheduler/test_scheduler.py b/nova/tests/unit/scheduler/test_scheduler.py index b79bbffefd1b..d61cf56d6686 100644 --- a/nova/tests/unit/scheduler/test_scheduler.py +++ b/nova/tests/unit/scheduler/test_scheduler.py @@ -18,6 +18,7 @@ Tests For Scheduler """ import mock +import oslo_messaging as messaging import nova.conf from nova import context @@ -127,67 +128,45 @@ class SchedulerManagerTestCase(test.NoDBTestCase): @mock.patch('nova.scheduler.utils.resources_from_request_spec') @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' 'get_allocation_candidates') - def test_select_destination_old_placement(self, mock_get_ac, mock_rfrs): - """Tests that we will pass None for the provider_summaries parameter to - the scheduler driver select_destinations() method when the scheduler - report client's get_allocation_candidates() returns None, None as it - would if placement service hasn't been upgraded before scheduler. - """ + def _test_select_destination(self, get_allocation_candidates_response, + mock_get_ac, mock_rfrs): fake_spec = objects.RequestSpec() fake_spec.instance_uuid = uuids.instance - place_res = (None, None) + place_res = get_allocation_candidates_response mock_get_ac.return_value = place_res with mock.patch.object(self.manager.driver, 'select_destinations' ) as select_destinations: - self.manager.select_destinations(None, spec_obj=fake_spec, + self.assertRaises(messaging.rpc.dispatcher.ExpectedException, + self.manager.select_destinations, None, spec_obj=fake_spec, instance_uuids=[fake_spec.instance_uuid]) - select_destinations.assert_called_once_with(None, fake_spec, - [fake_spec.instance_uuid], None, None) + select_destinations.assert_not_called() mock_get_ac.assert_called_once_with(mock_rfrs.return_value) - @mock.patch('nova.scheduler.utils.resources_from_request_spec') - @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' - 'get_allocation_candidates', return_value=None) - def test_select_destination_placement_connect_fails( - self, mock_get_ac, mock_rfrs): - """Tests that we will pass None for the provider_summaries parameter to - the scheduler driver select_destinations() method when the scheduler + def test_select_destination_old_placement(self): + """Tests that we will raise NoValidhost when the scheduler + report client's get_allocation_candidates() returns None, None as it + would if placement service hasn't been upgraded before scheduler. + """ + place_res = (None, None) + self._test_select_destination(place_res) + + def test_select_destination_placement_connect_fails(self): + """Tests that we will raise NoValidHost when the scheduler report client's get_allocation_candidates() returns None, which it would if the connection to Placement failed and the safe_connect decorator returns None. """ - fake_spec = objects.RequestSpec() - fake_spec.instance_uuid = uuids.instance - with mock.patch.object(self.manager.driver, - 'select_destinations') as select_destinations: - self.manager.select_destinations( - self.context, spec_obj=fake_spec, - instance_uuids=[fake_spec.instance_uuid]) - select_destinations.assert_called_once_with( - self.context, fake_spec, [fake_spec.instance_uuid], None, None) - mock_get_ac.assert_called_once_with(mock_rfrs.return_value) + place_res = None + self._test_select_destination(place_res) - @mock.patch('nova.scheduler.utils.resources_from_request_spec') - @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' - 'get_allocation_candidates') - def test_select_destination_no_candidates(self, mock_get_ac, mock_rfrs): - """Tests that we will pass None for the provider_summaries parameter to - the scheduler driver select_destinations() method when the scheduler + def test_select_destination_no_candidates(self): + """Tests that we will raise NoValidHost when the scheduler report client's get_allocation_candidates() returns [], {} which it would if placement service hasn't yet had compute nodes populate inventory. """ - fake_spec = objects.RequestSpec() - fake_spec.instance_uuid = uuids.instance place_res = ([], {}) - mock_get_ac.return_value = place_res - with mock.patch.object(self.manager.driver, 'select_destinations' - ) as select_destinations: - self.manager.select_destinations(None, spec_obj=fake_spec, - instance_uuids=[fake_spec.instance_uuid]) - select_destinations.assert_called_once_with(None, fake_spec, - [fake_spec.instance_uuid], None, None) - mock_get_ac.assert_called_once_with(mock_rfrs.return_value) + self._test_select_destination(place_res) @mock.patch('nova.scheduler.utils.resources_from_request_spec') @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'