Fix new format of cisco_ios show cdp neigh detail - #2260
Conversation
… options - and add tests
|
@Ardeck While not necessarily "bad", there is quite a bit of change here to the data structure. This non-backwards compatible nature of things constitutes a breaking change (example definition). We want to also keep in mind to maintain naming parity with LLDP for example. Admittedly I haven't combed through each piece of your PR yet. There might be a middle ground where we can reduce the level of incompatibility (as we're a ways off from a major release where breaking changes could be included). |
Hello, I completely understand, I hesitated before touching it but after I started, I tried to get the most of it. A ) New Format, Values
-TLV /Power Power is displayed differently, I did not find "Power Available TLV:"
I am not sure in which version these change was made are the previous template is probably very old. -Management address section was not captured B) Values changed
It was creating conflicts as Interface IP and Management IP can be different cf. 4.raw But there is confusion between IPv4 and IPv6 Same the regex for the value can be preswerved to the old values C) Value removed NEIGHBOR_DESCRIPTION is more problematic as I completely rewrote it for several reasons :
|
jvanderaa
left a comment
There was a problem hiding this comment.
I think the rest of these are making sense on the names, we may have to go update some of the other templates as we get this one merged in.
I'm a little worried about the NEIGHBOR_DESCRIPTION getting broken down, will we need to be updating this template quite a bit as new devices that come online since it is all free form fields.
| Value CAPABILITIES (.+?) | ||
| Value INTERFACE_IP (\d+\.\d+\.\d+\.\d+) | ||
| Value List INTERFACE_IPV6 ([0-9a-fA-F:]+) | ||
| Value MGMT_IP_ADDRESS (\d+\.\d+\.\d+\.\d+) |
There was a problem hiding this comment.
I'd like to keep the MGMT_ADDRESS in lock step with LLDP neighbor outputs as well. So that these remain consistent across neighbor templates.
There was a problem hiding this comment.
I'd like to keep the MGMT_ADDRESS in lock step with LLDP neighbor outputs as well. So that these remain consistent across neighbor templates.
I have reverted the breaking changes. At least the naming is not critical.
@Ardeck My reason for asking is to see if I have access to hardware that demonstrates these new format changes. Thank you for details! |
I have mainly tested on IOS-XE version 17.6.5+, mainly various model of cat9K There are also various neighbor, mostly Cisco other catalyst and Nexus but also other vendors. |
| version: | ||
| - "Technical Support: http://www.cisco.com/techsupport" | ||
| - "Copyright (c) 1986-2010 by Cisco Systems, Inc." | ||
| - "Compiled Mon 15-Mar-10 23:32 by prod_rel_team" |
There was a problem hiding this comment.
This isn't version information.
| ^\s*${SOFTWARE}\s*,\s*${SOFTWARE_FAMILY}\s*,\s*[^,]*\bVersion\s*:?\s*${SOFTWARE_VERSION}\s*,*\s*.*$$ | ||
| ^\s*${SOFTWARE}\s*,*\s*${SOFTWARE_FAMILY}\s*,*s*[^,]*\bVersion\s*:?\s*${SOFTWARE_VERSION}\s*,*\s*.*$$ | ||
| # get whatever is provided | ||
| ^${VERSION} |
There was a problem hiding this comment.
Something seems a little off.
Plus, there are three capture groups with version in them.
VTP_VERSIONstands on its own, so no issue there- There's
SOFTWARE_VERSIONandVERSIONthat are possibly duplicates - The LLDP equivalent has capture group names with abbreviations (not sure how much normalization we'd do there -- depending on community preference)
| # Nexus Neighbor provides Serial Number | ||
| ^Device ID\s*:\s*${NEIGHBOR_NAME}\s*\(${SERIAL}\)\s*$$ |
There was a problem hiding this comment.
@Ardeck
I looked for a Nexus or NX-OS neighbor in the test data, but didn't find one.
Did I miss it?
matt852
left a comment
There was a problem hiding this comment.
Recommendation: Changes Suggested
Breaking Change: No
Thanks @Ardeck — this is a big usability upgrade for the CDP detail template, and the added fixtures cover a nice range of platforms. A few suggestions before merge:
-
Fix the
,*s*typo in the secondNewVersionsoftware rule. It matches optional literalscharacters where whitespace was intended — and it's the rule doing the actual software capture today. Inntc_templates/templates/cisco_ios_show_cdp_neighbors_detail.textfsm:- ^\s*${SOFTWARE}\s*,*\s*${SOFTWARE_FAMILY}\s*,*s*[^,]*\bVersion\s*:?\s*${SOFTWARE_VERSION}\s*,*\s*.*$$ + ^\s*${SOFTWARE}\s*,*\s*${SOFTWARE_FAMILY}\s*,*\s*[^,]*\bVersion\s*:?\s*${SOFTWARE_VERSION}\s*,*\s*.*$$
-
Remove the unreachable second
^Versionrule inStart. It's identical to the rule above it, so TextFSM can never reach it:^Version\s*:\s*$$ -> GetVersion - ^Version\s*:\s*$$ -> NewVersion -
Rename
INTERFACE_IPV6toIPV6_ADDRESSES. The data model (docs/dev/data_model.md) standardizesIPV6_ADDRESSESfor lists of IPv6 addresses. The Value declaration:-Value List INTERFACE_IPV6 ([0-9a-fA-F:]+) +Value List IPV6_ADDRESSES ([0-9a-fA-F:]+)
The matching rule in
ParseIP:- ^\s*IPv6 address:\s*${INTERFACE_IPV6}\s*\(.*\)\s*$$ + ^\s*IPv6 address:\s*${IPV6_ADDRESSES}\s*\(.*\)\s*$$
And in the fixtures:
- interface_ipv6: [] + ipv6_addresses: []
Apply the same key rename to every other
interface_ipv6:line in all five.ymlfiles. -
Confirm
VTP_VERSIONshould not be renamed toCDP_VERSION. I had this in the Claude review output but would like to confirm it is accurate before suggesting you change it: "The rule feeding it matchesadvertisement version: 2, which is the CDP protocol version the neighbor advertises — not anything VTP-related — so the current name will mislead consumers."
If this is accurate, the following changes are needed. Otherwise let me know and I'll update the model.
-Value VTP_VERSION (\d+)
+Value CDP_VERSION (\d+)The matching rule in Start:
- ^advertisement\s+version\s*:\s*${VTP_VERSION}\s*$$
+ ^advertisement\s+version\s*:\s*${CDP_VERSION}\s*$$And rename every vtp_version: key to cdp_version: in all five .yml files.
-
End the new states with
^. -> Error. Perdocs/dev/dev_parser.md, each state should end with^. -> Errorso unaccounted-for output fails loudly instead of being silently swallowed (e.g. a record separator reached while still inParseMgmtIPwould currently eat the following record). Add as the last rule ofParseIP,NewVersion,ParseMgmtIP, andPwrAvail— for example inParseIP:^\s*Platform\s*:\s*${PLATFORM}\s*,\s*Capabilities\s*:\s*${CAPABILITIES}\s*$$ -> Start + ^. -> Error(
GetVersiondoesn't need it since${NEIGHBOR_DESCRIPTION}already matches every line.) This passes against all five of your fixtures as-is. -
SERIALnever captures in any fixture. The^Device ID\s*:\s*${NEIGHBOR_NAME}\s*\(${SERIAL}\)\s*$$rule isn't exercised —serialis empty in all 23 fixture rows. Could you add a raw sample with aDevice ID: <name>(<serial>)neighbor (e.g. a Nexus peer), or drop the rule until real output is available? -
The software fields only populate for legacy two-line version headers. Because
GetVersionconsumes the first line of the version block asNEIGHBOR_DESCRIPTION, the modern single-line header (e.g. your 17.9.5 samples) never reaches theNewVersionsoftware rules —software,software_family, andsoftware_versioncome out empty for exactly the outputs this PR targets, andsoftware_familyis empty in every row. It's worth either restructuring so those fields populate consistently, or dropping them from this PR until they can be captured reliably.
Thanks!
(reviewed with Claude)
… options - and add tests
Summary
Update
cisco_ios_show_cdp_neighbors_detail.textfsmto handle new format and many new fieldsChanges
ntc_templates/templates/indexfor correct mapping.rawand.ymltest cases undertests/cisco_ios/show_cdp_neighbors_detail/Motivation
Current template is missing many parameters.
Validation
poetry run invoke testslocally — all tests pass..rawsamples.Notes
Scope limited to Cisco IOS show cdp neighbors detail.