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

OSPF instances and missing mandatory fields fix attempt #15712

Merged
merged 5 commits into from Jan 15, 2024

Conversation

PipoCanaja
Copy link
Contributor

@PipoCanaja PipoCanaja commented Jan 8, 2024

We have a lot of columns that cannot be NULL, so we need to check that before trying to createOrUpdate

id int(10) unsigned NO PRI NULL auto_increment
ospfRouterId varchar(32) NO NULL
ospfAdminStat varchar(32) NO NULL
ospfVersionNumber varchar(32) NO NULL
ospfAreaBdrRtrStatus varchar(32) NO NULL
ospfASBdrRtrStatus varchar(32) NO NULL
ospfExternLsaCount int(10) unsigned NO NULL
ospfExternLsaCksumSum int(11) NO NULL
ospfTOSSupport varchar(32) NO NULL
ospfOriginateNewLsas int(10) unsigned NO NULL
ospfRxNewLsas int(10) unsigned NO NULL

fixes #15701

DO NOT DELETE THE UNDERLYING TEXT

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.

@PipoCanaja PipoCanaja changed the title dirty fix OSPF instances and missing mandatory fields fix attempt Jan 8, 2024
@PipoCanaja PipoCanaja self-assigned this Jan 8, 2024
Copy link
Contributor

@vhuk vhuk left a comment

Choose a reason for hiding this comment

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

duplicate ospfExternLsaCount, otherwise looks good to me.

@@ -87,6 +87,11 @@ public function poll(OS $os, DataStorageInterface $datastore): void
if (empty($ospf_entry['ospfRouterId'])) {
continue; // skip invalid data
}
foreach (['ospfRxNewLsas', 'ospfOriginateNewLsas', 'ospfAreaBdrRtrStatus', 'ospfExternLsaCount', 'ospfTOSSupport', 'ospfExternLsaCksumSum', 'ospfExternLsaCount', 'ospfASBdrRtrStatus', 'ospfVersionNumber', 'ospfAdminStat'] as $column) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ospfExternLsaCount is twice on the list

Copy link
Contributor Author

@PipoCanaja PipoCanaja Jan 8, 2024

Choose a reason for hiding this comment

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

On my way to fix that. I suppose double check is not necessary 🤣 Did you check it in your instance to confirm the issue is gone ?

LibreNMS/Modules/Ospf.php Outdated Show resolved Hide resolved
Copy link
Contributor

@vhuk vhuk left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thank you very much!

@PipoCanaja
Copy link
Contributor Author

Great, then ready for review !

Copy link
Member

@laf laf left a comment

Choose a reason for hiding this comment

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

LGTM

@laf laf merged commit f115de9 into librenms:master Jan 15, 2024
8 checks passed
@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ospf module fails with Dell N2048 if OSPF is not running
4 participants