From 8803a7b921efa09748f581e0f59a3a305f1b39fa Mon Sep 17 00:00:00 2001 From: =?utf8?q?Stephan=20M=C3=BCller?= Date: Fri, 24 Jan 2020 16:36:48 +0100 Subject: [PATCH] mgr/dashboard: Prevent dashboard breakdown on bad pool selection MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The problem was that if a pool was created that outsizes the max available OSDs the pool gets stuck with the "creating+incomplete" pg state. If this pool than is selected to get it's details, the method the dashboard is calling to get the pools RBD configuration will get stuck, therefore the dashboard gets stuck. This is the issue to the related bug: https://tracker.ceph.com/issues/43771 ATM this is only a workaround it won't fix the underlying problem, it will just ensure that the dashboard just won't call the method if the pools pg state is in the mentioned state. Fixes: https://tracker.ceph.com/issues/43765 Signed-off-by: Stephan Müller (cherry picked from commit e174b91d6b7670ed575577ddff18edc354be69fb) Conflicts: src/pybind/mgr/dashboard/services/rbd.py - Import for ceph_service was missing - Filters out 'unknown' state instead of 'incomplete' state, as pools in nautilus wait for PGs to be there. src/pybind/mgr/dashboard/services/ceph_service.py - Import conflict src/pybind/mgr/dashboard/tests/test_rbd_service.py - Test file wasn't there before --- .../mgr/dashboard/services/ceph_service.py | 21 +++++- src/pybind/mgr/dashboard/services/rbd.py | 15 +++++ .../mgr/dashboard/tests/test_ceph_service.py | 65 +++++++++++++++++++ .../mgr/dashboard/tests/test_rbd_service.py | 37 +++++++++++ 4 files changed, 135 insertions(+), 3 deletions(-) create mode 100644 src/pybind/mgr/dashboard/tests/test_ceph_service.py create mode 100644 src/pybind/mgr/dashboard/tests/test_rbd_service.py diff --git a/src/pybind/mgr/dashboard/services/ceph_service.py b/src/pybind/mgr/dashboard/services/ceph_service.py index f222eb7cc51..9b50c093a8e 100644 --- a/src/pybind/mgr/dashboard/services/ceph_service.py +++ b/src/pybind/mgr/dashboard/services/ceph_service.py @@ -11,7 +11,7 @@ from mgr_util import get_time_series_rates, get_most_recent_rate from .. import logger, mgr try: - from typing import Dict, Any # pylint: disable=unused-import + from typing import Dict, Any, Union # pylint: disable=unused-import except ImportError: pass # For typing only @@ -117,12 +117,27 @@ class CephService(object): @classmethod def get_pool_name_from_id(cls, pool_id): + # type: (int) -> Union[str, None] + pool = cls.get_pool_by_attribute('pool', pool_id) + return pool['pool_name'] if pool is not None else None + + @classmethod + def get_pool_by_attribute(cls, attribute, value): + # type: (str, Any) -> Union[dict, None] pool_list = cls.get_pool_list() for pool in pool_list: - if pool['pool'] == pool_id: - return pool['pool_name'] + if attribute in pool and pool[attribute] == value: + return pool return None + @classmethod + def get_pool_pg_status(cls, pool_name): + # type: (str) -> dict + pool = cls.get_pool_by_attribute('pool_name', pool_name) + if pool is None: + return {} + return mgr.get("pg_summary")['by_pool'][pool['pool'].__str__()] + @classmethod def send_command(cls, srv_type, prefix, srv_spec='', **kwargs): """ diff --git a/src/pybind/mgr/dashboard/services/rbd.py b/src/pybind/mgr/dashboard/services/rbd.py index ec35a50f814..fbec61049b9 100644 --- a/src/pybind/mgr/dashboard/services/rbd.py +++ b/src/pybind/mgr/dashboard/services/rbd.py @@ -6,6 +6,7 @@ import six import rbd from .. import mgr +from .ceph_service import CephService RBD_FEATURES_NAME_MAPPING = { @@ -85,6 +86,20 @@ class RbdConfiguration(object): except rbd.ImageNotFound: result = [] else: # pool config + pg_status = list(CephService.get_pool_pg_status(self._pool_name).keys()) + if len(pg_status) == 1 and 'unknown' in pg_status[0]: + # If config_list would be called with ioctx if it's a bad pool, + # the dashboard would stop working, waiting for the response + # that would not happen. + # + # This is only a workaround for https://tracker.ceph.com/issues/43771 which + # already got rejected as not worth the effort. + # + # Are more complete workaround for the dashboard will be implemented with + # https://tracker.ceph.com/issues/44224 + # + # @TODO: If #44224 is addressed remove this workaround + return [] result = self._rbd.config_list(ioctx) return list(result) diff --git a/src/pybind/mgr/dashboard/tests/test_ceph_service.py b/src/pybind/mgr/dashboard/tests/test_ceph_service.py new file mode 100644 index 00000000000..baf8588117b --- /dev/null +++ b/src/pybind/mgr/dashboard/tests/test_ceph_service.py @@ -0,0 +1,65 @@ +# -*- coding: utf-8 -*- +# pylint: disable=dangerous-default-value,too-many-public-methods +from __future__ import absolute_import + +import unittest +try: + import mock +except ImportError: + import unittest.mock as mock + +from ..services.ceph_service import CephService + + +class CephServiceTest(unittest.TestCase): + pools = [{ + 'pool_name': 'good_pool', + 'pool': 1, + }, { + 'pool_name': 'bad_pool', + 'pool': 2, + 'flaky': 'option_x' + }] + + def setUp(self): + # Mock get_pool_list + self.list_patch = mock.patch('dashboard.services.ceph_service.CephService.get_pool_list') + self.list = self.list_patch.start() + self.list.return_value = self.pools + # Mock mgr.get + self.mgr_patch = mock.patch('dashboard.mgr.get') + self.mgr = self.mgr_patch.start() + self.mgr.return_value = { + 'by_pool': { + '1': {'active+clean': 16}, + '2': {'creating+incomplete': 16}, + } + } + self.service = CephService() + + def tearDown(self): + self.list_patch.stop() + self.mgr_patch.stop() + + def test_get_pool_by_attribute_with_match(self): + self.assertEqual(self.service.get_pool_by_attribute('pool', 1), self.pools[0]) + self.assertEqual(self.service.get_pool_by_attribute('pool_name', 'bad_pool'), self.pools[1]) + + def test_get_pool_by_attribute_without_a_match(self): + self.assertEqual(self.service.get_pool_by_attribute('pool', 3), None) + self.assertEqual(self.service.get_pool_by_attribute('not_there', 'sth'), None) + + def test_get_pool_by_attribute_matching_a_not_always_set_attribute(self): + self.assertEqual(self.service.get_pool_by_attribute('flaky', 'option_x'), self.pools[1]) + + def test_get_pool_name_from_id_with_match(self): + self.assertEqual(self.service.get_pool_name_from_id(1), 'good_pool') + + def test_get_pool_name_from_id_without_match(self): + self.assertEqual(self.service.get_pool_name_from_id(3), None) + + def test_get_pool_pg_status(self): + self.assertEqual(self.service.get_pool_pg_status('good_pool'), {'active+clean': 16}) + + def test_get_pg_status_without_match(self): + self.assertEqual(self.service.get_pool_pg_status('no-pool'), {}) diff --git a/src/pybind/mgr/dashboard/tests/test_rbd_service.py b/src/pybind/mgr/dashboard/tests/test_rbd_service.py new file mode 100644 index 00000000000..bd0da4020c1 --- /dev/null +++ b/src/pybind/mgr/dashboard/tests/test_rbd_service.py @@ -0,0 +1,37 @@ +# -*- coding: utf-8 -*- +# pylint: disable=dangerous-default-value,too-many-public-methods +from __future__ import absolute_import + +import unittest +try: + import mock +except ImportError: + import unittest.mock as mock + +from ..services.rbd import RbdConfiguration + + +class RbdServiceTest(unittest.TestCase): + + @mock.patch('dashboard.services.rbd.RbdConfiguration._rbd.config_list') + @mock.patch('dashboard.mgr.get') + @mock.patch('dashboard.services.ceph_service.CephService.get_pool_list') + def test_pool_rbd_configuration_with_different_pg_states(self, get_pool_list, get, config_list): + get_pool_list.return_value = [{ + 'pool_name': 'good-pool', + 'pool': 1, + }, { + 'pool_name': 'bad-pool', + 'pool': 2, + }] + get.return_value = { + 'by_pool': { + '1': {'active+clean': 32}, + '2': {'unknown': 32}, + } + } + config_list.return_value = [1, 2, 3] + config = RbdConfiguration('bad-pool') + self.assertEqual(config.list(), []) + config = RbdConfiguration('good-pool') + self.assertEqual(config.list(), [1, 2, 3]) -- 2.47.3