mgr/dashboard: Prevent dashboard breakdown on bad pool selection

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 <smueller@suse.com>

diff --git a/src/pybind/mgr/dashboard/services/ceph_service.py b/src/pybind/mgr/dashboard/services/ceph_service.py
index e0d583d8c01e78f90f8b40a16c00f7ba14de3671..5e4a5c15342d13bb5727c8a186616f5b3b250740 100644 (file)
--- a/src/pybind/mgr/dashboard/services/ceph_service.py
+++ b/src/pybind/mgr/dashboard/services/ceph_service.py
@@ -12,7 +12,7 @@ from .. import mgr
 from ..exceptions import DashboardException
 
 try:
-    from typing import Dict  # pylint: disable=unused-import
+    from typing import Dict, Any, Union  # pylint: disable=unused-import
 except ImportError:
     pass  # For typing only
 
@@ -138,12 +138,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 d4a38f22b290fdc6d3a6de0c21a2193b79de0c0c..1734ca3bac7ac1b18569807a14da1c8337a15d00 100644 (file)
--- a/src/pybind/mgr/dashboard/services/rbd.py
+++ b/src/pybind/mgr/dashboard/services/rbd.py
@@ -126,6 +126,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 'incomplete' 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 (file)
index 0000000..baf8588
--- /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
index d1a02ca7619e73379420b1a2cbf20954e6492c23..caee4029180f52f5e53fa8c9994cbee719d2a232 100644 (file)
--- a/src/pybind/mgr/dashboard/tests/test_rbd_service.py
+++ b/src/pybind/mgr/dashboard/tests/test_rbd_service.py
@@ -3,8 +3,12 @@
 from __future__ import absolute_import
 
 import unittest
+try:
+    import mock
+except ImportError:
+    import unittest.mock as mock
 
-from ..services.rbd import get_image_spec, parse_image_spec
+from ..services.rbd import get_image_spec, parse_image_spec, RbdConfiguration
 
 
 class RbdServiceTest(unittest.TestCase):
@@ -16,3 +20,26 @@ class RbdServiceTest(unittest.TestCase):
     def test_parse_image_spec(self):
         self.assertEqual(parse_image_spec('mypool/myns/myimage'), ('mypool', 'myns', 'myimage'))
         self.assertEqual(parse_image_spec('mypool/myimage'), ('mypool', None, 'myimage'))
+
+    @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': {'creating+incomplete': 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])