From a26c2225b656b3c5e057f4b02e17fb70385c40a6 Mon Sep 17 00:00:00 2001 From: Feilong Wang Date: Tue, 5 Jun 2018 15:08:49 +1200 Subject: [PATCH] Deprecate send_cluster_metrics Currently, Magnum is running periodic tasks to collect k8s cluster metrics to message bus. Unfortunately, it's collecting pods info only from "default" namespace which makes this function useless. What's more, even Magnum can get all pods from all namespaces, it doesn't make much sense to keep this function in Magnum. Because operators only care about the health of cluster nodes. If they want to know the status of pods, they can use heapster or other tools to get that. Task: 22619 Story: 1775116 Change-Id: I3ca0f2e96fe63870406cc5323f08fa018ac6e8be --- magnum/conf/drivers.py | 5 ++++- magnum/service/periodic.py | 2 ++ magnum/tests/unit/service/test_periodic.py | 8 +++++--- ...cate-send_cluster_metrics-8adaac64a979f720.yaml | 14 ++++++++++++++ 4 files changed, 25 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/deprecate-send_cluster_metrics-8adaac64a979f720.yaml diff --git a/magnum/conf/drivers.py b/magnum/conf/drivers.py index e4e32d91ea..2006c97f58 100644 --- a/magnum/conf/drivers.py +++ b/magnum/conf/drivers.py @@ -31,7 +31,10 @@ drivers_opts = [ help='Path to the OpenStack CA-bundle file to pass and ' 'install in all cluster nodes.'), cfg.BoolOpt('send_cluster_metrics', - default=True, + default=False, + deprecated_for_removal=True, + deprecated_reason='It does not make sense only collecting ' + 'metrics from the "default" namespcae.', help='Allow periodic tasks to pull COE data and send to ' 'ceilometer.'), cfg.ListOpt('disabled_drivers', diff --git a/magnum/service/periodic.py b/magnum/service/periodic.py index 2191d02593..5e66c77598 100755 --- a/magnum/service/periodic.py +++ b/magnum/service/periodic.py @@ -16,6 +16,7 @@ import functools from oslo_log import log +from oslo_log.versionutils import deprecated from oslo_service import loopingcall from oslo_service import periodic_task @@ -140,6 +141,7 @@ class MagnumPeriodicTasks(periodic_task.PeriodicTasks): @periodic_task.periodic_task(run_immediately=True) @set_context + @deprecated(as_of=deprecated.ROCKY) def _send_cluster_metrics(self, ctx): if not CONF.drivers.send_cluster_metrics: LOG.debug('Skip sending cluster metrics') diff --git a/magnum/tests/unit/service/test_periodic.py b/magnum/tests/unit/service/test_periodic.py index 416305d8c5..4d50765a33 100644 --- a/magnum/tests/unit/service/test_periodic.py +++ b/magnum/tests/unit/service/test_periodic.py @@ -224,6 +224,7 @@ class PeriodicTestCase(base.TestCase): mock_get_notifier, mock_cluster_list, mock_create_monitor): """Test if RPC notifier receives the expected message""" + CONF.set_override('send_cluster_metrics', True, group='drivers') mock_make_admin_context.return_value = self.context notifier = mock.MagicMock() mock_get_notifier.return_value = notifier @@ -269,6 +270,7 @@ class PeriodicTestCase(base.TestCase): def test_send_cluster_metrics_compute_metric_raise( self, mock_make_admin_context, mock_get_notifier, mock_cluster_list, mock_create_monitor): + CONF.set_override('send_cluster_metrics', True, group='drivers') mock_make_admin_context.return_value = self.context notifier = mock.MagicMock() mock_get_notifier.return_value = notifier @@ -300,6 +302,7 @@ class PeriodicTestCase(base.TestCase): def test_send_cluster_metrics_pull_data_raise( self, mock_make_admin_context, mock_get_notifier, mock_cluster_list, mock_create_monitor): + CONF.set_override('send_cluster_metrics', True, group='drivers') mock_make_admin_context.return_value = self.context notifier = mock.MagicMock() mock_get_notifier.return_value = notifier @@ -321,6 +324,7 @@ class PeriodicTestCase(base.TestCase): def test_send_cluster_metrics_monitor_none( self, mock_make_admin_context, mock_get_notifier, mock_cluster_list, mock_create_monitor): + CONF.set_override('send_cluster_metrics', True, group='drivers') mock_make_admin_context.return_value = self.context notifier = mock.MagicMock() mock_get_notifier.return_value = notifier @@ -340,6 +344,7 @@ class PeriodicTestCase(base.TestCase): def test_send_cluster_metrics_disable_pull_data( self, mock_make_admin_context, mock_get_notifier, mock_cluster_list, mock_create_monitor): + mock_make_admin_context.return_value = self.context notifier = mock.MagicMock() mock_get_notifier.return_value = notifier @@ -352,9 +357,6 @@ class PeriodicTestCase(base.TestCase): monitor.get_metric_unit.return_value = '%' mock_create_monitor.return_value = monitor - CONF.set_override('send_cluster_metrics', - False, group='drivers') - periodic.MagnumPeriodicTasks(CONF)._send_cluster_metrics(self.context) self.assertEqual(0, mock_create_monitor.call_count) diff --git a/releasenotes/notes/deprecate-send_cluster_metrics-8adaac64a979f720.yaml b/releasenotes/notes/deprecate-send_cluster_metrics-8adaac64a979f720.yaml new file mode 100644 index 0000000000..247b9f4b15 --- /dev/null +++ b/releasenotes/notes/deprecate-send_cluster_metrics-8adaac64a979f720.yaml @@ -0,0 +1,14 @@ +--- +deprecations: + - | + Currently, Magnum is running periodic tasks to collect k8s cluster + metrics to message bus. Unfortunately, it's collecting pods info + only from "default" namespace which makes this function useless. + What's more, even Magnum can get all pods from all namespaces, it + doesn't make much sense to keep this function in Magnum. Because + operators only care about the health of cluster nodes. If they + want to know the status of pods, they can use heapster or other + tools to get that. So the feauture is being deprecated now and will be + removed in Stein release. And the default value is changed to False, which + means won't send the metrics. +