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

device: Added state sensor support for HPE MSA devices #7808

Merged
merged 3 commits into from Dec 13, 2017

Conversation

Projects
None yet
4 participants
@laf
Member

laf commented Nov 27, 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.

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: #7719

Untested as the walk in the issue didn't have the data but from the MIB, it should be ok.

@laf laf added the Device 🖥 label Nov 27, 2017

@kkrumm1

This comment has been minimized.

Show comment
Hide comment
@kkrumm1

kkrumm1 Nov 27, 2017

Member

i can try the walk again if you like

Member

kkrumm1 commented Nov 27, 2017

i can try the walk again if you like

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Nov 27, 2017

Member

' .....Wz........' wth!

I'll need the full snmpwalk from this device to test further. Add .1.3.6.1.3.94.1. to the end of the walk command rather than .

Member

laf commented Nov 27, 2017

' .....Wz........' wth!

I'll need the full snmpwalk from this device to test further. Add .1.3.6.1.3.94.1. to the end of the walk command rather than .

@kkrumm1

This comment has been minimized.

Show comment
Hide comment
@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Nov 27, 2017

Member

I feel like I mentioned this before but connUnitSensorTable -> connUnitSensorEntry
INDEX { connUnitSensorUnitId, connUnitSensorIndex }
connUnitSensorUnitId = FcGlobalId ::= OCTET STRING (SIZE(16))

So, the index contains a string, maybe the mib is out of date?

Member

murrant commented Nov 27, 2017

I feel like I mentioned this before but connUnitSensorTable -> connUnitSensorEntry
INDEX { connUnitSensorUnitId, connUnitSensorIndex }
connUnitSensorUnitId = FcGlobalId ::= OCTET STRING (SIZE(16))

So, the index contains a string, maybe the mib is out of date?

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Nov 29, 2017

Member

@kkrumm1 do you have access to a newer mib? I can't find one online so not sure one exists.

Member

laf commented Nov 29, 2017

@kkrumm1 do you have access to a newer mib? I can't find one online so not sure one exists.

@kkrumm1

This comment has been minimized.

Show comment
Hide comment
@kkrumm1

kkrumm1 Nov 29, 2017

Member

hmm let me check HPE website.

Member

kkrumm1 commented Nov 29, 2017

hmm let me check HPE website.

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Nov 29, 2017

Member

It's got an older date but seems to have more in it, save it to /opt/librenms/mibs/hp/FCMGMT-MIB with this patch applied and try a discovery again.

Member

laf commented Nov 29, 2017

It's got an older date but seems to have more in it, save it to /opt/librenms/mibs/hp/FCMGMT-MIB with this patch applied and try a discovery again.

@kkrumm1

This comment has been minimized.

Show comment
Hide comment
@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Dec 1, 2017

Member

I'm pretty sure this is implemented as intended. You could change the snmpflags.

Member

murrant commented Dec 1, 2017

I'm pretty sure this is implemented as intended. You could change the snmpflags.

@kkrumm1

This comment has been minimized.

Show comment
Hide comment
@kkrumm1

kkrumm1 Dec 1, 2017

Member

@murrant how do we add SNMPflags?

Member

kkrumm1 commented Dec 1, 2017

@murrant how do we add SNMPflags?

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Dec 2, 2017

Member

I'm pretty sure this is implemented as intended.

@murrant you mean the PR seems correct?

Member

laf commented Dec 2, 2017

I'm pretty sure this is implemented as intended.

@murrant you mean the PR seems correct?

@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Dec 8, 2017

Member

No, I mean the data seems correct, even if it is less than useful.

Member

murrant commented Dec 8, 2017

No, I mean the data seems correct, even if it is less than useful.

@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Dec 8, 2017

Member

Set the snmp options to -OQUsbe That should store the whole oid after as the index right?

Member

murrant commented Dec 8, 2017

Set the snmp options to -OQUsbe That should store the whole oid after as the index right?

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Dec 8, 2017

Member

@kkrumm1 can you retry this patch please and post the output of ./discovery.php -h HOSTNAME -d -m sensors if it doesn't work still please.

Member

laf commented Dec 8, 2017

@kkrumm1 can you retry this patch please and post the output of ./discovery.php -h HOSTNAME -d -m sensors if it doesn't work still please.

@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Dec 9, 2017

Member

Oopsie, I missed the -Ob option above, please try with my correction @kkrumm1.

