From bc2c3359b32f452cd772e510d4c139a8e4594900 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Tue, 5 Mar 2019 17:16:23 -0500 Subject: [PATCH] Fix WeighedHost logging regression Change I8666e0af3f057314f6b06939a108411b8a88d64b in Pike refactored some code in the FilterScheduler which accidentally changed how the list of weighed hosts are logged, which caused the wrapped HostState objects to be logged rather than the WeighedHost objects, which contain the actual "weight" attribute which is useful for debugging weigher configuration and scheduling decisions. This fixes the regression by logging the weighed hosts before stripping off the WeighedHost wrapper and adds a simple wrinkle to an existing test to assert we are logging the correct object. Change-Id: I528794b4b6f0007efc1238ad28dc402456664f86 Closes-Bug: #1816360 (cherry picked from commit 84533f5eb3c5b4ab7598d7c278b53524acc1c6e0) (cherry picked from commit 31b74bfa4063d68d0f5ea9e883cad8cbcb70ab09) --- nova/scheduler/filter_scheduler.py | 5 +++-- nova/tests/unit/scheduler/test_filter_scheduler.py | 13 ++++++++++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/nova/scheduler/filter_scheduler.py b/nova/scheduler/filter_scheduler.py index 5de9ed3aea55..8a50f9c8e8ac 100644 --- a/nova/scheduler/filter_scheduler.py +++ b/nova/scheduler/filter_scheduler.py @@ -418,11 +418,12 @@ class FilterScheduler(driver.Scheduler): if w.weight == weighed_hosts[0].weight] random.shuffle(best_hosts) weighed_hosts = best_hosts + weighed_hosts[len(best_hosts):] + # Log the weighed hosts before stripping off the wrapper class so that + # the weight value gets logged. + LOG.debug("Weighed %(hosts)s", {'hosts': weighed_hosts}) # Strip off the WeighedHost wrapper class... weighed_hosts = [h.obj for h in weighed_hosts] - LOG.debug("Weighed %(hosts)s", {'hosts': weighed_hosts}) - # We randomize the first element in the returned list to alleviate # congestion where the same host is consistently selected among # numerous potential hosts for similar request specs. diff --git a/nova/tests/unit/scheduler/test_filter_scheduler.py b/nova/tests/unit/scheduler/test_filter_scheduler.py index 4cbf75caa2fe..139132204990 100644 --- a/nova/tests/unit/scheduler/test_filter_scheduler.py +++ b/nova/tests/unit/scheduler/test_filter_scheduler.py @@ -539,10 +539,11 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): self.assertEqual(['host2', 'host1'], ig.hosts) self.assertEqual({}, ig.obj_get_changes()) + @mock.patch('nova.scheduler.filter_scheduler.LOG.debug') @mock.patch('random.choice', side_effect=lambda x: x[1]) @mock.patch('nova.scheduler.host_manager.HostManager.get_weighed_hosts') @mock.patch('nova.scheduler.host_manager.HostManager.get_filtered_hosts') - def test_get_sorted_hosts(self, mock_filt, mock_weighed, mock_rand): + def test_get_sorted_hosts(self, mock_filt, mock_weighed, mock_rand, debug): """Tests the call that returns a sorted list of hosts by calling the host manager's filtering and weighing routines """ @@ -557,8 +558,18 @@ class FilterSchedulerTestCase(test_scheduler.SchedulerTestCase): weights.WeighedHost(hs1, 1.0), weights.WeighedHost(hs2, 1.0), ] + # Make sure that when logging the weighed hosts we are logging them + # with the WeighedHost wrapper class rather than the HostState objects. + def fake_debug(message, *args, **kwargs): + if message.startswith('Weighed'): + self.assertEqual(1, len(args)) + for weighed_host in args[0]['hosts']: + self.assertIsInstance(weighed_host, weights.WeighedHost) + debug.side_effect = fake_debug + results = self.driver._get_sorted_hosts(mock.sentinel.spec, all_host_states, mock.sentinel.index) + debug.assert_called() mock_filt.assert_called_once_with(all_host_states, mock.sentinel.spec, mock.sentinel.index)