diff --git a/changelog/58108.fixed.md b/changelog/58108.fixed.md new file mode 100644 index 00000000000..021cfc57909 --- /dev/null +++ b/changelog/58108.fixed.md @@ -0,0 +1 @@ +Fixed ``salt '*' napalm.junos_cli`` (and other Junos calls) raising ``TypeError``/``RuntimeError`` when no timeout was requested. ``napalm.junos_cli`` forwards ``dev_timeout=None`` by default, and the Junos ``_timeout_decorator``/``_timeout_decorator_cleankwargs`` wrappers treated that as a real value, so ``max(None, 0)`` raised (and setting the connection timeout to ``None`` is rejected by junos-eznc). The wrappers now coalesce ``None`` to ``0`` and only override the connection timeout when a real (>0) ``dev_timeout``/``timeout`` is given. diff --git a/salt/modules/junos.py b/salt/modules/junos.py index 4aa8da83ffb..52ddff328a4 100644 --- a/salt/modules/junos.py +++ b/salt/modules/junos.py @@ -153,8 +153,14 @@ def __exit__(self, exc_type, exc_value, exc_traceback): def _timeout_decorator(function): @wraps(function) def wrapper(*args, **kwargs): - if "dev_timeout" in kwargs or "timeout" in kwargs: - ldev_timeout = max(kwargs.pop("dev_timeout", 0), kwargs.pop("timeout", 0)) + # Callers such as napalm.junos_cli pass dev_timeout=None when no timeout + # was requested; coalesce None to 0 so max() does not raise, and only + # override the connection timeout when a real (>0) value is given + # (junos-eznc rejects a None/0 timeout). + ldev_timeout = max( + kwargs.pop("dev_timeout", 0) or 0, kwargs.pop("timeout", 0) or 0 + ) + if ldev_timeout: conn = __proxy__["junos.conn"]() restore_timeout = conn.timeout conn.timeout = ldev_timeout @@ -174,8 +180,12 @@ def wrapper(*args, **kwargs): def _timeout_decorator_cleankwargs(function): @wraps(function) def wrapper(*args, **kwargs): - if "dev_timeout" in kwargs or "timeout" in kwargs: - ldev_timeout = max(kwargs.pop("dev_timeout", 0), kwargs.pop("timeout", 0)) + # See _timeout_decorator: dev_timeout=None must not raise, and the + # connection timeout is only overridden when a real (>0) value is given. + ldev_timeout = max( + kwargs.pop("dev_timeout", 0) or 0, kwargs.pop("timeout", 0) or 0 + ) + if ldev_timeout: conn = __proxy__["junos.conn"]() restore_timeout = conn.timeout conn.timeout = ldev_timeout diff --git a/tests/pytests/unit/modules/test_junos.py b/tests/pytests/unit/modules/test_junos.py index cfc792f5dd3..95ddb235ab9 100644 --- a/tests/pytests/unit/modules/test_junos.py +++ b/tests/pytests/unit/modules/test_junos.py @@ -200,6 +200,25 @@ def function(x): mock_timeout.assert_has_calls(calls) +def test__timeout_decorator_none_dev_timeout(): + # napalm.junos_cli (and friends) forward dev_timeout=None when no timeout + # was requested. That must not raise, and must leave the connection timeout + # untouched -- only a real (>0) value overrides it (#58108). + with patch("jnpr.junos.Device.timeout", new_callable=PropertyMock) as mock_timeout: + mock_timeout.return_value = 30 + + def function(x): + return x + + decorator = junos._timeout_decorator(function) + assert decorator("Test Mock", dev_timeout=None) == "Test Mock" + mock_timeout.assert_not_called() + + decorator = junos._timeout_decorator_cleankwargs(function) + assert decorator("Test Mock", dev_timeout=None, __pub_args="abc") == "Test Mock" + mock_timeout.assert_not_called() + + def test_facts_refresh(): with patch("salt.modules.saltutil.sync_grains") as mock_sync_grains: ret = {