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

Coriant Network Hardware Page. #6187

Merged
merged 24 commits into from Apr 12, 2017

Conversation

Projects
None yet
8 participants
@xbeaudouin
Contributor

xbeaudouin commented Mar 13, 2017

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.

  • Have you signed the Contributors agreement - please do NOT submit a pull request unless you have (signing the agreement in the same pull request is fine). Your commit message for signing the agreement must appear as per the docs.
  • Have you followed our code guidelines?

Testers

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

Coriant TNMS support

This will add a new page that allow Coriant Hardware to be seen and get the right status.

@mention-bot

This comment has been minimized.

Show comment
Hide comment
@mention-bot

mention-bot Mar 13, 2017

Thank you for submitting a PR @xbeaudouin! We have found the following @murrant, @laf and @InsaneSplash based on the history of these files to review this PR.

Thank you for submitting a PR @xbeaudouin! We have found the following @murrant, @laf and @InsaneSplash based on the history of these files to review this PR.

@xbeaudouin xbeaudouin changed the title from Coriant tnms1 to Coriant Network Hardware Page. Mar 13, 2017

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Mar 13, 2017

Member

I did mention in the other PR that it wouldn't be good to see mef-this, mef-that poller/discovery modules - this is what I mean. How comes this is separate, what is it actually giving the user?

Member

laf commented Mar 13, 2017

I did mention in the other PR that it wouldn't be good to see mef-this, mef-that poller/discovery modules - this is what I mean. How comes this is separate, what is it actually giving the user?

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Mar 13, 2017

Member

Also the sql schema file should be all on one line.

Member

laf commented Mar 13, 2017

Also the sql schema file should be all on one line.

Xavier Beaudouin
@xbeaudouin

This comment has been minimized.

Show comment
Hide comment
@xbeaudouin

xbeaudouin Mar 14, 2017

Contributor

@laf: the SQL is fixed. About MEF, this is constructor idenpendant stuff, totaly outside Coriant. So that's why this is yet another poller/discovery stuff.

Contributor

xbeaudouin commented Mar 14, 2017

@laf: the SQL is fixed. About MEF, this is constructor idenpendant stuff, totaly outside Coriant. So that's why this is yet another poller/discovery stuff.

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Mar 14, 2017

Member

What exactly does this bring? Could components be used?

Member

murrant commented Mar 14, 2017

What exactly does this bring? Could components be used?

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Mar 14, 2017

Member

That doesn't explain what this gives though?

Member

laf commented Mar 14, 2017

That doesn't explain what this gives though?

@xbeaudouin

This comment has been minimized.

Show comment
Hide comment
@xbeaudouin

xbeaudouin Mar 15, 2017

Contributor

Here some screen shot, that explain better the output.
capture d ecran 2017-03-15 a 16 01 25

Coriant TNMS is somewhat an proxy to several devices, this give us status to each devices and allow us to raize alerts :)

Contributor

xbeaudouin commented Mar 15, 2017

Here some screen shot, that explain better the output.
capture d ecran 2017-03-15 a 16 01 25

Coriant TNMS is somewhat an proxy to several devices, this give us status to each devices and allow us to raize alerts :)

@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Mar 16, 2017

Member

Do you think any other device os will benefit from this module?

If not, it might just make sense to put this in the os polling file includes/polling/os/coriant.inc.php

Member

murrant commented Mar 16, 2017

Do you think any other device os will benefit from this module?

If not, it might just make sense to put this in the os polling file includes/polling/os/coriant.inc.php

@Rosiak

This comment has been minimized.

Show comment
Hide comment
@Rosiak

Rosiak Mar 16, 2017

Contributor

@murrant
Wireless controllers? Not saying it directly could benefit from this MEC stuff, but the general approach could perhaps be used? List connected AP's, their state etc.

Contributor

Rosiak commented Mar 16, 2017

@murrant
Wireless controllers? Not saying it directly could benefit from this MEC stuff, but the general approach could perhaps be used? List connected AP's, their state etc.

Xavier Beaudouin added some commits Mar 17, 2017

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Mar 17, 2017

Member

