Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changes/423.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed `EOSDevice.close()` leaking the Netmiko SSH session opened by the file-transfer methods.
16 changes: 14 additions & 2 deletions pyntc/devices/eos_device.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
11 changes: 0 additions & 11 deletions pyntc/devices/eos_ssh_device.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
44 changes: 44 additions & 0 deletions tests/unit/test_devices/test_eos_device.py
Original file line number Diff line number Diff line change
Expand Up @@ -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``."""

Expand Down
Loading