Fix exception when trying to get environmental data from certain Nexus devices #1133
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…es that postfix some json keys with the device series name (shorthand) in the show environment output, specifically under the powersup key.
…hat key is available, otherwise, fall back to the old method of getting the PSU's capacity by parsing the ps_model information.
There was a problem hiding this comment.
Some Nexus devices postfix the names of the keys with the device's series information, ie _n3k in the case for Nexus 3k devices in the show environment output.
Ewwww 🤢
Thanks for this patch @ironick09, looks good! Would you please be able to add a test case for this, to ensure compatibility on the long term?
|
FYI: we are planning to release NAPALM 3.0.0 soon. If you'd like your patch to be included in this version, please address the comments above. Thanks! |
|
I will get the tests added this morning. Thanks :) |
|
Hi @mirceaulinic, I have added tests / mock data for my changes, here are the results: (.venv)$ cat report.json | jq
{
"data": [
{
"type": "report",
"id": 1,
"attributes": {
"environment": {
"Python": "3.7.6",
"Platform": "Darwin-19.3.0-x86_64-i386-64bit"
},
"summary": {
"passed": 3,
"num_tests": 3,
"duration": 0.05885815620422363
},
"created_at": "2020-03-03 09:39:45.653403"
},
"relationships": {
"tests": {
"data": [
{
"id": 1,
"type": "test"
},
{
"id": 2,
"type": "test"
},
{
"id": 3,
"type": "test"
}
]
}
}
}
],
"included": [
{
"id": 1,
"type": "test",
"attributes": {
"name": "test/nxos/test_getters.py::TestGetter::test_get_environment[pre_7.0]",
"duration": 0.012677907943725586,
"run_index": 0,
"setup": {
"name": "setup",
"duration": 0.005444049835205078,
"outcome": "passed"
},
"call": {
"name": "call",
"duration": 0.0015869140625,
"outcome": "passed"
},
"teardown": {
"name": "teardown",
"duration": 0.0002028942108154297,
"outcome": "passed"
},
"outcome": "passed"
}
},
{
"id": 2,
"type": "test",
"attributes": {
"name": "test/nxos/test_getters.py::TestGetter::test_get_environment[nxos_3k]",
"duration": 0.0018322467803955078,
"run_index": 1,
"setup": {
"name": "setup",
"duration": 0.00022721290588378906,
"outcome": "passed"
},
"call": {
"name": "call",
"duration": 0.0012319087982177734,
"outcome": "passed"
},
"teardown": {
"name": "teardown",
"duration": 0.00014591217041015625,
"outcome": "passed"
},
"outcome": "passed"
}
},
{
"id": 3,
"type": "test",
"attributes": {
"name": "test/nxos/test_getters.py::TestGetter::test_get_environment[normal]",
"duration": 0.0019218921661376953,
"run_index": 2,
"setup": {
"name": "setup",
"duration": 0.00020694732666015625,
"outcome": "passed"
},
"call": {
"name": "call",
"duration": 0.0012021064758300781,
"outcome": "passed"
},
"teardown": {
"name": "teardown",
"duration": 0.0003058910369873047,
"outcome": "passed"
},
"outcome": "passed"
}
}
]
} |
|
Could you try re-running the job? It looks like it failed because of a connection error/timeout when installing the python packages. Thanks |
|
@ironick09 Re-started test... |
neelimapp
pushed a commit
to neelimapp/napalm
that referenced
this pull request
Mar 20, 2020
…s devices (napalm-automation#1133) * Fix bug when trying to get environmental information from Nexus devices that postfix some json keys with the device series name (shorthand) in the show environment output, specifically under the powersup key. * Use ot_capa to get the PSU's capacity for newer versions of nxos if that key is available, otherwise, fall back to the old method of getting the PSU's capacity by parsing the ps_model information. * Reformat line length * Run black formatter to fix CI issues * Add tests for nxos_3k (some software versions)
bharath-ravindranath
pushed a commit
to bharath-ravindranath/napalm
that referenced
this pull request
Apr 19, 2020
…s devices (napalm-automation#1133) * Fix bug when trying to get environmental information from Nexus devices that postfix some json keys with the device series name (shorthand) in the show environment output, specifically under the powersup key. * Use ot_capa to get the PSU's capacity for newer versions of nxos if that key is available, otherwise, fall back to the old method of getting the PSU's capacity by parsing the ps_model information. * Reformat line length * Run black formatter to fix CI issues * Add tests for nxos_3k (some software versions)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some Nexus devices postfix the names of the keys with the device's series information, ie
_n3kin the case for Nexus 3k devices in theshow environmentoutput. This causes aKeyErrorexception to be thrown when trying to retrieve environmental data from the device using theget_environment()method.The currently method assumes that the key in the resulting JSON structure is
TABLE_psinfoand subsequentlyROW_psinfowhich is not true for all Nexus devices.This fix uses the strategy deployed here to find the keys that start with
TABLE_psinfoandROW_psinfoand use those to index the JSON structure to find the environmental information.Additionally, newer nxos releases have changed the information provided in the JSON output of show environment, along with this, the format of the
ps_modelis not standard, so that relying on the format of the power supply's model name to be consistent enough to parse to determine the max capacity of the PSU is no longer reliable. Instead, newer nxos releases provide a new key,tot_capawhich specifies the total capacity of the PSU in Watts.The second fix/change here is to use the
tot_capakey to retrieve the PSU's total capacity if that key exists, otherwise we fall back to the old method.I only have Nexus 3000 and Nexus 9000 devices in my environment to test these changes on, and they work there.
I tested this on Nexus 3k's running the following NX-OS versions:
And on Nexus 9k's running the following NX-OS versions:
On my Nexus 3k devices, they output is slightly different than the Nexus 9k (below), specifically the
TABLE_psinfoandROW_psinfokeys are postfixed with_n3kand causes a KeyError exception to be thrown when attempting to index the JSON structure for the keyTABLE_psinfo(..andROW_psinfo):On my 9k Nexus device,
get_environment()can parse theTABLE_psinfoandROW_psinfokeys because they are not postfixed with the device's series number (_n9k) (but failed when attempting to parse the psmodel number to determine the capacity of the PSU):Here is some testing:
get_environment():get_environment():get_environment():get_environment():