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

Add quanta lb6m #6816

Merged
merged 16 commits into from Jun 13, 2017

Conversation

Projects
None yet
6 participants
@segfault
Contributor

segfault commented Jun 10, 2017

Adding Quanta LB6M stats.

cpu usage, mempool, fan speed, psu status, and temperature

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

segfault added some commits Jun 9, 2017

initial quanta cpu, fan, temp and mem pool support.
ubnt edgeswitch is using the same broadcom fastpath core so the fetches are the same.
adding the lb6m power supply state
fixed the comment in the fanspeed file
@mention-bot

This comment has been minimized.

Show comment
Hide comment
@mention-bot

mention-bot Jun 10, 2017

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

mention-bot commented Jun 10, 2017

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

@CLAassistant

This comment has been minimized.

Show comment
Hide comment
@CLAassistant

CLAassistant Jun 10, 2017

CLA assistant check
All committers have signed the CLA.

CLAassistant commented Jun 10, 2017

CLA assistant check
All committers have signed the CLA.

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment

LibreNMS-CI commented Jun 10, 2017

Auto-Deploy finished, Test PR at http://6816.ci.librenms.org or https://6816.ci.librenms.org

Show outdated Hide outdated includes/discovery/sensors/state/quanta.inc.php
}
}
$idx = 0;

This comment has been minimized.

@segfault

segfault Jun 10, 2017

Contributor

I'm sure there's a better way for me to accomplish this. Any advice would be great.

@segfault

segfault Jun 10, 2017

Contributor

I'm sure there's a better way for me to accomplish this. Any advice would be great.

@laf

Looks great overall. I've put some comments in but the general is:

Try not to use snmp_walk() and use snmpwalk_cache_* instead. It gives you better formatted data.

Use named OIDs instead of numerical in all the snmp calls if you can, it makes reading the code and what data to expect easier.

For the mibs, you've got a lot of what's already available in the main mibs folder, like bgp.my (BGP4-MIB). You will need to remove all of those.

Thanks for submitting the PR :)

Show outdated Hide outdated includes/discovery/mempools/quanta.inc.php
if ($device['os'] == "quanta") {
d_echo('Quanta Memory:');
//QUANTA-LB6M-REF-MIB::agentSwitchCpuProcessMemFree
$avail = snmp_get($device, '.1.3.6.1.4.1.4413.1.1.1.1.4.1.0', '-Oqv');

This comment has been minimized.

@laf

laf Jun 10, 2017

Member

You should use the name and MIB seeing as you have it. Same goes for throughout the code, makes reading what it's doing easier.

@laf

laf Jun 10, 2017

Member

You should use the name and MIB seeing as you have it. Same goes for throughout the code, makes reading what it's doing easier.

Show outdated Hide outdated includes/discovery/sensors.inc.php
@@ -37,6 +37,11 @@
include 'includes/discovery/sensors/state/hp.inc.php';
}
if ($device['os'] == 'quanta') {

This comment has been minimized.

@laf

laf Jun 10, 2017

Member

You don't need to have this four lines of text. Those files will be pulled in on discovery.

@laf

laf Jun 10, 2017

Member

You don't need to have this four lines of text. Those files will be pulled in on discovery.

Show outdated Hide outdated includes/discovery/sensors/fanspeed/quanta.inc.php
$sensor_type = 'quanta_fan';
//FASTPATH-BOXSERVICES-PRIVATE-MIB::boxServicesFanSpeed
$sensors_id_oid = '.1.3.6.1.4.1.4413.1.1.43.1.6.1.4';
$sensors_values = snmp_walk($device, $sensors_id_oid, '-Ovq');

This comment has been minimized.

@laf

laf Jun 10, 2017

Member

You should use one of the snmpwalk_cache_* calls instead, this gives you an array rather than a string of data you need to split.

@laf

laf Jun 10, 2017

Member

You should use one of the snmpwalk_cache_* calls instead, this gives you an array rather than a string of data you need to split.

Show outdated Hide outdated includes/discovery/sensors/state/quanta.inc.php
dbInsert($insert, 'state_translations');
}
}
discover_sensor($valid['sensor'], 'state', $device, $chassis_state_oid, 1, $state_name, $descr, '1', '1', null, null, null, null, $state, 'snmp', 1);

