Merge pull request #32829 from Devp00l/wip-43765

Reviewed-by: Alfonso Martínez <almartin@redhat.com>
Reviewed-by: Tatjana Dehler <tdehler@suse.com>
Reviewed-by: Volker Theile <vtheile@suse.com>

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):
         """

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)
 

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'), {})

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])