From 66755b4338ce1c4d8301b83be75fb889bb9f58bd Mon Sep 17 00:00:00 2001 From: Josh VanDeraa Date: Wed, 12 Aug 2026 10:07:57 -0500 Subject: [PATCH 1/2] Fix EOSDevice.close() leaking the Netmiko SSH session close() was a no-op, but open() creates a real SSH session for the file-transfer paths (file_copy, check_file_exists, get_remote_checksum, remote_file_copy). A caller holding many device objects accumulated one open socket per device. Disconnect when _connected is set, matching IOSDevice.close(). The guard also keeps close() safe on a device that only ever spoke eAPI, since native_ssh is assigned inside open(). Removes EOSSSHDevice.close(), now an exact duplicate of the parent: native_ssh on that class is a property returning native, so both bodies make the same call. --- pyntc/devices/eos_device.py | 16 +++++++- pyntc/devices/eos_ssh_device.py | 11 ------ tests/unit/test_devices/test_eos_device.py | 44 ++++++++++++++++++++++ 3 files changed, 58 insertions(+), 13 deletions(-) diff --git a/pyntc/devices/eos_device.py b/pyntc/devices/eos_device.py index 31d445ae..997f918e 100644 --- a/pyntc/devices/eos_device.py +++ b/pyntc/devices/eos_device.py @@ -250,8 +250,20 @@ def checkpoint(self, checkpoint_file): self.show(f"copy running-config {checkpoint_file}") def close(self): - """Not implemented. Just ``passes``.""" - pass # pylint: disable=unnecessary-pass + """Release the Netmiko SSH session opened by ``open``. + + The eAPI transport itself is stateless HTTP and needs no teardown, but + ``open`` builds a real SSH session for the file-transfer paths + (``file_copy``, ``check_file_exists``, ``get_remote_checksum``, + ``remote_file_copy``). Without this, a caller holding many device objects + accumulates one open socket per device. + + Does nothing when no SSH session was ever opened. + """ + if self._connected: + self.native_ssh.disconnect() + self._connected = False + log.debug("Host %s: Connection closed.", self.host) def config(self, commands): """Send configuration commands to a device. diff --git a/pyntc/devices/eos_ssh_device.py b/pyntc/devices/eos_ssh_device.py index 5cbe100e..c6beddc0 100644 --- a/pyntc/devices/eos_ssh_device.py +++ b/pyntc/devices/eos_ssh_device.py @@ -184,17 +184,6 @@ def open(self): log.debug("Host %s: Connection to device was opened successfully.", self.host) - def close(self): - """Disconnect from the device. - - Note this differs from ``EOSDevice.close``, which is a no-op because eAPI is - stateless. An SSH session holds a real socket that should be released. - """ - if self._connected: - self.native.disconnect() - self._connected = False - log.debug("Host %s: Connection closed.", self.host) - def show(self, commands, raw_text=False): """Send show command(s) to the device. diff --git a/tests/unit/test_devices/test_eos_device.py b/tests/unit/test_devices/test_eos_device.py index 05f3ea5c..066f9aca 100644 --- a/tests/unit/test_devices/test_eos_device.py +++ b/tests/unit/test_devices/test_eos_device.py @@ -469,6 +469,50 @@ def test_init_pass_port_and_timeout(mock_eos_connect): ) +def test_close_disconnects_netmiko_session(eos_device): + # open() creates a real SSH session for the file-transfer paths; close() has to + # release it or long-lived callers leak a socket per device. + eos_device.native_ssh = mock.MagicMock() + eos_device._connected = True + + eos_device.close() + + eos_device.native_ssh.disconnect.assert_called_once() + assert eos_device._connected is False + + +def test_close_is_noop_when_never_opened(eos_device): + # native_ssh is only assigned inside open(), so close() must not reach for it + # on a device that has only ever spoken eAPI. + assert not hasattr(eos_device, "native_ssh") + + eos_device.close() + + assert eos_device._connected is False + + +def test_close_is_idempotent(eos_device): + eos_device.native_ssh = mock.MagicMock() + eos_device._connected = True + + eos_device.close() + eos_device.close() + + eos_device.native_ssh.disconnect.assert_called_once() + + +@mock.patch("pyntc.devices.eos_device.ConnectHandler") +def test_close_does_not_strand_the_device(mock_connect_handler, eos_device): + # Closing must stay safe for a reusable object: every SSH-backed method calls + # open() first, and open() has to rebuild the session a previous close() tore down. + eos_device.open() + eos_device.close() + eos_device.open() + + assert mock_connect_handler.call_count == 2 + assert eos_device._connected is True + + class EOSDeviceMockedTestCase(unittest.TestCase): """Base test case wiring a mocked ``pyeapi`` node onto an ``EOSDevice``.""" From e5ee5e07f9c5235ee0f3be01f7085e98f33a8042 Mon Sep 17 00:00:00 2001 From: Josh VanDeraa Date: Wed, 12 Aug 2026 10:56:04 -0500 Subject: [PATCH 2/2] Add changelog fragment for #423 --- changes/423.fixed | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/423.fixed diff --git a/changes/423.fixed b/changes/423.fixed new file mode 100644 index 00000000..216d8dd1 --- /dev/null +++ b/changes/423.fixed @@ -0,0 +1 @@ +Fixed `EOSDevice.close()` leaking the Netmiko SSH session opened by the file-transfer methods.