mgr/dashboard: fix exception in get_smart_data_by_daemon

- Fix string substitution exception.
- Add more test coverage.

Fixes: https://tracker.ceph.com/issues/48245
Signed-off-by: Kiefer Chang <kiefer.chang@suse.com>
(cherry picked from commit d2ddea5a79a54086c4ee104aea4f59f10d174735)

 Conflicts:
	src/pybind/mgr/dashboard/requirements-lint.txt
	src/pybind/mgr/dashboard/tests/test_ceph_service.py
 - Added pytest; fixed and re-arranged imports; removed isort
   since it breaks make check in octopus

diff --git a/src/pybind/mgr/dashboard/requirements-lint.txt b/src/pybind/mgr/dashboard/requirements-lint.txt
index 8f68ad3856ecc46aaa43d7d6e649a6fedd9b9a13..54e187dd20cbdd9b4885b91eb0d266bb4c5175e3 100644 (file)
--- a/src/pybind/mgr/dashboard/requirements-lint.txt
+++ b/src/pybind/mgr/dashboard/requirements-lint.txt
@@ -9,3 +9,4 @@ flake8-colors==0.1.6; python_version >= '3'
 rstcheck==3.3.1; python_version >= '3'
 autopep8; python_version >= '3'
 pyfakefs; python_version >= '3'
+pytest<4

diff --git a/src/pybind/mgr/dashboard/services/ceph_service.py b/src/pybind/mgr/dashboard/services/ceph_service.py
index ea918b6b3f953683437d95622811d87c0df2f338..ea794a4474eff1d9234e2d3724ef4d1330077b10 100644 (file)
--- a/src/pybind/mgr/dashboard/services/ceph_service.py
+++ b/src/pybind/mgr/dashboard/services/ceph_service.py
@@ -303,7 +303,7 @@ class CephService(object):
                         CephService._get_smart_data_by_device(device))
         else:
             msg = '[SMART] could not retrieve device list from daemon with type %s and ' +\
-                'with ID %d'
+                'with ID %s'
             logger.debug(msg, daemon_type, daemon_id)
         return smart_data
 

diff --git a/src/pybind/mgr/dashboard/tests/test_ceph_service.py b/src/pybind/mgr/dashboard/tests/test_ceph_service.py
index 5111e68d492bd893e1710b21ae06323b698a7258..3b2aa8b1332343fbf9e149c9cea58476065f91db 100644 (file)
--- a/src/pybind/mgr/dashboard/tests/test_ceph_service.py
+++ b/src/pybind/mgr/dashboard/tests/test_ceph_service.py
@@ -2,11 +2,12 @@
 # pylint: disable=dangerous-default-value,too-many-public-methods
 from __future__ import absolute_import
 
+import logging
 import unittest
-try:
-    import mock
-except ImportError:
-    import unittest.mock as mock
+from unittest import mock
+from contextlib import contextmanager
+
+import pytest
 
 from ..services.ceph_service import CephService
 
@@ -65,3 +66,44 @@ class CephServiceTest(unittest.TestCase):
 
     def test_get_pg_status_without_match(self):
         self.assertEqual(self.service.get_pool_pg_status('no-pool'), {})
+
+
+@contextmanager
+def mock_smart_data(data):
+    devices = [{'devid': devid} for devid in data]
+
+    def _get_smart_data(d):
+        return {d['devid']: data[d['devid']]}
+
+    with mock.patch.object(CephService, '_get_smart_data_by_device', side_effect=_get_smart_data), \
+            mock.patch.object(CephService, 'get_devices_by_host', return_value=devices), \
+            mock.patch.object(CephService, 'get_devices_by_daemon', return_value=devices):
+        yield
+
+
+@pytest.mark.parametrize(
+    "by,args,log",
+    [
+        ('host', ('osd0',), 'from host osd0'),
+        ('daemon', ('osd', '1'), 'with ID 1')
+    ]
+)
+def test_get_smart_data(caplog, by, args, log):
+    # pylint: disable=protected-access
+    expected_data = {
+        'aaa': {'device': {'name': '/dev/sda'}},
+        'bbb': {'device': {'name': '/dev/sdb'}},
+    }
+    with mock_smart_data(expected_data):
+        smart_data = getattr(CephService, 'get_smart_data_by_{}'.format(by))(*args)
+        getattr(CephService, 'get_devices_by_{}'.format(by)).assert_called_with(*args)
+        CephService._get_smart_data_by_device.assert_called()
+        assert smart_data == expected_data
+
+    with caplog.at_level(logging.DEBUG):
+        with mock_smart_data([]):
+            smart_data = getattr(CephService, 'get_smart_data_by_{}'.format(by))(*args)
+            getattr(CephService, 'get_devices_by_{}'.format(by)).assert_called_with(*args)
+            CephService._get_smart_data_by_device.assert_not_called()
+            assert smart_data == {}
+            assert log in caplog.text