Perhaps, this should either be made a little more generic or not be a module.

Member

murrant commented Mar 17, 2017

Perhaps, this should either be made a little more generic or not be a module.

@xbeaudouin

This comment has been minimized.

Show comment
Hide comment
@xbeaudouin

xbeaudouin Mar 17, 2017

Contributor

I am opened to suggestions. Coriant snmp stuff is really a mess IMHO

Contributor

xbeaudouin commented Mar 17, 2017

I am opened to suggestions. Coriant snmp stuff is really a mess IMHO

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Mar 22, 2017

Member

Maybe a module called hardware?

Member

laf commented Mar 22, 2017

Maybe a module called hardware?

@xbeaudouin

This comment has been minimized.

Show comment
Hide comment
@xbeaudouin

xbeaudouin Mar 22, 2017

Contributor

@laf: as you wish, I am opened to all suggestions on your side.

Contributor

xbeaudouin commented Mar 22, 2017

@laf: as you wish, I am opened to all suggestions on your side.

Xavier Beaudouin added some commits Mar 22, 2017

Xavier Beaudouin
Fix another conflict file
Merge branch 'master' of https://github.com/librenms/librenms into coriant_tnms1
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Mar 22, 2017

Member

Let's get a consensus on it first.

Member

laf commented Mar 22, 2017

Let's get a consensus on it first.

Xavier Beaudouin added some commits Mar 22, 2017

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Mar 22, 2017

Member

I feel like just putting this in the os polling module is correct it doesn't seem like it can be shared with other os.

Member

murrant commented Mar 22, 2017

I feel like just putting this in the os polling module is correct it doesn't seem like it can be shared with other os.

@xbeaudouin

This comment has been minimized.

Show comment
Hide comment
@xbeaudouin

xbeaudouin Mar 23, 2017

Contributor

Since Coriant SNMP implementation is mostly working only on Coriant (the snmp this stuff is kinda strange BTW), I don't think we can share anything on other OS / hardware...

Contributor

xbeaudouin commented Mar 23, 2017

Since Coriant SNMP implementation is mostly working only on Coriant (the snmp this stuff is kinda strange BTW), I don't think we can share anything on other OS / hardware...

@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Mar 23, 2017

Member

So you agree, move the module polling code to includes/polling/os/coriant.inc.php

Member

murrant commented Mar 23, 2017

So you agree, move the module polling code to includes/polling/os/coriant.inc.php

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Mar 23, 2017

Member

I personally don't as os module should be simply for that purpose - yes I know we have other cruft in now but that will need tidying up as well.

Member

laf commented Mar 23, 2017

I personally don't as os module should be simply for that purpose - yes I know we have other cruft in now but that will need tidying up as well.

@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Mar 23, 2017

Member

It seems like a reasonable place for something like this. I don't want a poller module for everything.

Member

murrant commented Mar 23, 2017

It seems like a reasonable place for something like this. I don't want a poller module for everything.

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Mar 23, 2017

Member

Neither do I but this seems like an excessive use of the os poller module.

Member

laf commented Mar 23, 2017

Neither do I but this seems like an excessive use of the os poller module.

@xbeaudouin

This comment has been minimized.

Show comment
Hide comment
@xbeaudouin

xbeaudouin Mar 24, 2017

Contributor

@laf: the Coriant TNMS SNMP is somewhat brain damage, I can provide in private the documentation of this. Since this PR is mostly the begining of the support.

Contributor

xbeaudouin commented Mar 24, 2017

@laf: the Coriant TNMS SNMP is somewhat brain damage, I can provide in private the documentation of this. Since this PR is mostly the begining of the support.

Xavier Beaudouin
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
Xavier Beaudouin
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@guilhermefigueiredo

This comment has been minimized.

Show comment
Hide comment
@guilhermefigueiredo

guilhermefigueiredo Mar 29, 2017

Hello!

How to do to test and add a coriant host in my librenms? I already did github-apply 6187 but apparently do not have the option yet .. remembering that we need the context in snmpv3, could they help me?

Hello!

