From 3e1fe5f7e45c66f606b179b4c7e57a5d2bbc6e75 Mon Sep 17 00:00:00 2001
From: digambar <digambarpatil15@yahoo.co.in>
Date: Tue, 30 Oct 2018 11:57:38 -0500
Subject: [PATCH] Fix CPU count returned by introspection in Ironic iDRAC
 driver

If list_cpus in dracclient, it returns below values -

[CPU(id='CPU.Socket.1', cores=14, ht__enabled=True),
CPU(id='CPU.Socket.2', cores=14, ht_enabled=True)]

Each CPU socket has cores with hyperthreading enabled, so if we
calculate the cpu per socket, we will get actual cpu count per socket,
So objective of this patch to fetch actual CPU's per socket and sum it

Story: #2004155
Change-Id: Ia86b0462a3e73af7a2ce0a6439286f7071f74caa
---
 ironic/drivers/modules/drac/inspect.py        | 15 +++++++++++-
 .../unit/drivers/modules/drac/test_inspect.py | 24 +++++++++++++++----
 .../notes/fix-cpu-count-8904a4e1a24456f4.yaml |  7 ++++++
 3 files changed, 41 insertions(+), 5 deletions(-)
 create mode 100644 releasenotes/notes/fix-cpu-count-8904a4e1a24456f4.yaml

diff --git a/ironic/drivers/modules/drac/inspect.py b/ironic/drivers/modules/drac/inspect.py
index e6029a242a..bdccfd0188 100644
--- a/ironic/drivers/modules/drac/inspect.py
+++ b/ironic/drivers/modules/drac/inspect.py
@@ -80,7 +80,8 @@ class DracInspect(base.InspectInterface):
                 [memory.size_mb for memory in client.list_memory()])
             cpus = client.list_cpus()
             if cpus:
-                properties['cpus'] = len(cpus)
+                properties['cpus'] = sum(
+                    [self._calculate_cpus(cpu) for cpu in cpus])
                 properties['cpu_arch'] = 'x86_64' if cpus[0].arch64 else 'x86'
 
             virtual_disks = client.list_virtual_disks()
@@ -148,3 +149,15 @@ class DracInspect(base.InspectInterface):
         for disk in disks:
             if disk.size_mb >= min_size_required_mb:
                 return disk
+
+    def _calculate_cpus(self, cpu):
+        """Find actual CPU count.
+
+        :param cpu: Pass cpu.
+
+        :returns: returns total cpu count.
+        """
+        if cpu.ht_enabled:
+            return cpu.cores * 2
+        else:
+            return cpu.cores
diff --git a/ironic/tests/unit/drivers/modules/drac/test_inspect.py b/ironic/tests/unit/drivers/modules/drac/test_inspect.py
index 2c6b259360..1d650e29fe 100644
--- a/ironic/tests/unit/drivers/modules/drac/test_inspect.py
+++ b/ironic/tests/unit/drivers/modules/drac/test_inspect.py
@@ -63,7 +63,7 @@ class DracInspectionTestCase(test_utils.BaseDracTest):
                  'speed': 2400,
                  'model': 'Intel(R) Xeon(R) CPU E5-2620 v3 @ 2.40GHz',
                  'state': 'ok',
-                 'ht_enabled': True,
+                 'ht_enabled': False,
                  'turbo_enabled': True,
                  'vt_enabled': True,
                  'arch64': True}]
@@ -141,7 +141,7 @@ class DracInspectionTestCase(test_utils.BaseDracTest):
         expected_node_properties = {
             'memory_mb': 32768,
             'local_gb': 1116,
-            'cpus': 2,
+            'cpus': 18,
             'cpu_arch': 'x86_64'}
         mock_client = mock.Mock()
         mock_get_drac_client.return_value = mock_client
@@ -184,7 +184,7 @@ class DracInspectionTestCase(test_utils.BaseDracTest):
         expected_node_properties = {
             'memory_mb': 32768,
             'local_gb': 279,
-            'cpus': 2,
+            'cpus': 18,
             'cpu_arch': 'x86_64'}
         mock_client = mock.Mock()
         mock_get_drac_client.return_value = mock_client
@@ -229,7 +229,7 @@ class DracInspectionTestCase(test_utils.BaseDracTest):
         expected_node_properties = {
             'memory_mb': 32768,
             'local_gb': 1116,
-            'cpus': 2,
+            'cpus': 18,
             'cpu_arch': 'x86_64'}
         mock_client = mock.Mock()
         mock_get_drac_client.return_value = mock_client
@@ -255,3 +255,19 @@ class DracInspectionTestCase(test_utils.BaseDracTest):
                 self.physical_disks)
 
             self.assertEqual(285888, root_disk.size_mb)
+
+    def test__calculate_cpus(self):
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=True) as task:
+            cpu = task.driver.inspect._calculate_cpus(
+                self.cpus[0])
+
+            self.assertEqual(12, cpu)
+
+    def test__calculate_cpus_without_ht_enabled(self):
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=True) as task:
+            cpu = task.driver.inspect._calculate_cpus(
+                self.cpus[1])
+
+            self.assertEqual(6, cpu)
diff --git a/releasenotes/notes/fix-cpu-count-8904a4e1a24456f4.yaml b/releasenotes/notes/fix-cpu-count-8904a4e1a24456f4.yaml
new file mode 100644
index 0000000000..e609f2ab54
--- /dev/null
+++ b/releasenotes/notes/fix-cpu-count-8904a4e1a24456f4.yaml
@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Fixes a bug where the number of CPU sockets was being returned by the
+    ``idrac`` hardware type during introspection, instead of the number of
+    virtual CPUs. See bug `2004155
+    <https://storyboard.openstack.org/#!/story/2004155>`_ for details.