Member

murrant commented Dec 9, 2017

Oopsie, I missed the -Ob option above, please try with my correction @kkrumm1.

@scrutinizer-notifier

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

scrutinizer-notifier Dec 9, 2017

The inspection completed: No new issues

scrutinizer-notifier commented Dec 9, 2017

The inspection completed: No new issues

@kkrumm1

This comment has been minimized.

Show comment
Hide comment
@kkrumm1

kkrumm1 Dec 9, 2017

Member

@murrant @laf when i try and apply the patch i get this error on the mib
image

Member

kkrumm1 commented Dec 9, 2017

@murrant @laf when i try and apply the patch i get this error on the mib
image

@murrant

This comment has been minimized.

Show comment
Hide comment
@murrant

murrant Dec 9, 2017

Member

@kkrumm1 you deleted the file?

Member

murrant commented Dec 9, 2017

@kkrumm1 you deleted the file?

@kkrumm1

This comment has been minimized.

Show comment
Hide comment
@kkrumm1

kkrumm1 Dec 9, 2017

Member

@murrant I think I did back when I was testing the mibs out. Added it back :)

Member

kkrumm1 commented Dec 9, 2017

@murrant I think I did back when I was testing the mibs out. Added it back :)

@kkrumm1

This comment has been minimized.

Show comment
Hide comment
@kkrumm1

kkrumm1 Dec 9, 2017

Member

Progress yay!

Looks like it added some
image

But not all the other sensors show up in the web ui.

@laf @murrant https://p.libren.ms/view/8a82c31a

Member

kkrumm1 commented Dec 9, 2017

Progress yay!

Looks like it added some
image

But not all the other sensors show up in the web ui.

@laf @murrant https://p.libren.ms/view/8a82c31a

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Dec 9, 2017

Member

Check logs/librenms.log after running it, lots of those sensors are trying to be added so I expect it's a restriction on the db.

Member

laf commented Dec 9, 2017

Check logs/librenms.log after running it, lots of those sensors are trying to be added so I expect it's a restriction on the db.

@kkrumm1

This comment has been minimized.

Show comment
Hide comment
@kkrumm1

kkrumm1 Dec 9, 2017

Member

no errors in the log but here is the debug for discovery https://p.libren.ms/view/8a82c31a

Member

kkrumm1 commented Dec 9, 2017

no errors in the log but here is the debug for discovery https://p.libren.ms/view/8a82c31a

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Dec 9, 2017

Member

That's got lots of

INSERT INTO `sensors`

Which says it's trying to add them. Run a few of those inserts to test.

Member

laf commented Dec 9, 2017

That's got lots of

INSERT INTO `sensors`

Which says it's trying to add them. Run a few of those inserts to test.

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Dec 9, 2017

Member

Yeah's the index size being too big.

Member

laf commented Dec 9, 2017

Yeah's the index size being too big.

@kkrumm1

This comment has been minimized.

Show comment
Hide comment
@kkrumm1

kkrumm1 Dec 11, 2017

Member

Yeah's the index size being too big.

any way to fix this?

Member

kkrumm1 commented Dec 11, 2017

Yeah's the index size being too big.

any way to fix this?

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Dec 11, 2017

Member

Run this and see if it works for you:

ALTER TABLE sensors CHANGE sensor_index sensor_index VARCHAR( 128 );

Member

laf commented Dec 11, 2017

Run this and see if it works for you:

ALTER TABLE sensors CHANGE sensor_index sensor_index VARCHAR( 128 );

@kkrumm1

This comment has been minimized.

Show comment
Hide comment
@kkrumm1

kkrumm1 Dec 11, 2017

Member

@laf that did the trick :)

image

Member

kkrumm1 commented Dec 11, 2017

@laf that did the trick :)

image

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Dec 11, 2017

Member

Ace. I'm going to move that schema change to PR #7714 so we only deploy one schema change to sensors table.

Member

laf commented Dec 11, 2017

Ace. I'm going to move that schema change to PR #7714 so we only deploy one schema change to sensors table.

@murrant murrant merged commit 569d456 into librenms:master Dec 13, 2017

2 checks passed

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

@laf laf deleted the laf:device/issue-7719 branch Dec 13, 2017

@lock

This comment has been minimized.

Show comment
Hide comment
@lock

lock bot May 16, 2018

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

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.