From 5686dc52c7ce1c1a2ad97d6be4eb443a31be6927 Mon Sep 17 00:00:00 2001 From: Sebastian Wagner Date: Fri, 8 Feb 2019 18:45:22 +0100 Subject: [PATCH] mgr/dashboard: Merge OrchClient with OrchestratorClientMixin Fixed missing error propagation from the orchestrator to the dashboard Adapted orchestrator integration in NFS Signed-off-by: Sebastian Wagner (cherry picked from commit 17e541a6c9a1b1b9842676f44865376da9bd7625) Conflicts: src/pybind/mgr/dashboard/tools.py Conflct was: Nautilus doesn't use `ipaddress.IPv6Address()` yet --- src/pybind/mgr/dashboard/controllers/iscsi.py | 4 +- src/pybind/mgr/dashboard/services/ganesha.py | 31 ++++++---- .../mgr/dashboard/services/iscsi_config.py | 4 +- .../mgr/dashboard/services/orchestrator.py | 57 +++++++------------ .../mgr/dashboard/tests/test_ganesha.py | 5 +- src/pybind/mgr/dashboard/tests/test_iscsi.py | 2 +- src/pybind/mgr/orchestrator.py | 41 +++++++++---- 7 files changed, 77 insertions(+), 67 deletions(-) diff --git a/src/pybind/mgr/dashboard/controllers/iscsi.py b/src/pybind/mgr/dashboard/controllers/iscsi.py index da25f4c70a6d9..9f0e982901452 100644 --- a/src/pybind/mgr/dashboard/controllers/iscsi.py +++ b/src/pybind/mgr/dashboard/controllers/iscsi.py @@ -36,7 +36,7 @@ class IscsiUi(BaseController): status['message'] = 'There are no gateways defined' return status try: - for gateway in gateways.keys(): + for gateway in gateways: try: IscsiClient.instance(gateway_name=gateway).ping() except RequestException: @@ -67,7 +67,7 @@ class IscsiUi(BaseController): def portals(self): portals = [] gateways_config = IscsiGatewaysConfig.get_gateways_config() - for name in gateways_config['gateways'].keys(): + for name in gateways_config['gateways']: ip_addresses = IscsiClient.instance(gateway_name=name).get_ip_addresses() portals.append({'name': name, 'ip_addresses': ip_addresses['data']}) return sorted(portals, key=lambda p: '{}.{}'.format(p['name'], p['ip_addresses'])) diff --git a/src/pybind/mgr/dashboard/services/ganesha.py b/src/pybind/mgr/dashboard/services/ganesha.py index 8a81e5bb35b05..4053d20c1ca10 100644 --- a/src/pybind/mgr/dashboard/services/ganesha.py +++ b/src/pybind/mgr/dashboard/services/ganesha.py @@ -3,6 +3,7 @@ from __future__ import absolute_import import re +from orchestrator import OrchestratorError from .cephfs import CephFS from .cephx import CephX from .orchestrator import OrchClient @@ -65,11 +66,18 @@ class Ganesha(object): def get_ganesha_clusters(cls): return [cluster_id for cluster_id in cls._get_clusters_locations()] + @staticmethod + def _get_orch_nfs_instances(): + try: + return OrchClient().list_service_info("nfs") + except (RuntimeError, OrchestratorError, ImportError): + return [] + @classmethod def get_daemons_status(cls): - if not OrchClient.instance().available(): + instances = cls._get_orch_nfs_instances() + if not instances: return None - instances = OrchClient.instance().list_service_info("nfs") result = {} for instance in instances: @@ -105,14 +113,13 @@ class Ganesha(object): @classmethod def get_pool_and_namespace(cls, cluster_id): - if OrchClient.instance().available(): - instances = OrchClient.instance().list_service_info("nfs") - # we assume that every instance stores there configuration in the - # same RADOS pool/namespace - if instances: - location = instances[0].rados_config_location - pool, ns, _ = cls.parse_rados_url(location) - return pool, ns + instances = cls._get_orch_nfs_instances() + # we assume that every instance stores there configuration in the + # same RADOS pool/namespace + if instances: + location = instances[0].rados_config_location + pool, ns, _ = cls.parse_rados_url(location) + return pool, ns locations = cls._get_clusters_locations() if cluster_id not in locations: raise NFSException("Cluster not found: cluster_id={}" @@ -122,7 +129,7 @@ class Ganesha(object): @classmethod def reload_daemons(cls, cluster_id, daemons_id): logger.debug("[NFS] issued reload of daemons: %s", daemons_id) - if not OrchClient.instance().available(): + if not OrchClient().available(): logger.debug("[NFS] orchestrator not available") return reload_list = [] @@ -135,7 +142,7 @@ class Ganesha(object): continue if daemons[cluster_id][daemon_id] == 1: reload_list.append((cluster_id, daemon_id)) - OrchClient.instance().reload_service("nfs", reload_list) + OrchClient().reload_service("nfs", reload_list) @classmethod def fsals_available(cls): diff --git a/src/pybind/mgr/dashboard/services/iscsi_config.py b/src/pybind/mgr/dashboard/services/iscsi_config.py index e4a78f6b626ef..bb6de51a99e82 100644 --- a/src/pybind/mgr/dashboard/services/iscsi_config.py +++ b/src/pybind/mgr/dashboard/services/iscsi_config.py @@ -49,7 +49,7 @@ _ISCSI_STORE_KEY = "_iscsi_config" class IscsiGatewaysConfig(object): @classmethod def _load_config(cls): - if OrchClient.instance().available(): + if OrchClient().available(): raise ManagedByOrchestratorException() json_db = mgr.get_store(_ISCSI_STORE_KEY, '{"gateways": {}}') @@ -89,7 +89,7 @@ class IscsiGatewaysConfig(object): config = cls._load_config() except ManagedByOrchestratorException: config = {'gateways': {}} - instances = OrchClient.instance().list_service_info("iscsi") + instances = OrchClient().list_service_info("iscsi") for instance in instances: config['gateways'][instance.nodename] = { 'service_url': instance.service_url diff --git a/src/pybind/mgr/dashboard/services/orchestrator.py b/src/pybind/mgr/dashboard/services/orchestrator.py index 4a19afef9d48e..366ff7de735c5 100644 --- a/src/pybind/mgr/dashboard/services/orchestrator.py +++ b/src/pybind/mgr/dashboard/services/orchestrator.py @@ -1,55 +1,38 @@ # -*- coding: utf-8 -*- from __future__ import absolute_import -import time - +from orchestrator import OrchestratorClientMixin, raise_if_exception, OrchestratorError from .. import mgr, logger -class NoOrchestratorConfiguredException(Exception): - pass - - -class OrchClient(object): - _instance = None - - @classmethod - def instance(cls): - if cls._instance is None: - cls._instance = OrchClient() - return cls._instance - - def _call(self, method, *args, **kwargs): - _backend = mgr.get_module_option_ex("orchestrator_cli", "orchestrator") - if not _backend: - raise NoOrchestratorConfiguredException() - return mgr.remote(_backend, method, *args, **kwargs) - - def _wait(self, completions): - while not self._call("wait", completions): - if any(c.should_wait for c in completions): - time.sleep(5) - else: - break +# pylint: disable=abstract-method +class OrchClient(OrchestratorClientMixin): + def __init__(self): + super(OrchClient, self).__init__() + self.set_mgr(mgr) def list_service_info(self, service_type): - completion = self._call("describe_service", service_type, None, None) - self._wait([completion]) + # type: (str) -> list + completion = self.describe_service(service_type, None, None) + self._orchestrator_wait([completion]) + raise_if_exception(completion) return completion.result def available(self): - _backend = mgr.get_module_option_ex("orchestrator_cli", "orchestrator") - if not _backend: + try: + status, desc = super(OrchClient, self).available() + logger.info("[ORCH] is orchestrator available: %s, %s", status, desc) + return status + except (RuntimeError, OrchestratorError, ImportError): return False - status, desc = self._call("available") - logger.info("[ORCH] is orchestrator available: %s, %s", status, desc) - return status def reload_service(self, service_type, service_ids): if not isinstance(service_ids, list): service_ids = [service_ids] - completion_list = [self._call("service_action", 'reload', service_type, - service_name, service_id) + completion_list = [self.service_action('reload', service_type, + service_name, service_id) for service_name, service_id in service_ids] - self._wait(completion_list) + self._orchestrator_wait(completion_list) + for c in completion_list: + raise_if_exception(c) diff --git a/src/pybind/mgr/dashboard/tests/test_ganesha.py b/src/pybind/mgr/dashboard/tests/test_ganesha.py index aacbfcf5c2aef..5dced126424b6 100644 --- a/src/pybind/mgr/dashboard/tests/test_ganesha.py +++ b/src/pybind/mgr/dashboard/tests/test_ganesha.py @@ -5,6 +5,7 @@ import unittest from mock import MagicMock, Mock +import orchestrator from . import KVStoreMockMixin from .. import mgr from ..settings import Settings @@ -127,7 +128,9 @@ EXPORT mgr.rados = MagicMock() mgr.rados.open_ioctx.return_value = ioctx_mock - mgr.remote.return_value = False, None + + # pylint: disable=protected-access + mgr._select_orchestrator.side_effect = orchestrator.NoOrchestrator() ganesha.CephX = MagicMock() ganesha.CephX.list_clients.return_value = ['ganesha'] diff --git a/src/pybind/mgr/dashboard/tests/test_iscsi.py b/src/pybind/mgr/dashboard/tests/test_iscsi.py index 949bf629b50bf..e371ef368622a 100644 --- a/src/pybind/mgr/dashboard/tests/test_iscsi.py +++ b/src/pybind/mgr/dashboard/tests/test_iscsi.py @@ -17,7 +17,7 @@ class IscsiTest(ControllerTestCase, CLICommandTestMixin): @classmethod def setup_server(cls): - OrchClient.instance().available = lambda: False + OrchClient().available = lambda: False mgr.rados.side_effect = None # pylint: disable=protected-access Iscsi._cp_config['tools.authenticate.on'] = False diff --git a/src/pybind/mgr/orchestrator.py b/src/pybind/mgr/orchestrator.py index fc27cd8eb8f43..9e5d2edd48b41 100644 --- a/src/pybind/mgr/orchestrator.py +++ b/src/pybind/mgr/orchestrator.py @@ -11,6 +11,7 @@ import uuid import six +from mgr_module import MgrModule from mgr_util import format_bytes try: @@ -230,7 +231,7 @@ class Orchestrator(object): return True def available(self): - # type: () -> Tuple[Optional[bool], Optional[str]] + # type: () -> Tuple[bool, str] """ Report whether we can talk to the orchestrator. This is the place to give the user a meaningful message if the orchestrator @@ -242,13 +243,16 @@ class Orchestrator(object): (e.g. based on a periodic background ping of the orchestrator) if that's necessary to make this method fast. - Do not override this method if you don't have a meaningful - status to return: the default None, None return value is used - to indicate that a module is unable to indicate its availability. + ..note:: `True` doesn't mean that the desired functionality + is actually available in the orchestrator. I.e. this + won't work as expected:: + + >>> if OrchestratorClientMixin().available()[0]: # wrong. + ... OrchestratorClientMixin().get_hosts() :return: two-tuple of boolean, string """ - return None, None + raise NotImplementedError() def wait(self, completions): """ @@ -305,7 +309,7 @@ class Orchestrator(object): raise NotImplementedError() def describe_service(self, service_type=None, service_id=None, node_name=None): - # type: (str, str, str) -> ReadCompletion[List[ServiceDescription]] + # type: (Optional[str], Optional[str], Optional[str]) -> ReadCompletion[List[ServiceDescription]] """ Describe a service (of any kind) that is already configured in the orchestrator. For example, when viewing an OSD in the dashboard @@ -643,7 +647,7 @@ class DriveGroupSpec(object): def __init__(self, host_pattern, data_devices=None, db_devices=None, wal_devices=None, journal_devices=None, data_directories=None, osds_per_device=None, objectstore='bluestore', encrypted=False, db_slots=None, wal_slots=None): - # type: (str, Optional[DeviceSelection], Optional[DeviceSelection], Optional[DeviceSelection], Optional[DeviceSelection], Optional[List[str]], int, str, bool, int, int) -> () + # type: (str, Optional[DeviceSelection], Optional[DeviceSelection], Optional[DeviceSelection], Optional[DeviceSelection], Optional[List[str]], int, str, bool, int, int) -> None # concept of applying a drive group to a (set) of hosts is tightly # linked to the drive group itself @@ -905,24 +909,37 @@ class OrchestratorClientMixin(Orchestrator): ... self.log.debug(completion.result) """ + + def set_mgr(self, mgr): + # type: (MgrModule) -> None + """ + Useable in the Dashbord that uses a global ``mgr`` + """ + + self.__mgr = mgr # Make sure we're not overwriting any other `mgr` properties + def _oremote(self, meth, args, kwargs): """ Helper for invoking `remote` on whichever orchestrator is enabled :raises RuntimeError: If the remote method failed. - :raises NoOrchestrator: + :raises OrchestratorError: orchestrator failed to perform :raises ImportError: no `orchestrator_cli` module or backend not found. """ try: - o = self._select_orchestrator() + mgr = self.__mgr + except AttributeError: + mgr = self + try: + o = mgr._select_orchestrator() except AttributeError: - o = self.remote('orchestrator_cli', '_select_orchestrator') + o = mgr.remote('orchestrator_cli', '_select_orchestrator') if o is None: raise NoOrchestrator() - self.log.debug("_oremote {} -> {}.{}(*{}, **{})".format(self.module_name, o, meth, args, kwargs)) - return self.remote(o, meth, *args, **kwargs) + mgr.log.debug("_oremote {} -> {}.{}(*{}, **{})".format(mgr.module_name, o, meth, args, kwargs)) + return mgr.remote(o, meth, *args, **kwargs) def _update_completion_progress(self, completion, force_progress=None): # type: (WriteCompletion, Optional[float]) -> None -- 2.39.5