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 HSRP state sensors for Cisco IOS on L3 switches #15809

Merged
merged 8 commits into from Feb 10, 2024

Conversation

rudybroersma
Copy link
Contributor

@rudybroersma rudybroersma commented Feb 8, 2024

This patch contains an addition to the IOS discovery YAML to add HSRP state sensor support.

eg:
sample 1:
image
sample 2:
image

This allows LibreNMS to detect HSRP changes in the network

My Cisco lab consists of Catalyst 3560 switches.

Special thanks to Christiaan de Jonge :)

Please note

Please read this information carefully. You can run ./lnms dev:check to check your code before submitting.

  • Have you followed our code guidelines?
  • If my Pull Request does some changes/fixes/enhancements in the WebUI, I have inserted a screenshot of it.
  • If my Pull Request makes discovery/polling/yaml changes, I have added/updated test data.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926
After you are done testing, you can remove the changes with ./scripts/github-remove. If there are schema changes, you can ask on discord how to revert.

@electrocret electrocret added the Needs Tests 🦄 https://docs.librenms.org/Developing/os/Test-Units/ label Feb 9, 2024
Copy link

github-actions bot commented Feb 9, 2024

Please add test data so we can ensure your change is not broken in the future.
Read the docs to find out how: https://docs.librenms.org/Developing/os/Test-Units

@rudybroersma
Copy link
Contributor Author

@electrocret Tests done!

@Jellyfrog Jellyfrog added the Device 🖥️ New or added device support label Feb 9, 2024
@PipoCanaja PipoCanaja removed the Needs Tests 🦄 https://docs.librenms.org/Developing/os/Test-Units/ label Feb 9, 2024
@PipoCanaja
Copy link
Contributor

Hi @rudybroersma
Looks good to me. Might be a little bit verbose on bigger chassis with a lot of Vlan Interfaces but it can be filtered by the users.

That being said, I think it would make sense to go a little bit further and create a dedicated module for HSRP/VRRP/etc that would complement the port page and replace these sensors.

Copy link
Contributor

@PipoCanaja PipoCanaja left a comment

Choose a reason for hiding this comment

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

LGTM

@rudybroersma
Copy link
Contributor Author

Hi @rudybroersma Looks good to me. Might be a little bit verbose on bigger chassis with a lot of Vlan Interfaces but it can be filtered by the users.

That being said, I think it would make sense to go a little bit further and create a dedicated module for HSRP/VRRP/etc that would complement the port page and replace these sensors.

Alright, just so I get this right.. You mean a new page in the tabs like Overview, Graph, Ports, VLANs, Neighbors, STP and alike?

I'll work on that, but might take me a while. Limited free time and all. If in the meantime this PR can be approved that would be great, I'll remove this sensor when my new code is ready.

@PipoCanaja
Copy link
Contributor

Hi @rudybroersma Looks good to me. Might be a little bit verbose on bigger chassis with a lot of Vlan Interfaces but it can be filtered by the users.
That being said, I think it would make sense to go a little bit further and create a dedicated module for HSRP/VRRP/etc that would complement the port page and replace these sensors.

Alright, just so I get this right.. You mean a new page in the tabs like Overview, Graph, Ports, VLANs, Neighbors, STP and alike?

2 angles:

I'll work on that, but might take me a while. Limited free time and all. If in the meantime this PR can be approved that would be great, I'll remove this sensor when my new code is ready.

Yep, sounds good.

@PipoCanaja PipoCanaja merged commit 374482d into librenms:master Feb 10, 2024
10 checks passed
@rudybroersma rudybroersma deleted the ciscohsrp branch February 12, 2024 21:51
@librenms-bot
Copy link

This pull request has been mentioned on LibreNMS Community. There might be relevant details there:

https://community.librenms.org/t/24-2-0-changelog/23721/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Device 🖥️ New or added device support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants