From 418a5668a4c947326f28cf13e962e914ebadc012 Mon Sep 17 00:00:00 2001
From: Julia Kreger <juliaashleykreger@gmail.com>
Date: Fri, 29 Mar 2019 10:34:40 -0700
Subject: [PATCH] ipmi: Ignore sensor debug data

When the conductor has debugging enabled, that command is
passed to ipmitool to enable debugging of other commands.

However, this means tons of extra data is dumped as part of
the sensor data collection for ironic, which breaks string
parsing and ultimately metrics collection.

Since we can identify these lines, lets ignore them.

Change-Id: Ife77707210f8289d8f2e0223fb9ee1909d798546
Story: 2005332
Task: 30267
---
 ironic/drivers/modules/ipmitool.py            |  3 +
 .../unit/drivers/modules/test_ipmitool.py     | 74 +++++++++++++++++++
 ...ata-fix-for-ipmitool-eb13e80ccdd984db.yaml | 10 +++
 3 files changed, 87 insertions(+)
 create mode 100644 releasenotes/notes/debug-sensor-data-fix-for-ipmitool-eb13e80ccdd984db.yaml

diff --git a/ironic/drivers/modules/ipmitool.py b/ironic/drivers/modules/ipmitool.py
index 15a66af4e9..bb37b44c85 100644
--- a/ironic/drivers/modules/ipmitool.py
+++ b/ironic/drivers/modules/ipmitool.py
@@ -660,6 +660,9 @@ def _process_sensor(sensor_data):
     for field in sensor_data_fields:
         if not field:
             continue
+        if field.startswith('<<'):
+            # This is debug data, and can be safely ignored for this.
+            continue
         kv_value = field.split(':')
         if len(kv_value) != 2:
             continue
diff --git a/ironic/tests/unit/drivers/modules/test_ipmitool.py b/ironic/tests/unit/drivers/modules/test_ipmitool.py
index 27cfdff987..8aa3c3a2c1 100644
--- a/ironic/tests/unit/drivers/modules/test_ipmitool.py
+++ b/ironic/tests/unit/drivers/modules/test_ipmitool.py
@@ -2404,6 +2404,80 @@ class IPMIToolDriverTestCase(Base):
 
         self.assertEqual(expected_return, ret)
 
+    def test__parse_ipmi_sensor_data_debug(self):
+        fake_sensors_data = """
+<<  Message tag                        : 0x00
+<<  RMCP+ status                       : no errors
+<<  Maximum privilege level            : admin
+<<  Console Session ID                 : 0xa0a2a3a4
+<<  BMC Session ID                     : 0x02006a01
+<<  Negotiated authenticatin algorithm : hmac_sha1
+<<  Negotiated integrity algorithm     : hmac_sha1_96
+<<  Negotiated encryption algorithm    : aes_cbc_128
+
+
+                            Sensor ID              : Temp (0x2)
+                             Entity ID             : 3.2 (Processor)
+                             Sensor Type (Analog)  : Temperature
+                             Sensor Reading        : 50 (+/- 1) degrees C
+                             Status                : ok
+                             Nominal Reading       : 50.000
+                             Normal Minimum        : 11.000
+                             Normal Maximum        : 69.000
+                             Upper critical        : 90.000
+                             Upper non-critical    : 85.000
+                             Positive Hysteresis   : 1.000
+                             Negative Hysteresis   : 1.000
+
+                            Sensor ID              : FAN MOD 1A RPM (0x30)
+                             Entity ID             : 7.1 (System Board)
+                             Sensor Type (Analog)  : Fan
+                             Sensor Reading        : 8400 (+/- 75) RPM
+                             Status                : ok
+                             Nominal Reading       : 5325.000
+                             Normal Minimum        : 10425.000
+                             Normal Maximum        : 14775.000
+                             Lower critical        : 4275.000
+                             Positive Hysteresis   : 375.000
+                             Negative Hysteresis   : 375.000
+                             """
+        expected_return = {
+            'Fan': {
+                'FAN MOD 1A RPM (0x30)': {
+                    'Status': 'ok',
+                    'Sensor Reading': '8400 (+/- 75) RPM',
+                    'Entity ID': '7.1 (System Board)',
+                    'Normal Minimum': '10425.000',
+                    'Positive Hysteresis': '375.000',
+                    'Normal Maximum': '14775.000',
+                    'Sensor Type (Analog)': 'Fan',
+                    'Lower critical': '4275.000',
+                    'Negative Hysteresis': '375.000',
+                    'Sensor ID': 'FAN MOD 1A RPM (0x30)',
+                    'Nominal Reading': '5325.000'
+                }
+            },
+            'Temperature': {
+                'Temp (0x2)': {
+                    'Status': 'ok',
+                    'Sensor Reading': '50 (+/- 1) degrees C',
+                    'Entity ID': '3.2 (Processor)',
+                    'Normal Minimum': '11.000',
+                    'Positive Hysteresis': '1.000',
+                    'Upper non-critical': '85.000',
+                    'Normal Maximum': '69.000',
+                    'Sensor Type (Analog)': 'Temperature',
+                    'Negative Hysteresis': '1.000',
+                    'Upper critical': '90.000',
+                    'Sensor ID': 'Temp (0x2)',
+                    'Nominal Reading': '50.000'
+                }
+            }
+        }
+        ret = ipmi._parse_ipmi_sensors_data(self.node, fake_sensors_data)
+
+        self.assertEqual(expected_return, ret)
+
     def test__parse_ipmi_sensor_data_failed(self):
         fake_sensors_data = "abcdef"
         self.assertRaises(exception.FailedToParseSensorData,
diff --git a/releasenotes/notes/debug-sensor-data-fix-for-ipmitool-eb13e80ccdd984db.yaml b/releasenotes/notes/debug-sensor-data-fix-for-ipmitool-eb13e80ccdd984db.yaml
new file mode 100644
index 0000000000..a12707c185
--- /dev/null
+++ b/releasenotes/notes/debug-sensor-data-fix-for-ipmitool-eb13e80ccdd984db.yaml
@@ -0,0 +1,10 @@
+---
+fixes:
+  - |
+    Fixes an issue where the sensor data parsing method for the ``ipmitool``
+    interface lacked the ability to handle the automatically included
+    `ipmitool` debugging information when the ``debug`` option is set to
+    ``True`` in the ironic.conf file. As such, extra debugging information
+    supplied by the underlying ``ipmitool`` command is disregarded.
+    More information can be found in
+    `story 2005331 <https://storyboard.openstack.org/#!/story/2005332>`_.