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

Major Processors rewrite #8066

Merged
merged 8 commits into from Feb 5, 2018

Conversation

Projects
None yet
3 participants
@murrant
Member

murrant commented Jan 9, 2018

If you are having issues with this change, make sure you update again, a fix has been merged: #8202

Generic changes

YAML definition support!
HOST-RESOURCES-MIB is now only tried if no other processors are found. (before was always ran)
UCD-SNMP-MIB is tried if if HOST-RESOURCES-MIB finds no processors (before was never tried, except manually selected os)
More generic and reusable discovery/poller module infrastructure.

Processor types that are fixed

bdcom
binos
riverbed
saf-integra
ftos/dnos C Series
powerconnect Some models (changed index too)
enterasys (changed index too)
zynos (maybe)

New support added

dnos Z Series

Processors that will lose historical data

You can restore the old data by renaming the file to the new file name after the change.

Changed index

fiberhome

Changed to standard hr mib

axos
calix
dsm

Changed to standard ucd mib

hpblmos

Missing test data

vrp

  • Most have guessed data based on previous processor discovery files. This is usually indicated by usage % of 42.

DO NOT DELETE THIS TEXT

Please note

Please read this information carefully. You can run ./scripts/pre-commit.php to check your code before submitting.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926

fixes: #8098

@murrant murrant added the Blocker 🚫 label Jan 9, 2018

@TheMysteriousX TheMysteriousX referenced this pull request Jan 10, 2018

Merged

Add support for ArbOS #8055

1 of 1 task complete
@laf

This comment has been minimized.

Member

laf commented Jan 11, 2018

I'll run this overnight to test.

One question, is this worth implementing (and in wireless as well) a cleanup function so we can run either when someone disables a module in the webui (or maybe just when poller next runs) which will drop the data from the db for this module and the device?

@laf

This comment has been minimized.

Member

laf commented Jan 11, 2018

I have just merged in #7850 which added more cpu data so this will need to be taken in to account here. Sorry about that :(

@murrant

This comment has been minimized.

Member

murrant commented Jan 12, 2018

Sure we can do that, but lets keep it separate, this PR is already way to large.

@laf

This comment has been minimized.

Member

laf commented Jan 12, 2018

I'll get some debug info soon but:

  | 2018-01-12 09:08:15 |   | Processor added: type hr index 196608 descr Intel Xeon E3-12xx v2 (Ivy Bridge)
-- | -- | -- | --
  | 2018-01-12 09:08:15 |   | Processor added: type hr index 196609 descr Intel Xeon E3-12xx v2 (Ivy Bridge)
  | 2018-01-12 00:33:27 |   | Processor Removed: hr 196608 Intel Xeon E3-12xx v2 (Ivy Bridge)
  | 2018-01-12 00:33:27 |   | Processor Removed: hr 196609 Intel Xeon E3-12xx v2 (Ivy Bridge)

Latest entry is me switching back to master.

image

@laf

This comment has been minimized.

Member

laf commented Jan 13, 2018

Helpful at all? https://p.libren.ms/view/1071c01c

It's just a standard linux vm so nothing special

@murrant

This comment has been minimized.

Member

murrant commented Jan 13, 2018

@laf Seems fine until this:
SQL[DELETE T FROM `processors` T LEFT JOIN `devices` ON `devices`.`device_id` = T.`device_id` WHERE `devices`.`device_id` IS NULL]

@murrant murrant changed the title from WIP Processor refactor to Major Processors rewrite Jan 14, 2018

@murrant murrant removed the Blocker 🚫 label Jan 14, 2018

@laf

This comment has been minimized.

Member

laf commented Jan 14, 2018

I'll run this overnight.

One big blocker for me is the loss in data, we shouldn't be doing that to users. If it's being done to tidy things up then we should allow those OSs that would lose data to stick with slightly less than ideal code to keep the historical data until such time we can find a way to rename files for all users.

It's definitely an amazing step forward so don't let the above annoy you, it's just we set out not to break installs where we can and this seems like it's not a small break for quite a few users.

HOST-RESOURCES-MIB is now only tried if no other processors are found. (before was always ran)

Do you think we might hit instances where the old way was required or is that just most likely duplicate processors that were discovered previously.

@murrant

This comment has been minimized.

Member

murrant commented Jan 15, 2018

Renaming sounds fine, but I don't want it in the poller like it has been done previously. Perhaps another process in the daily.sh. I have minimized the amount of rrd name changes.
Reasons:

  • avaya-ers, sgos: zero-padded the index and are in a table, it seemed unreasonable to add a lot of code/yaml complexity for these two
  • fiberhome: processors are in a table, was previously hardcoded with two processors and with a mis-matched index
  • The rest are due to migrating to standard code instead of custom code.

I saw very little evidence that any of our os code implemented both custom HR and a custom MIB. I don't expect anyone will lose anything here.

@laf

