diff --git a/etc/ironic/ironic.conf.sample b/etc/ironic/ironic.conf.sample index ce5dc10200..753e394aa9 100644 --- a/etc/ironic/ironic.conf.sample +++ b/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] diff --git a/ironic/conf/snmp.py b/ironic/conf/snmp.py index 1dd84181b6..1a7026e2cc 100644 --- a/ironic/conf/snmp.py +++ b/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.')), ] diff --git a/ironic/drivers/modules/snmp.py b/ironic/drivers/modules/snmp.py index 5b9ffb6514..1e2f2a73f8 100644 --- a/ironic/drivers/modules/snmp.py +++ b/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. diff --git a/ironic/tests/unit/drivers/modules/test_snmp.py b/ironic/tests/unit/drivers/modules/test_snmp.py index ca2da7ea50..43f80e7f4e 100644 --- a/ironic/tests/unit/drivers/modules/test_snmp.py +++ b/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) diff --git a/releasenotes/notes/snmp-driver-udp-transport-settings-67419be988fcff40.yaml b/releasenotes/notes/snmp-driver-udp-transport-settings-67419be988fcff40.yaml new file mode 100644 index 0000000000..eae9c8271f --- /dev/null +++ b/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.