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