How to do to test and add a coriant host in my librenms? I already did github-apply 6187 but apparently do not have the option yet .. remembering that we need the context in snmpv3, could they help me?

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Mar 29, 2017

Member

I've pushed an update to this to move the module into the os polling instead.

For me to approve this needs the devices table page moving to http://www.jquery-bootgrid.com/. We shouldn't have unpaginaged tables.

I also don't see why we'd have SqlSNMP(), looks like the function wrapper can be removed and we can just call the code.

Member

laf commented Mar 29, 2017

I've pushed an update to this to move the module into the os polling instead.

For me to approve this needs the devices table page moving to http://www.jquery-bootgrid.com/. We shouldn't have unpaginaged tables.

I also don't see why we'd have SqlSNMP(), looks like the function wrapper can be removed and we can just call the code.

@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Mar 30, 2017

Member

Thanks for the updates laf.

@xbeaudouin
I would like this to be converted to use "Components":
http://docs.librenms.org/Extensions/Component/

Member

murrant commented Mar 30, 2017

Thanks for the updates laf.

@xbeaudouin
I would like this to be converted to use "Components":
http://docs.librenms.org/Extensions/Component/

@xbeaudouin

This comment has been minimized.

Show comment
Hide comment
@xbeaudouin

xbeaudouin Mar 30, 2017

Contributor

@guilhermefigueiredo: you need to have access to TNMS system and enable SNMP
@laf: the "SqlNMP" function is a basis of the way Coriant store the data... eg :
capture d ecran 2017-03-14 a 15 00 54
(Yes this is like a database)
@murrant: Will have a look about Components, thanks for the hint.

Contributor

xbeaudouin commented Mar 30, 2017

@guilhermefigueiredo: you need to have access to TNMS system and enable SNMP
@laf: the "SqlNMP" function is a basis of the way Coriant store the data... eg :
capture d ecran 2017-03-14 a 15 00 54
(Yes this is like a database)
@murrant: Will have a look about Components, thanks for the hint.

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Mar 30, 2017

Member

@murrant I'm not a huge fan of components, it makes alerting that much more difficult for people to understand and use.

@xbeaudouin I don't see the relevance still. All you are doing is running an snmpwalk and looping through the data to insert/delete/update.

Member

laf commented Mar 30, 2017

@murrant I'm not a huge fan of components, it makes alerting that much more difficult for people to understand and use.

@xbeaudouin I don't see the relevance still. All you are doing is running an snmpwalk and looping through the data to insert/delete/update.

@xbeaudouin

This comment has been minimized.

Show comment
Hide comment
@xbeaudouin

xbeaudouin Mar 30, 2017

Contributor

@laf: Started to read the Components, I am not sure this can be used with Coriant SNMP. I have several snmp tables with relational data between them. The fact is I have to add all data into a DB to make join between them to get some more data. I have some documentation from Coriant that really give me an headache. I can show you in private

Contributor

xbeaudouin commented Mar 30, 2017

@laf: Started to read the Components, I am not sure this can be used with Coriant SNMP. I have several snmp tables with relational data between them. The fact is I have to add all data into a DB to make join between them to get some more data. I have some documentation from Coriant that really give me an headache. I can show you in private

@guilhermefigueiredo

This comment has been minimized.

Show comment
Hide comment
@guilhermefigueiredo

guilhermefigueiredo Mar 30, 2017

@xbeaudouin ,

I have access to TNMS and snmp is already configured (version 3), but I need to add the "context" (-n option in snmpwalk) or else I can not do the collection.

root@nms:/opt/librenms# snmpwalk -v3 -a MD5 -l authPriv -u Administrator -A passWorD -x AES -X passWorD X.X.Y.Y
Error in packet.
Reason: noAccess
root@nms:/opt/librenms#