This comment has been minimized.

Member

laf commented Jan 15, 2018

Renaming sounds fine, but I don't want it in the poller like it has been done previously. Perhaps another process in the daily.sh. I have minimized the amount of rrd name changes.

Daily would be fine running alongside something like the schema updates. The biggest issue is rrdtool that now supports rrdcached without a shared nfs meaning we have no direct access to files.

If you're adamant that the best way forward with those OS' is to break the data we store then you'll need to do a notification to users warning them of this so they can manually rename if needs be.

@murrant murrant referenced this pull request Jan 17, 2018

Closed

pre-commit.php issues #8098

1 of 5 tasks complete
@laf

This comment has been minimized.

Member

laf commented Jan 25, 2018

Is this now ready to go? Are we doing a notification to warn people some data is expected to be lost?

@murrant

This comment has been minimized.

Member

murrant commented Jan 25, 2018

I am fine with a notification. Based on my testing very few should be affected negatively. I'll work up a notification, how much lead time? A week?

@laf

This comment has been minimized.

Member

laf commented Jan 26, 2018

~5 days?

Also, could you add docs for adding processors via yaml?

murrant added a commit to librenms/librenms.github.io that referenced this pull request Jan 28, 2018

@laf laf referenced this pull request Jan 29, 2018

Merged

StoneOs improvements #8155

1 of 1 task complete
@murrant

This comment has been minimized.

Member

murrant commented Jan 30, 2018

Merge Feb 5.

murrant added some commits Nov 30, 2017

Extract DiscoveryItem and move some things to better places.
Extract model class
Fix up model construction.  I have problem with construction...
Makeshift model working.  Switch constructor to factory.  discover() and create()
Support legacy discovery.
Remove uneeded custom pollers
Remove netonix custom detection as we try ucd on all os now.
Add a few yaml procs.  Fix a couple things.
More processor discovery conversions
Move Calix e7 to standard hrProcessorLoad, but it doesn't fully implement the HR-MIB, move things around to make it work.
Add a few yaml procs.  Fix a couple things. Correct some stupid mib stuff.
Move more, drop php 5.3
Add netscaler which uses string indexes.  Port fiberhome to yaml and use skip_values
More conversions.  BroadcomProcessorUsage Trait
Serveriron and Ironware share some mibs.  Create a common abstract os for them.
Add yaml support for mib specification in each data entry
Make legacy discover_processor() set 0 for hrDeviceIndex

Untangle Dell switch OS processors

Use use shared OS for groups if they don't have a specific group.
fix silly mib mistake

Make index optional

Move HR and UCD to Traits and out of Processor.
@scrutinizer-notifier

This comment has been minimized.

scrutinizer-notifier commented Jan 30, 2018

The inspection completed: 724 Issues, 36 Patches

@laf laf added the Blocker 🚫 label Jan 31, 2018

@laf laf added the Refactor label Jan 31, 2018

@murrant murrant removed the Blocker 🚫 label Feb 5, 2018

@murrant murrant merged commit 11147d3 into librenms:master Feb 5, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@murrant murrant deleted the murrant:processor-class branch Feb 5, 2018

@nenad007 nenad007 referenced this pull request Feb 6, 2018

Closed

All Device Processors got Removed today #8205

5 of 5 tasks complete

inetAnt added a commit to criteo-forks/librenms that referenced this pull request Mar 19, 2018

Major Processors rewrite (librenms#8066)
* Extract DiscoveryItem and move some things to better places.
Extract model class
Fix up model construction.  I have problem with construction...
Makeshift model working.  Switch constructor to factory.  discover() and create()
Support legacy discovery.
Remove uneeded custom pollers
Remove netonix custom detection as we try ucd on all os now.
Add a few yaml procs.  Fix a couple things.
More processor discovery conversions
Move Calix e7 to standard hrProcessorLoad, but it doesn't fully implement the HR-MIB, move things around to make it work.
Add a few yaml procs.  Fix a couple things. Correct some stupid mib stuff.
Move more, drop php 5.3
Add netscaler which uses string indexes.  Port fiberhome to yaml and use skip_values
More conversions.  BroadcomProcessorUsage Trait
Serveriron and Ironware share some mibs.  Create a common abstract os for them.
Add yaml support for mib specification in each data entry
Make legacy discover_processor() set 0 for hrDeviceIndex

Untangle Dell switch OS processors

Use use shared OS for groups if they don't have a specific group.
fix silly mib mistake

Make index optional

Move HR and UCD to Traits and out of Processor.

* forgot to update the fortiswitch index

* Make sgos and avaya-ers match the old index.

* fix comware test data

* fix merge errors

* fix dsm and remove pointless empty modules

* file not found exception is in the wrong place.

* Updated processor development docs
@lock

This comment has been minimized.

lock bot commented May 16, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed.

@lock lock bot locked as resolved and limited conversation to collaborators May 16, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.