Merge "Fix MetricWeigher to use MonitorMetricList" into stable/liberty

4 files changed, 94 insertions, 67 deletions

diff --git a/nova/scheduler/host_manager.py b/nova/scheduler/host_manager.py
index 248521087c..34413650f2 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 602a779945..ca53551296 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 or []}
         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 605c4ef42c..6da49a608b 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 6832d95fbf..23f404f227 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]
@@ -65,49 +74,67 @@ class MetricsWeigherTestCase(test.NoDBTestCase):
         self.assertEqual(expected_weight, weighed_host.weight)
         self.assertEqual(expected_host, weighed_host.obj.host)
 
+    def test_single_resource_no_metrics(self):
+        setting = [idle + '=1']
+        hostinfo_list = [fakes.FakeHostState('host1', 'node1',
+                                             {'metrics': None}),
+                         fakes.FakeHostState('host2', 'node2',
+                                             {'metrics': None})]
+        self.assertRaises(exception.ComputeHostMetricNotFound,
+                          self._get_weighed_host, hostinfo_list, setting)
+
     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 +147,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 +171,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')