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

Added Perle OS Support for IOLAN SCS #9866

Merged
merged 13 commits into from Feb 25, 2019

Conversation

Projects
None yet
3 participants
@esundberg
Copy link
Contributor

commented Feb 24, 2019

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
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.

esundberg added some commits Feb 24, 2019

There was an existing Perle OS, updated it to include the Perle IOLAN…
… SCS model .1.3.6.1.4.1.1966.12. Removed Files the script created that were no longer needed
There was an existing Perle OS, updated it to include the Perle IOLAN…
… SCS model .1.3.6.1.4.1.1966.12. Removed Files the script created that were no longer needed. Round 2
@CLAassistant

This comment has been minimized.

Copy link

commented Feb 24, 2019

CLA assistant check
All committers have signed the CLA.

@esundberg

This comment has been minimized.

Copy link
Contributor Author

commented Feb 24, 2019

Note the snmpsim file is named perle_iolan_scs.snmprec there was already a perle.snmprec for another device

@esundberg

This comment has been minimized.

Copy link
Contributor Author

commented Feb 24, 2019

I realize this might of been a mistake to put the snmprec file as perle_iolan_scs.snmprec when the os is call perle

How do you want me to handle this? Create a new device call perle_iolan_scs?

@PipoCanaja

This comment has been minimized.

Copy link
Contributor

commented Feb 24, 2019

Hi @esundberg
Thanx for your PR. First step would be to ensure that only the files linked to this perle change are added to the PR. Could you please correct the permissions so that the other files do not get added ?

In order to get the original files, you can do :

git checkout master bootstrap/cache/.gitignore 
git checkout master includes/definitions/discovery/perle.yaml
git checkout master logs/.gitignore
git checkout master rrd/.gitignore 
git checkout master storage/app/.gitignore 
git checkout master storage/app/public/.gitignore 
git checkout master storage/debugbar/.gitignore 
git checkout master storage/framework/cache/.gitignore  
git checkout master storage/framework/cache/data/.gitignore  
git checkout master storage/framework/sessions/.gitignore  
git checkout master storage/framework/testing/.gitignore  
git checkout master storage/framework/views/.gitignore  
git checkout master storage/logs/.gitignore  

For the discovery itself, I would only keep the sysObjectID and get rid of the sysDescr. 1966 includes all Perle devices anyway.

@esundberg

This comment has been minimized.

Copy link
Contributor Author

commented Feb 24, 2019

@PipoCanaja

I am getting a git error when i try to fix the .gitignore issue.

librenms@librenms-devel:~$ git pull upstream bootstrap/cache/.gitignore
fatal: Invalid refspec 'bootstrap/cache/.gitignore'


So the sysdesc was existing, it looks like someone previous added support for the perle MCR MGT:
https://www.perle.com/downloads/software/media-converter/perle-mcr-mgt-unix.mib

The device I am adding is Perle IOLAN SCS
https://www.perle.com/downloads/software/iolan/perle-sds.mib

I am not a MIB expert, but it looks like both use the same SNMP Enterprise object Identifier 1966.
perle OBJECT IDENTIFIER ::= { enterprises 1966 }

I think the we might be able to do this to catch there two different models with the folllow, but I want verify with you guys first.
Remove sysDescr section

  • sysObjectID:
  • .1.3.6.1.4.1.1966.12 #Perle IOLAN SCS
  • .1.3.6.1.4.1.1966.20 #Perle MCR-MGT
@PipoCanaja

This comment has been minimized.

Copy link
Contributor

commented Feb 24, 2019

Depending on what sensors,etc you add later, you might need to split perle into perle-mcr and perle-iolan if they are too different.
But for the time being, I would suggest to match ".1.3.6.1.4.1.1966." directly which would identify all perle devices, whatever their model is. From there, the existing test data for perle MCR and the replies from your perle-iolan will help us decide if we have to split or not.

@PipoCanaja

This comment has been minimized.

Copy link
Contributor

commented Feb 24, 2019

Could you remove the sysDescr part ? That should help Travis with the YAML validity check.

esundberg and others added some commits Feb 25, 2019

Update exiting includes/definitions/perle.yaml to catch all device by…
… Perle. Removed the .gitignore files from the branch
Restore the sysDescr
The existing testdata relies on this, as sysObjectID is not collected
@PipoCanaja

This comment has been minimized.

Copy link
Contributor

commented Feb 25, 2019

I had to restore the sysDescr check cause the actual Perle MCR testdata relies on it to succeed. Looks better now.

@PipoCanaja PipoCanaja changed the title Issue 9847 - Add Perle OS Support for IOLAN SCS Added Perle OS Support for IOLAN SCS Feb 25, 2019

@PipoCanaja PipoCanaja merged commit 8929cdb into librenms:master Feb 25, 2019

6 checks passed

Inspection Summary
Details
Node: analysis
Details
Travis CI - Pull Request Build Passed
Details
WIP Ready for review
Details
codeclimate All good!
Details
license/cla Contributor License Agreement is signed.
Details

@lock lock bot locked as resolved and limited conversation to collaborators Apr 26, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.