From 973f31212ad0719a5c1a20540c38563b37bc8873 Mon Sep 17 00:00:00 2001 From: Jay Pipes Date: Fri, 26 Jun 2015 14:33:03 -0400 Subject: [PATCH] Use stevedore for loading monitor extensions The compute monitor plugins were being loaded using the nova.loadables module. This patch uses the stevedore library to load monitor extensions. We keep the same semantics as the previous loading mechanism: we use the CONF.compute_monitors configuration option to winnow the set of all monitor plugins, and we ensure that no two monitors that return the same set of metrics will be loaded. However, this patch deprecates the CONF.compute_available_monitors configuration option, since stevedore and setuptools entry points now allow a set of plugins to be specified without any further configuration options. Change-Id: I97bf8bcd43faf9f3fe40983497c2360233d5f599 Fixes-bug: 1468012 DocImpact: deprecates the CONF.compute_available_monitors option. --- nova/compute/monitors/__init__.py | 108 ++++++++---------- nova/compute/monitors/cpu/virt_driver.py | 6 +- nova/compute/resource_tracker.py | 4 +- .../unit/compute/monitors/cpu/__init__.py | 0 .../test_virt_driver.py} | 32 +++--- .../unit/compute/monitors/test_monitors.py | 69 +++-------- .../unit/compute/test_resource_tracker.py | 30 +++-- setup.cfg | 2 + 8 files changed, 101 insertions(+), 150 deletions(-) create mode 100644 nova/tests/unit/compute/monitors/cpu/__init__.py rename nova/tests/unit/compute/monitors/{test_cpu_monitor.py => cpu/test_virt_driver.py} (77%) diff --git a/nova/compute/monitors/__init__.py b/nova/compute/monitors/__init__.py index 405a13ef9e60..b6736a524521 100644 --- a/nova/compute/monitors/__init__.py +++ b/nova/compute/monitors/__init__.py @@ -19,20 +19,24 @@ Resource monitor API specification. from oslo_config import cfg from oslo_log import log as logging +from stevedore import enabled -import nova.compute.monitors.base from nova.i18n import _LW -from nova import loadables compute_monitors_opts = [ cfg.MultiStrOpt('compute_available_monitors', - default=['nova.compute.monitors.all_monitors'], + deprecated_for_removal=True, + default=None, help='Monitor classes available to the compute which may ' - 'be specified more than once.'), + 'be specified more than once. This option is ' + 'DEPRECATED and no longer used. Use setuptools entry ' + 'points to list available monitor plugins.'), cfg.ListOpt('compute_monitors', default=[], help='A list of monitors that can be used for getting ' - 'compute metrics.'), + 'compute metrics. You can use the alias/name from ' + 'the setuptools entry points for nova.compute.monitors.* ' + 'namespaces.'), ] CONF = cfg.CONF @@ -40,64 +44,42 @@ CONF.register_opts(compute_monitors_opts) LOG = logging.getLogger(__name__) -# TODO(jaypipes): Replace the use of loadables with stevedore. -class ResourceMonitorHandler(loadables.BaseLoader): - """Base class to handle loading monitor classes. - """ - def __init__(self): - super(ResourceMonitorHandler, self).__init__( - nova.compute.monitors.base.MonitorBase) +class MonitorHandler(object): - def choose_monitors(self, manager): - """This function checks the monitor names and metrics names against a - predefined set of acceptable monitors. - """ - monitor_classes = self.get_matching_classes( - CONF.compute_available_monitors) - monitor_class_map = {cls.__name__: cls for cls in monitor_classes} - monitor_cls_names = CONF.compute_monitors - good_monitors = [] - bad_monitors = [] - metric_names = set() - for monitor_name in monitor_cls_names: - if monitor_name not in monitor_class_map: - bad_monitors.append(monitor_name) - continue + def __init__(self, resource_tracker): + self.cpu_monitor_loaded = False - try: - # make sure different monitors do not have the same - # metric name - monitor = monitor_class_map[monitor_name](manager) - metric_names_tmp = monitor.get_metric_names() - overlap = metric_names & metric_names_tmp - if not overlap: - metric_names = metric_names | metric_names_tmp - good_monitors.append(monitor) - else: - msg = (_LW("Excluding monitor %(monitor_name)s due to " - "metric name overlap; overlapping " - "metrics: %(overlap)s") % - {'monitor_name': monitor_name, - 'overlap': ', '.join(overlap)}) - LOG.warn(msg) - bad_monitors.append(monitor_name) - except Exception as ex: - msg = (_LW("Monitor %(monitor_name)s cannot be used: %(ex)s") % - {'monitor_name': monitor_name, 'ex': ex}) - LOG.warn(msg) - bad_monitors.append(monitor_name) + ns = 'nova.compute.monitors.cpu' + cpu_plugin_mgr = enabled.EnabledExtensionManager( + namespace=ns, + invoke_on_load=True, + check_func=self.check_enabled_cpu_monitor, + invoke_args=(resource_tracker,) + ) + self.monitors = [obj.obj for obj in cpu_plugin_mgr] - if bad_monitors: - LOG.warning(_LW("The following monitors have been disabled: %s"), - ', '.join(bad_monitors)) - - return good_monitors - - -def all_monitors(): - """Return a list of monitor classes found in this directory. - - This method is used as the default for available monitors - and should return a list of all monitor classes available. - """ - return ResourceMonitorHandler().get_all_classes() + def check_enabled_cpu_monitor(self, ext): + if self.cpu_monitor_loaded is not False: + msg = _LW("Excluding CPU monitor %(monitor_name)s. Already " + "loaded %(loaded_cpu_monitor)s.") + msg = msg % { + 'monitor_name': ext.name, + 'loaded_cpu_monitor': self.cpu_monitor_loaded + } + LOG.warn(msg) + return False + # TODO(jaypipes): Right now, we only have CPU monitors, so we don't + # need to check if the plugin is a CPU monitor or not. Once non-CPU + # monitors are added, change this to check either the base class or + # the set of metric names returned to ensure only a single CPU + # monitor is loaded at any one time. + if ext.name in CONF.compute_monitors: + self.cpu_monitor_loaded = ext.name + return True + msg = _LW("Excluding CPU monitor %(monitor_name)s. Not in the " + "list of enabled monitors (CONF.compute_monitors).") + msg = msg % { + 'monitor_name': ext.name, + } + LOG.warn(msg) + return False diff --git a/nova/compute/monitors/cpu/virt_driver.py b/nova/compute/monitors/cpu/virt_driver.py index 7d5b04cfa978..6c6539910c7d 100644 --- a/nova/compute/monitors/cpu/virt_driver.py +++ b/nova/compute/monitors/cpu/virt_driver.py @@ -33,10 +33,10 @@ LOG = logging.getLogger(__name__) class Monitor(base.CPUMonitorBase): """CPU monitor that uses the virt driver's get_host_cpu_stats() call.""" - def __init__(self, compute_manager): - super(Monitor, self).__init__(compute_manager) + def __init__(self, resource_tracker): + super(Monitor, self).__init__(resource_tracker) self.source = CONF.compute_driver - self.driver = self.compute_manager.driver + self.driver = resource_tracker.driver self._data = {} self._cpu_stats = {} diff --git a/nova/compute/resource_tracker.py b/nova/compute/resource_tracker.py index 7c35a52cf072..6c8a147d437c 100644 --- a/nova/compute/resource_tracker.py +++ b/nova/compute/resource_tracker.py @@ -80,8 +80,8 @@ class ResourceTracker(object): self.tracked_instances = {} self.tracked_migrations = {} self.conductor_api = conductor.API() - monitor_handler = monitors.ResourceMonitorHandler() - self.monitors = monitor_handler.choose_monitors(self) + monitor_handler = monitors.MonitorHandler(self) + self.monitors = monitor_handler.monitors self.ext_resources_handler = \ ext_resources.ResourceHandler(CONF.compute_resources) self.old_resources = objects.ComputeNode() diff --git a/nova/tests/unit/compute/monitors/cpu/__init__.py b/nova/tests/unit/compute/monitors/cpu/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/nova/tests/unit/compute/monitors/test_cpu_monitor.py b/nova/tests/unit/compute/monitors/cpu/test_virt_driver.py similarity index 77% rename from nova/tests/unit/compute/monitors/test_cpu_monitor.py rename to nova/tests/unit/compute/monitors/cpu/test_virt_driver.py index 710afef95dcb..08b3d40316ec 100644 --- a/nova/tests/unit/compute/monitors/test_cpu_monitor.py +++ b/nova/tests/unit/compute/monitors/cpu/test_virt_driver.py @@ -20,25 +20,24 @@ from nova import objects from nova import test -class ComputeDriverCPUMonitorTestCase(test.NoDBTestCase): - def setUp(self): - super(ComputeDriverCPUMonitorTestCase, self).setUp() +class FakeDriver(object): + def get_host_cpu_stats(self): + return {'kernel': 5664160000000, + 'idle': 1592705190000000, + 'frequency': 800, + 'user': 26728850000000, + 'iowait': 6121490000000} - class FakeDriver(object): - def get_host_cpu_stats(self): - return {'kernel': 5664160000000, - 'idle': 1592705190000000, - 'frequency': 800, - 'user': 26728850000000, - 'iowait': 6121490000000} - class FakeComputeManager(object): - driver = FakeDriver() +class FakeResourceTracker(object): + driver = FakeDriver() - self.monitor = virt_driver.Monitor(FakeComputeManager()) + +class VirtDriverCPUMonitorTestCase(test.NoDBTestCase): def test_get_metric_names(self): - names = self.monitor.get_metric_names() + monitor = virt_driver.Monitor(FakeResourceTracker()) + names = monitor.get_metric_names() self.assertEqual(10, len(names)) self.assertIn("cpu.frequency", names) self.assertIn("cpu.user.time", names) @@ -53,8 +52,9 @@ class ComputeDriverCPUMonitorTestCase(test.NoDBTestCase): def test_get_metrics(self): metrics = objects.MonitorMetricList() - self.monitor.add_metrics_to_list(metrics) - names = self.monitor.get_metric_names() + monitor = virt_driver.Monitor(FakeResourceTracker()) + monitor.add_metrics_to_list(metrics) + names = monitor.get_metric_names() for metric in metrics.objects: self.assertIn(metric.name, names) diff --git a/nova/tests/unit/compute/monitors/test_monitors.py b/nova/tests/unit/compute/monitors/test_monitors.py index 9c7bbe3367af..d7c66f34502d 100644 --- a/nova/tests/unit/compute/monitors/test_monitors.py +++ b/nova/tests/unit/compute/monitors/test_monitors.py @@ -15,66 +15,25 @@ """Tests for resource monitors.""" -from oslo_utils import timeutils +import mock from nova.compute import monitors -from nova.compute.monitors import base -from nova.objects import fields from nova import test -class CPUMonitor1(base.MonitorBase): - - NOW_TS = timeutils.utcnow() - - def __init__(self, *args): - super(CPUMonitor1, self).__init__(*args) - self.source = 'CPUMonitor1' - - def get_metric_names(self): - return set([ - fields.MonitorMetricType.CPU_FREQUENCY - ]) - - def get_metric(self, name): - return 100, CPUMonitor1.NOW_TS - - -class CPUMonitor2(base.MonitorBase): - - def get_metric_names(self): - return set([ - fields.MonitorMetricType.CPU_FREQUENCY - ]) - - def get_metric(self, name): - # This should never be called since the CPU metrics overlap - # with the ones in the CPUMonitor1. - pass - - -class ResourceMonitorsTestCase(test.NoDBTestCase): +class MonitorsTestCase(test.NoDBTestCase): """Test case for monitors.""" - def setUp(self): - super(ResourceMonitorsTestCase, self).setUp() - self.monitor_handler = monitors.ResourceMonitorHandler() - fake_monitors = [ - 'nova.tests.unit.compute.monitors.test_monitors.CPUMonitor1', - 'nova.tests.unit.compute.monitors.test_monitors.CPUMonitor2'] - self.flags(compute_available_monitors=fake_monitors) + @mock.patch('stevedore.enabled.EnabledExtensionManager') + def test_check_enabled_cpu_monitor(self, _mock_ext_manager): + class FakeExt(object): + def __init__(self, name): + self.name = name - def test_choose_monitors_not_found(self): - self.flags(compute_monitors=['CPUMonitor1', 'CPUMonitorb']) - monitor_classes = self.monitor_handler.choose_monitors(self) - self.assertEqual(len(monitor_classes), 1) - - def test_choose_monitors_bad(self): - self.flags(compute_monitors=['CPUMonitor1', 'CPUMonitor2']) - monitor_classes = self.monitor_handler.choose_monitors(self) - self.assertEqual(len(monitor_classes), 1) - - def test_choose_monitors_none(self): - self.flags(compute_monitors=[]) - monitor_classes = self.monitor_handler.choose_monitors(self) - self.assertEqual(len(monitor_classes), 0) + # We check to ensure only one CPU monitor is loaded... + self.flags(compute_monitors=['cpu_mon1', 'cpu_mon2']) + handler = monitors.MonitorHandler(None) + ext_cpu_mon1 = FakeExt('cpu_mon1') + ext_cpu_mon2 = FakeExt('cpu_mon2') + self.assertTrue(handler.check_enabled_cpu_monitor(ext_cpu_mon1)) + self.assertFalse(handler.check_enabled_cpu_monitor(ext_cpu_mon2)) diff --git a/nova/tests/unit/compute/test_resource_tracker.py b/nova/tests/unit/compute/test_resource_tracker.py index 2181962bc860..f79861ded649 100644 --- a/nova/tests/unit/compute/test_resource_tracker.py +++ b/nova/tests/unit/compute/test_resource_tracker.py @@ -24,6 +24,7 @@ from oslo_config import cfg from oslo_serialization import jsonutils from oslo_utils import timeutils +from nova.compute.monitors import base as monitor_base from nova.compute import resource_tracker from nova.compute import resources from nova.compute import task_states @@ -36,7 +37,6 @@ from nova.objects import base as obj_base from nova.objects import pci_device_pool from nova import rpc from nova import test -from nova.tests.unit.compute.monitors import test_monitors from nova.tests.unit.pci import fakes as pci_fakes from nova.virt import driver @@ -201,7 +201,8 @@ class FakeVirtDriver(driver.ComputeDriver): class BaseTestCase(test.TestCase): - def setUp(self): + @mock.patch('stevedore.enabled.EnabledExtensionManager') + def setUp(self, _mock_ext_mgr): super(BaseTestCase, self).setUp() self.flags(reserved_host_disk_mb=0, @@ -1276,10 +1277,6 @@ class OrphanTestCase(BaseTrackerTestCase): class ComputeMonitorTestCase(BaseTestCase): def setUp(self): super(ComputeMonitorTestCase, self).setUp() - fake_monitors = [ - 'nova.tests.unit.compute.monitors.test_monitors.CPUMonitor1', - 'nova.tests.unit.compute.monitors.test_monitors.CPUMonitor2'] - self.flags(compute_available_monitors=fake_monitors) self.tracker = self._tracker() self.node_name = 'nodename' self.user_id = 'fake' @@ -1289,7 +1286,6 @@ class ComputeMonitorTestCase(BaseTestCase): self.project_id) def test_get_host_metrics_none(self): - self.flags(compute_monitors=[]) self.tracker.monitors = [] metrics = self.tracker._get_host_metrics(self.context, self.node_name) @@ -1307,9 +1303,21 @@ class ComputeMonitorTestCase(BaseTestCase): self.assertEqual(0, len(metrics)) def test_get_host_metrics(self): - class1 = test_monitors.CPUMonitor1(self.tracker) - self.tracker.monitors = [class1] + class FakeCPUMonitor(monitor_base.MonitorBase): + NOW_TS = timeutils.utcnow() + + def __init__(self, *args): + super(FakeCPUMonitor, self).__init__(*args) + self.source = 'FakeCPUMonitor' + + def get_metric_names(self): + return set(["cpu.frequency"]) + + def get_metric(self, name): + return 100, self.NOW_TS + + self.tracker.monitors = [FakeCPUMonitor(None)] mock_notifier = mock.Mock() with mock.patch.object(rpc, 'get_notifier', @@ -1322,10 +1330,10 @@ class ComputeMonitorTestCase(BaseTestCase): expected_metrics = [ { 'timestamp': timeutils.strtime( - test_monitors.CPUMonitor1.NOW_TS), + FakeCPUMonitor.NOW_TS), 'name': 'cpu.frequency', 'value': 100, - 'source': 'CPUMonitor1' + 'source': 'FakeCPUMonitor' }, ] diff --git a/setup.cfg b/setup.cfg index e0327553fc8a..cc04c8e5ee85 100644 --- a/setup.cfg +++ b/setup.cfg @@ -41,6 +41,8 @@ oslo.config.opts = nova.openstack.common.sslutils = nova.openstack.common.sslutils:list_opts nova.openstack.common.versionutils = nova.openstack.common.versionutils:list_opts +nova.compute.monitors.cpu = + virt_driver = nova.compute.monitors.cpu.virt_driver:Monitor nova.compute.resources = vcpu = nova.compute.resources.vcpu:VCPU nova.image.download.modules =