From d2ddea5a79a54086c4ee104aea4f59f10d174735 Mon Sep 17 00:00:00 2001
From: Kiefer Chang <kiefer.chang@suse.com>
Date: Wed, 18 Nov 2020 19:35:02 +0800
Subject: [PATCH] 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>
---
 .../mgr/dashboard/requirements-lint.txt       |  1 +
 .../mgr/dashboard/services/ceph_service.py    |  2 +-
 .../mgr/dashboard/tests/test_ceph_service.py  | 49 +++++++++++++++++--
 3 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/src/pybind/mgr/dashboard/requirements-lint.txt b/src/pybind/mgr/dashboard/requirements-lint.txt
index 85821253725..87beb5ce5a3 100644
--- a/src/pybind/mgr/dashboard/requirements-lint.txt
+++ b/src/pybind/mgr/dashboard/requirements-lint.txt
@@ -8,3 +8,4 @@ rstcheck==3.3.1; python_version >= '3'
 autopep8; python_version >= '3'
 pyfakefs; python_version >= '3'
 isort==5.5.3
+pytest
diff --git a/src/pybind/mgr/dashboard/services/ceph_service.py b/src/pybind/mgr/dashboard/services/ceph_service.py
index 42d511c1b50..fa97b33ea34 100644
--- 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 6c793631fd9..3433443b170 100644
--- a/src/pybind/mgr/dashboard/tests/test_ceph_service.py
+++ b/src/pybind/mgr/dashboard/tests/test_ceph_service.py
@@ -2,12 +2,12 @@
 # pylint: disable=dangerous-default-value,too-many-public-methods
 from __future__ import absolute_import
 
+import logging
 import unittest
+from contextlib import contextmanager
+from unittest import mock
 
-try:
-    import mock
-except ImportError:
-    import unittest.mock as mock
+import pytest
 
 from ..services.ceph_service import CephService
 
@@ -66,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
-- 
2.39.5