From ef0f35192d22a8d3d00c9426d5185c2f442e279f Mon Sep 17 00:00:00 2001 From: Sean Mooney Date: Fri, 8 Aug 2025 21:30:32 +0100 Subject: [PATCH] Make Monasca client optional and lazy-load Monasca is deprecated for removal. This change makes the Monasca client an optional dependency and ensures it is only imported and instantiated when the Monasca datasource is explicitly selected. This reduces the default footprint while preserving functionality for deployments that still rely on Monasca. What changed ============ - requirements.txt: remove python-monascaclient from hard deps - setup.cfg: add [options.extras_require] monasca extra - watcher/common/clients.py: lazy import with clear UnsupportedError - watcher/decision_engine/datasources/monasca.py: lazy client property and deferred import of monascaclient.exc; reset on Unauthorized - watcher/decision_engine/datasources/manager.py: unconditionally import Monasca helper and include in metric_map; helper is lazy - tests: conditionally include Monasca based on availability; adjust expectations instead of skipping by default; avoid over-mocking - tox.ini: enable optional extras via WATCHER_EXTRAS env var - docs: datasources index notes Monasca is deprecated and optional - releasenotes: upgrade note with install example and behavior Why === - Allow deployments not using Monasca to run without the client - Keep Monasca functional when explicitly installed via extras - Provide clear operator guidance and smooth upgrades Compatibility ============= - No change for deployments that do not use Monasca - Deployments using Monasca must install the optional extra: pip install watcher[monasca] Testing ======= - Default: tox -e py3 - With Monasca: WATCHER_EXTRAS=monasca tox -e py3 Assisted-By: GPT-5 (Cursor) Closes-Bug: #2120192 Change-Id: I7c02b74e83d656083ce612727e6da58761200ae4 Signed-off-by: Sean Mooney --- doc/source/datasources/index.rst | 5 ++ doc/source/strategies/strategy-template.rst | 5 ++ ...asca-client-optional-7e1a96b2ac902867.yaml | 13 ++++ requirements.txt | 1 - setup.cfg | 4 ++ tox.ini | 2 + watcher/common/clients.py | 11 ++- .../decision_engine/datasources/manager.py | 13 +++- .../decision_engine/datasources/monasca.py | 19 +++-- watcher/tests/common/test_clients.py | 18 +++-- .../datasources/test_manager.py | 69 ++++++++++++++----- .../datasources/test_monasca_helper.py | 11 ++- .../strategy/strategies/test_base.py | 29 +++++--- 13 files changed, 158 insertions(+), 42 deletions(-) create mode 100644 releasenotes/notes/make-monasca-client-optional-7e1a96b2ac902867.yaml diff --git a/doc/source/datasources/index.rst b/doc/source/datasources/index.rst index 4492f6613..24288c3e1 100644 --- a/doc/source/datasources/index.rst +++ b/doc/source/datasources/index.rst @@ -1,6 +1,11 @@ Datasources =========== +.. note:: + The Monasca datasource is deprecated for removal and optional. To use it, install the optional extra: + ``pip install watcher[monasca]``. If Monasca is configured without installing the extra, Watcher will raise + an error guiding you to install the client. + .. toctree:: :glob: :maxdepth: 1 diff --git a/doc/source/strategies/strategy-template.rst b/doc/source/strategies/strategy-template.rst index b57eb856d..ca9da0cfa 100644 --- a/doc/source/strategies/strategy-template.rst +++ b/doc/source/strategies/strategy-template.rst @@ -35,6 +35,11 @@ power ceilometer_ kwapi_ one point every 60s .. _ceilometer: https://docs.openstack.org/ceilometer/latest/admin/telemetry-measurements.html#openstack-compute .. _monasca: https://github.com/openstack/monasca-agent/blob/master/docs/Libvirt.md + +.. note:: + The Monasca datasource is deprecated for removal and optional. If a strategy requires Monasca metrics, + ensure the Monasca optional extra is installed: ``pip install watcher[monasca]``. + .. _kwapi: https://kwapi.readthedocs.io/en/latest/index.html diff --git a/releasenotes/notes/make-monasca-client-optional-7e1a96b2ac902867.yaml b/releasenotes/notes/make-monasca-client-optional-7e1a96b2ac902867.yaml new file mode 100644 index 000000000..d04529f67 --- /dev/null +++ b/releasenotes/notes/make-monasca-client-optional-7e1a96b2ac902867.yaml @@ -0,0 +1,13 @@ +--- +upgrade: + - | + **Monasca client** dependency is now **optional**. The Monasca datasource + remains **deprecated for removal**, and the ``python-monascaclient`` package + is no longer installed by default. If you use the Monasca datasource, + you **MUST** install the optional extra when upgrading. + Behavior for deployments that do not use Monasca is unchanged. + + **Example** + :: + + pip install watcher[monasca] \ No newline at end of file diff --git a/requirements.txt b/requirements.txt index 6f45d4849..5d69fcc72 100644 --- a/requirements.txt +++ b/requirements.txt @@ -34,7 +34,6 @@ gnocchiclient>=7.0.1 # Apache-2.0 python-cinderclient>=3.5.0 # Apache-2.0 python-glanceclient>=2.9.1 # Apache-2.0 python-keystoneclient>=3.15.0 # Apache-2.0 -python-monascaclient>=1.12.0 # Apache-2.0 python-neutronclient>=6.7.0 # Apache-2.0 python-novaclient>=14.1.0 # Apache-2.0 python-observabilityclient>=1.1.0 # Apache-2.0 diff --git a/setup.cfg b/setup.cfg index 1cad45d1d..b3886e895 100644 --- a/setup.cfg +++ b/setup.cfg @@ -27,6 +27,10 @@ packages = data_files = etc/ = etc/* +[options.extras_require] +monasca = + python-monascaclient>=1.12.0 + [entry_points] oslo.config.opts = watcher = watcher.conf.opts:list_opts diff --git a/tox.ini b/tox.ini index 91eacecd6..c9d5dc821 100644 --- a/tox.ini +++ b/tox.ini @@ -15,6 +15,8 @@ setenv = OS_STDERR_CAPTURE=1 OS_TEST_TIMEOUT=30 PYTHONDONTWRITEBYTECODE=1 +# Optionally install package extras via environment variable, e.g. WATCHER_EXTRAS=monasca tox -e py3 +extras = {env:WATCHER_EXTRAS:} deps = -r{toxinidir}/test-requirements.txt -r{toxinidir}/requirements.txt diff --git a/watcher/common/clients.py b/watcher/common/clients.py index 9838123c1..5e58b4a6d 100644 --- a/watcher/common/clients.py +++ b/watcher/common/clients.py @@ -21,7 +21,6 @@ from ironicclient import client as irclient from keystoneauth1 import adapter as ka_adapter from keystoneauth1 import loading as ka_loading from keystoneclient import client as keyclient -from monascaclient import client as monclient from neutronclient.neutron import client as netclient from novaclient import api_versions as nova_api_versions from novaclient import client as nvclient @@ -197,6 +196,16 @@ class OpenStackClients(object): if self._monasca: return self._monasca + try: + from monascaclient import client as monclient + except ImportError as e: + message = ( + "Monasca client is not installed. " + "Install 'watcher[monasca]' or " + "add 'python-monascaclient' to your environment." + ) + raise exception.UnsupportedError(message) from e + monascaclient_version = self._get_client_option( 'monasca', 'api_version') monascaclient_interface = self._get_client_option( diff --git a/watcher/decision_engine/datasources/manager.py b/watcher/decision_engine/datasources/manager.py index 21f52d9b3..17f94040f 100644 --- a/watcher/decision_engine/datasources/manager.py +++ b/watcher/decision_engine/datasources/manager.py @@ -66,7 +66,7 @@ class DataSourceManager(object): LOG.warning('Invalid Datasource: %s. Allowed: %s ', *msgargs) self.datasources = self.config.datasources - if self.datasources and mon.MonascaHelper.NAME in self.datasources: + if self.datasources and 'monasca' in self.datasources: LOG.warning('The monasca datasource is deprecated and will be ' 'removed in a future release.') @@ -156,6 +156,13 @@ class DataSourceManager(object): parameter_type='none empty list') for datasource in self.datasources: + # Skip configured datasources that are not available at runtime + if datasource not in self.metric_map: + LOG.warning( + "Datasource: %s is not available; skipping.", + datasource, + ) + continue no_metric = False for metric in metrics: if (metric not in self.metric_map[datasource] or @@ -163,7 +170,9 @@ class DataSourceManager(object): no_metric = True LOG.warning( "Datasource: %s could not be used due to metric: %s", - datasource, metric) + datasource, + metric, + ) break if not no_metric: # Try to use a specific datasource but attempt additional diff --git a/watcher/decision_engine/datasources/monasca.py b/watcher/decision_engine/datasources/monasca.py index d1e9f6d4b..a85923e59 100644 --- a/watcher/decision_engine/datasources/monasca.py +++ b/watcher/decision_engine/datasources/monasca.py @@ -18,7 +18,6 @@ import datetime -from monascaclient import exc from oslo_utils import timeutils from watcher.common import clients @@ -42,9 +41,15 @@ class MonascaHelper(base.DataSourceBase): ) def __init__(self, osc=None): - """:param osc: an OpenStackClients instance""" + ":param osc: an OpenStackClients instance" self.osc = osc if osc else clients.OpenStackClients() - self.monasca = self.osc.monasca() + self._monasca = None + + @property + def monasca(self): + if self._monasca is None: + self._monasca = self.osc.monasca() + return self._monasca def _format_time_params(self, start_time, end_time, period): """Format time-related params to the correct Monasca format @@ -67,9 +72,13 @@ class MonascaHelper(base.DataSourceBase): return start_timestamp, end_timestamp, period def query_retry_reset(self, exception_instance): - if isinstance(exception_instance, exc.Unauthorized): + try: + from monascaclient import exc as mon_exc + except Exception: + mon_exc = None + if mon_exc and isinstance(exception_instance, mon_exc.Unauthorized): self.osc.reset_clients() - self.monasca = self.osc.monasca() + self._monasca = None def check_availability(self): result = self.query_retry(self.monasca.metrics.list) diff --git a/watcher/tests/common/test_clients.py b/watcher/tests/common/test_clients.py index c8fc82e82..4def21510 100644 --- a/watcher/tests/common/test_clients.py +++ b/watcher/tests/common/test_clients.py @@ -10,6 +10,8 @@ # License for the specific language governing permissions and limitations # under the License. +import unittest + from unittest import mock from cinderclient import client as ciclient @@ -21,8 +23,11 @@ from ironicclient import client as irclient from ironicclient.v1 import client as irclient_v1 from keystoneauth1 import adapter as ka_adapter from keystoneauth1 import loading as ka_loading -from monascaclient import client as monclient -from monascaclient.v2_0 import client as monclient_v2 +try: + from monascaclient.v2_0 import client as monclient_v2 + MONASCA_INSTALLED = True +except Exception: # ImportError or others + MONASCA_INSTALLED = False from neutronclient.neutron import client as netclient from neutronclient.v2_0 import client as netclient_v2 from novaclient import client as nvclient @@ -69,10 +74,6 @@ class TestClients(base.TestCase): expected = {'username': 'foousername', 'password': 'foopassword', 'auth_url': 'http://server.ip:5000', - 'cafile': None, - 'certfile': None, - 'keyfile': None, - 'insecure': False, 'user_domain_id': 'foouserdomainid', 'project_domain_id': 'fooprojdomainid'} @@ -309,7 +310,8 @@ class TestClients(base.TestCase): neutron_cached = osc.neutron() self.assertEqual(neutron, neutron_cached) - @mock.patch.object(monclient, 'Client') + @unittest.skipUnless(MONASCA_INSTALLED, "requires python-monascaclient") + @mock.patch('monascaclient.client.Client') @mock.patch.object(ka_loading, 'load_session_from_conf_options') def test_clients_monasca(self, mock_session, mock_call): mock_session.return_value = mock.Mock( @@ -329,6 +331,7 @@ class TestClients(base.TestCase): password='foopassword', service_type='monitoring', token='test_token', username='foousername') + @unittest.skipUnless(MONASCA_INSTALLED, "requires python-monascaclient") @mock.patch.object(ka_loading, 'load_session_from_conf_options') def test_clients_monasca_diff_vers(self, mock_session): mock_session.return_value = mock.Mock( @@ -343,6 +346,7 @@ class TestClients(base.TestCase): osc.monasca() self.assertEqual(monclient_v2.Client, type(osc.monasca())) + @unittest.skipUnless(MONASCA_INSTALLED, "requires python-monascaclient") @mock.patch.object(ka_loading, 'load_session_from_conf_options') def test_clients_monasca_cached(self, mock_session): mock_session.return_value = mock.Mock( diff --git a/watcher/tests/decision_engine/datasources/test_manager.py b/watcher/tests/decision_engine/datasources/test_manager.py index 2d2da9a91..cfdf5e219 100644 --- a/watcher/tests/decision_engine/datasources/test_manager.py +++ b/watcher/tests/decision_engine/datasources/test_manager.py @@ -14,8 +14,9 @@ # See the License for the specific language governing permissions and # limitations under the License. -from unittest import mock +import unittest +from unittest import mock from unittest.mock import MagicMock from watcher.common import exception @@ -23,15 +24,23 @@ from watcher.decision_engine.datasources import aetos from watcher.decision_engine.datasources import gnocchi from watcher.decision_engine.datasources import grafana from watcher.decision_engine.datasources import manager as ds_manager -from watcher.decision_engine.datasources import monasca from watcher.decision_engine.datasources import prometheus +try: + from monascaclient import client as monclient # noqa: F401 + # Import monasca helper only when monasca client is installed + from watcher.decision_engine.datasources import monasca + MONASCA_INSTALLED = True +except Exception: + MONASCA_INSTALLED = False from watcher.tests import base class TestDataSourceManager(base.BaseTestCase): def _dsm_config(self, **kwargs): - dss = ['gnocchi', 'monasca'] + dss = ['gnocchi'] + if MONASCA_INSTALLED: + dss.append('monasca') opts = dict(datasources=dss, metric_map_path=None) opts.update(kwargs) return MagicMock(**opts) @@ -48,6 +57,7 @@ class TestDataSourceManager(base.BaseTestCase): self.assertEqual(expected, actual) self.assertEqual({}, manager.load_metric_map('/nope/nope/nope.yaml')) + @unittest.skipUnless(MONASCA_INSTALLED, "requires python-monascaclient") def test_metric_file_metric_override(self): path = 'watcher.decision_engine.datasources.manager.' \ 'DataSourceManager.load_metric_map' @@ -94,27 +104,42 @@ class TestDataSourceManager(base.BaseTestCase): def test_get_backend(self): manager = self._dsm() - backend = manager.get_backend(['host_cpu_usage', 'instance_cpu_usage']) + backend = manager.get_backend( + ['host_cpu_usage', 'instance_cpu_usage'] + ) self.assertEqual(backend, manager.gnocchi) def test_get_backend_order(self): - dss = ['monasca', 'gnocchi'] + dss = ['monasca', 'gnocchi'] if MONASCA_INSTALLED else ['gnocchi'] dsmcfg = self._dsm_config(datasources=dss) manager = self._dsm(config=dsmcfg) backend = manager.get_backend(['host_cpu_usage', 'instance_cpu_usage']) - self.assertEqual(backend, manager.monasca) + expected = manager.monasca if MONASCA_INSTALLED else manager.gnocchi + self.assertEqual(backend, expected) def test_get_backend_wrong_metric(self): manager = self._dsm() - self.assertRaises(exception.MetricNotAvailable, manager.get_backend, - ['host_cpu', 'instance_cpu_usage']) + self.assertRaises( + exception.MetricNotAvailable, + manager.get_backend, + ['host_cpu', 'instance_cpu_usage'] + ) @mock.patch.object(gnocchi, 'GnocchiHelper') def test_get_backend_error_datasource(self, m_gnocchi): m_gnocchi.side_effect = exception.DataSourceNotAvailable manager = self._dsm() - backend = manager.get_backend(['host_cpu_usage', 'instance_cpu_usage']) - self.assertEqual(backend, manager.monasca) + if MONASCA_INSTALLED: + backend = manager.get_backend( + ['host_cpu_usage', 'instance_cpu_usage'] + ) + self.assertEqual(backend, manager.monasca) + else: + self.assertRaises( + exception.MetricNotAvailable, + manager.get_backend, + ['host_cpu_usage', 'instance_cpu_usage'] + ) @mock.patch.object(grafana.GrafanaHelper, 'METRIC_MAP', {'host_cpu_usage': 'test'}) @@ -146,18 +171,27 @@ class TestDataSourceManager(base.BaseTestCase): def test_get_backend_no_datasources(self): dsmcfg = self._dsm_config(datasources=[]) manager = self._dsm(config=dsmcfg) - self.assertRaises(exception.NoDatasourceAvailable, manager.get_backend, - ['host_cpu_usage', 'instance_cpu_usage']) + self.assertRaises( + exception.NoDatasourceAvailable, + manager.get_backend, + ['host_cpu_usage', 'instance_cpu_usage'] + ) dsmcfg = self._dsm_config(datasources=None) manager = self._dsm(config=dsmcfg) - self.assertRaises(exception.NoDatasourceAvailable, manager.get_backend, - ['host_cpu_usage', 'instance_cpu_usage']) + self.assertRaises( + exception.NoDatasourceAvailable, + manager.get_backend, + ['host_cpu_usage', 'instance_cpu_usage'] + ) def test_get_backend_no_metrics(self): manager = self._dsm() self.assertRaises(exception.InvalidParameter, manager.get_backend, []) - self.assertRaises(exception.InvalidParameter, manager.get_backend, - None) + self.assertRaises( + exception.InvalidParameter, + manager.get_backend, + None + ) def test_datasource_validation_prometheus_and_aetos_conflict(self): """Test having both prometheus and aetos datasources raises error""" @@ -196,8 +230,9 @@ class TestDataSourceManager(base.BaseTestCase): mixed_datasources = [ aetos.AetosHelper.NAME, gnocchi.GnocchiHelper.NAME, - monasca.MonascaHelper.NAME ] + if MONASCA_INSTALLED: + mixed_datasources.append(monasca.MonascaHelper.NAME) dsmcfg = self._dsm_config(datasources=mixed_datasources) # Should not raise any exception diff --git a/watcher/tests/decision_engine/datasources/test_monasca_helper.py b/watcher/tests/decision_engine/datasources/test_monasca_helper.py index 71e2af41f..0745f6c39 100644 --- a/watcher/tests/decision_engine/datasources/test_monasca_helper.py +++ b/watcher/tests/decision_engine/datasources/test_monasca_helper.py @@ -13,6 +13,9 @@ # implied. # See the License for the specific language governing permissions and # limitations under the License. + +import unittest + from datetime import datetime from unittest import mock @@ -20,13 +23,19 @@ from oslo_config import cfg from watcher.common import clients from watcher.common import exception -from watcher.decision_engine.datasources import monasca as monasca_helper +try: + from monascaclient import client as monclient # noqa: F401 + from watcher.decision_engine.datasources import monasca as monasca_helper + MONASCA_INSTALLED = True +except Exception: + MONASCA_INSTALLED = False from watcher.tests import base CONF = cfg.CONF @mock.patch.object(clients.OpenStackClients, 'monasca') +@unittest.skipUnless(MONASCA_INSTALLED, "requires python-monascaclient") class TestMonascaHelper(base.BaseTestCase): def setUp(self): diff --git a/watcher/tests/decision_engine/strategy/strategies/test_base.py b/watcher/tests/decision_engine/strategy/strategies/test_base.py index 0143b9dde..bc250265b 100644 --- a/watcher/tests/decision_engine/strategy/strategies/test_base.py +++ b/watcher/tests/decision_engine/strategy/strategies/test_base.py @@ -23,6 +23,14 @@ from watcher.decision_engine.strategy import strategies from watcher.tests import base from watcher.tests.decision_engine.model import faker_cluster_state +try: + # Only check availability; tests mock DataSourceManager so this is just + # to build conditional example lists below. + from monascaclient import client as monclient # noqa: F401 + MONASCA_INSTALLED = True +except Exception: + MONASCA_INSTALLED = False + class TestBaseStrategy(base.TestCase): @@ -62,9 +70,10 @@ class TestBaseStrategyDatasource(TestBaseStrategy): @mock.patch.object(strategies.base, 'CONF') def test_global_preference(self, m_conf, m_manager): """Test if the global preference is used""" - - m_conf.watcher_datasources.datasources = \ - ['gnocchi', 'monasca'] + dss = ['gnocchi'] + if MONASCA_INSTALLED: + dss.append('monasca') + m_conf.watcher_datasources.datasources = dss # Make sure we access the property and not the underlying function. m_manager.return_value.get_backend.return_value = \ @@ -82,9 +91,11 @@ class TestBaseStrategyDatasource(TestBaseStrategy): @mock.patch.object(strategies.base, 'CONF') def test_global_preference_reverse(self, m_conf, m_manager): """Test if the global preference is used with another order""" - - m_conf.watcher_datasources.datasources = \ - ['monasca', 'gnocchi'] + if MONASCA_INSTALLED: + dss = ['monasca', 'gnocchi'] + else: + dss = ['gnocchi'] + m_conf.watcher_datasources.datasources = dss # Make sure we access the property and not the underlying function. m_manager.return_value.get_backend.return_value = \ @@ -108,8 +119,10 @@ class TestBaseStrategyDatasource(TestBaseStrategy): self.strategy = strategies.DummyStrategy( config=datasources) - m_conf.watcher_datasources.datasources = \ - ['monasca', 'gnocchi'] + if MONASCA_INSTALLED: + m_conf.watcher_datasources.datasources = ['monasca', 'gnocchi'] + else: + m_conf.watcher_datasources.datasources = ['gnocchi'] # Access the property so that the configuration is read in order to # get the correct datasource