From 0f6f79f1d9c716e13f662151459fa107f651f158 Mon Sep 17 00:00:00 2001 From: Nizamudeen A Date: Mon, 18 Jul 2022 11:08:28 +0530 Subject: [PATCH] mgr/dashboard: do not recommend throughput for ssd's only cluster This is just a bug fix where we recommend the throughput option even if there are only ssd's are present in the cluster. Fixes: https://tracker.ceph.com/issues/56413 Signed-off-by: Nizamudeen A --- src/pybind/mgr/dashboard/controllers/osd.py | 9 +++-- src/pybind/mgr/dashboard/tests/test_osd.py | 43 +++++++++++++++++++-- 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/src/pybind/mgr/dashboard/controllers/osd.py b/src/pybind/mgr/dashboard/controllers/osd.py index dd3e80261acf6..49f234536ed5f 100644 --- a/src/pybind/mgr/dashboard/controllers/osd.py +++ b/src/pybind/mgr/dashboard/controllers/osd.py @@ -509,16 +509,17 @@ class OsdUi(Osd): if device.available: if device.human_readable_type == 'hdd': hdds += 1 + # SSDs and NVMe are both counted as 'ssd' + # so differentiating nvme using its path + elif '/dev/nvme' in device.path: + nvmes += 1 else: ssds += 1 - # we still don't know how to infer nvmes - # Tracker: https://tracker.ceph.com/issues/55728 - nvmes += 1 if hdds: res.options[OsdDeploymentOptions.COST_CAPACITY].available = True res.recommended_option = OsdDeploymentOptions.COST_CAPACITY - if ssds: + if hdds and ssds: res.options[OsdDeploymentOptions.THROUGHPUT].available = True res.recommended_option = OsdDeploymentOptions.THROUGHPUT if nvmes: diff --git a/src/pybind/mgr/dashboard/tests/test_osd.py b/src/pybind/mgr/dashboard/tests/test_osd.py index 9f83fa74b37cb..144a98e49af46 100644 --- a/src/pybind/mgr/dashboard/tests/test_osd.py +++ b/src/pybind/mgr/dashboard/tests/test_osd.py @@ -431,8 +431,8 @@ class OsdTest(ControllerTestCase): res = self._get_deployment_options(fake_client, devices_data) self.assertFalse(res['options'][OsdDeploymentOptions.COST_CAPACITY]['available']) - self.assertTrue(res['options'][OsdDeploymentOptions.THROUGHPUT]['available']) - self.assertEqual(res['recommended_option'], 'throughput_optimized') + self.assertFalse(res['options'][OsdDeploymentOptions.THROUGHPUT]['available']) + self.assertEqual(res['recommended_option'], None) @mock.patch('dashboard.controllers.orchestrator.OrchClient.instance') def test_deployment_options_throughput(self, instance): @@ -451,5 +451,42 @@ class OsdTest(ControllerTestCase): res = self._get_deployment_options(fake_client, devices_data) self.assertTrue(res['options'][OsdDeploymentOptions.COST_CAPACITY]['available']) self.assertTrue(res['options'][OsdDeploymentOptions.THROUGHPUT]['available']) - self.assertTrue(res['options'][OsdDeploymentOptions.IOPS]['available']) + self.assertFalse(res['options'][OsdDeploymentOptions.IOPS]['available']) assert res['recommended_option'] == OsdDeploymentOptions.THROUGHPUT + + @mock.patch('dashboard.controllers.orchestrator.OrchClient.instance') + def test_deployment_options_with_hdds_and_nvmes(self, instance): + fake_client = mock.Mock() + instance.return_value = fake_client + fake_client.get_missing_features.return_value = [] + + devices_data = [ + {'type': 'ssd', 'path': '/dev/nvme01', 'host': 'host1'}, + {'type': 'ssd', 'path': '/dev/nvme02', 'host': 'host1'}, + {'type': 'ssd', 'path': '/dev/nvme03', 'host': 'host2'}, + {'type': 'hdd', 'path': '/dev/sde', 'host': 'host1'}, + {'type': 'hdd', 'path': '/dev/sdd', 'host': 'host2'}, + ] + + res = self._get_deployment_options(fake_client, devices_data) + self.assertTrue(res['options'][OsdDeploymentOptions.COST_CAPACITY]['available']) + self.assertFalse(res['options'][OsdDeploymentOptions.THROUGHPUT]['available']) + self.assertTrue(res['options'][OsdDeploymentOptions.IOPS]['available']) + assert res['recommended_option'] == OsdDeploymentOptions.COST_CAPACITY + + @mock.patch('dashboard.controllers.orchestrator.OrchClient.instance') + def test_deployment_options_iops(self, instance): + fake_client = mock.Mock() + instance.return_value = fake_client + fake_client.get_missing_features.return_value = [] + + devices_data = [ + {'type': 'ssd', 'path': '/dev/nvme01', 'host': 'host1'}, + {'type': 'ssd', 'path': '/dev/nvme02', 'host': 'host1'}, + {'type': 'ssd', 'path': '/dev/nvme03', 'host': 'host2'} + ] + + res = self._get_deployment_options(fake_client, devices_data) + self.assertFalse(res['options'][OsdDeploymentOptions.COST_CAPACITY]['available']) + self.assertFalse(res['options'][OsdDeploymentOptions.THROUGHPUT]['available']) + self.assertTrue(res['options'][OsdDeploymentOptions.IOPS]['available']) -- 2.39.5