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

Convert OS mrv-od to yaml, discover more sensors, discover entity-physical #10266

Merged
merged 7 commits into from May 29, 2019

Conversation

Projects
None yet
2 participants
@martijn-schmidt
Copy link
Contributor

commented May 26, 2019

  • Dynamic entPhysicalIndex values for YamlDiscovery

  • Use degree symbol when displaying temperature values

  • Adjust SNMP options for new snmp_get_multi()

    The OS polling module for mrv-od was using snmpget commands with
    -Ovqs, rather than -OQUs, which meant that the equal sign was
    stripped in the snmpget output. But that equal sign is now used
    as the explode() delimiter so it broke the construction of the
    returned array for the snmp_get_multi() function.

    Additionally, I removed the NBS-CMMC-MIB:: prefix which is no
    longer returned by snmp_get_multi() as part of the OID. Now,
    the mrv-od.inc.php OS polling module is retrieving data from
    positions in the array with actual content.

  • Convert mrv-od to yaml, improve sensors discovery

    Converted sensors discovery for mrv-od to yaml, retaining all
    functionality for pre-cache, dbm, and temperature. Replaced specific
    SFF-8472 Digital Diagnostics state sensors with generic ones, given that
    we are now reading out detailed values through new dbm, current, voltage
    sensors.

    Reasoning: the generic Digital Diagnostics state sensors report state
    diagsOk, or else the worst severity level measured among all of the
    Digital Diagnostics values for that optical data point. You'd really
    only want to set a generic "alarm" so the box can report that it
    thinks something is wrong, whereafter a human can look at the graphs
    and/or the CLI to draw their conclusions about the alarm. Otherwise,
    things would quickly become too spammy as you are essentially duplicating
    every sensor across both regular measurements and state measurements.

    Further changes:

    • Increase maximum number of detected PSUs from 4 to 8 per the MIB.
    • State sensor to detect (in)sufficient power budget for all modules.
    • Use skip_values to hide state sensors with the notSupported value.
    • Use skip_values to hide sensors which are unsupported per the MIB,
      signified by the device returning special values -1 or -2147483648.
    • Add dBm, temperature "over" graphs to the mrv-od device header bar.
  • Add mrv-od entity-physical support

    MRV does not use the ENTITY-MIB data format, so we had to reconstruct
    the inventory from their proprietary format using the following tables
    from their NBS-CMMC-MIB:

    • nbsCmmcChassisTable
    • nbsCmmcSlotTable
    • nbsCmmcPortTable

    Additionally, since sensors didn't have their own ifindex in the
    NBS-CMMC-MIB, I went ahead and appended the last OID digit to
    the "containing" ifindex create a unique number. The same was
    done for the sensors in the yaml-discovery.

  • Moderated our IP addresses, ifaliases, serialnumbers (retaining original length) in the snmpsim data

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

martijn-schmidt added some commits May 26, 2019

Adjust SNMP options for new snmp_get_multi()
The OS polling module for mrv-od was using snmpget commands with
-Ovqs, rather than -OQUs, which meant that the equal sign was
stripped in the snmpget output. But that equal sign is now used
as the explode() delimiter so it broke the construction of the
returned array for the snmp_get_multi() function.

Additionally, I removed the NBS-CMMC-MIB:: prefix which is no
longer returned by snmp_get_multi() as part of the OID. Now,
the mrv-od.inc.php OS polling module is retrieving data from
positions in the array with actual content.
Convert mrv-od to yaml, improve sensors discovery
Converted sensors discovery for mrv-od to yaml, retaining all
functionality for pre-cache, dbm, and temperature. Replaced specific
SFF-8472 Digital Diagnostics state sensors with generic ones, given that
we are now reading out detailed values through new dbm, current, voltage
sensors.

Reasoning: the generic Digital Diagnostics state sensors report state
diagsOk, or else the worst severity level measured among all of the
Digital Diagnostics values for that optical data point. You'd really
only want to set a generic "alarm" so the box can report that it
thinks something is wrong, whereafter a human can look at the graphs
and/or the CLI to draw their conclusions about the alarm. Otherwise,
things would quickly become too spammy as you are essentially duplicating
every sensor across both regular measurements and state measurements.

Further changes:
- Increase maximum number of detected PSUs from 4 to 8 per the MIB.
- State sensor to detect (in)sufficient power budget for all modules.
- Use skip_values to hide state sensors with the notSupported value.
- Use skip_values to hide sensors which are unsupported per the MIB,
  signified by the device returning special values -1 or -2147483648.
- Add dBm, temperature "over" graphs to the mrv-od device header bar.
Add mrv-od entity-physical support
MRV does not use the ENTITY-MIB data format, so we had to reconstruct
the inventory from their proprietary format using the following tables
from their NBS-CMMC-MIB:

- nbsCmmcChassisTable
- nbsCmmcSlotTable
- nbsCmmcPortTable

Additionally, since sensors didn't have their own ifindex in the
NBS-CMMC-MIB, I went ahead and appended the last OID digit to
the "containing" ifindex create a unique number. The same was
done for the sensors in the yaml-discovery.

@martijn-schmidt martijn-schmidt marked this pull request as ready for review May 26, 2019

@martijn-schmidt martijn-schmidt changed the title WIP - convert OS mrv-od to yaml, discover more sensors, discover entity-physical Convert OS mrv-od to yaml, discover more sensors, discover entity-physical May 26, 2019

Fix entPhysicalIndex returning 0 instead of null
Some unit test breakage was caused by 62167eb which changed
the code to return 0 instead of null when {{ $index }} was
set as the entPhysicalIndex in the YamlDiscovery. So now we
detect if it returns some value which may be interpreted as
false, and if so we set null rather than 0.

@murrant murrant added the Device 🖥 label May 29, 2019

@murrant
Copy link
Member

left a comment

Thanks, great work!

@murrant murrant merged commit 9455a83 into librenms:master May 29, 2019

5 of 6 checks passed

codeclimate 6 issues to fix
Details
Inspection Summary
Details
Node: analysis
Details
Travis CI - Pull Request Build Passed
Details
WIP Ready for review
Details
license/cla Contributor License Agreement is signed.
Details

@martijn-schmidt martijn-schmidt deleted the martijn-schmidt:mrv-updates branch May 29, 2019

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