From ffe81367e10eaf8c6fecc96f3c442eb471354bbc Mon Sep 17 00:00:00 2001 From: Samuel Walladge Date: Mon, 6 Nov 2023 17:11:19 +1030 Subject: [PATCH] Add config option for rbd_stats_pools This allows configuration RBD IO statistics collection for RBD pools. Co-authored-by: Yoshi Kadokawa Closes-Bug: #2042405 Related-Bug: #1989648 Change-Id: I2252163533a312f0f53165f946711ab20bb0e3c9 --- config.yaml | 10 ++++++ src/ceph_hooks.py | 5 +++ src/ceph_metrics.py | 2 ++ src/utils.py | 11 +++++++ unit_tests/test_ceph_hooks.py | 55 ++++++++++++++++++++++++++++++++- unit_tests/test_ceph_metrics.py | 1 + 6 files changed, 83 insertions(+), 1 deletion(-) diff --git a/config.yaml b/config.yaml index 1a5375de..882e9b81 100644 --- a/config.yaml +++ b/config.yaml @@ -326,3 +326,13 @@ options: description: | The balancer mode used by the Ceph manager. Can only be set for Luminous or later versions, and only when the balancer module is enabled. + rbd-stats-pools: + type: string + default: "" + description: | + Set pools to collect RBD per-image IO statistics by enabling dynamic OSD performance counters. + It can be set to: + - a comma separated list of RBD pools to enable (eg. "pool1,pool2,poolN") + - "*" to enable for all RBD pools + - "" to disable statistics + For more information: https://docs.ceph.com/en/latest/mgr/prometheus/#rbd-io-statistics diff --git a/src/ceph_hooks.py b/src/ceph_hooks.py index a04fc7bd..6310e60f 100755 --- a/src/ceph_hooks.py +++ b/src/ceph_hooks.py @@ -92,6 +92,7 @@ from utils import ( get_rbd_features, get_ceph_osd_releases, execute_post_osd_upgrade_steps, + mgr_config_set_rbd_stats_pools, mgr_disable_module, mgr_enable_module, is_mgr_module_enabled, @@ -376,6 +377,9 @@ def config_changed(): try_disable_insecure_reclaim() for relid in relation_ids('dashboard'): dashboard_relation(relid) + + mgr_config_set_rbd_stats_pools() + return True @@ -502,6 +506,7 @@ def prometheus_relation(relid=None, unit=None, prometheus_permitted=None, mgr_enable_module('prometheus')) log("checking if prometheus module is enabled") if prometheus_permitted and module_enabled: + mgr_config_set_rbd_stats_pools() log("Updating prometheus") data = { 'hostname': get_relation_ip('prometheus'), diff --git a/src/ceph_metrics.py b/src/ceph_metrics.py index 0c320451..25077782 100644 --- a/src/ceph_metrics.py +++ b/src/ceph_metrics.py @@ -19,6 +19,7 @@ if TYPE_CHECKING: from charms.prometheus_k8s.v0 import prometheus_scrape from charms_ceph import utils as ceph_utils from ops.framework import BoundEvent +from utils import mgr_config_set_rbd_stats_pools logger = logging.getLogger(__name__) @@ -64,6 +65,7 @@ class CephMetricsEndpointProvider(prometheus_scrape.MetricsEndpointProvider): logger.debug( "is_leader and is_bootstrapped, running rel changed: %s", event ) + mgr_config_set_rbd_stats_pools() ceph_utils.mgr_enable_module("prometheus") logger.debug("module_enabled") self.update_alert_rules() diff --git a/src/utils.py b/src/utils.py index 9e41ea82..03fdb9de 100644 --- a/src/utils.py +++ b/src/utils.py @@ -19,6 +19,7 @@ import subprocess import errno import tenacity +from charms_ceph import utils as ceph_utils from charmhelpers.core.hookenv import ( DEBUG, cached, @@ -417,3 +418,13 @@ def _set_require_osd_release(release): msg = 'Unable to execute command <{}>'.format(call_error.cmd) log(message=msg, level='ERROR') raise OsdPostUpgradeError(call_error) + + +def mgr_config_set_rbd_stats_pools(): + """Update ceph mgr config with the value from rbd-status-pools config + """ + if is_leader() and ceph_utils.is_bootstrapped(): + ceph_utils.mgr_config_set( + 'mgr/prometheus/rbd_stats_pools', + config('rbd-stats-pools') + ) diff --git a/unit_tests/test_ceph_hooks.py b/unit_tests/test_ceph_hooks.py index b11bde57..497bb212 100644 --- a/unit_tests/test_ceph_hooks.py +++ b/unit_tests/test_ceph_hooks.py @@ -18,6 +18,7 @@ with patch('charmhelpers.contrib.hardening.harden.harden') as mock_dec: mock_dec.side_effect = (lambda *dargs, **dkwargs: lambda f: lambda *args, **kwargs: f(*args, **kwargs)) import ceph_hooks + import utils TO_PATCH = [ 'config', @@ -55,7 +56,8 @@ CHARM_CONFIG = {'config-flags': '', 'nagios_additional_checks': "", 'nagios_additional_checks_critical': False, 'nagios_check_num_osds': False, - 'disable-pg-max-object-skew': False} + 'disable-pg-max-object-skew': False, + 'rbd-stats-pools': 'foo'} class CephHooksTestCase(test_utils.CharmTestCase): @@ -313,6 +315,8 @@ class CephHooksTestCase(test_utils.CharmTestCase): ceph_hooks.get_client_application_name('rel:1', None), 'glance') + @patch.object(utils, 'is_leader', lambda: False) + @patch.object(ceph_hooks.ceph, 'mgr_config_set', lambda _key, _value: None) @patch.object(ceph_hooks.ceph, 'list_pools') @patch.object(ceph_hooks, 'mgr_enable_module') @patch.object(ceph_hooks, 'emit_cephconf') @@ -336,6 +340,8 @@ class CephHooksTestCase(test_utils.CharmTestCase): ceph_hooks.config_changed() mgr_enable_module.assert_not_called() + @patch.object(utils, 'is_leader', lambda: False) + @patch.object(ceph_hooks.ceph, 'mgr_config_set', lambda _key, _value: None) @patch.object(ceph_hooks.ceph, 'monitor_key_set') @patch.object(ceph_hooks.ceph, 'list_pools') @patch.object(ceph_hooks, 'mgr_enable_module') @@ -365,6 +371,8 @@ class CephHooksTestCase(test_utils.CharmTestCase): mgr_enable_module.assert_called_once_with('pg_autoscaler') monitor_key_set.assert_called_once_with('admin', 'autotune', 'true') + @patch.object(utils, 'is_leader', lambda: False) + @patch.object(ceph_hooks.ceph, 'mgr_config_set', lambda _key, _value: None) @patch.object(ceph_hooks.ceph, 'list_pools') @patch.object(ceph_hooks, 'mgr_enable_module') @patch.object(ceph_hooks, 'emit_cephconf') @@ -674,6 +682,7 @@ class BootstrapSourceTestCase(test_utils.CharmTestCase): self.assertRaises(AssertionError, ceph_hooks.bootstrap_source_relation_changed) + @patch.object(utils, 'is_leader', lambda: False) @patch.object(ceph_hooks.ceph, 'is_bootstrapped') @patch.object(ceph_hooks, 'emit_cephconf') @patch.object(ceph_hooks, 'leader_get') @@ -708,6 +717,50 @@ class BootstrapSourceTestCase(test_utils.CharmTestCase): _emit_cephconf.assert_called_once_with() _is_bootstrapped.assert_called_once_with() + @patch.object(utils, 'is_leader', lambda: True) + @patch.object(utils, 'config', lambda _: 'pool1') + @patch.object(utils.ceph_utils, 'mgr_config_set') + @patch.object(ceph_hooks.ceph, 'is_bootstrapped') + @patch.object(ceph_hooks, 'emit_cephconf') + @patch.object(ceph_hooks, 'leader_get') + @patch.object(ceph_hooks, 'is_leader') + @patch.object(ceph_hooks, 'relations_of_type') + @patch.object(ceph_hooks, 'get_mon_hosts') + @patch.object(ceph_hooks, 'check_for_upgrade') + @patch.object(ceph_hooks, 'config') + def test_config_changed_leader( + self, + _config, + _check_for_upgrade, + _get_mon_hosts, + _relations_of_type, + _is_leader, + _leader_get, + _emit_cephconf, + _is_bootstrapped, + _mgr_config_set + ): + config = copy.deepcopy(CHARM_CONFIG) + _config.side_effect = \ + lambda key=None: config.get(key, None) if key else config + _relations_of_type.return_value = False + _is_leader.return_value = True + _leader_get.side_effect = ['fsid', 'monsec', 'fsid', 'monsec'] + _is_bootstrapped.return_value = True + ceph_hooks.config_changed() + _check_for_upgrade.assert_called_once_with() + _get_mon_hosts.assert_called_once_with() + _leader_get.assert_has_calls([ + call('fsid'), + call('monitor-secret'), + ]) + _emit_cephconf.assert_called_once_with() + _is_bootstrapped.assert_has_calls([call(), call()]) + _mgr_config_set.assert_called_once_with( + 'mgr/prometheus/rbd_stats_pools', 'pool1' + ) + + @patch.object(utils, 'is_leader', lambda: False) @patch.object(ceph_hooks, 'emit_cephconf') @patch.object(ceph_hooks, 'create_sysctl') @patch.object(ceph_hooks, 'check_for_upgrade') diff --git a/unit_tests/test_ceph_metrics.py b/unit_tests/test_ceph_metrics.py index 0468d28d..f93141f1 100644 --- a/unit_tests/test_ceph_metrics.py +++ b/unit_tests/test_ceph_metrics.py @@ -52,6 +52,7 @@ class TestCephMetrics(unittest.TestCase): "metrics-endpoint", ) + @patch("ceph_metrics.mgr_config_set_rbd_stats_pools", lambda: None) @patch("ceph_metrics.ceph_utils.is_bootstrapped", return_value=True) @patch("ceph_metrics.ceph_utils.is_mgr_module_enabled", return_value=False) @patch("ceph_metrics.ceph_utils.mgr_enable_module")