From 3ab779c8976116db3b9c6edc5ea8227a19e41f3f Mon Sep 17 00:00:00 2001
From: Ramamani Yeleswarapu <ramamani.yeleswarapu@intel.com>
Date: Thu, 27 Jul 2017 07:29:00 -0700
Subject: [PATCH] Collect processor, memory and BIOS output of dmidecode -
 follow-up

Fixes nits, modifies unit tests.

This is a follow-up to commit c5544fb7a0d9d2a1481b3256c3587259368b599e.

Change-Id: Ibca82cc6e32311b1ff0be7137d8392f63e12639b
Closes-Bug: #1635057
---
 ironic_python_agent/dmi_inspector.py          | 46 ++++++++++---------
 .../tests/unit/test_dmi_inspector.py          | 14 ++++++
 2 files changed, 38 insertions(+), 22 deletions(-)

diff --git a/ironic_python_agent/dmi_inspector.py b/ironic_python_agent/dmi_inspector.py
index 8bd85736c..9db036260 100644
--- a/ironic_python_agent/dmi_inspector.py
+++ b/ironic_python_agent/dmi_inspector.py
@@ -40,7 +40,7 @@ def collect_dmidecode_info(data, failures):
     try:
         data['dmi'] = parse_dmi(shret)
     except (ValueError, IndexError) as exc:
-        LOG.warning('Failed to collect dmidecode info %s:', exc)
+        LOG.warning('Failed to collect dmidecode info: %s', exc)
 
 
 def parse_dmi(data):
@@ -69,28 +69,30 @@ def parse_dmi(data):
         if not len(infoblock):
             continue
 
-        if infoblock.startswith('Handle 0x'):
-            try:
-                # Determine DMI type value. Handle line will look like this:
-                # Handle 0x0018, DMI type 17, 27 bytes
-                dmi_type = int(infoblock.split(',', 2)[1].strip()[
-                               len('DMI type'):])
-            except (ValueError, IndexError) as exc:
-                LOG.warning('Failed to parse Handle type in dmi output: %s',
-                            exc)
-                continue
+        if not infoblock.startswith('Handle 0x'):
+            continue
 
-            if dmi_type in TYPE.values():
-                sectiondata = _parse_handle_block(infoblock)
+        try:
+            # Determine DMI type value. Handle line will look like this:
+            # Handle 0x0018, DMI type 17, 27 bytes
+            dmi_type = int(infoblock.split(',', 2)[1].strip()[
+                           len('DMI type'):])
+        except (ValueError, IndexError) as exc:
+            LOG.warning('Failed to parse Handle type in dmi output: %s',
+                        exc)
+            continue
 
-                if dmi_type == TYPE['bios']:
-                    dmi_info['bios'] = sectiondata
-                elif dmi_type == TYPE['cpu']:
-                    dmi_info['cpu'].append(sectiondata)
-                elif dmi_type == TYPE['memory']:
-                    memorydata.append(sectiondata)
-                elif dmi_type == TYPE['devices']:
-                    devicedata.append(sectiondata)
+        if dmi_type in TYPE.values():
+            sectiondata = _parse_handle_block(infoblock)
+
+            if dmi_type == TYPE['bios']:
+                dmi_info['bios'] = sectiondata
+            elif dmi_type == TYPE['cpu']:
+                dmi_info['cpu'].append(sectiondata)
+            elif dmi_type == TYPE['memory']:
+                memorydata.append(sectiondata)
+            elif dmi_type == TYPE['devices']:
+                devicedata.append(sectiondata)
 
     return _save_data(dmi_info, memorydata, devicedata)
 
@@ -124,7 +126,7 @@ def _save_data(dmi_info, memorydata, devicedata):
             dmi_info['memory'] = memorydata[0]
             dmi_info['memory']['Number Of Devices'] = device_count
             dmi_info['memory'].pop('Handle')
-        except (KeyError) as exc:
+        except KeyError as exc:
             LOG.warning('Failed to process memory dmi data: %s', exc)
             raise
 
diff --git a/ironic_python_agent/tests/unit/test_dmi_inspector.py b/ironic_python_agent/tests/unit/test_dmi_inspector.py
index 2a22fba91..0c8c69703 100644
--- a/ironic_python_agent/tests/unit/test_dmi_inspector.py
+++ b/ironic_python_agent/tests/unit/test_dmi_inspector.py
@@ -668,12 +668,26 @@ class TestCollectDmidecodeInfo(base.IronicAgentTest):
         ret = dmi_inspector._save_data(dmi_info, mem, devices)
         self.assertEqual(expected, ret)
 
+    def test_save_data_error_number_of_devices(self):
+        dmi_info = {}
+        dmi_info['bios'] = {}
+        dmi_info['cpu'] = []
+        dmi_info['memory'] = {}
+        dmi_info['memory']['devices'] = {}
+
         self.assertRaises(KeyError,
                           dmi_inspector._save_data,
                           dmi_info,
                           [{'foo': 'bar', 'Handle': '0x10'}],
                           [{'bar': 'foo'}, {'bar': 'foo'}])
 
+    def test_save_data_error_handle(self):
+        dmi_info = {}
+        dmi_info['bios'] = {}
+        dmi_info['cpu'] = []
+        dmi_info['memory'] = {}
+        dmi_info['memory']['devices'] = {}
+
         self.assertRaises(KeyError,
                           dmi_inspector._save_data,
                           dmi_info,