From fd2b868d41a632748939b82ed1b8927adcdfb113 Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Wed, 9 Sep 2015 11:20:19 -0400 Subject: [PATCH] Fix MetricWeigher to use MonitorMetricList The commit I4dfbea27ce6c3eecc1a8658b1f9dc0feb2298705 changes 'HostState.metrics' to 'objects.MonitorMetricList'. But MetricWeigher still use the old dictionary of 'host_manager.MetricItem'. This patch corrects the behavior of MetricWeigher with related tests, and delete unused MetricItem. Co-Authored-By: Sylvain Bauza Change-Id: I7c98c2e189318e85a733f0287ef507c7bad36472 Closes-Bug: #1493680 --- nova/scheduler/host_manager.py | 5 - nova/scheduler/weights/metrics.py | 5 +- .../unit/scheduler/test_caching_scheduler.py | 2 + .../scheduler/weights/test_weights_metrics.py | 140 ++++++++++-------- 4 files changed, 85 insertions(+), 67 deletions(-) diff --git a/nova/scheduler/host_manager.py b/nova/scheduler/host_manager.py index 248521087c82..34413650f2c2 100644 --- a/nova/scheduler/host_manager.py +++ b/nova/scheduler/host_manager.py @@ -105,11 +105,6 @@ class ReadOnlyDict(IterableUserDict): raise TypeError() -# Representation of a single metric value from a compute node. -MetricItem = collections.namedtuple( - 'MetricItem', ['value', 'timestamp', 'source']) - - @utils.expects_func_args('self', 'instance') def set_update_time_on_success(function): """Set updated time of HostState when consuming succeed.""" diff --git a/nova/scheduler/weights/metrics.py b/nova/scheduler/weights/metrics.py index 602a7799459c..c9cc4b4077d0 100644 --- a/nova/scheduler/weights/metrics.py +++ b/nova/scheduler/weights/metrics.py @@ -84,9 +84,12 @@ class MetricsWeigher(weights.BaseHostWeigher): def _weigh_object(self, host_state, weight_properties): value = 0.0 + # NOTE(sbauza): Keying a dict of Metrics per metric name given that we + # have a MonitorMetricList object + metrics_dict = {m.name: m for m in host_state.metrics} for (name, ratio) in self.setting: try: - value += host_state.metrics[name].value * ratio + value += metrics_dict[name].value * ratio except KeyError: if CONF.metrics.required: raise exception.ComputeHostMetricNotFound( diff --git a/nova/tests/unit/scheduler/test_caching_scheduler.py b/nova/tests/unit/scheduler/test_caching_scheduler.py index 605c4ef42c36..6da49a608b17 100644 --- a/nova/tests/unit/scheduler/test_caching_scheduler.py +++ b/nova/tests/unit/scheduler/test_caching_scheduler.py @@ -18,6 +18,7 @@ from oslo_utils import timeutils from six.moves import range from nova import exception +from nova import objects from nova.scheduler import caching_scheduler from nova.scheduler import host_manager from nova.tests.unit.scheduler import test_scheduler @@ -137,6 +138,7 @@ class CachingSchedulerTestCase(test_scheduler.SchedulerTestCase): } host_state.cpu_allocation_ratio = 16.0 host_state.ram_allocation_ratio = 1.5 + host_state.metrics = objects.MonitorMetricList(objects=[]) return host_state @mock.patch('nova.db.instance_extra_get_by_instance_uuid', diff --git a/nova/tests/unit/scheduler/weights/test_weights_metrics.py b/nova/tests/unit/scheduler/weights/test_weights_metrics.py index 6832d95fbfa9..526969f33cc1 100644 --- a/nova/tests/unit/scheduler/weights/test_weights_metrics.py +++ b/nova/tests/unit/scheduler/weights/test_weights_metrics.py @@ -14,13 +14,19 @@ Tests For Scheduler metrics weights. """ from nova import exception -from nova.scheduler import host_manager +from nova.objects import fields +from nova.objects import monitor_metric from nova.scheduler import weights from nova.scheduler.weights import metrics from nova import test from nova.tests.unit.scheduler import fakes +idle = fields.MonitorMetricType.CPU_IDLE_TIME +kernel = fields.MonitorMetricType.CPU_KERNEL_TIME +user = fields.MonitorMetricType.CPU_USER_TIME + + class MetricsWeigherTestCase(test.NoDBTestCase): def setUp(self): super(MetricsWeigherTestCase, self).setUp() @@ -36,25 +42,28 @@ class MetricsWeigherTestCase(test.NoDBTestCase): hosts, weight_properties)[0] def _get_all_hosts(self): - def fake_metric(value): - return host_manager.MetricItem(value=value, timestamp='fake-time', - source='fake-source') + def fake_metric(name, value): + return monitor_metric.MonitorMetric(name=name, value=value) + + def fake_list(objs): + m_list = [fake_metric(name, val) for name, val in objs] + return monitor_metric.MonitorMetricList(objects=m_list) host_values = [ - ('host1', 'node1', {'metrics': {'foo': fake_metric(512), - 'bar': fake_metric(1)}}), - ('host2', 'node2', {'metrics': {'foo': fake_metric(1024), - 'bar': fake_metric(2)}}), - ('host3', 'node3', {'metrics': {'foo': fake_metric(3072), - 'bar': fake_metric(1)}}), - ('host4', 'node4', {'metrics': {'foo': fake_metric(8192), - 'bar': fake_metric(0)}}), - ('host5', 'node5', {'metrics': {'foo': fake_metric(768), - 'bar': fake_metric(0), - 'zot': fake_metric(1)}}), - ('host6', 'node6', {'metrics': {'foo': fake_metric(2048), - 'bar': fake_metric(0), - 'zot': fake_metric(2)}}), + ('host1', 'node1', {'metrics': fake_list([(idle, 512), + (kernel, 1)])}), + ('host2', 'node2', {'metrics': fake_list([(idle, 1024), + (kernel, 2)])}), + ('host3', 'node3', {'metrics': fake_list([(idle, 3072), + (kernel, 1)])}), + ('host4', 'node4', {'metrics': fake_list([(idle, 8192), + (kernel, 0)])}), + ('host5', 'node5', {'metrics': fake_list([(idle, 768), + (kernel, 0), + (user, 1)])}), + ('host6', 'node6', {'metrics': fake_list([(idle, 2048), + (kernel, 0), + (user, 2)])}), ] return [fakes.FakeHostState(host, node, values) for host, node, values in host_values] @@ -66,48 +75,57 @@ class MetricsWeigherTestCase(test.NoDBTestCase): self.assertEqual(expected_host, weighed_host.obj.host) def test_single_resource(self): - # host1: foo=512 - # host2: foo=1024 - # host3: foo=3072 - # host4: foo=8192 + # host1: idle=512 + # host2: idle=1024 + # host3: idle=3072 + # host4: idle=8192 # so, host4 should win: - setting = ['foo=1'] + setting = [idle + '=1'] self._do_test(setting, 1.0, 'host4') def test_multiple_resource(self): - # host1: foo=512, bar=1 - # host2: foo=1024, bar=2 - # host3: foo=3072, bar=1 - # host4: foo=8192, bar=0 + # host1: idle=512, kernel=1 + # host2: idle=1024, kernel=2 + # host3: idle=3072, kernel=1 + # host4: idle=8192, kernel=0 # so, host2 should win: - setting = ['foo=0.0001', 'bar=1'] + setting = [idle + '=0.0001', kernel + '=1'] self._do_test(setting, 1.0, 'host2') + def test_single_resource_duplicate_setting(self): + # host1: idle=512 + # host2: idle=1024 + # host3: idle=3072 + # host4: idle=8192 + # so, host1 should win (sum of settings is negative): + setting = [idle + '=-2', idle + '=1'] + self._do_test(setting, 1.0, 'host1') + def test_single_resourcenegtive_ratio(self): - # host1: foo=512 - # host2: foo=1024 - # host3: foo=3072 - # host4: foo=8192 + # host1: idle=512 + # host2: idle=1024 + # host3: idle=3072 + # host4: idle=8192 # so, host1 should win: - setting = ['foo=-1'] + setting = [idle + '=-1'] self._do_test(setting, 1.0, 'host1') def test_multiple_resource_missing_ratio(self): - # host1: foo=512, bar=1 - # host2: foo=1024, bar=2 - # host3: foo=3072, bar=1 - # host4: foo=8192, bar=0 + # host1: idle=512, kernel=1 + # host2: idle=1024, kernel=2 + # host3: idle=3072, kernel=1 + # host4: idle=8192, kernel=0 # so, host4 should win: - setting = ['foo=0.0001', 'bar'] + setting = [idle + '=0.0001', kernel] self._do_test(setting, 1.0, 'host4') def test_multiple_resource_wrong_ratio(self): - # host1: foo=512, bar=1 - # host2: foo=1024, bar=2 - # host3: foo=3072, bar=1 - # host4: foo=8192, bar=0 + # host1: idle=512, kernel=1 + # host2: idle=1024, kernel=2 + # host3: idle=3072, kernel=1 + # host4: idle=8192, kernel=0 # so, host4 should win: - setting = ['foo=0.0001', 'bar = 2.0t'] + setting = [idle + '=0.0001', kernel + ' = 2.0t'] self._do_test(setting, 1.0, 'host4') def _check_parsing_result(self, weigher, setting, results): @@ -120,23 +138,23 @@ class MetricsWeigherTestCase(test.NoDBTestCase): def test_parse_setting(self): weigher = self.weighers[0] self._check_parsing_result(weigher, - ['foo=1'], - [('foo', 1.0)]) + [idle + '=1'], + [(idle, 1.0)]) self._check_parsing_result(weigher, - ['foo=1', 'bar=-2.1'], - [('foo', 1.0), ('bar', -2.1)]) + [idle + '=1', kernel + '=-2.1'], + [(idle, 1.0), (kernel, -2.1)]) self._check_parsing_result(weigher, - ['foo=a1', 'bar=-2.1'], - [('bar', -2.1)]) + [idle + '=a1', kernel + '=-2.1'], + [(kernel, -2.1)]) self._check_parsing_result(weigher, - ['foo', 'bar=-2.1'], - [('bar', -2.1)]) + [idle, kernel + '=-2.1'], + [(kernel, -2.1)]) self._check_parsing_result(weigher, - ['=5', 'bar=-2.1'], - [('bar', -2.1)]) + ['=5', kernel + '=-2.1'], + [(kernel, -2.1)]) def test_metric_not_found_required(self): - setting = ['foo=1', 'zot=2'] + setting = [idle + '=1', user + '=2'] self.assertRaises(exception.ComputeHostMetricNotFound, self._do_test, setting, @@ -144,13 +162,13 @@ class MetricsWeigherTestCase(test.NoDBTestCase): 'host4') def test_metric_not_found_non_required(self): - # host1: foo=512, bar=1 - # host2: foo=1024, bar=2 - # host3: foo=3072, bar=1 - # host4: foo=8192, bar=0 - # host5: foo=768, bar=0, zot=1 - # host6: foo=2048, bar=0, zot=2 + # host1: idle=512, kernel=1 + # host2: idle=1024, kernel=2 + # host3: idle=3072, kernel=1 + # host4: idle=8192, kernel=0 + # host5: idle=768, kernel=0, user=1 + # host6: idle=2048, kernel=0, user=2 # so, host5 should win: self.flags(required=False, group='metrics') - setting = ['foo=0.0001', 'zot=-1'] + setting = [idle + '=0.0001', user + '=-1'] self._do_test(setting, 1.0, 'host5')