Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add hp_comware_display_bgp_peer_ipv4 #2020

Merged

Conversation

CursedEnvy
Copy link
Contributor

Adding a template to read out the bgp ipv4 neighbours on a comware router
(it is also possible to use vprn-instance {instance_name} after the commando, output is the same (except for the neighbours of course))

Test file is the data as shown on a router (when I use my script to read out the config, the new line and spaces before BGP local router are gone, hence the use of * to cover this)
I choose to Filldown the top variables, because I noticed this was the case on the IOS template as well

Had to get a lot of help from a colleague as my first steps creating something from nothing, I am not good at regex, yet :)

…uter (it is also possible to use vprn-instance {instance_name} after the commando, output is the same (except for the neighbours of course))

Test file is the data as shown on a router (when I use my script to read out the config, the new line and spaces before BGP local router are gone, hence the use of * to cover this)
I choose to Filldown the top variables, because I noticed this was the case on the IOS template as well
@@ -0,0 +1,26 @@
Value Filldown LOCAL_ROUTER_ID (\d{1,3}(?:\.\d{1,3}){3})
Value Filldown LOCAL_AS_NUMBER (\d+)
Value Filldown TOTAL_PEERS (\d+)
Copy link
Contributor

@mjbear mjbear Feb 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edited:
Does it make sense to capture the Total Peers count?
(We'd already know how many peers based on the length of the list that is returned.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coincidentally had an discussion on that with my colleague yesterday, in the end I looked at some ios show ip bgp templates and saw them do this, but I still agree it felt weird and indeed now that you mention it, redundant. He was also in favour of leaving it out.

Only thing I now start to wonder is how will this handle multiple AS neighbours, maybe I'll have to search a test case today.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: actually don't think we have such a case of multiple AS neighbours anywhere or at least I'll be searching a needle in a haystack
Anyway, I removed the total and established peers!

Value Filldown LOCAL_ROUTER_ID (\d{1,3}(?:\.\d{1,3}){3})
Value Filldown LOCAL_AS_NUMBER (\d+)
Value Filldown TOTAL_PEERS (\d+)
Value Filldown PEERS_ESTABLISHED (\d+)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to Total Peers count, is the Established Peer count important since the State is already being captured?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, can be left out and handled for example with a property if you turn the dict into a basemodel later.

@mjbear
Copy link
Contributor

mjbear commented Feb 12, 2025

@CursedEnvy
Once we get through the above discussion items, I have some other thoughts.
Thank you.

Accepting mjbear his change to make the AS variable more descriptive

Co-authored-by: Michael Bear <[email protected]>
@CursedEnvy
Copy link
Contributor Author

@CursedEnvy Once we get through the above discussion items, I have some other thoughts. Thank you.

Sure, thank you for your input always, I am happy to hear your thoughts!

Accepting improvement to leave out state transition from mjbear

Co-authored-by: Michael Bear <[email protected]>
@jvanderaa jvanderaa merged commit 14c5e22 into networktocode:master Feb 13, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants