Fix nxos sh int status to support tunnels#2308
Conversation
matt852
left a comment
There was a problem hiding this comment.
Recommendation: Changes Suggested
Note: @mjbear the below review is written by an AI agent specifically to help with this repo. I'm interested in your feedback on this. Additionally, I have concerns with hardcoding these values and possibly missing some, or needing to update this already growing list over time, and your PR may end up breaking things (as you also pointed out). Thoughts?
Thanks @mjbear — nice catch on the tunnel-section parsing. One suggestion before merge, building directly on the caveat you flagged in the PR body:
Expand the STATUS enumeration to cover known NXOS values not present in current fixtures. The new list is exactly complete vs. the 8 status values across the committed .raw files, so anything else — err-disabled, inactive, monitoring, suspended, linkFlapErr, or the un-truncated notconnect/xcvrAbsent/channelDown/noOperMembers forms shown on some platforms — now hits ^. -> Error where it previously parsed. The sibling cisco_nxos_show_interface_status_err-disabled.textfsm already uses err-disabled|linkFlapErr and cisco_ios_show_interfaces_status.textfsm uses err-disabled|disabled|connected|notconnect|inactive|up|down|monitoring|suspended; the union below keeps every existing test green and reduces the forward-compat risk:
In ntc_templates/templates/cisco_nxos_show_interface_status.textfsm, the STATUS Value declaration:
-Value STATUS (connected|notconnec|down|disabled|sfpAbsent|xcvrAbsen|channelDo|noOperMem)
+Value STATUS (connected|notconnec|notconnect|down|disabled|sfpAbsent|xcvrAbsen|xcvrAbsent|channelDo|channelDown|noOperMem|noOperMembers|err-disabled|inactive|monitoring|suspended|up|linkFlapErr)Thanks!
It seems that hard coding the
Hi @matt852! I recognize the potential longer term maintenance component if there are any other status values that we don't cover with existing test data. That would be unfortunate, but hopefully the community would step forward. It's possibly ignore this PR and the linked Issue ticket ... or fix it and live with a possibility of having to amend the template. 🤷 Should we add values to the regex for which we do not have test data? |
|
@mjbear Great point on the adding values without test data. Let me follow up on this with you soon. |
resolves #2307
resolves google/textfsm/issues/138
Fix Cisco NXOS show interface status template to add support for tunnel interfaces
Note: It's possible the Reason or Reason description could in some cases be two words (when basing it off Cisco doc output for other more specificshow interface statuscommands). I'm not guessing at what the output might be - I've asked the community member for information.Update: In some cases the Status is two words - this along with regex matching the "reason description" caused me to hard code the status options. If the existing test data doesn't account for all status options, this hard coding could cause breakage. 😞