From 7348c156a8032c09e2046aa23d95fcf996b39db3 Mon Sep 17 00:00:00 2001
From: Danek Duvall <danek.duvall@oracle.com>
Date: Tue, 9 Aug 2016 14:15:16 -0700
Subject: [PATCH] ValueError exception when SNMP returns NoSuchObject

When an SNMP server doesn't implement a particular variable, pysnmp gives
us back a NoSuchObject object.  We can check for that when we fail to
convert the value to the type we're expecting it to be and safely return
None instead.

Change-Id: Ideb7ab68a0d3c6f0d133fafe020309c19cbdd7c7
Closes-Bug: #1611515
---
 ceilometer/hardware/inspector/snmp.py         | 23 +++++++++++----
 .../unit/hardware/inspector/test_snmp.py      | 29 +++++++++++++++++--
 2 files changed, 44 insertions(+), 8 deletions(-)

diff --git a/ceilometer/hardware/inspector/snmp.py b/ceilometer/hardware/inspector/snmp.py
index 69d1a2fdb2..c891010f8d 100644
--- a/ceilometer/hardware/inspector/snmp.py
+++ b/ceilometer/hardware/inspector/snmp.py
@@ -17,13 +17,18 @@
 
 import copy
 
+from oslo_log import log
 from pysnmp.entity.rfc3413.oneliner import cmdgen
+from pysnmp.proto import rfc1905
 import six
 import six.moves.urllib.parse as urlparse
 
 from ceilometer.hardware.inspector import base
 
 
+LOG = log.getLogger(__name__)
+
+
 class SNMPException(Exception):
     pass
 
@@ -187,18 +192,24 @@ class SNMPInspector(base.Inspector):
         return matched
 
     @staticmethod
-    def get_oid_value(oid_cache, oid_def, suffix=''):
+    def get_oid_value(oid_cache, oid_def, suffix='', host=None):
         oid, converter = oid_def
         value = oid_cache[oid + suffix]
         if converter:
-            value = converter(value)
+            try:
+                value = converter(value)
+            except ValueError:
+                if isinstance(value, rfc1905.NoSuchObject):
+                    LOG.debug("OID %s%s has no value" % (
+                        oid, " on %s" % host.hostname if host else ""))
+                    return None
         return value
 
     @classmethod
-    def construct_metadata(cls, oid_cache, meta_defs, suffix=''):
+    def construct_metadata(cls, oid_cache, meta_defs, suffix='', host=None):
         metadata = {}
         for key, oid_def in six.iteritems(meta_defs):
-            metadata[key] = cls.get_oid_value(oid_cache, oid_def, suffix)
+            metadata[key] = cls.get_oid_value(oid_cache, oid_def, suffix, host)
         return metadata
 
     @classmethod
@@ -243,11 +254,11 @@ class SNMPInspector(base.Inspector):
             suffix = oid[len(meter_def['metric_oid'][0]):]
             value = self.get_oid_value(oid_cache,
                                        meter_def['metric_oid'],
-                                       suffix)
+                                       suffix, host)
             # get the metadata for this sample value
             metadata = self.construct_metadata(oid_cache,
                                                meter_def['metadata'],
-                                               suffix)
+                                               suffix, host)
             extra_metadata = copy.deepcopy(input_extra_metadata) or {}
             # call post_op for special cases
             if meter_def['post_op']:
diff --git a/ceilometer/tests/unit/hardware/inspector/test_snmp.py b/ceilometer/tests/unit/hardware/inspector/test_snmp.py
index 28e584ecbd..d88640e33d 100644
--- a/ceilometer/tests/unit/hardware/inspector/test_snmp.py
+++ b/ceilometer/tests/unit/hardware/inspector/test_snmp.py
@@ -17,6 +17,7 @@
 import mock
 from oslo_utils import netutils
 from oslotest import mockpatch
+from pysnmp.proto.rfc1905 import noSuchObject
 
 from ceilometer.hardware.inspector import snmp
 from ceilometer.tests import base as test_base
@@ -34,8 +35,14 @@ class FakeObjectName(object):
 
 class FakeCommandGenerator(object):
     def getCmd(self, authData, transportTarget, *oids, **kwargs):
-        varBinds = [(FakeObjectName(oid),
-                    int(oid.split('.')[-1])) for oid in oids]
+        emptyOID = '1.3.6.1.4.1.2021.4.14.0'
+        varBinds = [
+            (FakeObjectName(oid), int(oid.split('.')[-1]))
+            for oid in oids
+            if oid != emptyOID
+        ]
+        if emptyOID in oids:
+            varBinds += [(FakeObjectName(emptyOID), noSuchObject)]
         return (None, None, 0, varBinds)
 
     def bulkCmd(authData, transportTarget, nonRepeaters, maxRepetitions,
@@ -65,6 +72,12 @@ class TestSNMPInspector(test_base.BaseTestCase):
             },
             'post_op': None,
         },
+        'test_nosuch': {
+            'matching_type': snmp.EXACT,
+            'metric_oid': ('1.3.6.1.4.1.2021.4.14.0', int),
+            'metadata': {},
+            'post_op': None,
+        },
     }
 
     def setUp(self):
@@ -99,6 +112,18 @@ class TestSNMPInspector(test_base.BaseTestCase):
         extra.update(project_id=2)
         return value
 
+    def test_inspect_no_such_object(self):
+        cache = {}
+        try:
+            # inspect_generic() is a generator, so we explicitly need to
+            # iterate through it in order to trigger the exception.
+            list(self.inspector.inspect_generic(self.host,
+                                                cache,
+                                                {},
+                                                self.mapping['test_nosuch']))
+        except ValueError:
+            self.fail("got ValueError when interpreting NoSuchObject return")
+
     def test_inspect_generic_exact(self):
         self.inspector._fake_post_op = self._fake_post_op
         cache = {}