NXOS netmiko transport change

.gitignore

@@ -1,3 +1,6 @@
+# Local working notes / drafts (not committed)
+ideas/
+
 # Ansible Retry Files
 *.retry
 

changes/388.changed

@@ -0,0 +1 @@
+Migrated `NXOSDevice` methods from NX-API to Netmiko SSH: `running_config`, `set_timeout`, `install_os`, `reboot`, `backup_running_config`, `checkpoint`, `rollback`, `redundancy_state`, `file_copy_remote_exists`, and `file_copy`. 

changes/388.deprecated

@@ -0,0 +1 @@
+Deprecated the `transport`, `verify`, and `port` constructor kwargs on `NXOSDevice`. Supplying any of these now emits a `DeprecationWarning`. These kwargs will be removed in a future release as `NXOSDevice` migrates to Netmiko SSH exclusively.

docs/user/lib_overview.md

@@ -15,7 +15,28 @@ It's main purpose is to simplify the execution of common tasks including:
 - Cisco AireOS - uses netmiko (SSH)
 - Cisco ASA - uses netmiko (SSH)
 - Cisco IOS platforms - uses netmiko (SSH)
-- Cisco NX-OS - uses pynxos (NX-API)
+- Cisco NX-OS - migrating from pynxos (NX-API) to netmiko (SSH); see [NXOS transport change](#nxos-transport-change) below
 - Arista EOS - uses pyeapi (eAPI)
 - Juniper Junos - uses PyEz (NETCONF)
 - F5 Networks - uses f5-sdk (ReST)
+
+## NXOS transport change
+
+`NXOSDevice` is migrating from the pynxos (NX-API) transport to Netmiko SSH. The migration is being delivered in phases:
+
+- The following methods have been reimplemented on Netmiko SSH: `save()`, `running_config`, `set_timeout()`, `install_os()` (the `terminal dont-ask` step), `reboot()`, `backup_running_config()`, `checkpoint()`, `rollback()`, `redundancy_state`, `file_copy_remote_exists()`, and `file_copy()`.
+- `file_copy()` now uses Netmiko's `file_transfer()` (SCP over the existing SSH session) instead of pynxos.
+- `reboot()` now catches `netmiko.exceptions.ReadTimeout` (raised by Netmiko when the SSH session drops during reload) instead of `requests.exceptions.ReadTimeout`.
+- `redundancy_state` now falls back to `"active"` when the underlying SSH command raises any `netmiko.exceptions.NetmikoBaseException`.
+- `file_copy_remote_exists()` now compares the local file's checksum against the remote file's checksum via `verify_file()`; it no longer delegates to pynxos.
+- `refresh()` now also invalidates the cached `redundancy_state`.
+- The constructor still accepts the `transport`, `verify`, and `port` kwargs for backwards compatibility, but supplying any of them now emits a `DeprecationWarning`. These kwargs will be removed in a future release and will be ignored once the migration completes.
+- The remaining NX-API call sites (`show`, `config`, `facts`-derived properties such as `hostname`/`uptime`/`os_version`, `boot_options`, `set_boot_options`, etc.) are still wired through pynxos and will be migrated in follow-up releases.
+
+### Behavioral differences to expect once migration completes
+
+- `show(..., raw_text=False)` will return TextFSM-parsed structures (via `ntc-templates`) rather than NX-API JSON. The result shape may differ for some commands; verify against `ntc-templates` output for the relevant `show` command.
+- Properties derived from facts (`uptime`, `hostname`, `model`, `os_version`, etc.) will issue an SSH round trip on first access rather than reading from an NX-API JSON payload, and may not be populated until called.
+- Niche `show` commands without an `ntc-templates` parser will return raw text; callers that relied on NX-API structured output may need to switch to `raw_text=True` and parse themselves.
+
+See the corresponding entry in the release notes for the change-tracking fragment.

pyntc/devices/nxos_device.py

@@ -3,15 +3,17 @@
 import os
 import re
 import time
+import warnings
 
-from netmiko import ConnectHandler
-from requests.exceptions import ConnectTimeout, ReadTimeout
+from netmiko import ConnectHandler, file_transfer
+from netmiko.exceptions import NetmikoBaseException, ReadTimeout
+from requests.exceptions import ConnectTimeout
+from requests.exceptions import ReadTimeout as RequestsReadTimeout
 
 from pyntc import log
 from pyntc.devices.base_device import BaseDevice, RollbackError, fix_docs
 from pyntc.devices.pynxos.device import Device as NXOSNative
 from pyntc.devices.pynxos.errors import CLIError
-from pyntc.devices.pynxos.features.file_copy import FileTransferError as NXOSFileTransferError
 from pyntc.errors import (
     CommandError,
     CommandListError,
@@ -49,6 +51,21 @@ def __init__(self, host, username, password, transport="http", timeout=30, port=
 
         """
         super().__init__(host, username, password, device_type="cisco_nxos_nxapi")
+        deprecated_kwargs = []
+        if transport != "http":
+            deprecated_kwargs.append("transport")
+        if port is not None:
+            deprecated_kwargs.append("port")
+        if verify is not True:
+            deprecated_kwargs.append("verify")
+        if deprecated_kwargs:
+            warnings.warn(
+                f"NXOSDevice kwargs {deprecated_kwargs} are deprecated and will be removed in a future release. "
+                "NXOSDevice is migrating to Netmiko SSH exclusively; these NX-API-only kwargs will no longer "
+                "be honored once the migration is complete.",
+                DeprecationWarning,
+                stacklevel=2,
+            )
         self.transport = transport
         self.timeout = timeout
         self.port = port
@@ -88,6 +105,7 @@ def refresh(self):
         """Refresh caches on device instance."""
         if hasattr(self.native, "_facts"):
             delattr(self.native, "_facts")
+        self._redundancy_state = None
         super().refresh()
 
     def backup_running_config(self, filename):
@@ -96,8 +114,11 @@ def backup_running_config(self, filename):
         Args:
             filename (str): Name of backup file.
         """
-        self.native.backup_running_config(filename)
-        log.debug("Host %s: Running config backed up.", self.host)
+        self.open()
+        output = self.native_ssh.send_command("show running-config")
+        with open(filename, "w", encoding="utf-8") as backup_file:
+            backup_file.write(output)
+        log.debug("Host %s: Running config backed up to %s.", self.host, filename)
 
     @property
     def boot_options(self):
@@ -115,9 +136,17 @@ def checkpoint(self, filename):
 
         Args:
             filename (str): The filename to save the checkpoint on the remote device.
+
+        Raises:
+            CommandError: If the device rejects the checkpoint command.
         """
+        self.open()
+        command = f"checkpoint file {filename}"
         log.debug("Host %s: checkpoint is %s.", self.host, filename)
-        return self.native.checkpoint(filename)
+        output = self.native_ssh.send_command(command)
+        if re.search(r"%\s*Error|ERROR:", output):
+            log.error("Host %s: Checkpoint failed for %s: %s", self.host, filename, output)
+            raise CommandError(command, output.strip())
 
     def close(self):
         """Disconnect from device."""
@@ -267,57 +296,59 @@ def serial_number(self):
         return self._serial_number
 
     def file_copy(self, src, dest=None, file_system="bootflash:"):
-        """Send a local file to the device.
+        """Send a local file to the device via SCP over the SSH session.
 
         Args:
             src (str): Path to the local file to send.
             dest (str, optional): The destination file path. Defaults to basename of source path.
-            file_system (str, optional): [The file system for the remote file. Defaults to "bootflash:".
+            file_system (str, optional): The file system for the remote file. Defaults to "bootflash:".
 
         Raises:
             FileTransferError: Error if transfer of file cannot be verified.
         """
+        dest = dest or os.path.basename(src)
+        if self.file_copy_remote_exists(src, dest, file_system):
+            return
+        self._check_free_space(os.path.getsize(src), file_system=file_system)
+        self.open()
+        try:
+            file_transfer(
+                self.native_ssh,
+                source_file=src,
+                dest_file=dest,
+                file_system=file_system,
+                direction="put",
+                overwrite_file=True,
+            )
+        except Exception as err:  # noqa: BLE001
+            log.error("Host %s: SCP file transfer error %s", self.host, str(err))
+            raise FileTransferError from err
+        log.info("Host %s: File %s transferred successfully.", self.host, src)
         if not self.file_copy_remote_exists(src, dest, file_system):
-            dest = dest or os.path.basename(src)
-            self._check_free_space(os.path.getsize(src), file_system=file_system)
-            try:
-                file_copy = self.native.file_copy(  # pylint: disable=assignment-from-no-return
-                    src, dest, file_system=file_system
-                )  # pylint: disable=assignment-from-no-return
-                log.info("Host %s: File %s transferred successfully.", self.host, src)
-                if not self.file_copy_remote_exists(src, dest, file_system):
-                    log.error(
-                        "Host %s: Attempted file copy, but could not validate file existed after transfer %s",
-                        self.host,
-                        FileTransferError.default_message,
-                    )
-                    raise FileTransferError
-                return file_copy
-
-            except NXOSFileTransferError as err:
-                log.error("Host %s: NXOS file transfer error %s", self.host, str(err))
-                raise FileTransferError
+            log.error(
+                "Host %s: Attempted file copy, but could not validate file existed after transfer %s",
+                self.host,
+                FileTransferError.default_message,
+            )
+            raise FileTransferError
 
     # TODO: Make this an internal method since exposing file_copy should be sufficient
     def file_copy_remote_exists(self, src, dest=None, file_system="bootflash:"):
-        """Check if a remote file exists.
+        """Check if a remote file exists and matches the local file's checksum.
 
         Args:
             src (str): Path to the local file to send.
             dest (str, optional): The destination file path to be saved on remote device. Defaults to basename of source path.
             file_system (str, optional): The file system for the remote file. Defaults to "bootflash:".
 
         Returns:
-            (bool): True if the remote file exists. Otherwise, false.
+            (bool): True if the remote file exists and its checksum matches the local file. Otherwise, false.
         """
         dest = dest or os.path.basename(src)
-        log.debug(
-            "Host %s: File %s exists on remote %s.",
-            self.host,
-            src,
-            self.native.file_copy_remote_exists(src, dest, file_system=file_system),
-        )
-        return self.native.file_copy_remote_exists(src, dest, file_system=file_system)
+        local_checksum = self.get_local_checksum(src)
+        result = self.verify_file(local_checksum, dest, file_system=file_system)
+        log.debug("Host %s: File %s exists on remote and matches local: %s", self.host, dest, result)
+        return result
 
     def _get_file_system(self):
         """Determine the default file system or directory for device.
@@ -470,7 +501,6 @@ def get_remote_checksum(self, filename, hashing_algorithm="md5", **kwargs):
                 command,
                 result,
             )
-            print(f"result: {result}")
             remote_checksum = result
             return remote_checksum
 
@@ -632,7 +662,7 @@ def install_os(self, image_name, reboot=True, **vendor_specifics):
         Returns:
             (bool): True if new image is boot option on device. Otherwise, false.
         """
-        self.native.show("terminal dont-ask")
+        self.native_ssh.send_command("terminal dont-ask")
         timeout = vendor_specifics.get("timeout", 3600)
         if not self._image_booted(image_name):
             log.info("Host %s: Setting Image %s in boot options.", self.host, image_name)
@@ -674,17 +704,17 @@ def redundancy_state(self):
         """
         if self._redundancy_state is None:
             try:
-                output = self.native.show("show redundancy state", raw_text=True)
-                # Parse the redundancy state from output
+                self.open()
+                output = self.native_ssh.send_command("show redundancy state")
                 # Example output: "Redundancy state = active"
                 match = re.search(r"Redundancy\s+state\s*=\s*(\w+)", output, re.IGNORECASE)
                 if match:
                     self._redundancy_state = match.group(1).lower()
                 else:
                     # If no redundancy info, device may not support HA
                     self._redundancy_state = "active"
-            except CLIError:
-                # If command fails, assume active (non-HA or error condition)
+            except NetmikoBaseException:
+                # If the SSH command fails, assume active (non-HA or error condition)
                 self._redundancy_state = "active"
 
         return self._redundancy_state
@@ -749,9 +779,8 @@ def reboot(self, wait_for_reload=False, **kwargs):
             log.warning("Passing 'confirm' to reboot method is deprecated.")
             raise DeprecationWarning("Passing 'confirm' to reboot method is deprecated.")
         try:
-            self.native.show_list(["terminal dont-ask", "reload"])
-            # The native reboot is not always properly disabling confirmation. Above is more consistent.
-            # self.native.reboot(confirm=True)
+            self.native_ssh.send_command("terminal dont-ask")
+            self.native_ssh.send_command("reload")
         except ReadTimeout as expected_exception:
             log.info("Host %s: Device rebooted.", self.host)
             log.info("Hit expected exception during reload: %s", expected_exception.__class__)
@@ -769,12 +798,13 @@ def rollback(self, filename):
         Raises:
             RollbackError: Error if rollback command is unsuccesfull.
         """
-        try:
-            self.native.rollback(filename)
-            log.info("Host %s: Rollback to %s.", self.host, filename)
-        except CLIError:
-            log.error("Host %s: Rollback unsuccessful. %s may not exist.", self.host, filename)
+        self.open()
+        command = f"rollback running-config file {filename}"
+        output = self.native_ssh.send_command(command)
+        if re.search(r"%\s*Error|ERROR:|Rollback failed|does not exist", output, re.IGNORECASE):
+            log.error("Host %s: Rollback unsuccessful. %s may not exist. Output: %s", self.host, filename, output)
             raise RollbackError(f"Rollback unsuccessful, {filename} may not exist.")
+        log.info("Host %s: Rollback to %s.", self.host, filename)
 
     @property
     def running_config(self):
@@ -784,7 +814,8 @@ def running_config(self):
             (str): Running configuration of device.
         """
         log.debug("Host %s: Show running config.", self.host)
-        return self.native.running_config
+        self.open()
+        return self.native_ssh.send_command("show running-config")
 
     def save(self, filename="startup-config"):
         """Save a device's running configuration.
@@ -795,8 +826,13 @@ def save(self, filename="startup-config"):
         Returns:
             (bool): True if configuration is saved.
         """
+        self.open()
+        command = f"copy running-config {filename}"
+        self.native_ssh.send_command_timing(command)
+        self.native_ssh.send_command_timing("\n", read_timeout=200)
+        self.native_ssh.find_prompt()
         log.debug("Host %s: Copy running config with name %s.", self.host, filename)
-        return self.native.save(filename=filename)
+        return True
 
     def set_boot_options(self, image_name, kickstart=None, reboot=True, **vendor_specifics):
         """Set boot variables.
@@ -829,7 +865,7 @@ def set_boot_options(self, image_name, kickstart=None, reboot=True, **vendor_spe
         image_name = file_system + image_name
         try:
             self.native.set_boot_options(image_name, kickstart=kickstart, reboot=reboot)
-        except (ReadTimeout, ConnectTimeout):
+        except (RequestsReadTimeout, ConnectTimeout):
             pass
         log.info("Host %s: boot options have been set to %s", self.host, image_name)
 
@@ -840,7 +876,8 @@ def set_timeout(self, timeout):
             timeout (int): Timeout value.
         """
         log.debug("Host %s: Timeout set to %s.", self.host, timeout)
-        self.native.timeout = timeout
+        self.timeout = timeout
+        self.native_ssh.timeout = timeout
 
     def show(self, command, raw_text=False):
         """Send a non-configuration command.