Handle poorly formed individual sensor readings

ipmitool returns a series of multiple readings per run. Some of
these readings may be of a form that parser code cannot handle. The
desired behavior in this case is for the single reading to be
dropped and for parsing to continue. This change ensures that
behavior by continuing through the loop instead of allowing
exceptions to cause the loop to exit.

Tests have been added which verify the correct behavior.

Note that though there was an opportunity to log each skipped
reading this would lead to a very large number of log messages as
many or even most samples from ipmitool are not parsed.

Change-Id: I6a3193d5a6e12c69ca5c548e5fb58d8bc9646348
Closes-Bug: #1381600

ceilometer/ipmi/pollsters/sensor.py

@@ -54,19 +54,24 @@ class SensorPollster(plugin.PollsterBase):
         sensor_type_data = self._get_sensor_types(stats, self.METRIC)
 
         for sensor_data in sensor_type_data:
+            # Continue if sensor_data is not parseable.
             try:
                 sensor_reading = sensor_data['Sensor Reading']
+                sensor_id = sensor_data['Sensor ID']
             except KeyError:
-                raise InvalidSensorData("missing 'Sensor Reading'")
+                continue
 
             if not parser.validate_reading(sensor_reading):
                 continue
 
-            volume, unit = parser.parse_reading(sensor_reading)
+            try:
+                volume, unit = parser.parse_reading(sensor_reading)
+            except parser.InvalidSensorData:
+                continue
 
             resource_id = '%(host)s-%(sensor-id)s' % {
                 'host': CONF.host,
-                'sensor-id': parser.transform_id(sensor_data['Sensor ID'])
+                'sensor-id': parser.transform_id(sensor_id)
             }
 
             metadata = {

ceilometer/tests/ipmi/pollsters/base.py

@@ -57,13 +57,15 @@ class TestPollsterBase(base.BaseTestCase):
 
         self.pollster = self.make_pollster()
 
-    def _verify_metering(self, length, expected_vol, node):
+    def _verify_metering(self, length, expected_vol=None, node=None):
         cache = {}
         resources = {}
 
         samples = list(self.pollster.get_samples(self.mgr, cache, resources))
         self.assertEqual(length, len(samples))
 
-        self.assertTrue(any(s.volume == expected_vol for s in samples))
+        if expected_vol:
-        self.assertTrue(any(s.resource_metadata['node'] == node
+            self.assertTrue(any(s.volume == expected_vol for s in samples))
-                            for s in samples))
+        if node:
+            self.assertTrue(any(s.resource_metadata['node'] == node
+                                for s in samples))

ceilometer/tests/ipmi/pollsters/test_sensor.py

@@ -41,6 +41,10 @@ VOLTAGE_SENSOR_DATA = {
     'Voltage': ipmi_test_data.VOLTAGE_DATA
 }
 
+MISSING_SENSOR_DATA = ipmi_test_data.MISSING_SENSOR['payload']['payload']
+MALFORMED_SENSOR_DATA = ipmi_test_data.BAD_SENSOR['payload']['payload']
+MISSING_ID_SENSOR_DATA = ipmi_test_data.NO_SENSOR_ID['payload']['payload']
+
 
 class TestTemperatureSensorPollster(base.TestPollsterBase):
 
@@ -61,6 +65,60 @@ class TestTemperatureSensorPollster(base.TestPollsterBase):
         self._verify_metering(10, float(32), CONF.host)
 
 
+class TestMissingSensorData(base.TestPollsterBase):
+
+    def fake_sensor_data(self, sensor_type):
+        return MISSING_SENSOR_DATA
+
+    def fake_data(self):
+        # No use for Sensor test
+        return None
+
+    def make_pollster(self):
+        return sensor.TemperatureSensorPollster()
+
+    @mock.patch('ceilometer.pipeline.setup_pipeline', mock.MagicMock())
+    def test_get_samples(self):
+        self._test_get_samples()
+        self._verify_metering(0)
+
+
+class TestMalformedSensorData(base.TestPollsterBase):
+
+    def fake_sensor_data(self, sensor_type):
+        return MALFORMED_SENSOR_DATA
+
+    def fake_data(self):
+        # No use for Sensor test
+        return None
+
+    def make_pollster(self):
+        return sensor.TemperatureSensorPollster()
+
+    @mock.patch('ceilometer.pipeline.setup_pipeline', mock.MagicMock())
+    def test_get_samples(self):
+        self._test_get_samples()
+        self._verify_metering(0)
+
+
+class TestMissingSensorId(base.TestPollsterBase):
+
+    def fake_sensor_data(self, sensor_type):
+        return MISSING_ID_SENSOR_DATA
+
+    def fake_data(self):
+        # No use for Sensor test
+        return None
+
+    def make_pollster(self):
+        return sensor.TemperatureSensorPollster()
+
+    @mock.patch('ceilometer.pipeline.setup_pipeline', mock.MagicMock())
+    def test_get_samples(self):
+        self._test_get_samples()
+        self._verify_metering(0)
+
+
 class TestFanSensorPollster(base.TestPollsterBase):
 
     def fake_sensor_data(self, sensor_type):