From 7ef22179fa664b9daac1cbd35fffcfc101281053 Mon Sep 17 00:00:00 2001 From: Helena McGough Date: Fri, 30 Jun 2017 13:47:24 +0000 Subject: [PATCH] Deprecated the libvirt meter - Libvirt meter is only enabled if configured - Added deployment code - Included a reno - Updated doc/source/usage.rst - Updated unit tests Change-Id: Ia4eb566e087ff341505896534296e7afd338f98b --- collectd_ceilometer/common/meters/storage.py | 8 +++- collectd_ceilometer/common/settings.py | 8 ++++ .../tests/common/test_config.py | 41 ++++++++++++++++++- devstack/libs/collectd | 5 +++ devstack/settings | 3 ++ doc/source/usage.rst | 12 ++++++ etc/collectd.conf.d/collectd-aodh-plugin.conf | 3 ++ .../collectd-ceilometer-plugin.conf | 2 + .../collectd-gnocchi-plugin.conf | 2 + .../deprecate-libvirt-c97487c99492949d.yaml | 6 +++ 10 files changed, 86 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/deprecate-libvirt-c97487c99492949d.yaml diff --git a/collectd_ceilometer/common/meters/storage.py b/collectd_ceilometer/common/meters/storage.py index 45f8780..1d30d19 100644 --- a/collectd_ceilometer/common/meters/storage.py +++ b/collectd_ceilometer/common/meters/storage.py @@ -21,6 +21,7 @@ import threading from collectd_ceilometer.common.meters.base import Meter from collectd_ceilometer.common.meters.libvirt import LibvirtMeter +from collectd_ceilometer.common.settings import Config class MeterStorage(object): @@ -36,8 +37,11 @@ class MeterStorage(object): self._default = Meter(collectd=collectd) # fill dict with specialized meters classes - self._meters = {key: meter_class(collectd=collectd) - for key, meter_class in six.iteritems(self._classes)} + if Config.instance().libvirt_enabled() is True: + # Deprecated: Enabled manually + self._meters = \ + {key: meter_class(collectd=collectd) + for key, meter_class in six.iteritems(self._classes)} def get(self, plugin): """Get meter for the collectd plugin""" diff --git a/collectd_ceilometer/common/settings.py b/collectd_ceilometer/common/settings.py index 41440e9..eb6eeb2 100644 --- a/collectd_ceilometer/common/settings.py +++ b/collectd_ceilometer/common/settings.py @@ -58,6 +58,7 @@ class Config(object): CfgParam('OS_PASSWORD', None, six.text_type), CfgParam('OS_TENANT_NAME', None, six.text_type), CfgParam('VERBOSE', False, bool), + CfgParam('LIBVIRT_METER_ENABLED', False, bool), CfgParam('LIBVIRT_CONN_URI', 'qemu:///system', six.text_type), ] @@ -75,6 +76,7 @@ class Config(object): # dictionary for user-defined units self._user_units = {} self._units = UNITS.copy() + self._libvirt_meter = False # dictionary for user defined severities self._alarm_severities = {} @@ -120,6 +122,10 @@ class Config(object): LOGGER.info('There is no user-defined severity for this alarm') return 'moderate' + def libvirt_enabled(self): + """Check if the libvirt meter is enabled""" + return self._libvirt_meter + def _read_node(self, node): """Read a configuration node @@ -168,6 +174,8 @@ class Config(object): val = '*****' LOGGER.info( 'Got configuration parameter: %s -> "%s"', key, val) + if key == 'LIBVIRT_METER_ENABLED': + self._libvirt_meter = val else: LOGGER.error('Unknown configuration parameter "%s"', key) diff --git a/collectd_ceilometer/tests/common/test_config.py b/collectd_ceilometer/tests/common/test_config.py index b86e3b1..2292930 100644 --- a/collectd_ceilometer/tests/common/test_config.py +++ b/collectd_ceilometer/tests/common/test_config.py @@ -26,7 +26,8 @@ from collectd_ceilometer.common import settings def config_module( - values, units=None, module_name="collectd_ceilometer.ceilometer.plugin"): + values, units=None, libvirt_meter=False, + module_name="collectd_ceilometer.ceilometer.plugin"): children = [config_value(key, value) for key, value in six.iteritems(values)] if units: @@ -81,7 +82,8 @@ class TestConfig(TestCase): CEILOMETER_TIMEOUT=1000, OS_USERNAME='tester', OS_PASSWORD='testpasswd', - OS_TENANT_NAME='service') + OS_TENANT_NAME='service', + LIBVIRT_METER_ENABLED=False) @mock.patch.object(settings, 'LOGGER', autospec=True) def test_default_configuration(self, LOGGER): @@ -234,3 +236,38 @@ class TestConfig(TestCase): 'Invalid unit configuration: %s',"NOT_UNITS") self.assertEqual('None', config.unit('age', None)) + def test_libvirt_meter_enabled(self): + """Test configuration change when enabling the libvirt meter + + Set-up: Create a node and set the LIBVIRT_METER_ENABLED value. + Test: Read the node and check the 'LIBVIRT_METER_ENABLED value. + Expected behaviour: When configured this value will return True. + """ + + node = config_module(values=dict(LIBVIRT_METER_ENABLED=True)) + config = settings.Config._decorated() + + config.read(config_module(values=dict(LIBVIRT_METER_ENABLED=True))) + self.assertEqual(True, config.LIBVIRT_METER_ENABLED) + + @mock.patch.object(settings, 'LOGGER', autospec=True) + def test_libvirt_meter_default_config(self, LOGGER): + """Test default configuration for enabling the libvirt meter + + Set-up: Create a default node with no alternative configurations set + Test: Read the defaults of this node. + Expected behaviour: The default value for LIBVIRT_METER_ENABLED is + false. + """ + + node = config_module(values=self.default_values) + config = settings.Config._decorated() + + for child in node.children: + if child.key == 'LIBVIRT_METER_ENABLED': + config.libvirt_meter = getattr(config, child.key) + + config.read(node) + + self.assertEqual(False, config.libvirt_meter) + LOGGER.error.assert_not_called() diff --git a/devstack/libs/collectd b/devstack/libs/collectd index 1242173..27ac600 100644 --- a/devstack/libs/collectd +++ b/devstack/libs/collectd @@ -128,6 +128,7 @@ function adapt_collectd_conf { sudo sed -i 's|CEILOMETER_TIMEOUT.*$|CEILOMETER_TIMEOUT "'$CEILOMETER_TIMEOUT'"|g' $COLLECTD_CONF_DIR/collectd-ceilometer-plugin.conf sudo sed -i 's|OS_PASSWORD.*$|OS_PASSWORD "'$SERVICE_PASSWORD'"|g' $COLLECTD_CONF_DIR/collectd-ceilometer-plugin.conf sudo sed -i 's|OS_TENANT_NAME.*$|OS_TENANT_NAME "'$SERVICE_TENANT_NAME'"|g' $COLLECTD_CONF_DIR/collectd-ceilometer-plugin.conf + sudo sed -i 's|LIBVIRT_METER_ENABLED.*$|LIBVIRT_METER_ENABLED "'$LIBVIRT_METER_ENABLED'"|g' $COLLECTD_CONF_DIR/collectd-ceilometer-plugin.conf config_custom_units "ceilometer" "$COLLECTD_CEILOMETER_UNITS" fi @@ -146,6 +147,8 @@ function adapt_collectd_conf { sudo sed -i 's|CEILOMETER_TIMEOUT.*$|CEILOMETER_TIMEOUT "'$CEILOMETER_TIMEOUT'"|g' $COLLECTD_CONF_DIR/collectd-gnocchi-plugin.conf sudo sed -i 's|OS_PASSWORD.*$|OS_PASSWORD "'$SERVICE_PASSWORD'"|g' $COLLECTD_CONF_DIR/collectd-gnocchi-plugin.conf sudo sed -i 's|OS_TENANT_NAME.*$|OS_TENANT_NAME "'$SERVICE_TENANT_NAME'"|g' $COLLECTD_CONF_DIR/collectd-gnocchi-plugin.conf + sudo sed -i 's|LIBVIRT_METER_ENABLED.*$|LIBVIRT_METER_ENABLED "'$LIBVIRT_METER_ENABLED'"|g' $COLLECTD_CONF_DIR/collectd-gnocchi-plugin.conf + config_custom_units "gnocchi" "$COLLECTD_GNOCCHI_UNITS" fi @@ -162,6 +165,8 @@ function adapt_collectd_conf { sudo sed -i 's|CEILOMETER_TIMEOUT.*$|CEILOMETER_TIMEOUT "'$CEILOMETER_TIMEOUT'"|g' $COLLECTD_CONF_DIR/collectd-aodh-plugin.conf sudo sed -i 's|OS_PASSWORD.*$|OS_PASSWORD "'$SERVICE_PASSWORD'"|g' $COLLECTD_CONF_DIR/collectd-aodh-plugin.conf sudo sed -i 's|OS_TENANT_NAME.*$|OS_TENANT_NAME "'$SERVICE_TENANT_NAME'"|g' $COLLECTD_CONF_DIR/collectd-aodh-plugin.conf + sudo sed -i 's|LIBVIRT_METER_ENABLED.*$|LIBVIRT_METER_ENABLED "'$LIBVIRT_METER_ENABLED'"|g' $COLLECTD_CONF_DIR/collectd-aodh-plugin.conf + config_custom_severities "aodh" "$COLLECTD_ALARM_SEVERITIES" fi diff --git a/devstack/settings b/devstack/settings index 179d61e..942785c 100644 --- a/devstack/settings +++ b/devstack/settings @@ -31,6 +31,9 @@ CEILOMETER_TIMEOUT=${CEILOMETER_TIMEOUT:-1000} OS_AUTH_URL="$KEYSTONE_SERVICE_URI/v$IDENTITY_API_VERSION" OS_IDENTITY_API_VERSION=${IDENTITY_API_VERSION:-3} +# Libvirt meter is deprecated +LIBVIRT_METER_ENABLED=$(trueorfalse False LIBVIRT_METER_ENABLED) + # Fall back to default conf dir if option is unset if [ -z $COLLECTD_CONF_DIR ]; then if is_ubuntu; then diff --git a/doc/source/usage.rst b/doc/source/usage.rst index 347316d..44de207 100644 --- a/doc/source/usage.rst +++ b/doc/source/usage.rst @@ -194,6 +194,18 @@ COLLECTD_ADDITIONAL_PACKAGES Example: COLLECTD_ADDITIONAL_PACKAGES="package1 package2 package3" +LIBVIRT_METER_ENABLED +~~~~~~~~~~~~~~~~~~~~ + (True|False) HostnameFormat needs to be set to UUID so that VMs can be + tracked across migrations and across multi-host deployments. This is + important if you want to use the network plugin, and only run + collectd-{ceilometer, gnocchi, aodh} on the "collectd server", as we query + nova on the local node that is being monitored, to map instance name to + UUID. You can enable the collectd virt plugin when setting up a + multi-node deployment, which will do this. + + Default: False + Authenticating using Identity Server API v3 ------------------------------------------- diff --git a/etc/collectd.conf.d/collectd-aodh-plugin.conf b/etc/collectd.conf.d/collectd-aodh-plugin.conf index c679b29..3310447 100644 --- a/etc/collectd.conf.d/collectd-aodh-plugin.conf +++ b/etc/collectd.conf.d/collectd-aodh-plugin.conf @@ -27,6 +27,9 @@ OS_PASSWORD "password" OS_TENANT_NAME "service" + # Libvirt meter enabled + LIBVIRT_METER_ENABLED False + diff --git a/etc/collectd.conf.d/collectd-ceilometer-plugin.conf b/etc/collectd.conf.d/collectd-ceilometer-plugin.conf index e83f1cc..c9c8d13 100644 --- a/etc/collectd.conf.d/collectd-ceilometer-plugin.conf +++ b/etc/collectd.conf.d/collectd-ceilometer-plugin.conf @@ -30,6 +30,8 @@ OS_PASSWORD "password" OS_TENANT_NAME "service" + # Libvirt meter enabled + LIBVIRT_METER_ENABLED False diff --git a/etc/collectd.conf.d/collectd-gnocchi-plugin.conf b/etc/collectd.conf.d/collectd-gnocchi-plugin.conf index f28f638..306d56e 100644 --- a/etc/collectd.conf.d/collectd-gnocchi-plugin.conf +++ b/etc/collectd.conf.d/collectd-gnocchi-plugin.conf @@ -30,6 +30,8 @@ OS_PASSWORD "password" OS_TENANT_NAME "service" + # Libvirt meter enabled + LIBVIRT_METER_ENABLED False diff --git a/releasenotes/notes/deprecate-libvirt-c97487c99492949d.yaml b/releasenotes/notes/deprecate-libvirt-c97487c99492949d.yaml new file mode 100644 index 0000000..9e20c10 --- /dev/null +++ b/releasenotes/notes/deprecate-libvirt-c97487c99492949d.yaml @@ -0,0 +1,6 @@ +--- +deprecations: + - | + Marked LibvirtMeter as deprecated. Included deployment code to configure it + for now, by default it is not enabled. In future releases it will be + removed.