Merge pull request #1461 from dkaplan1/nxos-improvements

Update NXOS response processing for more informative error messaging

napalm/nxapi_plumbing/api_client.py

@@ -82,15 +82,7 @@ def _send_request(self, commands, method):
             )
             raise NXAPIAuthError(msg)
 
-        if response.status_code not in [200]:
-            msg = """Invalid status code returned on NX-API POST
-commands: {}
-status_code: {}""".format(
-                commands, response.status_code
-            )
-            raise NXAPIPostError(msg)
-
-        return response.text
+        return response
 
 
 class RPCClient(RPCBase):
@@ -143,7 +135,7 @@ def _process_api_response(self, response, commands, raw_text=False):
         structured data.
         """
 
-        response_list = json.loads(response)
+        response_list = json.loads(response.text)
         if isinstance(response_list, dict):
             response_list = [response_list]
 
@@ -154,7 +146,7 @@ def _process_api_response(self, response, commands, raw_text=False):
         new_response = []
         for response in response_list:
 
-            # Dectect errors
+            # Detect errors
             self._error_check(response)
 
             # Some commands like "show run" can have a None result
@@ -239,7 +231,15 @@ def _build_payload(self, commands, method, xml_version="1.0", version="1.0"):
         return payload
 
     def _process_api_response(self, response, commands, raw_text=False):
-        xml_root = etree.fromstring(response)
+        if response.status_code not in [200]:
+            msg = """Invalid status code returned on NX-API POST
+commands: {}
+status_code: {}""".format(
+                commands, response.status_code
+            )
+            raise NXAPIPostError(msg)
+
+        xml_root = etree.fromstring(response.text)
         response_list = xml_root.xpath("outputs/output")
         if len(commands) != len(response_list):
             raise NXAPIXMLError(

napalm/nxos/nxos.py

@@ -1175,7 +1175,7 @@ def get_interfaces_ip(self):
             ipv6_interf_table_vrf = self._get_command_table(
                 ipv6_command, "TABLE_intf", "ROW_intf"
             )
-        except napalm.nxapi_plumbing.errors.NXAPIPostError:
+        except napalm.nxapi_plumbing.errors.NXAPICommandError:
             return interfaces_ip
 
         for interface in ipv6_interf_table_vrf:

test/nxapi_plumbing/conftest.py

@@ -39,6 +39,7 @@ def pytest_addoption(parser):
 @pytest.fixture(scope="module")
 def mock_pynxos_device(request):
     """Create a mock pynxos test device."""
+    response_status_code = getattr(request, "param", 200)
     device = {
         "host": "nxos1.fake.com",
         "username": "admin",
@@ -49,13 +50,14 @@ def mock_pynxos_device(request):
         "timeout": 60,
         "verify": False,
     }
-    conn = MockDevice(**device)
+    conn = MockDevice(response_status_code=response_status_code, **device)
     return conn
 
 
 @pytest.fixture(scope="module")
 def mock_pynxos_device_xml(request):
     """Create a mock pynxos test device."""
+    response_status_code = getattr(request, "param", 200)
     device = {
         "host": "nxos1.fake.com",
         "username": "admin",
@@ -66,7 +68,7 @@ def mock_pynxos_device_xml(request):
         "timeout": 60,
         "verify": False,
     }
-    conn = MockDevice(**device)
+    conn = MockDevice(response_status_code=response_status_code, **device)
     return conn
 
 

test/nxapi_plumbing/mock_device.py

@@ -61,6 +61,7 @@ def __init__(
         port=None,
         timeout=30,
         verify=True,
+        response_status_code=200,
     ):
         super().__init__(
             host,
@@ -81,6 +82,7 @@ def __init__(
                 port=port,
                 timeout=timeout,
                 verify=verify,
+                response_status_code=response_status_code,
             )
         elif api_format == "xml":
             self.api = MockXMLClient(
@@ -91,10 +93,33 @@ def __init__(
                 port=port,
                 timeout=timeout,
                 verify=verify,
+                response_status_code=response_status_code,
             )
 
 
 class MockRPCClient(RPCClient):
+    def __init__(
+        self,
+        host,
+        username,
+        password,
+        transport="https",
+        port=None,
+        timeout=30,
+        verify=True,
+        response_status_code=200,
+    ):
+        self.response_status_code = response_status_code
+        super().__init__(
+            host=host,
+            username=username,
+            password=password,
+            transport=transport,
+            port=port,
+            timeout=timeout,
+            verify=verify,
+        )
+
     def _send_request(self, commands, method="cli"):
         payload = self._build_payload(commands, method)
 
@@ -112,12 +137,34 @@ def _send_request(self, commands, method="cli"):
 
         response_obj = FakeResponse()
         response_obj.text = mock_response
-        response_obj.status_code = 200
+        response_obj.status_code = self.response_status_code
 
-        return response_obj.text
+        return response_obj
 
 
 class MockXMLClient(XMLClient):
+    def __init__(
+        self,
+        host,
+        username,
+        password,
+        transport="https",
+        port=None,
+        timeout=30,
+        verify=True,
+        response_status_code=200,
+    ):
+        self.response_status_code = response_status_code
+        super().__init__(
+            host=host,
+            username=username,
+            password=password,
+            transport=transport,
+            port=port,
+            timeout=timeout,
+            verify=verify,
+        )
+
     def _send_request(self, commands, method="cli_show"):
         payload = self._build_payload(commands, method)
 
@@ -135,6 +182,6 @@ def _send_request(self, commands, method="cli_show"):
 
         response_obj = FakeResponse()
         response_obj.text = mock_response
-        response_obj.status_code = 200
+        response_obj.status_code = self.response_status_code
 
-        return response_obj.text
+        return response_obj

test/nxapi_plumbing/test_config.py

@@ -1,3 +1,9 @@
+import pytest
+
+from napalm.nxapi_plumbing import NXAPICommandError
+from napalm.nxapi_plumbing.errors import NXAPIPostError
+
+
 def test_config_jsonrpc(mock_pynxos_device):
     result = mock_pynxos_device.config("logging history size 200")
     assert result is None
@@ -36,3 +42,31 @@ def test_config_xml_list(mock_pynxos_device_xml):
         msg = element.find("./msg")
         assert status_code.text == "200"
         assert msg.text == "Success"
+
+
+@pytest.mark.parametrize("mock_pynxos_device", [500], indirect=True)
+def test_config_jsonrpc_raises_NXAPICommandError_on_non_200_config_error(
+    mock_pynxos_device,
+):
+    with pytest.raises(
+        NXAPICommandError, match='The command "bogus command" gave the error'
+    ):
+        mock_pynxos_device.config("bogus command")
+
+
+@pytest.mark.parametrize("mock_pynxos_device_xml", [500], indirect=True)
+def test_config_xml_raises_NXAPIPostError_on_non_200_post_error(mock_pynxos_device_xml):
+    with pytest.raises(
+        NXAPIPostError, match="Invalid status code returned on NX-API POST"
+    ):
+        mock_pynxos_device_xml.config("logging history size 200")
+
+
+@pytest.mark.parametrize("mock_pynxos_device_xml", [200], indirect=True)
+def test_config_xml_raises_NXAPICommandError_on_200_config_error(
+    mock_pynxos_device_xml,
+):
+    with pytest.raises(
+        NXAPICommandError, match='The command "bogus command" gave the error'
+    ):
+        mock_pynxos_device_xml.config("bogus command")

test/nxos/conftest.py

@@ -2,10 +2,9 @@
 from builtins import super
 
 import pytest
+from napalm.base.mock import raise_exception
 from napalm.base.test import conftest as parent_conftest
-
 from napalm.base.test.double import BaseTestDouble
-
 from napalm.nxos import nxos
 
 
@@ -74,6 +73,8 @@ def show(self, command, raw_text=False):
             result = self.read_txt_file(full_path)
         else:
             result = self.read_json_file(full_path)
+            if "exception" in result:
+                raise_exception(result)
 
         return result
 

test/nxos/mocked_data/test_get_interfaces_ip/no_ipv6_support/show_ipv6_interface.json

@@ -1,3 +1,7 @@
 {
-    "exception": "nxapi_plumbing.api_client.NXAPIPostError"
-}
+    "exception": "napalm.nxapi_plumbing.errors.NXAPICommandError",
+    "kwargs": {
+        "command": "show ipv6 interface",
+        "message": ""
+    }
+}