Make SNMP UDP transport settings configurable

SNMP timeout and retries are now configurable through ironic.conf. This
allows to have custom configurations when certain PDUs are slower than
others.

The config defaults are the same defaults that cmdgen.UdpTransportTarget
uses (1.0 second timeout and 5 retries). The config is now reflecting
these defaults.

Closes-Bug: #1640932
Change-Id: I2490902ad2e39e8e4dc34484799a0dae96bc57f8
(cherry picked from commit 80b4424877)

etc/ironic/ironic.conf.sample

@@ -3609,6 +3609,17 @@
 # Minimum value: 0
 #reboot_delay = 0
 
+# Response timeout in seconds used for UDP transport. Timeout
+# should be a multiple of 0.5 seconds and is applicable to
+# each retry. (floating point value)
+# Minimum value: 0
+#udp_transport_timeout = 1.0
+
+# Maximum number of UDP request retries, 0 means just a single
+# request. (integer value)
+# Minimum value: 0
+#udp_transport_retries = 5
+
 
 [ssl]
 

ironic/conf/snmp.py

@@ -29,7 +29,18 @@ opts = [
                default=0,
                min=0,
                help=_('Time (in seconds) to sleep between when rebooting '
-                      '(powering off and on again)'))
+                      '(powering off and on again)')),
+    cfg.FloatOpt('udp_transport_timeout',
+                 default=1.0,
+                 min=0.0,
+                 help=_('Response timeout in seconds used for UDP transport. '
+                        'Timeout should be a multiple of 0.5 seconds and '
+                        'is applicable to each retry.')),
+    cfg.IntOpt('udp_transport_retries',
+               default=5,
+               min=0,
+               help=_('Maximum number of UDP request retries, '
+                      '0 means no retries.')),
 ]
 
 

ironic/drivers/modules/snmp.py

@@ -122,7 +122,10 @@ class SNMPClient(object):
         # The transport target accepts timeout and retries parameters, which
         # default to 1 (second) and 5 respectively. These are deemed sensible
         # enough to allow for an unreliable network or slow device.
-        return cmdgen.UdpTransportTarget((self.address, self.port))
+        return cmdgen.UdpTransportTarget(
+            (self.address, self.port),
+            timeout=CONF.snmp.udp_transport_timeout,
+            retries=CONF.snmp.udp_transport_retries)
 
     def get(self, oid):
         """Use PySNMP to perform an SNMP GET operation on a single object.

ironic/tests/unit/drivers/modules/test_snmp.py

@@ -75,7 +75,10 @@ class SNMPClientTestCase(base.TestCase):
         client = snmp.SNMPClient(self.address, self.port, snmp.SNMP_V3)
         client._get_transport()
         mock_cmdgen.assert_called_once_with()
-        mock_transport.assert_called_once_with((client.address, client.port))
+        mock_transport.assert_called_once_with(
+            (client.address, client.port),
+            retries=CONF.snmp.udp_transport_retries,
+            timeout=CONF.snmp.udp_transport_timeout)
 
     @mock.patch.object(cmdgen, 'UdpTransportTarget', autospec=True)
     def test__get_transport_err(self, mock_transport, mock_cmdgen):
@@ -83,7 +86,28 @@ class SNMPClientTestCase(base.TestCase):
         client = snmp.SNMPClient(self.address, self.port, snmp.SNMP_V3)
         self.assertRaises(snmp_error.PySnmpError, client._get_transport)
         mock_cmdgen.assert_called_once_with()
-        mock_transport.assert_called_once_with((client.address, client.port))
+        mock_transport.assert_called_once_with(
+            (client.address, client.port),
+            retries=CONF.snmp.udp_transport_retries,
+            timeout=CONF.snmp.udp_transport_timeout)
+
+    @mock.patch.object(cmdgen, 'UdpTransportTarget', autospec=True)
+    def test__get_transport_custom_timeout(self, mock_transport, mock_cmdgen):
+        self.config(udp_transport_timeout=2.0, group='snmp')
+        client = snmp.SNMPClient(self.address, self.port, snmp.SNMP_V3)
+        client._get_transport()
+        mock_cmdgen.assert_called_once_with()
+        mock_transport.assert_called_once_with((client.address, client.port),
+                                               retries=5, timeout=2.0)
+
+    @mock.patch.object(cmdgen, 'UdpTransportTarget', autospec=True)
+    def test__get_transport_custom_retries(self, mock_transport, mock_cmdgen):
+        self.config(udp_transport_retries=10, group='snmp')
+        client = snmp.SNMPClient(self.address, self.port, snmp.SNMP_V3)
+        client._get_transport()
+        mock_cmdgen.assert_called_once_with()
+        mock_transport.assert_called_once_with((client.address, client.port),
+                                               retries=10, timeout=1.0)
 
     @mock.patch.object(snmp.SNMPClient, '_get_transport', autospec=True)
     @mock.patch.object(snmp.SNMPClient, '_get_auth', autospec=True)

releasenotes/notes/snmp-driver-udp-transport-settings-67419be988fcff40.yaml

@@ -0,0 +1,7 @@
+---
+features:
+  - Adds SNMP request timeout and retries settings for the SNMP UDP transport.
+    Some SNMP devices take longer than others to respond.
+    The new Ironic configuration settings ``[snmp]/udp_transport_retries``
+    and ``[snmp]/udp_transport_timeout`` allow to change the number of
+    retries and the timeout values respectively for the the SNMP driver.