root@nms:/opt/librenms# snmpwalk -v3 -a MD5 -n tnms -l authPriv -u Administrator -A passWorD -x AES -X passWorD X.X.Y.Y
SNMPv2-MIB::sysDescr.0 = STRING: P42022-P5173-C547-01-50VB hiT 7300 5.40 70.02.00 IC0220101160620 Copyright 2016 Coriant. All rights reserved.
SNMPv2-MIB::sysObjectID.0 = OID: SNMPv2-SMI::enterprises.42229.1.1.2863311536
DISMAN-EVENT-MIB::sysUpTimeInstance = Timeticks: (846062685) 97 days, 22:10:26.85
SNMPv2-MIB::sysContact.0 = STRING: eng
SNMPv2-MIB::sysName.0 = STRING: CORIANT-F
SNMPv2-MIB::sysLocation.0 = STRING: F-SITE
SNMPv2-MIB::sysServices.0 = INTEGER: 1
SNMPv2-MIB::sysORLastChange.0 = Timeticks: (0) 0:00:00.00
Error in packet.
Reason: noAccess
Failed object: SNMPv2-MIB::sysORLastChange.0

root@nms:/opt/librenms#

@xbeaudouin ,

I have access to TNMS and snmp is already configured (version 3), but I need to add the "context" (-n option in snmpwalk) or else I can not do the collection.

root@nms:/opt/librenms# snmpwalk -v3 -a MD5 -l authPriv -u Administrator -A passWorD -x AES -X passWorD X.X.Y.Y
Error in packet.
Reason: noAccess
root@nms:/opt/librenms#

root@nms:/opt/librenms# snmpwalk -v3 -a MD5 -n tnms -l authPriv -u Administrator -A passWorD -x AES -X passWorD X.X.Y.Y
SNMPv2-MIB::sysDescr.0 = STRING: P42022-P5173-C547-01-50VB hiT 7300 5.40 70.02.00 IC0220101160620 Copyright 2016 Coriant. All rights reserved.
SNMPv2-MIB::sysObjectID.0 = OID: SNMPv2-SMI::enterprises.42229.1.1.2863311536
DISMAN-EVENT-MIB::sysUpTimeInstance = Timeticks: (846062685) 97 days, 22:10:26.85
SNMPv2-MIB::sysContact.0 = STRING: eng
SNMPv2-MIB::sysName.0 = STRING: CORIANT-F
SNMPv2-MIB::sysLocation.0 = STRING: F-SITE
SNMPv2-MIB::sysServices.0 = INTEGER: 1
SNMPv2-MIB::sysORLastChange.0 = Timeticks: (0) 0:00:00.00
Error in packet.
Reason: noAccess
Failed object: SNMPv2-MIB::sysORLastChange.0

root@nms:/opt/librenms#

@xbeaudouin

This comment has been minimized.

Show comment
Hide comment
@xbeaudouin

xbeaudouin Mar 30, 2017

Contributor

@guilhermefigueiredo: hum... I don't use snmp v3 with them (mostly because our TNMS is behind a private network), did you try with v2c ?
If it work you can ask them to fix their software ....

Contributor

xbeaudouin commented Mar 30, 2017

@guilhermefigueiredo: hum... I don't use snmp v3 with them (mostly because our TNMS is behind a private network), did you try with v2c ?
If it work you can ask them to fix their software ....

@guilhermefigueiredo

This comment has been minimized.

Show comment
Hide comment
@guilhermefigueiredo

guilhermefigueiredo Mar 30, 2017

@xbeaudouin So I understand hiT 7300 does not work with snmp v2c, only with v3

@xbeaudouin So I understand hiT 7300 does not work with snmp v2c, only with v3

@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Mar 30, 2017

Member

I think the core design for components is good, but it could use some Polish.

Member

murrant commented Mar 30, 2017

I think the core design for components is good, but it could use some Polish.

@xbeaudouin

This comment has been minimized.

Show comment
Hide comment
@xbeaudouin

xbeaudouin Mar 31, 2017

Contributor

@guilhermefigueiredo: hum... Seems we don't same version of TNMS server. I have TNMS version 16.10.4.24.981 on a Linux server. What is yours ?

Contributor

xbeaudouin commented Mar 31, 2017

@guilhermefigueiredo: hum... Seems we don't same version of TNMS server. I have TNMS version 16.10.4.24.981 on a Linux server. What is yours ?

@guilhermefigueiredo

This comment has been minimized.

Show comment
Hide comment
@guilhermefigueiredo