This comment has been minimized.

@laf

laf Jun 10, 2017

Member

Swap the 1 you've used for index to boxServicesTempUnitState on this line and the one below.

@laf

laf Jun 10, 2017

Member

Swap the 1 you've used for index to boxServicesTempUnitState on this line and the one below.

Show outdated Hide outdated includes/discovery/sensors/state/quanta.inc.php
$idx = 0;
foreach ($temp as $index => $entry) {
$descr = $tablevalue[3] . $idx;
$oid_for_entry = '.1.' . $index;

This comment has been minimized.

@laf

laf Jun 10, 2017

Member

Change this to $oid_for_entry = $tablevalue[2]. ".$index";

@laf

laf Jun 10, 2017

Member

Change this to $oid_for_entry = $tablevalue[2]. ".$index";

Show outdated Hide outdated includes/discovery/sensors/temperature/quanta.inc.php
$sensor_type = 'quanta_temp';
//FASTPATH-BOXSERVICES-PRIVATE-MIB::boxServicesTempSensorIndex
$sensors_id_oid = '.1.3.6.1.4.1.4413.1.1.43.1.8.1.4';
$sensors_values = snmp_walk($device, $sensors_id_oid, '-Ovq');

This comment has been minimized.

@laf

laf Jun 10, 2017

Member

Change snmp_walk to one of the other snmpwalk_cache_* calls.

@laf

laf Jun 10, 2017

Member

Change snmp_walk to one of the other snmpwalk_cache_* calls.

switching to names instead of numeric oids
cleaned up the mibs a bit
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment

LibreNMS-CI commented Jun 12, 2017

Auto-Deploy finished, Test PR at http://6816.ci.librenms.org or https://6816.ci.librenms.org

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment

LibreNMS-CI commented Jun 12, 2017

Auto-Deploy finished, Test PR at http://6816.ci.librenms.org or https://6816.ci.librenms.org

@laf

Few more things I've commented on.

Also the mibs, you've still got some that are already available, the ones I said last time were just example. So you'll need to remove those duplicates.

Show outdated Hide outdated includes/definitions/quanta.yaml
discovery:
- sysObjectId:
- .1.3.6.1.4.1.7244
-
sysObjectId:

This comment has been minimized.

@laf

laf Jun 12, 2017

Member

How come this was changed? I know it doesn't look much but moving this does change the way detection is done.

@laf

laf Jun 12, 2017

Member

How come this was changed? I know it doesn't look much but moving this does change the way detection is done.

This comment has been minimized.

@segfault

segfault Jun 12, 2017

Contributor

I'll revert it. What's the implication between the two?

@segfault

segfault Jun 12, 2017

Contributor

I'll revert it. What's the implication between the two?

This comment has been minimized.

@laf

laf Jun 12, 2017

Member

The original dictates an AND so both the second sysObjectId and sysDescr are required, the new change is just an OR.

@laf

laf Jun 12, 2017

Member

The original dictates an AND so both the second sysObjectId and sysDescr are required, the new change is just an OR.

This comment has been minimized.

@segfault

segfault Jun 12, 2017

Contributor

I believe I do want the OR. The first oid isn't provided by the lb6m.

@segfault

segfault Jun 12, 2017

Contributor

I believe I do want the OR. The first oid isn't provided by the lb6m.

This comment has been minimized.

@laf

laf Jun 12, 2017

Member

Actually I've just tested the change and it's the same for both so you can revert this (unless you really did find a reason to change it):

NEW
Array
(
    [discovery] => Array
        (
            [0] => Array
                (
                    [sysObjectId] => Array
                        (
                            [0] => .1.3.6.1.4.1.7244
                        )

                )

            [1] => Array
                (
                    [sysObjectId] => Array
                        (
                            [0] => .1.3.6.1.4.1.4413
                        )

                    [sysDescr_regex] => Array
                        (
                            [0] => /VxWorks/
                            [1] => /quanta/i
                        )

                )

        )

)
OLD:
Array
(
    [discovery] => Array
        (
            [0] => Array
                (
                    [sysObjectId] => Array
                        (
                            [0] => .1.3.6.1.4.1.7244
                        )

                )

            [1] => Array
                (
                    [sysObjectId] => Array
                        (
                            [0] => .1.3.6.1.4.1.4413
                        )

                    [sysDescr_regex] => Array
                        (
                            [0] => /VxWorks/
                            [1] => /quanta/i
                        )

                )

        )

)
@laf

laf Jun 12, 2017

Member

Actually I've just tested the change and it's the same for both so you can revert this (unless you really did find a reason to change it):

NEW
Array
(
    [discovery] => Array
        (
            [0] => Array
                (
                    [sysObjectId] => Array
                        (
                            [0] => .1.3.6.1.4.1.7244
                        )

                )

            [1] => Array
                (
                    [sysObjectId] => Array
                        (
                            [0] => .1.3.6.1.4.1.4413
                        )

                    [sysDescr_regex] => Array
                        (
                            [0] => /VxWorks/
                            [1] => /quanta/i
                        )

                )

        )

)
OLD:
Array
(
    [discovery] => Array
        (
            [0] => Array
                (
                    [sysObjectId] => Array
                        (
                            [0] => .1.3.6.1.4.1.7244
                        )

                )

            [1] => Array
                (
                    [sysObjectId] => Array
                        (
                            [0] => .1.3.6.1.4.1.4413
                        )

                    [sysDescr_regex] => Array
                        (
                            [0] => /VxWorks/
                            [1] => /quanta/i
                        )

                )

        )

)
Show outdated Hide outdated includes/polling/mempools/quanta.inc.php
* the source code distribution for details.
*/
$total = snmp_get($device, '.1.3.6.1.4.1.4413.1.1.1.1.4.2.0', '-Oqv');

This comment has been minimized.

@laf

laf Jun 12, 2017

Member

You can swap these to named OIDs here as well.

@laf

laf Jun 12, 2017

Member

You can swap these to named OIDs here as well.

Show outdated Hide outdated includes/polling/os/quanta.inc.php
@@ -1,3 +1,4 @@
<?php
list($hardware, $features, $version) = explode(',', $poll_device['sysDescr']);
$serial = trim(snmp_get($device, ".1.3.6.1.4.1.4413.1.1.1.1.1.4.0", "-Ovq"), '" ');

This comment has been minimized.

@laf

laf Jun 12, 2017

Member

Named OID here as well.

@laf

laf Jun 12, 2017

Member

Named OID here as well.

Show outdated Hide outdated includes/polling/processors/quanta.inc.php
d_echo('Quanta CPU usage:');
//SNMPv2-SMI::enterprises.4413.1.1.1.1.4.9.0
$proc_usage = snmp_get($device, '.1.3.6.1.4.1.4413.1.1.1.1.4.9.0', '-Ovq');

This comment has been minimized.

@laf

laf Jun 12, 2017

Member

Named OID here.

@laf

laf Jun 12, 2017

Member

Named OID here.

removing the units from the mempool output
reverting the quanta definition change as it's a no-op
removing more duplicate mibs
@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment

LibreNMS-CI commented Jun 12, 2017

Auto-Deploy finished, Test PR at http://6816.ci.librenms.org or https://6816.ci.librenms.org

@segfault

This comment has been minimized.

Show comment
Hide comment
@segfault

segfault Jun 13, 2017

Contributor

Hopefully all of the outstanding items have been sorted at this point. I have no further changes unless there's further cleanup required.

Contributor

segfault commented Jun 13, 2017

Hopefully all of the outstanding items have been sorted at this point. I have no further changes unless there's further cleanup required.

laf added some commits Jun 13, 2017

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment

LibreNMS-CI commented Jun 13, 2017

Auto-Deploy finished, Test PR at http://6816.ci.librenms.org or https://6816.ci.librenms.org

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment

LibreNMS-CI commented Jun 13, 2017

Auto-Deploy finished, Test PR at http://6816.ci.librenms.org or https://6816.ci.librenms.org

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Jun 13, 2017

Member

Still one more numerical OID that needs updating but you also seem to have duplicate MIBs still:
mibs/quanta/vlan.my as one example.

Member

laf commented Jun 13, 2017

Still one more numerical OID that needs updating but you also seem to have duplicate MIBs still:
mibs/quanta/vlan.my as one example.

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment

LibreNMS-CI commented Jun 13, 2017

Auto-Deploy finished, Test PR at http://6816.ci.librenms.org or https://6816.ci.librenms.org

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment

LibreNMS-CI commented Jun 13, 2017

Auto-Deploy finished, Test PR at http://6816.ci.librenms.org or https://6816.ci.librenms.org

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment

LibreNMS-CI commented Jun 13, 2017

Auto-Deploy finished, Test PR at http://6816.ci.librenms.org or https://6816.ci.librenms.org

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment

LibreNMS-CI commented Jun 13, 2017

Auto-Deploy finished, Test PR at http://6816.ci.librenms.org or https://6816.ci.librenms.org

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment

LibreNMS-CI commented Jun 13, 2017

Auto-Deploy finished, Test PR at http://6816.ci.librenms.org or https://6816.ci.librenms.org

@LibreNMS-CI

This comment has been minimized.

Show comment
Hide comment

LibreNMS-CI commented Jun 13, 2017

Auto-Deploy finished, Test PR at http://6816.ci.librenms.org or https://6816.ci.librenms.org

@segfault

This comment has been minimized.

Show comment
Hide comment
@segfault

segfault Jun 13, 2017

Contributor

Just pushed the last two items. Anything else I should clean up at this point?

Contributor

segfault commented Jun 13, 2017

Just pushed the last two items. Anything else I should clean up at this point?

one last numeric oid to name
removing the duplicate q-bridge mib
@scrutinizer-notifier

This comment has been minimized.

Show comment
Hide comment
@scrutinizer-notifier

scrutinizer-notifier Jun 13, 2017

The inspection completed: No new issues

scrutinizer-notifier commented Jun 13, 2017

The inspection completed: No new issues

@segfault

This comment has been minimized.

Show comment
Hide comment
@segfault

segfault Jun 13, 2017

Contributor

The build failure looks unrelated.

ERROR 1062 (23000) at line 1322: Duplicate entry 'device-pulse_users-firewall' for key 'PRIMARY'
7.59MiB 0:01:17
7.59MiB 0:01:17
mysqldump: Got errno 32 on write
ERROR: Cannot mirror master's database! MYSQL_USER=jenkins_librenms_pr-6816 MYSQL_DATABASE=jenkins_librenms_pr6816
Build step 'Execute shell' marked build as failure

Contributor

segfault commented Jun 13, 2017

The build failure looks unrelated.

ERROR 1062 (23000) at line 1322: Duplicate entry 'device-pulse_users-firewall' for key 'PRIMARY'
7.59MiB 0:01:17
7.59MiB 0:01:17
mysqldump: Got errno 32 on write
ERROR: Cannot mirror master's database! MYSQL_USER=jenkins_librenms_pr-6816 MYSQL_DATABASE=jenkins_librenms_pr6816
Build step 'Execute shell' marked build as failure

@laf

This comment has been minimized.

Show comment
Hide comment
@laf

laf Jun 13, 2017

Member

image

Member

laf commented Jun 13, 2017

image

@laf laf merged commit a5cf0b7 into librenms:master Jun 13, 2017

2 of 3 checks passed

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

@segfault segfault deleted the segfault:add-quanta-lb6m branch Jun 13, 2017

@lock

This comment has been minimized.

Show comment
Hide comment
@lock

lock bot May 17, 2018

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

lock bot commented May 17, 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 17, 2018

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