From 68c18a4099b2cabf15d6d93c2496e420d1bbfb48 Mon Sep 17 00:00:00 2001 From: Gavin Acosta <155584291+Defiantearth@users.noreply.github.com> Date: Wed, 27 May 2026 10:39:11 -0500 Subject: [PATCH 1/5] Fix reboot to wait for device --- pyntc/devices/jnpr_device.py | 72 +++++++++++++++++++++++++++--------- 1 file changed, 55 insertions(+), 17 deletions(-) diff --git a/pyntc/devices/jnpr_device.py b/pyntc/devices/jnpr_device.py index 70c8fcad..36c8114b 100644 --- a/pyntc/devices/jnpr_device.py +++ b/pyntc/devices/jnpr_device.py @@ -228,18 +228,53 @@ def _uptime_to_string(self, uptime_full_string): days, hours, minutes, seconds = self._uptime_components(uptime_full_string) return f"{days:02d}:{hours:02d}:{minutes:02d}:{seconds:02d}" - def _wait_for_device_reboot(self, timeout=3600): + def _wait_for_device_reboot(self, original_uptime, timeout=7200): + """Block until the device reboots and accepts a fresh connection. + + Drops the existing NETCONF session and polls for the device to come back. + The reboot is considered complete when a new connection succeeds and the + device reports an uptime lower than ``original_uptime`` (i.e., it has + booted since the reboot was issued). + + The pre-reboot session must be discarded first: once the device restarts, + PyEZ still reports it as connected even though the transport is dead, so it + is closed here to force each probe to establish a fresh connection. + + Args: + original_uptime (int): Device uptime in seconds captured before the reboot. + timeout (int, optional): Max seconds to wait for the device to return. Defaults to 2 hours. + """ start = time.time() - disconnected = False + + # Drop the pre-reboot NETCONF session so subsequent probes can't read from + # a stale connection PyEZ still reports as connected. + try: + self.close() + except Exception as close_exc: + log.debug("Host %s: Pre-reboot disconnect raised %s (ignored).", self.host, close_exc) + while time.time() - start < timeout: - if disconnected: - try: - self.open() + try: + self.open() + self._uptime = None + current_uptime = self.uptime + if current_uptime is not None and current_uptime < original_uptime: + log.info( + "Host %s: Device rebooted (uptime %ss < pre-reboot %ss).", + self.host, + current_uptime, + original_uptime, + ) return - except: # noqa E722 # nosec # pylint: disable=bare-except - pass - elif not self.connected: - disconnected = True + log.debug( + "Host %s: Reachable but uptime %ss >= pre-reboot %ss; still waiting.", + self.host, + current_uptime, + original_uptime, + ) + except Exception as exc: + log.debug("Host %s: Reboot probe failed (%s); will retry.", self.host, exc) + self.native.connected = False time.sleep(10) raise RebootTimeoutError(hostname=self.hostname, wait_time=timeout) @@ -318,12 +353,14 @@ def uptime(self): Returns: (int): Device uptime in seconds. """ - try: - native_uptime_string = self.native.facts["RE0"]["up_time"] - except (AttributeError, TypeError): - native_uptime_string = None - if self._uptime is None: + try: + # Bust PyEZ's cached facts so a cold cache always reflects the live device. + self.native.facts_refresh(keys="RE0") + native_uptime_string = self.native.facts["RE0"]["up_time"] + except (AttributeError, TypeError, KeyError): + native_uptime_string = None + if native_uptime_string is not None: self._uptime = self._uptime_to_seconds(native_uptime_string) @@ -505,13 +542,13 @@ def open(self): if not self.connected: self.native.open() - def reboot(self, wait_for_reload=False, timeout=3600, confirm=None): + def reboot(self, wait_for_reload=False, timeout=7200, confirm=None): """ Reload the controller or controller pair. Args: wait_for_reload (bool): Whether the reboot method should wait for the device to come back up before returning. Defaults to False. - timeout (int, optional): Time in seconds to wait for the device to return after reboot. Defaults to 1 hour. + timeout (int, optional): Time in seconds to wait for the device to return after reboot. Defaults to 2 hours. confirm (None): Not used. Deprecated since v0.17.0. Example: @@ -522,9 +559,10 @@ def reboot(self, wait_for_reload=False, timeout=3600, confirm=None): if confirm is not None: warnings.warn("Passing 'confirm' to reboot method is deprecated.", DeprecationWarning) + original_uptime = self.uptime self.sw.reboot(in_min=0) if wait_for_reload: - self._wait_for_device_reboot(timeout=timeout) + self._wait_for_device_reboot(original_uptime, timeout=timeout) def rollback(self, filename): """Rollback to a specific configuration file. From cadcf77bfdb42073137841b5fee18502a1509208 Mon Sep 17 00:00:00 2001 From: Gavin Acosta <155584291+Defiantearth@users.noreply.github.com> Date: Wed, 27 May 2026 14:47:53 -0500 Subject: [PATCH 2/5] Adding tests --- tests/unit/test_devices/test_jnpr_device.py | 55 +++++++++++++++++---- 1 file changed, 45 insertions(+), 10 deletions(-) diff --git a/tests/unit/test_devices/test_jnpr_device.py b/tests/unit/test_devices/test_jnpr_device.py index 5b9d325d..82a45cf9 100644 --- a/tests/unit/test_devices/test_jnpr_device.py +++ b/tests/unit/test_devices/test_jnpr_device.py @@ -244,18 +244,28 @@ def test_reboot(self): @mock.patch("pyntc.devices.jnpr_device.time.sleep") def test_wait_for_device_to_reboot(self, mock_sleep): - with mock.patch.object(self.device, "open") as mock_open: - # Emulate the device disconnected and reconnecting - type(self.device.native).connected = mock.PropertyMock(side_effect=[True, False, True]) - mock_open.side_effect = [Exception, Exception, True] - self.device.reboot(wait_for_reload=True, timeout=3) - mock_open.assert_has_calls([mock.call()] * 3) + """Reboot completes when the device returns with a lower uptime than the baseline.""" + with ( + mock.patch.object(self.device, "open") as mock_open, + mock.patch.object(self.device, "close"), + mock.patch.object(type(self.device), "uptime", new_callable=mock.PropertyMock) as mock_uptime, + ): + # First read is the pre-reboot baseline; second is the post-reboot uptime. + mock_uptime.side_effect = [455, 30] + self.device.reboot(wait_for_reload=True, timeout=30) + + self.device.sw.reboot.assert_called_with(in_min=0) + mock_open.assert_called() @mock.patch("pyntc.devices.jnpr_device.time.sleep") def test_wait_for_device_to_reboot_error(self, mock_sleep): - with mock.patch.object(self.device, "open") as mock_open: - type(self.device.native).connected = mock.PropertyMock(side_effect=[True, False]) - mock_open.side_effect = Exception + """Raise RebootTimeoutError when the device never reports a lower uptime within the timeout.""" + with ( + mock.patch.object(self.device, "open"), + mock.patch.object(self.device, "close"), + mock.patch.object(type(self.device), "uptime", new_callable=mock.PropertyMock) as mock_uptime, + ): + mock_uptime.return_value = 455 with pytest.raises(RebootTimeoutError): self.device.reboot(wait_for_reload=True, timeout=1) @@ -290,8 +300,33 @@ def test_checkpoint(self, mock_scp): self.device.show.assert_called_with("show config") def test_uptime(self): + """Cold cache (_uptime is None) refreshes facts and parses the uptime.""" + self.assertIsNone(self.device._uptime) uptime = self.device.uptime - assert uptime == 455 + self.assertEqual(uptime, 455) + self.device.native.facts_refresh.assert_called_once_with(keys="RE0") + + def test_uptime_cached(self): + """A populated cache is returned as-is, with no device round-trip.""" + self.device._uptime = 1234 + uptime = self.device.uptime + self.assertEqual(uptime, 1234) + self.device.native.facts_refresh.assert_not_called() + + def test_uptime_refreshes_after_cache_cleared(self): + """Clearing the cache forces a fresh read.""" + self.assertEqual(self.device.uptime, 455) + + self.device._uptime = None + self.device.native.facts = {"RE0": {"up_time": "30 seconds"}} + + self.assertEqual(self.device.uptime, 30) + + def test_uptime_none_when_facts_unavailable(self): + """Missing/unavailable facts return None gracefully instead of raising an Exception.""" + self.device._uptime = None + self.device.native.facts = {} + self.assertIsNone(self.device.uptime) def test_uptime_string(self): uptime_string = self.device.uptime_string From 2bd7f9ff8105313a768597ce7b7ebcf8c2b076d3 Mon Sep 17 00:00:00 2001 From: Gavin Acosta <155584291+Defiantearth@users.noreply.github.com> Date: Wed, 27 May 2026 14:55:36 -0500 Subject: [PATCH 3/5] changelog --- changes/394.fixed | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/394.fixed diff --git a/changes/394.fixed b/changes/394.fixed new file mode 100644 index 00000000..aa9dd1f4 --- /dev/null +++ b/changes/394.fixed @@ -0,0 +1 @@ +Fixed Junos reboots not being detected when waiting for the device to reload. From 6f0117049da82a2ba9bd82a0b3e99e12ca0e0a19 Mon Sep 17 00:00:00 2001 From: Gavin Acosta <155584291+Defiantearth@users.noreply.github.com> Date: Wed, 27 May 2026 15:17:16 -0500 Subject: [PATCH 4/5] pylint --- pyntc/devices/jnpr_device.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pyntc/devices/jnpr_device.py b/pyntc/devices/jnpr_device.py index 36c8114b..f8226920 100644 --- a/pyntc/devices/jnpr_device.py +++ b/pyntc/devices/jnpr_device.py @@ -250,7 +250,7 @@ def _wait_for_device_reboot(self, original_uptime, timeout=7200): # a stale connection PyEZ still reports as connected. try: self.close() - except Exception as close_exc: + except Exception as close_exc: # pylint: disable=broad-exception-caught log.debug("Host %s: Pre-reboot disconnect raised %s (ignored).", self.host, close_exc) while time.time() - start < timeout: @@ -272,7 +272,7 @@ def _wait_for_device_reboot(self, original_uptime, timeout=7200): current_uptime, original_uptime, ) - except Exception as exc: + except Exception as exc: # pylint: disable=broad-exception-caught log.debug("Host %s: Reboot probe failed (%s); will retry.", self.host, exc) self.native.connected = False time.sleep(10) From 86e5fa4a4d79f63ea5b1c33415d8e03864f05dca Mon Sep 17 00:00:00 2001 From: Gavin Acosta <155584291+Defiantearth@users.noreply.github.com> Date: Thu, 28 May 2026 08:35:30 -0500 Subject: [PATCH 5/5] suggestions --- changes/394.fixed | 1 + pyntc/devices/jnpr_device.py | 21 +++++++++++----- tests/unit/test_devices/test_jnpr_device.py | 27 ++++++++++++++++++++- 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/changes/394.fixed b/changes/394.fixed index aa9dd1f4..20126908 100644 --- a/changes/394.fixed +++ b/changes/394.fixed @@ -1 +1,2 @@ Fixed Junos reboots not being detected when waiting for the device to reload. +Increased the default Junos reboot wait timeout from 1 hour to 2 hours. diff --git a/pyntc/devices/jnpr_device.py b/pyntc/devices/jnpr_device.py index f8226920..b6431204 100644 --- a/pyntc/devices/jnpr_device.py +++ b/pyntc/devices/jnpr_device.py @@ -374,13 +374,16 @@ def uptime_string(self): Returns: (str): Device uptime. """ - try: - native_uptime_string = self.native.facts["RE0"]["up_time"] - except (AttributeError, TypeError): - native_uptime_string = None - if self._uptime_string is None: - self._uptime_string = self._uptime_to_string(native_uptime_string) + try: + # Bust PyEZ's cached facts so a cold cache always reflects the live device. + self.native.facts_refresh(keys="RE0") + native_uptime_string = self.native.facts["RE0"]["up_time"] + except (AttributeError, TypeError, KeyError): + native_uptime_string = None + + if native_uptime_string is not None: + self._uptime_string = self._uptime_to_string(native_uptime_string) return self._uptime_string @@ -559,7 +562,13 @@ def reboot(self, wait_for_reload=False, timeout=7200, confirm=None): if confirm is not None: warnings.warn("Passing 'confirm' to reboot method is deprecated.", DeprecationWarning) + self._uptime = None original_uptime = self.uptime + if original_uptime is None: + raise CommandError( + command="reboot", + message="Could not determine pre-reboot uptime; refusing to wait for reload.", + ) self.sw.reboot(in_min=0) if wait_for_reload: self._wait_for_device_reboot(original_uptime, timeout=timeout) diff --git a/tests/unit/test_devices/test_jnpr_device.py b/tests/unit/test_devices/test_jnpr_device.py index 82a45cf9..28271a64 100644 --- a/tests/unit/test_devices/test_jnpr_device.py +++ b/tests/unit/test_devices/test_jnpr_device.py @@ -329,8 +329,33 @@ def test_uptime_none_when_facts_unavailable(self): self.assertIsNone(self.device.uptime) def test_uptime_string(self): + """Cold cache (_uptime_string is None) refreshes facts and formats the uptime.""" + self.assertIsNone(self.device._uptime_string) uptime_string = self.device.uptime_string - assert uptime_string == "00:00:07:35" + self.assertEqual(uptime_string, "00:00:07:35") + self.device.native.facts_refresh.assert_called_once_with(keys="RE0") + + def test_uptime_string_cached(self): + """A populated cache is returned as-is, with no device round-trip.""" + self.device._uptime_string = "01:02:03:04" + uptime_string = self.device.uptime_string + self.assertEqual(uptime_string, "01:02:03:04") + self.device.native.facts_refresh.assert_not_called() + + def test_uptime_string_refreshes_after_cache_cleared(self): + """Clearing the cache forces a fresh read.""" + self.assertEqual(self.device.uptime_string, "00:00:07:35") + + self.device._uptime_string = None + self.device.native.facts = {"RE0": {"up_time": "30 seconds"}} + + self.assertEqual(self.device.uptime_string, "00:00:00:30") + + def test_uptime_string_none_when_facts_unavailable(self): + """Missing/unavailable facts return None gracefully instead of raising an Exception.""" + self.device._uptime_string = None + self.device.native.facts = {} + self.assertIsNone(self.device.uptime_string) def test_vendor(self): vendor = self.device.vendor