guilhermefigueiredo Apr 3, 2017

@xbeaudouin my version is 16 on a windows 7, but is the TNMS-NCT I think this version is some limited. We are trying to see with the coriant how to release access using snmp v2c.

@xbeaudouin my version is 16 on a windows 7, but is the TNMS-NCT I think this version is some limited. We are trying to see with the coriant how to release access using snmp v2c.

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Apr 5, 2017

Member

Any update on this?

Imho, this needs the tables moving to bootgrid and the code moving out of the function. I know you say it will change later but from everything you said I'm just not sure how practical some of the additional code will be.

Member

laf commented Apr 5, 2017

Any update on this?

Imho, this needs the tables moving to bootgrid and the code moving out of the function. I know you say it will change later but from everything you said I'm just not sure how practical some of the additional code will be.

@xbeaudouin

This comment has been minimized.

Show comment
Hide comment
@xbeaudouin

xbeaudouin Apr 6, 2017

Contributor

@laf: what do you mean by bootgrid ? (I wasn't sure this is for me).

Contributor

xbeaudouin commented Apr 6, 2017

@laf: what do you mean by bootgrid ? (I wasn't sure this is for me).

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Apr 6, 2017

Member

Basically we want to have paginated tables so that we don't end up with 100's of items on one page. It's not that difficult to do.

See:

html/pages/peering/ix-list.inc.php
html/includes/table/ix-list.inc.php

If you get stuck I can probably do it for you.

Member

laf commented Apr 6, 2017

Basically we want to have paginated tables so that we don't end up with 100's of items on one page. It's not that difficult to do.

See:

html/pages/peering/ix-list.inc.php
html/includes/table/ix-list.inc.php

If you get stuck I can probably do it for you.

@xbeaudouin

This comment has been minimized.

Show comment
Hide comment
@xbeaudouin

xbeaudouin Apr 7, 2017

Contributor

@laf: I started to convert it. Mostly working, but I have issue with formating with nice colors the labels like in the screen shot. Since I am not a guru of javascript... :p

Contributor

xbeaudouin commented Apr 7, 2017

@laf: I started to convert it. Mostly working, but I have issue with formating with nice colors the labels like in the screen shot. Since I am not a guru of javascript... :p

Xavier Beaudouin added some commits Apr 7, 2017

@xbeaudouin

This comment has been minimized.

Show comment
Hide comment
@xbeaudouin

xbeaudouin Apr 7, 2017

Contributor

@laf: just finished the conversion into bootgrid :p

Contributor

xbeaudouin commented Apr 7, 2017

@laf: just finished the conversion into bootgrid :p

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
Xavier Beaudouin
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@murrant

Pulled in new schema checks and added db_schema.yaml update.

Looks good to me, thanks for all the hard work @xbeaudouin !

@murrant murrant added the Schema label Apr 11, 2017

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

scrutinizer-notifier Apr 11, 2017

The inspection completed: 1 updated code elements

The inspection completed: 1 updated code elements

@laf laf merged commit 4b09726 into librenms:master Apr 12, 2017

2 checks passed

Auto-Deploy Build finished.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Apr 12, 2017

Member

image

Member

laf commented Apr 12, 2017

image

@xbeaudouin xbeaudouin referenced this pull request Apr 12, 2017

Merged

Fix #6187 with some missing definitions. #6403

2 of 2 tasks complete

murrant added a commit that referenced this pull request Apr 12, 2017

Fix #6187 with some missing definitions. (#6403)
* Fix the definition. Should be mef instead of mef-evc

* Added Coriant TNMS Hardware page.

* Polling and discovery modules for TNMS Network Equipements.

* SQL in one line.

* Only show when OS is coriant

* Fix conflict file

* Conflict file

* Added missing COLLATE

* Refactor poller code for more data to be inserted into DB in the future.
Removed the discovery module and merged it into poller code.

* Removed tnms-nbi discovery also in the yaml

* Removing debug

* Code updates and fixes + schema rename

* bootstrapify the code.
Renamed the sql file

* Bloody tabs !

* Fix missing information for the TNMS #6187

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

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